fix: correct cluster attributes being set when both legacy and new are passed in

common/domain/clusterattributes.go

@@ -4,7 +4,7 @@ import (
 	"github.com/uber/cadence/common/types"
 )
 
-func mergeActiveActiveScopes(existingDomain *types.ActiveClusters, incomingTask *types.ActiveClusters) (*types.ActiveClusters, bool) {
+func mergeActiveActiveScopes(existingDomain *types.ActiveClusters, incomingTask *types.ActiveClusters) (result *types.ActiveClusters, isChanged bool) {
 
 	if existingDomain == nil && incomingTask == nil {
 		return nil, false
@@ -20,7 +20,6 @@ func mergeActiveActiveScopes(existingDomain *types.ActiveClusters, incomingTask
 		return existingDomain, false
 	}
 
-	isChanged := false
 	mergedActiveClusters := &types.ActiveClusters{
 		AttributeScopes: make(map[string]types.ClusterAttributeScope),
 	}
@@ -40,6 +39,7 @@ func mergeActiveActiveScopes(existingDomain *types.ActiveClusters, incomingTask
 		_, existsAlready := mergedActiveClusters.AttributeScopes[newScope]
 		if !existsAlready {
 			mergedActiveClusters.AttributeScopes[newScope] = newScopeData
+			isChanged = true
 		}
 	}
 

common/domain/handler.go

@@ -1872,7 +1872,6 @@ func NewFailoverEvent(
 }
 
 func (d *handlerImpl) buildActiveActiveClusterScopesFromUpdateRequest(updateRequest *types.UpdateDomainRequest, config *persistence.DomainReplicationConfig, domainName string) (out *types.ActiveClusters, isChanged bool) {
-
 	var existing *types.ActiveClusters
 	if config != nil && config.ActiveClusters != nil {
 		existing = config.ActiveClusters

common/domain/handler_test.go

@@ -3673,6 +3673,130 @@ func TestBuildActiveActiveClustersFromUpdateRequest(t *testing.T) {
 	}
 }
 
+func TestBuildActiveActiveClustersFromUpdateRequestMissiongUpdateRepro(t *testing.T) {
+
+	testsCases := map[string]struct {
+		updateRequest          *types.UpdateDomainRequest
+		config                 *persistence.DomainReplicationConfig
+		domainName             string
+		handler                *handlerImpl
+		expectedActiveClusters *types.ActiveClusters
+		expectedIsChanged      bool
+	}{
+		"When both the ActiveClustersByRegion and AttributeScopes are being updated, but the updateScopes don't exist, the function should correctly return that there is a change being made": {
+			updateRequest: &types.UpdateDomainRequest{
+				ActiveClusters: &types.ActiveClusters{
+					ActiveClustersByRegion: map[string]types.ActiveClusterInfo{
+						"region0": {
+							ActiveClusterName: "cluster1",
+							FailoverVersion:   0,
+						},
+						"region1": {
+							ActiveClusterName: "cluster1",
+							FailoverVersion:   0,
+						},
+					},
+					AttributeScopes: map[string]types.ClusterAttributeScope{
+						"region": {
+							ClusterAttributes: map[string]types.ActiveClusterInfo{
+								"region0": {
+									ActiveClusterName: "cluster1",
+								},
+							},
+						},
+					},
+				},
+			},
+			config: &persistence.DomainReplicationConfig{
+				ActiveClusters: &types.ActiveClusters{
+					ActiveClustersByRegion: map[string]types.ActiveClusterInfo{
+						"region0": {
+							ActiveClusterName: "cluster1",
+							FailoverVersion:   2,
+						},
+						"region1": {
+							ActiveClusterName: "cluster1",
+							FailoverVersion:   2,
+						},
+					},
+					AttributeScopes: map[string]types.ClusterAttributeScope{},
+				},
+			},
+			expectedActiveClusters: &types.ActiveClusters{
+				// these are ignored in this function because they're updated on
+				// handler.updateReplicationConfig
+				//
+				// ActiveClustersByRegion: map[string]types.ActiveClusterInfo{
+				// 	"region0": {
+				// 		ActiveClusterName: "cluster1",
+				// 		FailoverVersion:   0,
+				// 	},
+				// 	"region1": {
+				// 		ActiveClusterName: "cluster1",
+				// 		FailoverVersion:   0,
+				// 	},
+				// },
+				AttributeScopes: map[string]types.ClusterAttributeScope{
+					"region": {
+						ClusterAttributes: map[string]types.ActiveClusterInfo{
+							"region0": {
+								ActiveClusterName: "cluster1",
+								FailoverVersion:   2,
+							},
+							// not defined in the test
+							// "region1": {
+							// 	ActiveClusterName: "cluster1",
+							// 	FailoverVersion:   0,
+							// },
+						},
+					},
+				},
+			},
+			expectedIsChanged: true,
+		},
+	}
+
+	for name, tc := range testsCases {
+		t.Run(name, func(t *testing.T) {
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockDomainManager := persistence.NewMockDomainManager(ctrl)
+
+			metadata := cluster.NewMetadata(
+				config.ClusterGroupMetadata{
+					FailoverVersionIncrement: 100,
+					ClusterGroup: map[string]config.ClusterInformation{
+						"cluster0": {
+							InitialFailoverVersion: 0,
+						},
+						"cluster1": {
+							InitialFailoverVersion: 2,
+						},
+					},
+				},
+				func(d string) bool { return false },
+				metrics.NewNoopMetricsClient(),
+				log.NewNoop(),
+			)
+
+			mockTimeSource := clock.NewMockedTimeSource()
+			handler := handlerImpl{
+				domainManager:    mockDomainManager,
+				clusterMetadata:  metadata,
+				archiverProvider: provider.NewArchiverProvider(nil, nil),
+				timeSource:       mockTimeSource,
+				logger:           log.NewNoop(),
+			}
+
+			activeClusters, isChanged := handler.buildActiveActiveClusterScopesFromUpdateRequest(tc.updateRequest, tc.config, tc.domainName)
+			assert.Equal(t, tc.expectedActiveClusters, activeClusters, "expected active clusters: %+v, actual active clusters: %+v", tc.expectedActiveClusters, activeClusters)
+			assert.Equal(t, tc.expectedIsChanged, isChanged, "expected is changed: %+v, actual is changed: %+v", tc.expectedIsChanged, isChanged)
+		})
+	}
+}
+
 func TestActiveClustersFromRegisterRequest(t *testing.T) {
 	tests := []struct {
 		name            string