From 65840cc8817613a74ded524b0d746c796293873b Mon Sep 17 00:00:00 2001
From: Vytautas <vytautas@uber.com>
Date: Fri, 24 Jul 2020 10:49:17 +0300
Subject: [PATCH] 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.
---
 internal/registry.go                        |  6 +++++
 internal/registry_test.go                   | 25 +++++++++++++++++++++
 test/fixtures/activity.cancel.sm.repro.json |  2 +-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/internal/registry.go b/internal/registry.go
index 911de3c01..a09ec275a 100644
--- a/internal/registry.go
+++ b/internal/registry.go
@@ -220,6 +220,9 @@ func (r *registry) getWorkflowAlias(fnName string) (string, bool) {
 func (r *registry) getWorkflowFn(fnName string) (interface{}, bool) {
 	r.Lock() // do not defer for Unlock to call next.getWorkflowFn without lock
 	fn, ok := r.workflowFuncMap[fnName]
+	if !ok { // if exact match is not found, check for backwards compatible name without -fm suffix
+		fn, ok = r.workflowFuncMap[strings.TrimSuffix(fnName, "-fm")]
+	}
 	if !ok && r.next != nil {
 		r.Unlock()
 		return r.next.getWorkflowFn(fnName)
@@ -263,6 +266,9 @@ func (r *registry) addActivityWithLock(fnName string, a activity) {
 func (r *registry) GetActivity(fnName string) (activity, bool) {
 	r.Lock() // do not defer for Unlock to call next.GetActivity without lock
 	a, ok := r.activityFuncMap[fnName]
+	if !ok { // if exact match is not found, check for backwards compatible name without -fm suffix
+		a, ok = r.activityFuncMap[strings.TrimSuffix(fnName, "-fm")]
+	}
 	if !ok && r.next != nil {
 		r.Unlock()
 		return r.next.GetActivity(fnName)
diff --git a/internal/registry_test.go b/internal/registry_test.go
index 2f296f1c5..e1376f6d6 100644
--- a/internal/registry_test.go
+++ b/internal/registry_test.go
@@ -27,10 +27,12 @@ import (
 )
 
 func TestWorkflowRegistration(t *testing.T) {
+	w := &testWorkflowStruct{}
 	tests := []struct {
 		msg               string
 		register          func(r *registry)
 		workflowType      string
+		altWorkflowType   string
 		resolveByFunction interface{}
 		resolveByAlias    string
 	}{
@@ -57,6 +59,13 @@ func TestWorkflowRegistration(t *testing.T) {
 			resolveByFunction: testWorkflowFunction,
 			resolveByAlias:    "workflow.alias",
 		},
+		{
+			msg:               "register workflow struct function (backwards compatible)",
+			register:          func(r *registry) { r.RegisterWorkflow(w.Method) },
+			workflowType:      "go.uber.org/cadence/internal.(*testWorkflowStruct).Method",
+			altWorkflowType:   "go.uber.org/cadence/internal.(*testWorkflowStruct).Method-fm",
+			resolveByFunction: w.Method,
+		},
 	}
 
 	for _, tt := range tests {
@@ -72,6 +81,12 @@ func TestWorkflowRegistration(t *testing.T) {
 			_, ok := r.getWorkflowFn(tt.workflowType)
 			require.True(t, ok)
 
+			// Verify workflow is resolved from alternative (backwards compatible) workflow type
+			if len(tt.altWorkflowType) > 0 {
+				_, ok = r.getWorkflowFn(tt.altWorkflowType)
+				require.True(t, ok)
+			}
+
 			// Verify resolving by function reference
 			workflowType = getWorkflowFunctionName(r, tt.resolveByFunction)
 			require.Equal(t, tt.workflowType, workflowType)
@@ -90,6 +105,7 @@ func TestActivityRegistration(t *testing.T) {
 		msg               string
 		register          func(r *registry)
 		activityType      string
+		altActivityType   string
 		resolveByFunction interface{}
 		resolveByAlias    string
 	}{
@@ -120,6 +136,7 @@ func TestActivityRegistration(t *testing.T) {
 			msg:               "register activity struct",
 			register:          func(r *registry) { r.RegisterActivity(&testActivityStruct{}) },
 			activityType:      "go.uber.org/cadence/internal.(*testActivityStruct).Method",
+			altActivityType:   "go.uber.org/cadence/internal.(*testActivityStruct).Method-fm",
 			resolveByFunction: (&testActivityStruct{}).Method,
 		},
 		{
@@ -153,6 +170,12 @@ func TestActivityRegistration(t *testing.T) {
 			_, ok := r.GetActivity(tt.activityType)
 			require.True(t, ok)
 
+			// Verify activity is resolved from alternative (backwards compatible) activity type
+			if len(tt.altActivityType) > 0 {
+				_, ok = r.GetActivity(tt.altActivityType)
+				require.True(t, ok)
+			}
+
 			// Verify resolving by function reference
 			activityType = getActivityFunctionName(r, tt.resolveByFunction)
 			require.Equal(t, tt.activityType, activityType, "resolve by function reference")
@@ -166,8 +189,10 @@ func TestActivityRegistration(t *testing.T) {
 	}
 }
 
+type testWorkflowStruct struct{}
 type testActivityStruct struct{}
 
+func (ts *testWorkflowStruct) Method(ctx Context) error { return nil }
 func (ts *testActivityStruct) Method() error { return nil }
 
 func testActivityFunction() error            { return nil }
diff --git a/test/fixtures/activity.cancel.sm.repro.json b/test/fixtures/activity.cancel.sm.repro.json
index d5c391241..e15a17d35 100755
--- a/test/fixtures/activity.cancel.sm.repro.json
+++ b/test/fixtures/activity.cancel.sm.repro.json
@@ -6,7 +6,7 @@
     "version": -24,
     "workflowExecutionStartedEventAttributes": {
       "workflowType": {
-        "name": "go.uber.org/cadence/test.(*Workflows).ActivityCancelRepro"
+        "name": "go.uber.org/cadence/test.(*Workflows).ActivityCancelRepro-fm"
       },
       "taskList": {
         "name": "tl-1"