Skip to content

Commit 1e2af3d

Browse files
fretz12dandavison
andauthored
Add handling of business ID policy for standalone activities (#8712)
## What changed? Add handling of business ID policy for standalone activities. Refactored standalone activity validations. ## Why? Required so that the chasm engine will handle the business ID policies based on the RPC request. ## How did you test it? - [X] built - [X] run locally and tested manually - [X] covered by existing tests - [X] added new unit test(s) - [X] added new functional test(s) ## Potential risks Will need to add more tests once we rebase standalone-activity with main. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds business ID reuse/conflict policy handling for standalone activities, refactors validation to operate on the full request with sane defaults, and updates tests accordingly. > > - **Activity Backend**: > - Map `enumspb` activity ID reuse/conflict policies to chasm policies and pass via `chasm.WithBusinessIDPolicy` in `StartActivityExecution`. > - Propagate `RequestId` via `chasm.WithRequestID`. > - **Validation**: > - Refactor `ValidateStandaloneActivity` to accept the entire `workflowservice.StartActivityExecutionRequest` and mutate in place (request ID, ID policies, input size, search attributes). > - Add `normalizeAndValidateIDPolicy` with defaults and incompatibility checks; require request ID when attaching completion callbacks. > - Minor tweaks to input/search attributes validation paths. > - **Frontend**: > - Update `validateAndPopulateStartRequest` to use new validation signatures and remove redundant vars. > - **Tests**: > - Add/extend unit tests for ID policy defaults/mismatch, input size warning/error, and request ID length. > - Add functional tests covering ID reuse (`REJECT_DUPLICATE`, `ALLOW_DUPLICATE_FAILED_ONLY`) and conflict policy failure on existing execution. > - Adjust cancellation tests to use unique `RequestId`; remove default `RequestId` from `startActivity` helper. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1225ccc. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Dan Davison <dan.davison@temporal.io>
1 parent 97d0e6c commit 1e2af3d

File tree

4 files changed

+188
-8
lines changed

4 files changed

+188
-8
lines changed

chasm/lib/activity/handler.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,29 @@ package activity
33
import (
44
"context"
55
"errors"
6+
"fmt"
67

8+
enumspb "go.temporal.io/api/enums/v1"
79
"go.temporal.io/api/serviceerror"
810
"go.temporal.io/api/workflowservice/v1"
911
"go.temporal.io/server/chasm"
1012
"go.temporal.io/server/chasm/lib/activity/gen/activitypb/v1"
1113
"go.temporal.io/server/common/contextutil"
1214
)
1315

16+
var (
17+
businessIDReusePolicyMap = map[enumspb.ActivityIdReusePolicy]chasm.BusinessIDReusePolicy{
18+
enumspb.ACTIVITY_ID_REUSE_POLICY_ALLOW_DUPLICATE: chasm.BusinessIDReusePolicyAllowDuplicate,
19+
enumspb.ACTIVITY_ID_REUSE_POLICY_ALLOW_DUPLICATE_FAILED_ONLY: chasm.BusinessIDReusePolicyAllowDuplicateFailedOnly,
20+
enumspb.ACTIVITY_ID_REUSE_POLICY_REJECT_DUPLICATE: chasm.BusinessIDReusePolicyRejectDuplicate,
21+
}
22+
23+
// TODO this will change once we rebase on main
24+
businessIDConflictPolicyMap = map[enumspb.ActivityIdConflictPolicy]chasm.BusinessIDConflictPolicy{
25+
enumspb.ACTIVITY_ID_CONFLICT_POLICY_FAIL: chasm.BusinessIDConflictPolicyFail,
26+
}
27+
)
28+
1429
type handler struct {
1530
activitypb.UnimplementedActivityServiceServer
1631
config *Config
@@ -23,6 +38,18 @@ func newHandler(config *Config) *handler {
2338
}
2439

2540
func (h *handler) StartActivityExecution(ctx context.Context, req *activitypb.StartActivityExecutionRequest) (*activitypb.StartActivityExecutionResponse, error) {
41+
frontendReq := req.GetFrontendRequest()
42+
43+
reusePolicy, ok := businessIDReusePolicyMap[frontendReq.GetIdReusePolicy()]
44+
if !ok {
45+
return nil, serviceerror.NewFailedPrecondition(fmt.Sprintf("unsupported ID reuse policy: %v", frontendReq.GetIdReusePolicy()))
46+
}
47+
48+
conflictPolicy, ok := businessIDConflictPolicyMap[frontendReq.GetIdConflictPolicy()]
49+
if !ok {
50+
return nil, serviceerror.NewFailedPrecondition(fmt.Sprintf("unsupported ID conflict policy: %v", frontendReq.GetIdConflictPolicy()))
51+
}
52+
2653
response, key, _, err := chasm.NewEntity(
2754
ctx,
2855
chasm.EntityKey{
@@ -47,6 +74,7 @@ func (h *handler) StartActivityExecution(ctx context.Context, req *activitypb.St
4774
},
4875
req.GetFrontendRequest(),
4976
chasm.WithRequestID(req.GetFrontendRequest().GetRequestId()),
77+
chasm.WithBusinessIDPolicy(reusePolicy, conflictPolicy),
5078
)
5179

5280
if err != nil {

chasm/lib/activity/validator.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
package activity
22

33
import (
4-
"fmt"
5-
64
"github.com/google/uuid"
75
activitypb "go.temporal.io/api/activity/v1"
86
commonpb "go.temporal.io/api/common/v1"
7+
enumspb "go.temporal.io/api/enums/v1"
98
"go.temporal.io/api/serviceerror"
109
"go.temporal.io/api/workflowservice/v1"
1110
"go.temporal.io/server/common"
@@ -43,7 +42,7 @@ func ValidateAndNormalizeActivityAttributes(
4342
runTimeout *durationpb.Duration,
4443
) error {
4544
if err := tqid.NormalizeAndValidate(options.TaskQueue, "", maxIDLengthLimit); err != nil {
46-
return fmt.Errorf("invalid TaskQueue: %w. ActivityId=%s ActivityType=%s", err, activityID, activityType)
45+
return err
4746
}
4847

4948
if activityID == "" {
@@ -54,7 +53,7 @@ func ValidateAndNormalizeActivityAttributes(
5453
}
5554

5655
if err := validateActivityRetryPolicy(namespaceID, options.RetryPolicy, getDefaultActivityRetrySettings); err != nil {
57-
return fmt.Errorf("invalid ActivityRetryPolicy: %w. ActivityId=%s ActivityType=%s", err, activityID, activityType)
56+
return err
5857
}
5958

6059
if len(activityID) > maxIDLengthLimit {
@@ -182,6 +181,10 @@ func validateAndNormalizeStartActivityExecutionRequest(
182181
return serviceerror.NewInvalidArgument("RequestID length exceeds limit.")
183182
}
184183

184+
if err := normalizeAndValidateIDPolicy(req); err != nil {
185+
return err
186+
}
187+
185188
if err := validateInputSize(
186189
req.GetActivityId(),
187190
req.GetActivityType().GetName(),
@@ -204,6 +207,18 @@ func validateAndNormalizeStartActivityExecutionRequest(
204207
return nil
205208
}
206209

210+
func normalizeAndValidateIDPolicy(req *workflowservice.StartActivityExecutionRequest) error {
211+
if req.GetIdReusePolicy() == enumspb.ACTIVITY_ID_REUSE_POLICY_UNSPECIFIED {
212+
req.IdReusePolicy = enumspb.ACTIVITY_ID_REUSE_POLICY_ALLOW_DUPLICATE
213+
}
214+
215+
if req.GetIdConflictPolicy() == enumspb.ACTIVITY_ID_CONFLICT_POLICY_UNSPECIFIED {
216+
req.IdConflictPolicy = enumspb.ACTIVITY_ID_CONFLICT_POLICY_FAIL
217+
}
218+
219+
return nil
220+
}
221+
207222
func validateInputSize(
208223
activityID string,
209224
blobSizeViolationTagValue string,

chasm/lib/activity/validator_test.go

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"github.com/stretchr/testify/require"
88
activitypb "go.temporal.io/api/activity/v1"
99
commonpb "go.temporal.io/api/common/v1"
10+
enumspb "go.temporal.io/api/enums/v1"
11+
"go.temporal.io/api/serviceerror"
1012
taskqueuepb "go.temporal.io/api/taskqueue/v1"
1113
"go.temporal.io/api/workflowservice/v1"
1214
"go.temporal.io/server/common/dynamicconfig"
@@ -209,7 +211,8 @@ func TestValidateFailures(t *testing.T) {
209211
tc.options,
210212
tc.priority,
211213
durationpb.New(0))
212-
require.Error(t, err)
214+
var invalidArgErr *serviceerror.InvalidArgument
215+
require.ErrorAs(t, err, &invalidArgErr)
213216
})
214217
}
215218
}
@@ -231,7 +234,8 @@ func TestValidateStandAloneRequestIDTooLong(t *testing.T) {
231234
log.NewNoopLogger(),
232235
defaultMaxIDLengthLimit,
233236
nil)
234-
require.Error(t, err)
237+
var invalidArgErr *serviceerror.InvalidArgument
238+
require.ErrorAs(t, err, &invalidArgErr)
235239
}
236240

237241
func TestValidateStandAloneInputTooLarge(t *testing.T) {
@@ -251,7 +255,8 @@ func TestValidateStandAloneInputTooLarge(t *testing.T) {
251255
log.NewNoopLogger(),
252256
defaultMaxIDLengthLimit,
253257
nil)
254-
require.Error(t, err)
258+
var invalidArgErr *serviceerror.InvalidArgument
259+
require.ErrorAs(t, err, &invalidArgErr)
255260
}
256261

257262
func TestValidateStandAloneInputWarningSizeShouldSucceed(t *testing.T) {
@@ -277,6 +282,28 @@ func TestValidateStandAloneInputWarningSizeShouldSucceed(t *testing.T) {
277282
require.NoError(t, err)
278283
}
279284

285+
func TestValidateStandAlone_IDPolicyShouldDefault(t *testing.T) {
286+
req := &workflowservice.StartActivityExecutionRequest{
287+
ActivityId: defaultActivityID,
288+
ActivityType: &commonpb.ActivityType{Name: defaultActivityType},
289+
Options: &defaultActivityOptions,
290+
Namespace: "default",
291+
RequestId: "test-request-id",
292+
}
293+
294+
err := validateAndNormalizeStartActivityExecutionRequest(
295+
req,
296+
defaultBlobSizeLimitError,
297+
defaultBlobSizeLimitWarn,
298+
log.NewNoopLogger(),
299+
defaultMaxIDLengthLimit,
300+
nil)
301+
302+
require.NoError(t, err)
303+
require.Equal(t, enumspb.ACTIVITY_ID_REUSE_POLICY_ALLOW_DUPLICATE, req.IdReusePolicy)
304+
require.Equal(t, enumspb.ACTIVITY_ID_CONFLICT_POLICY_FAIL, req.IdConflictPolicy)
305+
}
306+
280307
func TestModifiedActivityTimeouts(t *testing.T) {
281308
cases := []struct {
282309
name string
@@ -406,7 +433,8 @@ func TestModifiedActivityTimeouts(t *testing.T) {
406433
tc.runTimeout)
407434

408435
if tc.isErr {
409-
require.Error(t, err)
436+
var invalidArgErr *serviceerror.InvalidArgument
437+
require.ErrorAs(t, err, &invalidArgErr)
410438
return
411439
}
412440

tests/standalone_activity_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,115 @@ func (s *standaloneActivityTestSuite) SetupTest() {
6262
s.tv = testvars.New(s.T())
6363
}
6464

65+
func (s *standaloneActivityTestSuite) TestIDReusePolicy_RejectDuplicate() {
66+
t := s.T()
67+
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
68+
defer cancel()
69+
70+
activityID := s.tv.ActivityID()
71+
taskQueue := s.tv.TaskQueue().String()
72+
73+
startResp := s.startAndValidateActivity(ctx, t, activityID, taskQueue)
74+
runID := startResp.RunId
75+
76+
pollTaskResp := s.pollActivityTaskAndValidate(ctx, t, activityID, taskQueue, runID)
77+
78+
_, err := s.FrontendClient().RespondActivityTaskCompleted(ctx, &workflowservice.RespondActivityTaskCompletedRequest{
79+
Namespace: s.Namespace().String(),
80+
TaskToken: pollTaskResp.TaskToken,
81+
Result: defaultResult,
82+
Identity: "new-worker",
83+
})
84+
require.NoError(t, err)
85+
86+
s.validateCompletion(ctx, t, activityID, runID, "new-worker")
87+
88+
_, err = s.FrontendClient().StartActivityExecution(ctx, &workflowservice.StartActivityExecutionRequest{
89+
Namespace: s.Namespace().String(),
90+
ActivityId: activityID,
91+
ActivityType: s.tv.ActivityType(),
92+
Identity: s.tv.WorkerIdentity(),
93+
Input: defaultInput,
94+
Options: &activitypb.ActivityOptions{
95+
TaskQueue: &taskqueuepb.TaskQueue{
96+
Name: taskQueue,
97+
},
98+
StartToCloseTimeout: durationpb.New(1 * time.Minute),
99+
},
100+
IdReusePolicy: enumspb.ACTIVITY_ID_REUSE_POLICY_REJECT_DUPLICATE,
101+
})
102+
require.Error(t, err)
103+
}
104+
105+
func (s *standaloneActivityTestSuite) TestIDReusePolicy_AllowDuplicateFailedOnly() {
106+
t := s.T()
107+
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
108+
defer cancel()
109+
110+
activityID := s.tv.ActivityID()
111+
taskQueue := s.tv.TaskQueue().String()
112+
113+
startResp := s.startAndValidateActivity(ctx, t, activityID, taskQueue)
114+
runID := startResp.RunId
115+
116+
pollTaskResp := s.pollActivityTaskAndValidate(ctx, t, activityID, taskQueue, runID)
117+
118+
_, err := s.FrontendClient().RespondActivityTaskFailed(ctx, &workflowservice.RespondActivityTaskFailedRequest{
119+
Namespace: s.Namespace().String(),
120+
TaskToken: pollTaskResp.TaskToken,
121+
Failure: defaultFailure,
122+
Identity: "new-worker",
123+
})
124+
require.NoError(t, err)
125+
126+
s.validateFailure(ctx, t, activityID, runID, nil, "new-worker")
127+
128+
_, err = s.FrontendClient().StartActivityExecution(ctx, &workflowservice.StartActivityExecutionRequest{
129+
Namespace: s.Namespace().String(),
130+
ActivityId: activityID,
131+
ActivityType: s.tv.ActivityType(),
132+
Identity: s.tv.WorkerIdentity(),
133+
Input: defaultInput,
134+
Options: &activitypb.ActivityOptions{
135+
TaskQueue: &taskqueuepb.TaskQueue{
136+
Name: taskQueue,
137+
},
138+
StartToCloseTimeout: durationpb.New(1 * time.Minute),
139+
},
140+
IdReusePolicy: enumspb.ACTIVITY_ID_REUSE_POLICY_ALLOW_DUPLICATE_FAILED_ONLY,
141+
})
142+
require.NoError(t, err)
143+
}
144+
145+
func (s *standaloneActivityTestSuite) TestIDConflictPolicy_FailsIfExists() {
146+
t := s.T()
147+
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
148+
defer cancel()
149+
150+
activityID := s.tv.ActivityID()
151+
taskQueue := s.tv.TaskQueue().String()
152+
153+
s.startAndValidateActivity(ctx, t, activityID, taskQueue)
154+
155+
// By default, unspecified conflict policy should be set to ACTIVITY_ID_CONFLICT_POLICY_FAIL, so no need to set explicitly
156+
_, err := s.FrontendClient().StartActivityExecution(ctx, &workflowservice.StartActivityExecutionRequest{
157+
Namespace: s.Namespace().String(),
158+
ActivityId: activityID,
159+
ActivityType: s.tv.ActivityType(),
160+
Identity: s.tv.WorkerIdentity(),
161+
Input: defaultInput,
162+
Options: &activitypb.ActivityOptions{
163+
TaskQueue: &taskqueuepb.TaskQueue{
164+
Name: taskQueue,
165+
},
166+
StartToCloseTimeout: durationpb.New(1 * time.Minute),
167+
},
168+
})
169+
require.Error(t, err)
170+
}
171+
172+
// TODO(fred): add test for BusinessIDConflictPolicyUseExisting after rebasing on main
173+
65174
func (s *standaloneActivityTestSuite) TestActivityCompleted() {
66175
t := s.T()
67176
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second)

0 commit comments

Comments
 (0)