temporalio / temporalio/sdk-java

Activity options merging logic is not correct

Open
#2,042 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
433
Forks
249
Avg merge
5d 6h
Merged PRs (30d)
26

Description

Expected Behavior

There are six potential sources for activity options:

  1. WorkflowImplementationOptions#defaultActivityOptions
  2. WorkflowImplementationOptions#activityOptions map
  3. Workflow.setDefaultActivityOptions
  4. Workflow.applyActivityOptions map
  5. options argument passed to WorkflowInternal#newActivityStub
  6. 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:

https://github.com/temporalio/sdk-java/blob/8af4a2647dacab2cb0d4a3a613482e00bc297a39/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInternal.java#L303

and

https://github.com/temporalio/sdk-java/blob/16755a1bb7ebba29fb820b86a001c079cf4ffb62/temporal-sdk/src/main/java/io/temporal/internal/sync/ActivityInvocationHandler.java#L70

The short description is:

  1. options = options argument passed to [WorkflowInternal#newActivityStub] ? WorkflowImplementationOptions#defaultActivityOptions // not that they are not merged if the argument is present.

  2. WorkflowImplementationOptions#activityOptions map is overridden by options found in activityMethodOptions map argument passed to WorkflowInternal#newActivityStub

  3. 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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the referenced logic in temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInternal.java and ActivityInvocationHandler.java, then trace the six activity-option sources listed in the issue. Compare their current precedence with the expected order and verify that the resulting merge preserves each layer’s overrides. Done means the implementation follows the specified order for options supplied to the activity stub.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.