Skip to content

Commit 07fb281

Browse files
Fix registry to resolve functions with -fm suffix (#1012)
With recent changes -fm suffix was dropped from function names. This caused errors when resolving existing scheduled workflows and activities. With this fix we check for exact match and then check for registered function without -fm suffix for backwards compatibility.
1 parent 73287b6 commit 07fb281

File tree

3 files changed

+32
-1
lines changed

3 files changed

+32
-1
lines changed

internal/registry.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,9 @@ func (r *registry) getWorkflowAlias(fnName string) (string, bool) {
220220
func (r *registry) getWorkflowFn(fnName string) (interface{}, bool) {
221221
r.Lock() // do not defer for Unlock to call next.getWorkflowFn without lock
222222
fn, ok := r.workflowFuncMap[fnName]
223+
if !ok { // if exact match is not found, check for backwards compatible name without -fm suffix
224+
fn, ok = r.workflowFuncMap[strings.TrimSuffix(fnName, "-fm")]
225+
}
223226
if !ok && r.next != nil {
224227
r.Unlock()
225228
return r.next.getWorkflowFn(fnName)
@@ -263,6 +266,9 @@ func (r *registry) addActivityWithLock(fnName string, a activity) {
263266
func (r *registry) GetActivity(fnName string) (activity, bool) {
264267
r.Lock() // do not defer for Unlock to call next.GetActivity without lock
265268
a, ok := r.activityFuncMap[fnName]
269+
if !ok { // if exact match is not found, check for backwards compatible name without -fm suffix
270+
a, ok = r.activityFuncMap[strings.TrimSuffix(fnName, "-fm")]
271+
}
266272
if !ok && r.next != nil {
267273
r.Unlock()
268274
return r.next.GetActivity(fnName)

internal/registry_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@ import (
2727
)
2828

2929
func TestWorkflowRegistration(t *testing.T) {
30+
w := &testWorkflowStruct{}
3031
tests := []struct {
3132
msg string
3233
register func(r *registry)
3334
workflowType string
35+
altWorkflowType string
3436
resolveByFunction interface{}
3537
resolveByAlias string
3638
}{
@@ -57,6 +59,13 @@ func TestWorkflowRegistration(t *testing.T) {
5759
resolveByFunction: testWorkflowFunction,
5860
resolveByAlias: "workflow.alias",
5961
},
62+
{
63+
msg: "register workflow struct function (backwards compatible)",
64+
register: func(r *registry) { r.RegisterWorkflow(w.Method) },
65+
workflowType: "go.uber.org/cadence/internal.(*testWorkflowStruct).Method",
66+
altWorkflowType: "go.uber.org/cadence/internal.(*testWorkflowStruct).Method-fm",
67+
resolveByFunction: w.Method,
68+
},
6069
}
6170

6271
for _, tt := range tests {
@@ -72,6 +81,12 @@ func TestWorkflowRegistration(t *testing.T) {
7281
_, ok := r.getWorkflowFn(tt.workflowType)
7382
require.True(t, ok)
7483

84+
// Verify workflow is resolved from alternative (backwards compatible) workflow type
85+
if len(tt.altWorkflowType) > 0 {
86+
_, ok = r.getWorkflowFn(tt.altWorkflowType)
87+
require.True(t, ok)
88+
}
89+
7590
// Verify resolving by function reference
7691
workflowType = getWorkflowFunctionName(r, tt.resolveByFunction)
7792
require.Equal(t, tt.workflowType, workflowType)
@@ -90,6 +105,7 @@ func TestActivityRegistration(t *testing.T) {
90105
msg string
91106
register func(r *registry)
92107
activityType string
108+
altActivityType string
93109
resolveByFunction interface{}
94110
resolveByAlias string
95111
}{
@@ -120,6 +136,7 @@ func TestActivityRegistration(t *testing.T) {
120136
msg: "register activity struct",
121137
register: func(r *registry) { r.RegisterActivity(&testActivityStruct{}) },
122138
activityType: "go.uber.org/cadence/internal.(*testActivityStruct).Method",
139+
altActivityType: "go.uber.org/cadence/internal.(*testActivityStruct).Method-fm",
123140
resolveByFunction: (&testActivityStruct{}).Method,
124141
},
125142
{
@@ -153,6 +170,12 @@ func TestActivityRegistration(t *testing.T) {
153170
_, ok := r.GetActivity(tt.activityType)
154171
require.True(t, ok)
155172

173+
// Verify activity is resolved from alternative (backwards compatible) activity type
174+
if len(tt.altActivityType) > 0 {
175+
_, ok = r.GetActivity(tt.altActivityType)
176+
require.True(t, ok)
177+
}
178+
156179
// Verify resolving by function reference
157180
activityType = getActivityFunctionName(r, tt.resolveByFunction)
158181
require.Equal(t, tt.activityType, activityType, "resolve by function reference")
@@ -166,8 +189,10 @@ func TestActivityRegistration(t *testing.T) {
166189
}
167190
}
168191

192+
type testWorkflowStruct struct{}
169193
type testActivityStruct struct{}
170194

195+
func (ts *testWorkflowStruct) Method(ctx Context) error { return nil }
171196
func (ts *testActivityStruct) Method() error { return nil }
172197

173198
func testActivityFunction() error { return nil }

test/fixtures/activity.cancel.sm.repro.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"version": -24,
77
"workflowExecutionStartedEventAttributes": {
88
"workflowType": {
9-
"name": "go.uber.org/cadence/test.(*Workflows).ActivityCancelRepro"
9+
"name": "go.uber.org/cadence/test.(*Workflows).ActivityCancelRepro-fm"
1010
},
1111
"taskList": {
1212
"name": "tl-1"

0 commit comments

Comments
 (0)