Skip to content

Fix UnsupportedOperationException when merging ActivityOptions with i… #2749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nitishagar wants to merge 1 commit into
base: master
Choose a base branch
from
+89 −1
Open

Fix UnsupportedOperationException when merging ActivityOptions with i… #2749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion temporal-sdk/src/main/java/io/temporal/activity/ActivityOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.temporal.common.context.ContextPropagator;
import io.temporal.failure.CanceledFailure;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;

/** Options used to configure how an activity is invoked. */
Expand Down Expand Up @@ -282,7 +283,9 @@ public Builder mergeActivityOptions(ActivityOptions override) {
if (this.contextPropagators == null) {
this.contextPropagators = override.contextPropagators;
} else if (override.contextPropagators != null) {
this.contextPropagators.addAll(override.contextPropagators);
List<ContextPropagator> merged = new ArrayList<>(this.contextPropagators);
merged.addAll(override.contextPropagators);
this.contextPropagators = merged;
}
if (override.versioningIntent != VersioningIntent.VERSIONING_INTENT_UNSPECIFIED) {
this.versioningIntent = override.versioningIntent;
Expand Down
85 changes: 85 additions & 0 deletions temporal-sdk/src/test/java/io/temporal/activity/ActivityOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@

import static org.junit.Assert.*;

import io.temporal.api.common.v1.Payload;
import io.temporal.common.MethodRetry;
import io.temporal.common.RetryOptions;
import io.temporal.common.context.ContextPropagator;
import io.temporal.testing.TestActivityEnvironment;
import io.temporal.workflow.shared.TestActivities.TestActivity;
import io.temporal.workflow.shared.TestActivities.TestActivityImpl;
import java.lang.reflect.Method;
import java.time.Duration;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import org.junit.*;
import org.junit.rules.Timeout;
Expand Down Expand Up @@ -62,6 +66,87 @@ public void testActivityOptionsMerge() {
Assert.assertEquals(methodOps1, merged);
}

@Test
public void testActivityOptionsMergeWithImmutableContextPropagators() {
// Create simple test context propagators
ContextPropagator propagator1 =
new ContextPropagator() {
Comment on lines +72 to +73
Copy link
Contributor

@maciejdudko maciejdudko Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot of code duplication. It would be better to have a local class inside the test function:

public void testActivityOptionsMergeWithImmutableContextPropagators() {
  class MyContextPropagator implements ContextPropagator {
    MyContextPropagator(String name) {
      this.name = name;
    }

      @Override
    public String getName() {
      return name;
    }
  
    // etc.
  }

Then instantiate it twice:

ContextPropagator propagator1 = new MyContextPropagator("propagator 1");
ContextPropagator propagator2 = new MyContextPropagator("propagator 2");

@Override
public String getName() {
return "propagator1";
}

@Override
public Map<String, Payload> serializeContext(Object context) {
return Collections.emptyMap();
}

@Override
public Object deserializeContext(Map<String, Payload> context) {
return null;
}

@Override
public Object getCurrentContext() {
return null;
}

@Override
public void setCurrentContext(Object context) {}
};

ContextPropagator propagator2 =
new ContextPropagator() {
@Override
public String getName() {
return "propagator2";
}

@Override
public Map<String, Payload> serializeContext(Object context) {
return Collections.emptyMap();
}

@Override
public Object deserializeContext(Map<String, Payload> context) {
return null;
}

@Override
public Object getCurrentContext() {
return null;
}

@Override
public void setCurrentContext(Object context) {}
};

// Create options with immutable singleton lists
// This tests the fix for https://github.com/temporalio/sdk-java/issues/2482
ActivityOptions options1 =
ActivityOptions.newBuilder()
.setStartToCloseTimeout(Duration.ofSeconds(1))
.setContextPropagators(Collections.singletonList(propagator1))
.build();

ActivityOptions options2 =
ActivityOptions.newBuilder()
.setStartToCloseTimeout(Duration.ofSeconds(2))
.setContextPropagators(Collections.singletonList(propagator2))
.build();

// Merging should not throw UnsupportedOperationException
ActivityOptions merged =
ActivityOptions.newBuilder(options1).mergeActivityOptions(options2).build();

// Verify both context propagators are present in the merged result
List<ContextPropagator> mergedPropagators = merged.getContextPropagators();
assertNotNull(mergedPropagators);
assertEquals(2, mergedPropagators.size());
assertEquals("propagator1", mergedPropagators.get(0).getName());
assertEquals("propagator2", mergedPropagators.get(1).getName());
}

@Test
public void testActivityOptionsDefaultInstance() {
testEnv.registerActivitiesImplementations(new TestActivityImpl());
Expand Down