Add Temporal Nexus Operation Handler - #2842
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
a33ff01 to
7e61dab
Compare
7e61dab to
f2c65fc
Compare
VegetarianOrc
left a comment
VegetarianOrc
left a comment
There was a problem hiding this comment.
Looks great. Couple small questions mostly for my learning!
| if (Strings.isNullOrEmpty(token.getWorkflowId())) { | ||
| throw new IllegalArgumentException("Invalid workflow run token: missing workflow ID (wid)"); | ||
| throw new IllegalArgumentException("Invalid operation token: missing workflow ID (wid)"); |
There was a problem hiding this comment.
Should this check be moved into loadWorkflowRunOperationToken?
| /** | ||
| * Returns the synchronous result value, or null if this is an async result. | ||
| * | ||
| * @return the sync result value, or null | ||
| */ | ||
| @Nullable | ||
| public R getSyncResult() { | ||
| return syncResult; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the async operation token, or null if this is a sync result. | ||
| * | ||
| * @return the operation token, or null | ||
| */ | ||
| @Nullable | ||
| public String getAsyncOperationToken() { | ||
| return asyncOperationToken; | ||
| } |
There was a problem hiding this comment.
Just a style question to familiarize myself with Java a bit more: Should these throw if invoked on the wrong type of result? i.e. should getAsyncOperationToken throw if isSync == true?
There was a problem hiding this comment.
Agreed, it would be more idiomatic to check and throw. We can then remove @Nullable annotations from the methods too.
|
|
||
| public WorkflowRunOperationToken(String namespace, String workflowId) { | ||
| this.type = OperationTokenType.WORKFLOW_RUN; | ||
| public OperationToken(OperationTokenType type, String namespace, String workflowId) { |
There was a problem hiding this comment.
With more types of tokens upcoming, should we favor something like static constructors for this like OperationToken.NewWorkflowRunToken() that takes the required fields for the workflow run type?
I guess this is a bit like what OperationTokenUtil provides so feel free to dismiss this!
maciejdudko
left a comment
maciejdudko
left a comment
There was a problem hiding this comment.
Looks good, only a few small changes needed.
| /** | ||
| * Returns the synchronous result value, or null if this is an async result. | ||
| * | ||
| * @return the sync result value, or null | ||
| */ | ||
| @Nullable | ||
| public R getSyncResult() { | ||
| return syncResult; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the async operation token, or null if this is a sync result. | ||
| * | ||
| * @return the operation token, or null | ||
| */ | ||
| @Nullable | ||
| public String getAsyncOperationToken() { | ||
| return asyncOperationToken; | ||
| } |
There was a problem hiding this comment.
Agreed, it would be more idiomatic to check and throw. We can then remove @Nullable annotations from the methods too.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 10a94e4. Configure here.
10a94e4 to
71ab56f
Compare
3 participants
Add TemporalOperationHandler for generic Nexus operations
Goal
Provide a new base to make it easy to expose more Temporal actions as Nexus Operations
Summary
Test plan
Note
Medium Risk
Touches Nexus operation start/cancel and workflow lifecycle from handlers; behavior changes are mostly additive behind experimental APIs, but refactors WorkflowRunOperationImpl and token parsing used on cancel paths.
Overview
Adds
TemporalOperationHandler, a composable NexusOperationHandlerthat wires a user-defined start lambda to Temporal viaTemporalNexusClient(typed/untypedstartWorkflowoverloads) andTemporalOperationResult(sync vs async operation token). Cancel paths parse tokens with a generalizedOperationToken/loadOperationToken, dispatch workflow-run cancels through overridablecancelWorkflowRun, and default behavior cancels the backing workflow.Shared “start workflow + attach Nexus links” logic moves into
NexusStartWorkflowHelper, whichWorkflowRunOperationImplnow uses instead of inlined code.TemporalNexusClientImplenforces one async start per handler invocation (extra starts fail withBAD_REQUEST; the guard resets if start fails).All new public Nexus APIs are marked
@Experimental. Integration tests cover typed/untyped starts, void workflows, sync results, cancel, and double-start rejection.Reviewed by Cursor Bugbot for commit 71ab56f. Bugbot is set up for automated code reviews on this repo. Configure here.