From e9403f69ef9405cce64f3a54d3c31fa9e6041eeb Mon Sep 17 00:00:00 2001 From: David Porter Date: Mon, 24 Nov 2025 14:25:05 -0800 Subject: [PATCH] fix missing fields Signed-off-by: David Porter --- internal/compatibility/api_test.go | 140 ++++++++++++++++++++- internal/compatibility/proto/request.go | 12 +- internal/compatibility/proto/response.go | 32 +++++ internal/compatibility/testdata/service.go | 8 ++ internal/compatibility/thrift/request.go | 16 +++ 5 files changed, 204 insertions(+), 4 deletions(-) diff --git a/internal/compatibility/api_test.go b/internal/compatibility/api_test.go index 476d387d5..237d587f5 100644 --- a/internal/compatibility/api_test.go +++ b/internal/compatibility/api_test.go @@ -476,6 +476,137 @@ func TestDeprecateDomainRequest(t *testing.T) { FuzzOptions{}, ) } + +func TestFailoverDomainRequest(t *testing.T) { + // Test complete field mapping - all fields should now be properly mapped + t.Run("CompleteFieldMapping", func(t *testing.T) { + // Create a thrift FailoverDomainRequest with all fields + domainName := testdata.DomainName + clusterName := testdata.ClusterName1 + thriftRequest := &shared.FailoverDomainRequest{ + DomainName: &domainName, + DomainActiveClusterName: &clusterName, + ActiveClusters: thrift.ActiveClusters(testdata.ActiveClusters), + } + + // Convert to proto + protoRequest := proto.FailoverDomainRequest(thriftRequest) + + // Verify that all fields are mapped correctly + assert.Equal(t, testdata.DomainName, protoRequest.DomainName) + assert.Equal(t, testdata.ClusterName1, protoRequest.DomainActiveClusterName) + assert.NotNil(t, protoRequest.ActiveClusters, "ActiveClusters field should be mapped") + + // Test round-trip conversion + convertedBack := thrift.FailoverDomainRequest(protoRequest) + assert.NotNil(t, convertedBack) + assert.Equal(t, testdata.DomainName, convertedBack.GetDomainName()) + assert.Equal(t, testdata.ClusterName1, convertedBack.GetDomainActiveClusterName()) + assert.NotNil(t, convertedBack.ActiveClusters, "ActiveClusters should survive round-trip conversion") + }) + + // Test bidirectional conversion with standard test pattern + t.Run("BidirectionalConversion", func(t *testing.T) { + for _, item := range []*apiv1.FailoverDomainRequest{nil, {}, &testdata.FailoverDomainRequest} { + assert.Equal(t, item, proto.FailoverDomainRequest(thrift.FailoverDomainRequest(item))) + } + }) + + // Test with nil and empty cases + t.Run("NilAndEmptyCases", func(t *testing.T) { + assert.Nil(t, proto.FailoverDomainRequest(nil)) + assert.Nil(t, thrift.FailoverDomainRequest(nil)) + + emptyRequest := &shared.FailoverDomainRequest{} + converted := proto.FailoverDomainRequest(emptyRequest) + assert.NotNil(t, converted) + assert.Equal(t, "", converted.DomainName) + assert.Equal(t, "", converted.DomainActiveClusterName) + assert.Nil(t, converted.ActiveClusters) + }) + + // Fuzz test to ensure robustness + runFuzzTest(t, + thrift.FailoverDomainRequest, + proto.FailoverDomainRequest, + FuzzOptions{ + ExcludedFields: []string{ + "RegionToCluster", // [DEPRECATED] This field is deprecated and not mapped in conversion functions + }, + }, + ) +} + +func TestFailoverDomainResponse(t *testing.T) { + // Test complete bidirectional conversion + t.Run("BidirectionalConversion", func(t *testing.T) { + // Test nil case + assert.Nil(t, proto.FailoverDomainResponse(thrift.FailoverDomainResponse(nil))) + + // Test empty case - empty response should create a response with nil domain + emptyResponse := &apiv1.FailoverDomainResponse{} + converted := proto.FailoverDomainResponse(thrift.FailoverDomainResponse(emptyResponse)) + assert.Nil(t, converted) // thrift.FailoverDomainResponse returns nil for empty response + + // Test full response + fullResponse := &testdata.FailoverDomainResponse + assert.Equal(t, fullResponse, proto.FailoverDomainResponse(thrift.FailoverDomainResponse(fullResponse))) + }) + + // Test that both conversion directions work + t.Run("ConversionDirections", func(t *testing.T) { + // Test proto -> thrift + thriftResponse := thrift.FailoverDomainResponse(&testdata.FailoverDomainResponse) + assert.NotNil(t, thriftResponse) + assert.NotNil(t, thriftResponse.DomainInfo) + assert.Equal(t, testdata.DomainName, *thriftResponse.DomainInfo.Name) + + // Test thrift -> proto + protoResponse := proto.FailoverDomainResponse(thriftResponse) + assert.NotNil(t, protoResponse) + assert.NotNil(t, protoResponse.Domain) + assert.Equal(t, testdata.DomainName, protoResponse.Domain.Name) + }) + + // Fuzz test for robustness + runFuzzTest(t, + thrift.FailoverDomainResponse, + proto.FailoverDomainResponse, + FuzzOptions{ + CustomFuncs: []interface{}{ + func(status *apiv1.DomainStatus, c fuzz.Continue) { + validValues := []apiv1.DomainStatus{ + apiv1.DomainStatus_DOMAIN_STATUS_INVALID, + apiv1.DomainStatus_DOMAIN_STATUS_REGISTERED, + apiv1.DomainStatus_DOMAIN_STATUS_DEPRECATED, + apiv1.DomainStatus_DOMAIN_STATUS_DELETED, + } + *status = validValues[c.Intn(len(validValues))] + }, + func(status *apiv1.ArchivalStatus, c fuzz.Continue) { + validValues := []apiv1.ArchivalStatus{ + apiv1.ArchivalStatus_ARCHIVAL_STATUS_INVALID, + apiv1.ArchivalStatus_ARCHIVAL_STATUS_DISABLED, + apiv1.ArchivalStatus_ARCHIVAL_STATUS_ENABLED, + } + *status = validValues[c.Intn(len(validValues))] + }, + // WorkflowExecutionRetentionPeriod - must be day-precision + func(domain *apiv1.Domain, c fuzz.Continue) { + if domain.WorkflowExecutionRetentionPeriod != nil { + days := c.Int63n(MaxDurationSeconds / (24 * 3600)) + domain.WorkflowExecutionRetentionPeriod.Seconds = days * 24 * 3600 + domain.WorkflowExecutionRetentionPeriod.Nanos = 0 + } + }, + }, + ExcludedFields: []string{ + "RegionToCluster", // [DEPRECATED] This field is deprecated and not mapped in conversion functions + }, + }, + ) +} + func TestDescribeDomainRequest(t *testing.T) { for _, item := range []*apiv1.DescribeDomainRequest{ &testdata.DescribeDomainRequest_ID, @@ -3047,7 +3178,7 @@ func clearFieldsIf(obj interface{}, shouldClear func(fieldName string) bool) { field.Set(reflect.Zero(field.Type())) } - // Recursively clear fields in nested structs and slices + // Recursively clear fields in nested structs, slices, and maps if field.CanInterface() { switch field.Kind() { case reflect.Ptr: @@ -3063,6 +3194,13 @@ func clearFieldsIf(obj interface{}, shouldClear func(fieldName string) bool) { clearFieldsIf(elem.Interface(), shouldClear) } } + case reflect.Map: + for _, key := range field.MapKeys() { + elem := field.MapIndex(key) + if elem.CanInterface() { + clearFieldsIf(elem.Interface(), shouldClear) + } + } } } } diff --git a/internal/compatibility/proto/request.go b/internal/compatibility/proto/request.go index 819c38554..5c004959a 100644 --- a/internal/compatibility/proto/request.go +++ b/internal/compatibility/proto/request.go @@ -698,10 +698,16 @@ func FailoverDomainRequest(t *shared.FailoverDomainRequest) *apiv1.FailoverDomai if t == nil { return nil } - return &apiv1.FailoverDomainRequest{ - DomainName: t.GetDomainName(), - DomainActiveClusterName: *t.DomainActiveClusterName, + request := &apiv1.FailoverDomainRequest{ + DomainName: t.GetDomainName(), } + if t.DomainActiveClusterName != nil { + request.DomainActiveClusterName = *t.DomainActiveClusterName + } + if t.ActiveClusters != nil { + request.ActiveClusters = ActiveClusters(t.ActiveClusters) + } + return request } func ListFailoverHistoryRequest(t *shared.ListFailoverHistoryRequest) *apiv1.ListFailoverHistoryRequest { diff --git a/internal/compatibility/proto/response.go b/internal/compatibility/proto/response.go index ed202230c..e99aec66f 100644 --- a/internal/compatibility/proto/response.go +++ b/internal/compatibility/proto/response.go @@ -350,6 +350,38 @@ func UpdateDomainResponse(t *shared.UpdateDomainResponse) *apiv1.UpdateDomainRes } } +func FailoverDomainResponse(t *shared.FailoverDomainResponse) *apiv1.FailoverDomainResponse { + if t == nil { + return nil + } + domain := apiv1.Domain{ + FailoverVersion: t.GetFailoverVersion(), + IsGlobalDomain: t.GetIsGlobalDomain(), + } + if info := t.DomainInfo; info != nil { + domain.Id = info.GetUUID() + domain.Name = info.GetName() + domain.Status = DomainStatus(info.Status) + domain.Description = info.GetDescription() + domain.OwnerEmail = info.GetOwnerEmail() + domain.Data = info.Data + } + if config := t.Configuration; config != nil { + domain.WorkflowExecutionRetentionPeriod = daysToDuration(config.WorkflowExecutionRetentionPeriodInDays) + domain.BadBinaries = BadBinaries(config.BadBinaries) + domain.HistoryArchivalStatus = ArchivalStatus(config.HistoryArchivalStatus) + domain.HistoryArchivalUri = config.GetHistoryArchivalURI() + domain.VisibilityArchivalStatus = ArchivalStatus(config.VisibilityArchivalStatus) + domain.VisibilityArchivalUri = config.GetVisibilityArchivalURI() + } + if repl := t.ReplicationConfiguration; repl != nil { + domain.ActiveClusterName = repl.GetActiveClusterName() + domain.Clusters = ClusterReplicationConfigurationArray(repl.Clusters) + domain.ActiveClusters = ActiveClusters(repl.ActiveClusters) + } + return &apiv1.FailoverDomainResponse{Domain: &domain} +} + func ListFailoverHistoryResponse(t *shared.ListFailoverHistoryResponse) *apiv1.ListFailoverHistoryResponse { if t == nil { return nil diff --git a/internal/compatibility/testdata/service.go b/internal/compatibility/testdata/service.go index e0f8a7d28..979cdfdd1 100644 --- a/internal/compatibility/testdata/service.go +++ b/internal/compatibility/testdata/service.go @@ -107,6 +107,14 @@ var ( Name: DomainName, SecurityToken: SecurityToken, } + FailoverDomainRequest = apiv1.FailoverDomainRequest{ + DomainName: DomainName, + DomainActiveClusterName: ClusterName1, + ActiveClusters: ActiveClusters, + } + FailoverDomainResponse = apiv1.FailoverDomainResponse{ + Domain: &Domain, + } ListWorkflowExecutionsRequest = apiv1.ListWorkflowExecutionsRequest{ Domain: DomainName, PageSize: PageSize, diff --git a/internal/compatibility/thrift/request.go b/internal/compatibility/thrift/request.go index b0f9a6142..f75a3f592 100644 --- a/internal/compatibility/thrift/request.go +++ b/internal/compatibility/thrift/request.go @@ -604,6 +604,22 @@ func ListOpenWorkflowExecutionsRequest(r *apiv1.ListOpenWorkflowExecutionsReques } } +func FailoverDomainRequest(t *apiv1.FailoverDomainRequest) *shared.FailoverDomainRequest { + if t == nil { + return nil + } + request := &shared.FailoverDomainRequest{ + DomainName: &t.DomainName, + } + if t.DomainActiveClusterName != "" { + request.DomainActiveClusterName = &t.DomainActiveClusterName + } + if t.ActiveClusters != nil { + request.ActiveClusters = ActiveClusters(t.ActiveClusters) + } + return request +} + func ListFailoverHistoryRequest(t *apiv1.ListFailoverHistoryRequest) *shared.ListFailoverHistoryRequest { if t == nil { return nil