-
Notifications
You must be signed in to change notification settings - Fork 169
Description
Expected Behavior
There are six potential sources for activity options:
- WorkflowImplementationOptions#defaultActivityOptions
- WorkflowImplementationOptions#activityOptions map
- Workflow.setDefaultActivityOptions
- Workflow.applyActivityOptions map
- options argument passed to WorkflowInternal#newActivityStub
- activityMethodOptions map argument passed to WorkflowInternal#newActivityStub
When all these options are specified for the given activity type, the logical behavior is to merge them in the same order as specified above.
Actual Behavior
The current logic is split between:
sdk-java/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInternal.java
Line 303 in 8af4a26
options = (options == null) ? context.getDefaultActivityOptions() : options; |
and
sdk-java/temporal-sdk/src/main/java/io/temporal/internal/sync/ActivityInvocationHandler.java
Line 70 in 16755a1
ActivityOptions merged = |
The short description is:
-
options = options argument passed to [WorkflowInternal#newActivityStub] ? WorkflowImplementationOptions#defaultActivityOptions // not that they are not merged if the argument is present.
-
WorkflowImplementationOptions#activityOptions map is overridden by options found in activityMethodOptions map argument passed to WorkflowInternal#newActivityStub
-
options (from step 1) are overridden by a value (with the key equal to the activity type) from the map generated by step (2)
I believe that we should fix the merging logic to match the intuitive behavior explained in the "Expected Behavior" section.