diff --git a/common/domain/clusterattributes.go b/common/domain/clusterattributes.go index 38b6f34989b..cddc1b8b760 100644 --- a/common/domain/clusterattributes.go +++ b/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 } } diff --git a/common/domain/handler.go b/common/domain/handler.go index 35b1e901949..3a4493f31bd 100644 --- a/common/domain/handler.go +++ b/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 diff --git a/common/domain/handler_test.go b/common/domain/handler_test.go index 83777d0e4ab..922de532bba 100644 --- a/common/domain/handler_test.go +++ b/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