Skip to content

Commit c3d7b47

Browse files
Backwards compatible activity type names (#994)
* Backwards compatible activity type names Previously merged shortened activity type names of form {Method} is not compatible with current users of this client, since multiple activities in different packages with the same name results in naming conflicts. This change reverts shortening when no activity alias (prefix) is provided. The use of prefix shortens the name, assuming the users responsiblity to choose proper prefix. * Add EnableShortName option for workflow registration
1 parent 80e222f commit c3d7b47

15 files changed

+239
-36
lines changed

evictiontest/workflow_cache_eviction_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func (s *CacheEvictionSuite) TestResetStickyOnEviction() {
130130
ret := &m.PollForDecisionTaskResponse{
131131
TaskToken: make([]byte, 5),
132132
WorkflowExecution: &m.WorkflowExecution{WorkflowId: workflowID, RunId: runID},
133-
WorkflowType: &m.WorkflowType{Name: common.StringPtr("testReplayWorkflow")},
133+
WorkflowType: &m.WorkflowType{Name: common.StringPtr("go.uber.org/cadence/evictiontest.testReplayWorkflow")},
134134
History: &m.History{Events: testEvents},
135135
PreviousStartedEventId: common.Int64Ptr(5)}
136136
return ret, nil

internal/activity.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ type (
6161
// When an activity is part of a structure then each member of the structure becomes an activity with
6262
// this Name as a prefix + activity function name.
6363
Name string
64+
// Activity type name is equal to function name instead of fully qualified
65+
// name including function package (and struct type if used).
66+
// This option has no effect when explicit Name is provided.
67+
EnableShortName bool
6468
DisableAlreadyRegisteredCheck bool
6569
}
6670

internal/internal_worker.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,8 +1091,6 @@ func getFunctionName(i interface{}) string {
10911091
return fullName
10921092
}
10931093
fullName := runtime.FuncForPC(reflect.ValueOf(i).Pointer()).Name()
1094-
elements := strings.Split(fullName, ".")
1095-
shortName := elements[len(elements)-1]
10961094
// This allows to call activities by method pointer
10971095
// Compiler adds -fm suffix to a function name which has a receiver
10981096
// Note that this works even if struct pointer used to get the function is nil
@@ -1101,7 +1099,7 @@ func getFunctionName(i interface{}) string {
11011099
// var a *Activities
11021100
// ExecuteActivity(ctx, a.Foo)
11031101
// will call this function which is going to return "Foo"
1104-
return strings.TrimSuffix(shortName, "-fm")
1102+
return strings.TrimSuffix(fullName, "-fm")
11051103
}
11061104

11071105
func getActivityFunctionName(r *registry, i interface{}) string {

internal/internal_worker_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func (s *internalWorkerTestSuite) TestReplayWorkflowHistory() {
219219
taskList := "taskList1"
220220
testEvents := []*shared.HistoryEvent{
221221
createTestEventWorkflowExecutionStarted(1, &shared.WorkflowExecutionStartedEventAttributes{
222-
WorkflowType: &shared.WorkflowType{Name: common.StringPtr("testReplayWorkflow")},
222+
WorkflowType: &shared.WorkflowType{Name: common.StringPtr("go.uber.org/cadence/internal.testReplayWorkflow")},
223223
TaskList: &shared.TaskList{Name: common.StringPtr(taskList)},
224224
Input: testEncodeFunctionArgs(getDefaultDataConverter()),
225225
}),
@@ -261,7 +261,7 @@ func (s *internalWorkerTestSuite) TestReplayWorkflowHistory_LocalActivity() {
261261
taskList := "taskList1"
262262
testEvents := []*shared.HistoryEvent{
263263
createTestEventWorkflowExecutionStarted(1, &shared.WorkflowExecutionStartedEventAttributes{
264-
WorkflowType: &shared.WorkflowType{Name: common.StringPtr("testReplayWorkflowLocalActivity")},
264+
WorkflowType: &shared.WorkflowType{Name: common.StringPtr("go.uber.org/cadence/internal.testReplayWorkflowLocalActivity")},
265265
TaskList: &shared.TaskList{Name: common.StringPtr(taskList)},
266266
Input: testEncodeFunctionArgs(getDefaultDataConverter()),
267267
}),
@@ -379,7 +379,7 @@ func (s *internalWorkerTestSuite) testDecisionTaskHandlerHelper(params workerExe
379379
createTestEventDecisionTaskStarted(3),
380380
}
381381

382-
workflowType := "testReplayWorkflow"
382+
workflowType := "go.uber.org/cadence/internal.testReplayWorkflow"
383383
workflowID := "testID"
384384
runID := "testRunID"
385385

@@ -799,30 +799,30 @@ func (w activitiesCallingOptionsWorkflow) Execute(ctx Context, input []byte) (re
799799
require.True(w.t, **rStruct2Ptr == testActivityResult{Index: 10})
800800

801801
// By names.
802-
err = ExecuteActivity(ctx, "testActivityByteArgs", input).Get(ctx, nil)
802+
err = ExecuteActivity(ctx, "go.uber.org/cadence/internal.testActivityByteArgs", input).Get(ctx, nil)
803803
require.NoError(w.t, err, err)
804804

805805
err = ExecuteActivity(ctx, "testActivityMultipleArgs", 2, []string{"test"}, true).Get(ctx, nil)
806806
require.NoError(w.t, err, err)
807807

808-
err = ExecuteActivity(ctx, "testActivityNoResult", 2, "test").Get(ctx, nil)
808+
err = ExecuteActivity(ctx, "go.uber.org/cadence/internal.testActivityNoResult", 2, "test").Get(ctx, nil)
809809
require.NoError(w.t, err, err)
810810

811-
err = ExecuteActivity(ctx, "testActivityNoContextArg", 2, "test").Get(ctx, nil)
811+
err = ExecuteActivity(ctx, "go.uber.org/cadence/internal.testActivityNoContextArg", 2, "test").Get(ctx, nil)
812812
require.NoError(w.t, err, err)
813813

814-
f = ExecuteActivity(ctx, "testActivityReturnString")
814+
f = ExecuteActivity(ctx, "go.uber.org/cadence/internal.testActivityReturnString")
815815
err = f.Get(ctx, &rString)
816816
require.NoError(w.t, err, err)
817817
require.Equal(w.t, "testActivity", rString, rString)
818818

819-
f = ExecuteActivity(ctx, "testActivityReturnEmptyString")
819+
f = ExecuteActivity(ctx, "go.uber.org/cadence/internal.testActivityReturnEmptyString")
820820
var r2sString string
821821
err = f.Get(ctx, &r2String)
822822
require.NoError(w.t, err, err)
823823
require.Equal(w.t, "", r2sString)
824824

825-
f = ExecuteActivity(ctx, "testActivityReturnEmptyStruct")
825+
f = ExecuteActivity(ctx, "go.uber.org/cadence/internal.testActivityReturnEmptyStruct")
826826
err = f.Get(ctx, &r2Struct)
827827
require.NoError(w.t, err, err)
828828
require.Equal(w.t, testActivityResult{}, r2Struct)

internal/internal_workflow_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ func returnPanicWorkflow(ctx Context) (err error) {
166166

167167
func (s *WorkflowUnitTest) Test_SplitJoinActivityWorkflow() {
168168
env := s.NewTestWorkflowEnvironment()
169+
env.RegisterWorkflowWithOptions(splitJoinActivityWorkflow, RegisterWorkflowOptions{Name: "splitJoinActivityWorkflow"})
169170
env.RegisterActivityWithOptions(testAct, RegisterActivityOptions{Name: "testActivityWithOptions"})
170171
env.OnActivity(testAct, mock.Anything).Return(func(ctx context.Context) (string, error) {
171172
activityID := GetActivityInfo(ctx).ActivityID

internal/internal_workflow_testsuite_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func (s *WorkflowTestSuiteUnitTest) Test_OnActivityStartedListener() {
163163

164164
env := s.NewTestWorkflowEnvironment()
165165
env.RegisterWorkflow(workflowFn)
166-
env.RegisterActivity(testActivityHello)
166+
env.RegisterActivityWithOptions(testActivityHello, RegisterActivityOptions{Name: "testActivityHello"})
167167

168168
var activityCalls []string
169169
env.SetOnActivityStartedListener(func(activityInfo *ActivityInfo, ctx context.Context, args Values) {
@@ -745,7 +745,7 @@ func (s *WorkflowTestSuiteUnitTest) Test_ChildWorkflow_Mock_Panic_GetChildWorkfl
745745
}
746746

747747
env := s.NewTestWorkflowEnvironment()
748-
env.RegisterWorkflow(testWorkflowHello)
748+
env.RegisterWorkflowWithOptions(testWorkflowHello, RegisterWorkflowOptions{Name: "testWorkflowHello"})
749749
env.RegisterWorkflow(workflowFn)
750750
env.OnWorkflow(testWorkflowHello, mock.Anything, mock.Anything, mock.Anything).
751751
Return("mock_result", nil, "extra_argument") // extra arg causes panic
@@ -803,7 +803,7 @@ func (s *WorkflowTestSuiteUnitTest) Test_ChildWorkflow_Listener() {
803803

804804
env := s.NewTestWorkflowEnvironment()
805805
env.RegisterWorkflow(workflowFn)
806-
env.RegisterWorkflow(testWorkflowHello)
806+
env.RegisterWorkflowWithOptions(testWorkflowHello, RegisterWorkflowOptions{Name: "testWorkflowHello"})
807807
env.RegisterActivity(testActivityHello)
808808

809809
var childWorkflowName, childWorkflowResult string
@@ -1524,7 +1524,7 @@ func (s *WorkflowTestSuiteUnitTest) Test_WorkflowFriendlyName() {
15241524

15251525
env := s.NewTestWorkflowEnvironment()
15261526
env.RegisterWorkflow(workflowFn)
1527-
env.RegisterWorkflow(testWorkflowHello)
1527+
env.RegisterWorkflowWithOptions(testWorkflowHello, RegisterWorkflowOptions{Name: "testWorkflowHello"})
15281528
env.RegisterActivity(testActivityHello)
15291529
var called []string
15301530
env.SetOnChildWorkflowStartedListener(func(workflowInfo *WorkflowInfo, ctx Context, args Values) {

internal/registry.go

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ func (r *registry) RegisterWorkflowWithOptions(
8080
fnName := getFunctionName(wf)
8181
alias := options.Name
8282
registerName := fnName
83+
84+
if options.EnableShortName {
85+
registerName = getShortFunctionName(fnName)
86+
}
8387
if len(alias) > 0 {
8488
registerName = alias
8589
}
@@ -93,8 +97,8 @@ func (r *registry) RegisterWorkflowWithOptions(
9397
}
9498
}
9599
r.workflowFuncMap[registerName] = wf
96-
if len(alias) > 0 {
97-
r.workflowAliasMap[fnName] = alias
100+
if len(alias) > 0 || options.EnableShortName {
101+
r.workflowAliasMap[fnName] = registerName
98102
}
99103
}
100104

@@ -125,6 +129,10 @@ func (r *registry) registerActivityFunction(af interface{}, options RegisterActi
125129
fnName := getFunctionName(af)
126130
alias := options.Name
127131
registerName := fnName
132+
133+
if options.EnableShortName {
134+
registerName = getShortFunctionName(fnName)
135+
}
128136
if len(alias) > 0 {
129137
registerName = alias
130138
}
@@ -138,8 +146,8 @@ func (r *registry) registerActivityFunction(af interface{}, options RegisterActi
138146
}
139147
}
140148
r.activityFuncMap[registerName] = &activityExecutor{registerName, af}
141-
if len(alias) > 0 {
142-
r.activityAliasMap[fnName] = alias
149+
if len(alias) > 0 || options.EnableShortName {
150+
r.activityAliasMap[fnName] = registerName
143151
}
144152

145153
return nil
@@ -159,19 +167,28 @@ func (r *registry) registerActivityStruct(aStruct interface{}, options RegisterA
159167
if method.PkgPath != "" {
160168
continue
161169
}
162-
methodName := method.Name
163-
structPrefix := options.Name
170+
methodName := getFunctionName(method.Func.Interface())
164171
if err := validateFnFormat(method.Type, false); err != nil {
165172
return fmt.Errorf("failed to register activity method %v of %v: %e", methodName, structType.Name(), err)
166173
}
167-
registerName := structPrefix + methodName
174+
175+
structPrefix := options.Name
176+
registerName := methodName
177+
178+
if options.EnableShortName {
179+
registerName = getShortFunctionName(methodName)
180+
}
181+
if len(structPrefix) > 0 {
182+
registerName = structPrefix + getShortFunctionName(methodName)
183+
}
184+
168185
if !options.DisableAlreadyRegisteredCheck {
169186
if _, ok := r.getActivityNoLock(registerName); ok {
170187
return fmt.Errorf("activity type \"%v\" is already registered", registerName)
171188
}
172189
}
173190
r.activityFuncMap[registerName] = &activityExecutor{registerName, methodValue.Interface()}
174-
if len(structPrefix) > 0 {
191+
if len(structPrefix) > 0 || options.EnableShortName {
175192
r.activityAliasMap[methodName] = registerName
176193
}
177194
count++
@@ -184,6 +201,11 @@ func (r *registry) registerActivityStruct(aStruct interface{}, options RegisterA
184201
return nil
185202
}
186203

204+
func getShortFunctionName(fnName string) string {
205+
elements := strings.Split(fnName, ".")
206+
return elements[len(elements)-1]
207+
}
208+
187209
func (r *registry) getWorkflowAlias(fnName string) (string, bool) {
188210
r.Lock() // do not defer for Unlock to call next.getWorkflowAlias without lock
189211
alias, ok := r.workflowAliasMap[fnName]

0 commit comments

Comments
 (0)