From e8cda72f1f2f434cff719b8878e7a56c52272a3c Mon Sep 17 00:00:00 2001 From: Jakob Haahr Taankvist Date: Thu, 31 Oct 2024 13:14:40 +0100 Subject: [PATCH 1/5] Refactored and tested SerializeForLogging --- common/authorization/authorizer.go | 22 ++ common/types/admin.go | 105 ------- common/types/admin_test.go | 152 +--------- common/types/shared.go | 266 ------------------ common/{types => types_test}/shared_test.go | 164 ++--------- .../frontend/templates/accesscontrolled.tmpl | 2 +- .../accesscontrolled/admin_generated.go | 66 ++--- .../accesscontrolled/api_generated.go | 60 ++-- 8 files changed, 118 insertions(+), 719 deletions(-) rename common/{types => types_test}/shared_test.go (53%) diff --git a/common/authorization/authorizer.go b/common/authorization/authorizer.go index 2796d3e3f36..31849da088c 100644 --- a/common/authorization/authorizer.go +++ b/common/authorization/authorizer.go @@ -115,6 +115,28 @@ type FilteredRequestBody interface { SerializeForLogging() (string, error) } +type simpleRequestLogWrapper struct { + request interface{} +} + +func (f *simpleRequestLogWrapper) SerializeForLogging() (string, error) { + return types.SerializeRequest(f.request) +} + +type nullRequestLogWrapper struct{} + +func (f *nullRequestLogWrapper) SerializeForLogging() (string, error) { + return "", nil +} + +func NewFilteredRequestBody(request interface{}) FilteredRequestBody { + if request == nil { + return &nullRequestLogWrapper{} + } + + return &simpleRequestLogWrapper{request} +} + func validatePermission(claims *JWTClaims, attributes *Attributes, data domainData) error { if (attributes.Permission < PermissionRead) || (attributes.Permission > PermissionAdmin) { return fmt.Errorf("permission %v is not supported", attributes.Permission) diff --git a/common/types/admin.go b/common/types/admin.go index 2e2de36e424..cfeb9d2e3a2 100644 --- a/common/types/admin.go +++ b/common/types/admin.go @@ -28,13 +28,6 @@ type AddSearchAttributeRequest struct { SecurityToken string `json:"securityToken,omitempty"` } -func (v *AddSearchAttributeRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetSearchAttribute is an internal getter (TBD...) func (v *AddSearchAttributeRequest) GetSearchAttribute() (o map[string]IndexedValueType) { if v != nil && v.SearchAttribute != nil { @@ -56,13 +49,6 @@ type AdminDescribeWorkflowExecutionRequest struct { Execution *WorkflowExecution `json:"execution,omitempty"` } -func (v *AdminDescribeWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *AdminDescribeWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -107,13 +93,6 @@ type GetWorkflowExecutionRawHistoryV2Request struct { NextPageToken []byte `json:"nextPageToken,omitempty"` } -func (v *GetWorkflowExecutionRawHistoryV2Request) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *GetWorkflowExecutionRawHistoryV2Request) GetDomain() (o string) { if v != nil { @@ -229,13 +208,6 @@ type ResendReplicationTasksRequest struct { EndVersion *int64 `json:"endVersion,omitempty"` } -func (v *ResendReplicationTasksRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetWorkflowID is an internal getter (TBD...) func (v *ResendReplicationTasksRequest) GetWorkflowID() (o string) { if v != nil { @@ -272,13 +244,6 @@ type GetDynamicConfigRequest struct { Filters []*DynamicConfigFilter `json:"filters,omitempty"` } -func (v *GetDynamicConfigRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type GetDynamicConfigResponse struct { Value *DataBlob `json:"value,omitempty"` } @@ -288,25 +253,11 @@ type UpdateDynamicConfigRequest struct { ConfigValues []*DynamicConfigValue `json:"configValues,omitempty"` } -func (v *UpdateDynamicConfigRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type RestoreDynamicConfigRequest struct { ConfigName string `json:"configName,omitempty"` Filters []*DynamicConfigFilter `json:"filters,omitempty"` } -func (v *RestoreDynamicConfigRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // AdminDeleteWorkflowRequest is an internal type (TBD...) type AdminDeleteWorkflowRequest struct { Domain string `json:"domain,omitempty"` @@ -314,13 +265,6 @@ type AdminDeleteWorkflowRequest struct { SkipErrors bool `json:"skipErrors,omitempty"` } -func (v *AdminDeleteWorkflowRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - func (v *AdminDeleteWorkflowRequest) GetDomain() (o string) { if v != nil { return v.Domain @@ -356,13 +300,6 @@ type ListDynamicConfigRequest struct { ConfigName string `json:"configName,omitempty"` } -func (v *ListDynamicConfigRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type ListDynamicConfigResponse struct { Entries []*DynamicConfigEntry `json:"entries,omitempty"` } @@ -434,13 +371,6 @@ func FromIsolationGroupPartitionList(in []IsolationGroupPartition) IsolationGrou type GetGlobalIsolationGroupsRequest struct{} -func (v *GetGlobalIsolationGroupsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type GetGlobalIsolationGroupsResponse struct { IsolationGroups IsolationGroupConfiguration } @@ -449,26 +379,12 @@ type UpdateGlobalIsolationGroupsRequest struct { IsolationGroups IsolationGroupConfiguration } -func (v *UpdateGlobalIsolationGroupsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type UpdateGlobalIsolationGroupsResponse struct{} type GetDomainIsolationGroupsRequest struct { Domain string } -func (v *GetDomainIsolationGroupsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type GetDomainIsolationGroupsResponse struct { IsolationGroups IsolationGroupConfiguration } @@ -478,26 +394,12 @@ type UpdateDomainIsolationGroupsRequest struct { IsolationGroups IsolationGroupConfiguration } -func (v *UpdateDomainIsolationGroupsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type UpdateDomainIsolationGroupsResponse struct{} type GetDomainAsyncWorkflowConfiguratonRequest struct { Domain string } -func (v *GetDomainAsyncWorkflowConfiguratonRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type GetDomainAsyncWorkflowConfiguratonResponse struct { Configuration *AsyncWorkflowConfiguration } @@ -525,12 +427,5 @@ type UpdateDomainAsyncWorkflowConfiguratonRequest struct { Configuration *AsyncWorkflowConfiguration } -func (v *UpdateDomainAsyncWorkflowConfiguratonRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type UpdateDomainAsyncWorkflowConfiguratonResponse struct { } diff --git a/common/types/admin_test.go b/common/types/admin_test.go index 7b37943ade7..535b8eb5022 100644 --- a/common/types/admin_test.go +++ b/common/types/admin_test.go @@ -24,6 +24,7 @@ package types import ( "testing" + "unsafe" "github.com/stretchr/testify/assert" ) @@ -172,54 +173,6 @@ func TestAsyncWorkflowConfigurationDeepCopy(t *testing.T) { } } -func TestAddSearchAttributeRequest_SerializeForLogging(t *testing.T) { - tests := []struct { - name string - request *AddSearchAttributeRequest - want string - wantErr bool - }{ - { - name: "Nil request", - request: nil, - want: "", - wantErr: false, - }, - { - name: "Empty SearchAttribute", - request: &AddSearchAttributeRequest{ - SearchAttribute: nil, - SecurityToken: "", - }, - want: `{}`, - wantErr: false, - }, - { - name: "With SearchAttribute", - request: &AddSearchAttributeRequest{ - SearchAttribute: map[string]IndexedValueType{ - "attr1": 1, - }, - SecurityToken: "token", - }, - want: `{"searchAttribute":{"attr1":"KEYWORD"},"securityToken":"token"}`, - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := tt.request.SerializeForLogging() - if tt.wantErr { - assert.Error(t, err, "SerializeForLogging() error expected") - } else { - assert.NoError(t, err, "SerializeForLogging() error unexpected") - } - assert.Equal(t, tt.want, got, "SerializeForLogging() result mismatch") - }) - } -} - func TestAddSearchAttributeRequest_GetSearchAttribute(t *testing.T) { tests := []struct { name string @@ -261,52 +214,6 @@ func TestAddSearchAttributeRequest_GetSearchAttribute(t *testing.T) { } } -func TestAdminDescribeWorkflowExecutionRequest_SerializeForLogging(t *testing.T) { - tests := []struct { - name string - request *AdminDescribeWorkflowExecutionRequest - want string - wantErr bool - }{ - { - name: "Nil request", - request: nil, - want: "", - wantErr: false, - }, - { - name: "Empty request", - request: &AdminDescribeWorkflowExecutionRequest{}, - want: `{}`, - wantErr: false, - }, - { - name: "With Domain and Execution", - request: &AdminDescribeWorkflowExecutionRequest{ - Domain: "test-domain", - Execution: &WorkflowExecution{ - WorkflowID: "test-workflow", - RunID: "test-run", - }, - }, - want: `{"domain":"test-domain","execution":{"workflowId":"test-workflow","runId":"test-run"}}`, - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := tt.request.SerializeForLogging() - if tt.wantErr { - assert.Error(t, err, "SerializeForLogging() error expected") - } else { - assert.NoError(t, err, "SerializeForLogging() error unexpected") - } - assert.Equal(t, tt.want, got, "SerializeForLogging() result mismatch") - }) - } -} - func TestAdminDescribeWorkflowExecutionRequest_GetDomain(t *testing.T) { tests := []struct { name string @@ -406,58 +313,6 @@ func TestAdminDescribeWorkflowExecutionResponse_GetMutableStateInDatabase(t *tes } } -func TestGetWorkflowExecutionRawHistoryV2Request_SerializeForLogging(t *testing.T) { - tests := []struct { - name string - request *GetWorkflowExecutionRawHistoryV2Request - want string - wantErr bool - }{ - { - name: "Nil request", - request: nil, - want: "", - wantErr: false, - }, - { - name: "Empty request", - request: &GetWorkflowExecutionRawHistoryV2Request{}, - want: `{}`, - wantErr: false, - }, - { - name: "Full request", - request: &GetWorkflowExecutionRawHistoryV2Request{ - Domain: "test-domain", - Execution: &WorkflowExecution{ - WorkflowID: "workflow-id", - RunID: "run-id", - }, - StartEventID: ptrInt64(100), - StartEventVersion: ptrInt64(1), - EndEventID: ptrInt64(200), - EndEventVersion: ptrInt64(2), - MaximumPageSize: 50, - NextPageToken: []byte("token"), - }, - want: `{"domain":"test-domain","execution":{"workflowId":"workflow-id","runId":"run-id"},"startEventId":100,"startEventVersion":1,"endEventId":200,"endEventVersion":2,"maximumPageSize":50,"nextPageToken":"dG9rZW4="}`, - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := tt.request.SerializeForLogging() - if tt.wantErr { - assert.Error(t, err, "SerializeForLogging() error expected") - } else { - assert.NoError(t, err, "SerializeForLogging() error unexpected") - } - assert.Equal(t, tt.want, got, "SerializeForLogging() result mismatch") - }) - } -} - func TestGetWorkflowExecutionRawHistoryV2Request_GetDomain(t *testing.T) { tests := []struct { name string @@ -764,3 +619,8 @@ func TestGetWorkflowExecutionRawHistoryV2Response_GetVersionHistory(t *testing.T func ptrInt64(i int64) *int64 { return &i } + +// identicalByteArray returns true if a and b are the same slice, false otherwise. +func identicalByteArray(a, b []byte) bool { + return len(a) == len(b) && unsafe.SliceData(a) == unsafe.SliceData(b) +} diff --git a/common/types/shared.go b/common/types/shared.go index f8f4098cc64..b5f691a5af0 100644 --- a/common/types/shared.go +++ b/common/types/shared.go @@ -632,13 +632,6 @@ type CloseShardRequest struct { ShardID int32 `json:"shardID,omitempty"` } -func (v *CloseShardRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetShardID is an internal getter (TBD...) func (v *CloseShardRequest) GetShardID() (o int32) { if v != nil { @@ -802,13 +795,6 @@ type CountWorkflowExecutionsRequest struct { Query string `json:"query,omitempty"` } -func (v *CountWorkflowExecutionsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *CountWorkflowExecutionsRequest) GetDomain() (o string) { if v != nil { @@ -1511,13 +1497,6 @@ type DeprecateDomainRequest struct { SecurityToken string `json:"securityToken,omitempty"` } -func (v *DeprecateDomainRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetName is an internal getter (TBD...) func (v *DeprecateDomainRequest) GetName() (o string) { if v != nil { @@ -1532,13 +1511,6 @@ type DescribeDomainRequest struct { UUID *string `json:"uuid,omitempty"` } -func (v *DescribeDomainRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetName is an internal getter (TBD...) func (v *DescribeDomainRequest) GetName() (o string) { if v != nil && v.Name != nil { @@ -1604,26 +1576,12 @@ type DescribeHistoryHostRequest struct { ExecutionForHost *WorkflowExecution `json:"executionForHost,omitempty"` } -func (v *DescribeHistoryHostRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // DescribeShardDistributionRequest is an internal type (TBD...) type DescribeShardDistributionRequest struct { PageSize int32 `json:"pageSize,omitempty"` PageID int32 `json:"pageID,omitempty"` } -func (v *DescribeShardDistributionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetHostAddress is an internal getter (TBD...) func (v *DescribeHistoryHostRequest) GetHostAddress() (o string) { if v != nil && v.HostAddress != nil { @@ -1662,13 +1620,6 @@ type DescribeQueueRequest struct { Type *int32 `json:"type,omitempty"` } -func (v *DescribeQueueRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetShardID is an internal getter (TBD...) func (v *DescribeQueueRequest) GetShardID() (o int32) { if v != nil { @@ -1706,13 +1657,6 @@ type DescribeTaskListRequest struct { IncludeTaskListStatus bool `json:"includeTaskListStatus,omitempty"` } -func (v *DescribeTaskListRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *DescribeTaskListRequest) GetDomain() (o string) { if v != nil { @@ -1773,13 +1717,6 @@ type DescribeWorkflowExecutionRequest struct { Execution *WorkflowExecution `json:"execution,omitempty"` } -func (v *DescribeWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *DescribeWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -2559,13 +2496,6 @@ type GetWorkflowExecutionHistoryRequest struct { SkipArchival bool `json:"skipArchival,omitempty"` } -func (v *GetWorkflowExecutionHistoryRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *GetWorkflowExecutionHistoryRequest) GetDomain() (o string) { if v != nil { @@ -3291,13 +3221,6 @@ type ListArchivedWorkflowExecutionsRequest struct { Query string `json:"query,omitempty"` } -func (v *ListArchivedWorkflowExecutionsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *ListArchivedWorkflowExecutionsRequest) GetDomain() (o string) { if v != nil { @@ -3347,13 +3270,6 @@ type ListClosedWorkflowExecutionsRequest struct { StatusFilter *WorkflowExecutionCloseStatus `json:"statusFilter,omitempty"` } -func (v *ListClosedWorkflowExecutionsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *ListClosedWorkflowExecutionsRequest) GetDomain() (o string) { if v != nil { @@ -3398,13 +3314,6 @@ type ListDomainsRequest struct { NextPageToken []byte `json:"nextPageToken,omitempty"` } -func (v *ListDomainsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetPageSize is an internal getter (TBD...) func (v *ListDomainsRequest) GetPageSize() (o int32) { if v != nil { @@ -3445,13 +3354,6 @@ type ListOpenWorkflowExecutionsRequest struct { TypeFilter *WorkflowTypeFilter `json:"typeFilter,omitempty"` } -func (v *ListOpenWorkflowExecutionsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *ListOpenWorkflowExecutionsRequest) GetDomain() (o string) { if v != nil { @@ -3488,13 +3390,6 @@ type ListTaskListPartitionsRequest struct { TaskList *TaskList `json:"taskList,omitempty"` } -func (v *ListTaskListPartitionsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *ListTaskListPartitionsRequest) GetDomain() (o string) { if v != nil { @@ -3522,13 +3417,6 @@ type GetTaskListsByDomainRequest struct { Domain string `json:"domain,omitempty"` } -func (v *GetTaskListsByDomainRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *GetTaskListsByDomainRequest) GetDomain() (o string) { if v != nil { @@ -3567,13 +3455,6 @@ type ListWorkflowExecutionsRequest struct { Query string `json:"query,omitempty"` } -func (v *ListWorkflowExecutionsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *ListWorkflowExecutionsRequest) GetDomain() (o string) { if v != nil { @@ -3973,13 +3854,6 @@ type PollForActivityTaskRequest struct { TaskListMetadata *TaskListMetadata `json:"taskListMetadata,omitempty"` } -func (v *PollForActivityTaskRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *PollForActivityTaskRequest) GetDomain() (o string) { if v != nil { @@ -4040,13 +3914,6 @@ type PollForDecisionTaskRequest struct { BinaryChecksum string `json:"binaryChecksum,omitempty"` } -func (v *PollForDecisionTaskRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *PollForDecisionTaskRequest) GetDomain() (o string) { if v != nil { @@ -4408,13 +4275,6 @@ type QueryWorkflowRequest struct { QueryConsistencyLevel *QueryConsistencyLevel `json:"queryConsistencyLevel,omitempty"` } -func (v *QueryWorkflowRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *QueryWorkflowRequest) GetDomain() (o string) { if v != nil { @@ -4484,13 +4344,6 @@ type ReapplyEventsRequest struct { Events *DataBlob `json:"events,omitempty"` } -func (v *ReapplyEventsRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomainName is an internal getter (TBD...) func (v *ReapplyEventsRequest) GetDomainName() (o string) { if v != nil { @@ -4564,13 +4417,6 @@ type RecordActivityTaskHeartbeatRequest struct { Identity string `json:"identity,omitempty"` } -func (v *RecordActivityTaskHeartbeatRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // RecordActivityTaskHeartbeatResponse is an internal type (TBD...) type RecordActivityTaskHeartbeatResponse struct { CancelRequested bool `json:"cancelRequested,omitempty"` @@ -4597,13 +4443,6 @@ type RefreshWorkflowTasksRequest struct { Execution *WorkflowExecution `json:"execution,omitempty"` } -func (v *RefreshWorkflowTasksRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *RefreshWorkflowTasksRequest) GetDomain() (o string) { if v != nil { @@ -4638,13 +4477,6 @@ type RegisterDomainRequest struct { VisibilityArchivalURI string `json:"visibilityArchivalURI,omitempty"` } -func (v *RegisterDomainRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetName is an internal getter (TBD...) func (v *RegisterDomainRequest) GetName() (o string) { if v != nil { @@ -4740,13 +4572,6 @@ type RemoveTaskRequest struct { ClusterName string `json:"clusterName,omitempty"` } -func (v *RemoveTaskRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetShardID is an internal getter (TBD...) func (v *RemoveTaskRequest) GetShardID() (o int32) { if v != nil { @@ -4917,13 +4742,6 @@ type RequestCancelWorkflowExecutionRequest struct { FirstExecutionRunID string `json:"first_execution_run_id,omitempty"` } -func (v *RequestCancelWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *RequestCancelWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -5026,13 +4844,6 @@ type ResetQueueRequest struct { Type *int32 `json:"type,omitempty"` } -func (v *ResetQueueRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetShardID is an internal getter (TBD...) func (v *ResetQueueRequest) GetShardID() (o int32) { if v != nil { @@ -5063,13 +4874,6 @@ type ResetStickyTaskListRequest struct { Execution *WorkflowExecution `json:"execution,omitempty"` } -func (v *ResetStickyTaskListRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *ResetStickyTaskListRequest) GetDomain() (o string) { if v != nil { @@ -5100,13 +4904,6 @@ type ResetWorkflowExecutionRequest struct { SkipSignalReapply bool `json:"skipSignalReapply,omitempty"` } -func (v *ResetWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *ResetWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -5877,13 +5674,6 @@ type SignalWithStartWorkflowExecutionRequest struct { FirstRunAtTimestamp *int64 `json:"firstRunAtTimestamp,omitempty"` } -func (v *SignalWithStartWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *SignalWithStartWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -6000,13 +5790,6 @@ type SignalWorkflowExecutionRequest struct { Control []byte `json:"control,omitempty"` } -func (v *SignalWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *SignalWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -6262,13 +6045,6 @@ type RestartWorkflowExecutionRequest struct { Identity string `json:"identity,omitempty"` } -func (v *RestartWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *RestartWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -6291,13 +6067,6 @@ type DiagnoseWorkflowExecutionRequest struct { Identity string `json:"identity,omitempty"` } -func (v *DiagnoseWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain returns the domain func (v *DiagnoseWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -6357,13 +6126,6 @@ type StartWorkflowExecutionRequest struct { FirstRunAtTimeStamp *int64 `json:"firstRunAtTimeStamp,omitempty"` } -func (v *StartWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *StartWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -6729,13 +6491,6 @@ type TerminateWorkflowExecutionRequest struct { FirstExecutionRunID string `json:"first_execution_run_id,omitempty"` } -func (v *TerminateWorkflowExecutionRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetDomain is an internal getter (TBD...) func (v *TerminateWorkflowExecutionRequest) GetDomain() (o string) { if v != nil { @@ -6936,13 +6691,6 @@ type UpdateDomainRequest struct { FailoverTimeoutInSeconds *int32 `json:"failoverTimeoutInSeconds,omitempty"` } -func (v *UpdateDomainRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetName is an internal getter (TBD...) func (v *UpdateDomainRequest) GetName() (o string) { if v != nil { @@ -8256,13 +8004,6 @@ type GetCrossClusterTasksRequest struct { TargetCluster string `json:"targetCluster,omitempty"` } -func (v *GetCrossClusterTasksRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetShardIDs is an internal getter (TBD...) func (v *GetCrossClusterTasksRequest) GetShardIDs() (o []int32) { if v != nil && v.ShardIDs != nil { @@ -8301,13 +8042,6 @@ type RespondCrossClusterTasksCompletedRequest struct { FetchNewTasks bool `json:"fetchNewTasks,omitempty"` } -func (v *RespondCrossClusterTasksCompletedRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetShardID is an internal getter (TBD...) func (v *RespondCrossClusterTasksCompletedRequest) GetShardID() (o int32) { if v != nil { diff --git a/common/types/shared_test.go b/common/types_test/shared_test.go similarity index 53% rename from common/types/shared_test.go rename to common/types_test/shared_test.go index a1a6d41739c..6453c5a4a75 100644 --- a/common/types/shared_test.go +++ b/common/types_test/shared_test.go @@ -20,18 +20,21 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -package types +package types_test import ( "testing" "unsafe" "github.com/stretchr/testify/assert" + + "github.com/uber/cadence/common/authorization" + "github.com/uber/cadence/common/types" ) func TestSignalWithStartWorkflowExecutionRequestSerializeForLogging(t *testing.T) { tests := map[string]struct { - input *SignalWithStartWorkflowExecutionRequest + input *types.SignalWithStartWorkflowExecutionRequest expectedOutput string expectedErrorOutput error }{ @@ -42,123 +45,7 @@ func TestSignalWithStartWorkflowExecutionRequestSerializeForLogging(t *testing.T }, "empty request without error": { - input: &SignalWithStartWorkflowExecutionRequest{}, - expectedOutput: "{}", - expectedErrorOutput: nil, - }, - - "nil request without error": { - input: nil, - expectedOutput: "", - expectedErrorOutput: nil, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - assert.NotPanics(t, func() { - output, err := test.input.SerializeForLogging() - assert.Equal(t, test.expectedOutput, output) - assert.Equal(t, test.expectedErrorOutput, err) - assert.NotContains(t, output, "PII") - }) - }) - } -} - -func TestSignalWorkflowExecutionRequestSerializeForLogging(t *testing.T) { - tests := map[string]struct { - input *SignalWorkflowExecutionRequest - expectedOutput string - expectedErrorOutput error - }{ - "complete request without error": { - input: &SignalWorkflowExecutionRequest{ - Domain: "testDomain", - Input: []byte("testInputPII"), - Identity: "testIdentity", - RequestID: "DF66E35D-A5B0-425D-8731-6AAC4A4B6368", - SignalName: "testRequest", - Control: []byte("testControl"), - }, - expectedOutput: "{\"domain\":\"testDomain\",\"signalName\":\"testRequest\",\"identity\":\"testIdentity\",\"requestId\":\"DF66E35D-A5B0-425D-8731-6AAC4A4B6368\",\"control\":\"dGVzdENvbnRyb2w=\"}", - expectedErrorOutput: nil, - }, - - "empty request without error": { - input: &SignalWorkflowExecutionRequest{}, - expectedOutput: "{}", - expectedErrorOutput: nil, - }, - - "nil request without error": { - input: nil, - expectedOutput: "", - expectedErrorOutput: nil, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - assert.NotPanics(t, func() { - output, err := test.input.SerializeForLogging() - assert.Equal(t, test.expectedOutput, output) - assert.Equal(t, test.expectedErrorOutput, err) - assert.NotContains(t, output, "PII") - }) - }) - } -} - -func TestStartWorkflowExecutionRequestRequestSerializeForLogging(t *testing.T) { - testTasklistKind := TaskListKind(1) - testExecutionStartToCloseTimeoutSeconds := int32(1) - testTaskStartToCloseTimeoutSeconds := int32(1) - testWorkflowIDReusePolicy := WorkflowIDReusePolicy(1) - testDelayStartSeconds := int32(1) - testJitterStartSeconds := int32(1) - - tests := map[string]struct { - input *StartWorkflowExecutionRequest - expectedOutput string - expectedErrorOutput error - }{ - "complete request without error": { - input: &StartWorkflowExecutionRequest{ - Domain: "testDomain", - WorkflowID: "testWorkflowID", - WorkflowType: &WorkflowType{Name: "testWorkflowType"}, - TaskList: &TaskList{ - Name: "testTaskList", - Kind: &testTasklistKind, - }, - Input: []byte("testInputPII"), - ExecutionStartToCloseTimeoutSeconds: &testExecutionStartToCloseTimeoutSeconds, - TaskStartToCloseTimeoutSeconds: &testTaskStartToCloseTimeoutSeconds, - Identity: "testIdentity", - RequestID: "DF66E35D-A5B0-425D-8731-6AAC4A4B6368", - WorkflowIDReusePolicy: &testWorkflowIDReusePolicy, - RetryPolicy: &RetryPolicy{ - InitialIntervalInSeconds: 1, - BackoffCoefficient: 1, - MaximumIntervalInSeconds: 1, - MaximumAttempts: 1, - NonRetriableErrorReasons: []string{"testArray"}, - ExpirationIntervalInSeconds: 1, - }, - CronSchedule: "testSchedule", - Memo: &Memo{Fields: map[string][]byte{}}, - SearchAttributes: &SearchAttributes{IndexedFields: map[string][]byte{}}, - Header: &Header{Fields: map[string][]byte{}}, - DelayStartSeconds: &testDelayStartSeconds, - JitterStartSeconds: &testJitterStartSeconds, - }, - expectedOutput: "{\"domain\":\"testDomain\",\"workflowId\":\"testWorkflowID\",\"workflowType\":{\"name\":\"testWorkflowType\"},\"taskList\":{\"name\":\"testTaskList\",\"kind\":\"STICKY\"},\"executionStartToCloseTimeoutSeconds\":1,\"taskStartToCloseTimeoutSeconds\":1,\"identity\":\"testIdentity\",\"requestId\":\"DF66E35D-A5B0-425D-8731-6AAC4A4B6368\",\"workflowIdReusePolicy\":\"AllowDuplicate\",\"retryPolicy\":{\"initialIntervalInSeconds\":1,\"backoffCoefficient\":1,\"maximumIntervalInSeconds\":1,\"maximumAttempts\":1,\"nonRetriableErrorReasons\":[\"testArray\"],\"expirationIntervalInSeconds\":1},\"cronSchedule\":\"testSchedule\",\"header\":{},\"delayStartSeconds\":1,\"jitterStartSeconds\":1}", - expectedErrorOutput: nil, - }, - - "empty request without error": { - input: &StartWorkflowExecutionRequest{}, + input: &types.SignalWithStartWorkflowExecutionRequest{}, expectedOutput: "{}", expectedErrorOutput: nil, }, @@ -173,7 +60,8 @@ func TestStartWorkflowExecutionRequestRequestSerializeForLogging(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { assert.NotPanics(t, func() { - output, err := test.input.SerializeForLogging() + wrappedInput := authorization.NewFilteredRequestBody(test.input) + output, err := wrappedInput.SerializeForLogging() assert.Equal(t, test.expectedOutput, output) assert.Equal(t, test.expectedErrorOutput, err) assert.NotContains(t, output, "PII") @@ -185,7 +73,7 @@ func TestStartWorkflowExecutionRequestRequestSerializeForLogging(t *testing.T) { func TestSerializeRequest(t *testing.T) { // test serializing a normal request testReq := createNewSignalWithStartWorkflowExecutionRequest() - serializeRes, err := SerializeRequest(testReq) + serializeRes, err := types.SerializeRequest(testReq) expectRes := "{\"domain\":\"testDomain\",\"workflowId\":\"testWorkflowID\",\"workflowType\":{\"name\":\"testWorkflowType\"}," + "\"taskList\":{\"name\":\"testTaskList\",\"kind\":\"STICKY\"},\"executionStartToCloseTimeoutSeconds\":1," + @@ -200,16 +88,16 @@ func TestSerializeRequest(t *testing.T) { assert.Equal(t, expectErr, err) assert.NotPanics(t, func() { - SerializeRequest(nil) + types.SerializeRequest(nil) }) } -func createNewSignalWithStartWorkflowExecutionRequest() *SignalWithStartWorkflowExecutionRequest { - testTasklistKind := TaskListKind(1) +func createNewSignalWithStartWorkflowExecutionRequest() *types.SignalWithStartWorkflowExecutionRequest { + testTasklistKind := types.TaskListKind(1) testExecutionStartToCloseTimeoutSeconds := int32(1) testTaskStartToCloseTimeoutSeconds := int32(1) - testWorkflowIDReusePolicy := WorkflowIDReusePolicy(1) + testWorkflowIDReusePolicy := types.WorkflowIDReusePolicy(1) testDelayStartSeconds := int32(1) testJitterStartSeconds := int32(1) testFirstRunAtTimestamp := int64(1) @@ -217,11 +105,11 @@ func createNewSignalWithStartWorkflowExecutionRequest() *SignalWithStartWorkflow piiTestMap := make(map[string][]byte) piiTestMap["PII"] = piiTestArray - testReq := &SignalWithStartWorkflowExecutionRequest{ + testReq := &types.SignalWithStartWorkflowExecutionRequest{ Domain: "testDomain", WorkflowID: "testWorkflowID", - WorkflowType: &WorkflowType{Name: "testWorkflowType"}, - TaskList: &TaskList{ + WorkflowType: &types.WorkflowType{Name: "testWorkflowType"}, + TaskList: &types.TaskList{ Name: "testTaskList", Kind: &testTasklistKind, }, @@ -234,7 +122,7 @@ func createNewSignalWithStartWorkflowExecutionRequest() *SignalWithStartWorkflow SignalName: "testRequest", SignalInput: piiTestArray, Control: []byte("testControl"), - RetryPolicy: &RetryPolicy{ + RetryPolicy: &types.RetryPolicy{ InitialIntervalInSeconds: 1, BackoffCoefficient: 1, MaximumIntervalInSeconds: 1, @@ -243,9 +131,9 @@ func createNewSignalWithStartWorkflowExecutionRequest() *SignalWithStartWorkflow ExpirationIntervalInSeconds: 1, }, CronSchedule: "testSchedule", - Memo: &Memo{Fields: piiTestMap}, - SearchAttributes: &SearchAttributes{IndexedFields: piiTestMap}, - Header: &Header{Fields: map[string][]byte{}}, + Memo: &types.Memo{Fields: piiTestMap}, + SearchAttributes: &types.SearchAttributes{IndexedFields: piiTestMap}, + Header: &types.Header{Fields: map[string][]byte{}}, DelayStartSeconds: &testDelayStartSeconds, JitterStartSeconds: &testJitterStartSeconds, FirstRunAtTimestamp: &testFirstRunAtTimestamp, @@ -256,7 +144,7 @@ func createNewSignalWithStartWorkflowExecutionRequest() *SignalWithStartWorkflow func TestDataBlobDeepCopy(t *testing.T) { tests := []struct { name string - input *DataBlob + input *types.DataBlob }{ { name: "nil", @@ -264,19 +152,19 @@ func TestDataBlobDeepCopy(t *testing.T) { }, { name: "empty", - input: &DataBlob{}, + input: &types.DataBlob{}, }, { name: "thrift ok", - input: &DataBlob{ - EncodingType: EncodingTypeThriftRW.Ptr(), + input: &types.DataBlob{ + EncodingType: types.EncodingTypeThriftRW.Ptr(), Data: []byte("some thrift data"), }, }, { name: "json ok", - input: &DataBlob{ - EncodingType: EncodingTypeJSON.Ptr(), + input: &types.DataBlob{ + EncodingType: types.EncodingTypeJSON.Ptr(), Data: []byte("some json data"), }, }, diff --git a/service/frontend/templates/accesscontrolled.tmpl b/service/frontend/templates/accesscontrolled.tmpl index 40e7966cfd9..b6cd454e17b 100644 --- a/service/frontend/templates/accesscontrolled.tmpl +++ b/service/frontend/templates/accesscontrolled.tmpl @@ -104,7 +104,7 @@ func (a *{{$decorator}}) {{$method.Declaration}} { Permission: authorization.{{get $permissionMap $method.Name}}, {{- end}} {{- if ge (len $method.Params) 2}} - RequestBody: {{(index $method.Params 1).Name}}, + RequestBody: authorization.NewFilteredRequestBody( {{(index $method.Params 1).Name}} ), {{- if not (or (has $method.Name $nonDomainAuthAPIs) (eq $interfaceType "admin.Handler"))}} DomainName: {{(index $method.Params 1).Name}}.GetDomain(), {{- else if eq $method.Name "DescribeDomain"}} diff --git a/service/frontend/wrappers/accesscontrolled/admin_generated.go b/service/frontend/wrappers/accesscontrolled/admin_generated.go index ca3fe36f21f..85bc87db6c9 100644 --- a/service/frontend/wrappers/accesscontrolled/admin_generated.go +++ b/service/frontend/wrappers/accesscontrolled/admin_generated.go @@ -64,7 +64,7 @@ func (a *adminHandler) AddSearchAttribute(ctx context.Context, ap1 *types.AddSea attr := &authorization.Attributes{ APIName: "AddSearchAttribute", Permission: authorization.PermissionAdmin, - RequestBody: ap1, + RequestBody: authorization.NewFilteredRequestBody(ap1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -80,7 +80,7 @@ func (a *adminHandler) CloseShard(ctx context.Context, cp1 *types.CloseShardRequ attr := &authorization.Attributes{ APIName: "CloseShard", Permission: authorization.PermissionAdmin, - RequestBody: cp1, + RequestBody: authorization.NewFilteredRequestBody(cp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -96,7 +96,7 @@ func (a *adminHandler) CountDLQMessages(ctx context.Context, cp1 *types.CountDLQ attr := &authorization.Attributes{ APIName: "CountDLQMessages", Permission: authorization.PermissionAdmin, - RequestBody: cp1, + RequestBody: authorization.NewFilteredRequestBody(cp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -112,7 +112,7 @@ func (a *adminHandler) DeleteWorkflow(ctx context.Context, ap1 *types.AdminDelet attr := &authorization.Attributes{ APIName: "DeleteWorkflow", Permission: authorization.PermissionAdmin, - RequestBody: ap1, + RequestBody: authorization.NewFilteredRequestBody(ap1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -143,7 +143,7 @@ func (a *adminHandler) DescribeHistoryHost(ctx context.Context, dp1 *types.Descr attr := &authorization.Attributes{ APIName: "DescribeHistoryHost", Permission: authorization.PermissionAdmin, - RequestBody: dp1, + RequestBody: authorization.NewFilteredRequestBody(dp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -159,7 +159,7 @@ func (a *adminHandler) DescribeQueue(ctx context.Context, dp1 *types.DescribeQue attr := &authorization.Attributes{ APIName: "DescribeQueue", Permission: authorization.PermissionAdmin, - RequestBody: dp1, + RequestBody: authorization.NewFilteredRequestBody(dp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -175,7 +175,7 @@ func (a *adminHandler) DescribeShardDistribution(ctx context.Context, dp1 *types attr := &authorization.Attributes{ APIName: "DescribeShardDistribution", Permission: authorization.PermissionAdmin, - RequestBody: dp1, + RequestBody: authorization.NewFilteredRequestBody(dp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -191,7 +191,7 @@ func (a *adminHandler) DescribeWorkflowExecution(ctx context.Context, ap1 *types attr := &authorization.Attributes{ APIName: "DescribeWorkflowExecution", Permission: authorization.PermissionAdmin, - RequestBody: ap1, + RequestBody: authorization.NewFilteredRequestBody(ap1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -207,7 +207,7 @@ func (a *adminHandler) GetCrossClusterTasks(ctx context.Context, gp1 *types.GetC attr := &authorization.Attributes{ APIName: "GetCrossClusterTasks", Permission: authorization.PermissionAdmin, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -223,7 +223,7 @@ func (a *adminHandler) GetDLQReplicationMessages(ctx context.Context, gp1 *types attr := &authorization.Attributes{ APIName: "GetDLQReplicationMessages", Permission: authorization.PermissionAdmin, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -239,7 +239,7 @@ func (a *adminHandler) GetDomainAsyncWorkflowConfiguraton(ctx context.Context, g attr := &authorization.Attributes{ APIName: "GetDomainAsyncWorkflowConfiguraton", Permission: authorization.PermissionAdmin, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -255,7 +255,7 @@ func (a *adminHandler) GetDomainIsolationGroups(ctx context.Context, request *ty attr := &authorization.Attributes{ APIName: "GetDomainIsolationGroups", Permission: authorization.PermissionAdmin, - RequestBody: request, + RequestBody: authorization.NewFilteredRequestBody(request), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -271,7 +271,7 @@ func (a *adminHandler) GetDomainReplicationMessages(ctx context.Context, gp1 *ty attr := &authorization.Attributes{ APIName: "GetDomainReplicationMessages", Permission: authorization.PermissionAdmin, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -287,7 +287,7 @@ func (a *adminHandler) GetDynamicConfig(ctx context.Context, gp1 *types.GetDynam attr := &authorization.Attributes{ APIName: "GetDynamicConfig", Permission: authorization.PermissionAdmin, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -303,7 +303,7 @@ func (a *adminHandler) GetGlobalIsolationGroups(ctx context.Context, request *ty attr := &authorization.Attributes{ APIName: "GetGlobalIsolationGroups", Permission: authorization.PermissionAdmin, - RequestBody: request, + RequestBody: authorization.NewFilteredRequestBody(request), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -319,7 +319,7 @@ func (a *adminHandler) GetReplicationMessages(ctx context.Context, gp1 *types.Ge attr := &authorization.Attributes{ APIName: "GetReplicationMessages", Permission: authorization.PermissionAdmin, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -335,7 +335,7 @@ func (a *adminHandler) GetWorkflowExecutionRawHistoryV2(ctx context.Context, gp1 attr := &authorization.Attributes{ APIName: "GetWorkflowExecutionRawHistoryV2", Permission: authorization.PermissionAdmin, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -351,7 +351,7 @@ func (a *adminHandler) ListDynamicConfig(ctx context.Context, lp1 *types.ListDyn attr := &authorization.Attributes{ APIName: "ListDynamicConfig", Permission: authorization.PermissionAdmin, - RequestBody: lp1, + RequestBody: authorization.NewFilteredRequestBody(lp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -367,7 +367,7 @@ func (a *adminHandler) MaintainCorruptWorkflow(ctx context.Context, ap1 *types.A attr := &authorization.Attributes{ APIName: "MaintainCorruptWorkflow", Permission: authorization.PermissionAdmin, - RequestBody: ap1, + RequestBody: authorization.NewFilteredRequestBody(ap1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -383,7 +383,7 @@ func (a *adminHandler) MergeDLQMessages(ctx context.Context, mp1 *types.MergeDLQ attr := &authorization.Attributes{ APIName: "MergeDLQMessages", Permission: authorization.PermissionAdmin, - RequestBody: mp1, + RequestBody: authorization.NewFilteredRequestBody(mp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -399,7 +399,7 @@ func (a *adminHandler) PurgeDLQMessages(ctx context.Context, pp1 *types.PurgeDLQ attr := &authorization.Attributes{ APIName: "PurgeDLQMessages", Permission: authorization.PermissionAdmin, - RequestBody: pp1, + RequestBody: authorization.NewFilteredRequestBody(pp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -415,7 +415,7 @@ func (a *adminHandler) ReadDLQMessages(ctx context.Context, rp1 *types.ReadDLQMe attr := &authorization.Attributes{ APIName: "ReadDLQMessages", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -431,7 +431,7 @@ func (a *adminHandler) ReapplyEvents(ctx context.Context, rp1 *types.ReapplyEven attr := &authorization.Attributes{ APIName: "ReapplyEvents", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -447,7 +447,7 @@ func (a *adminHandler) RefreshWorkflowTasks(ctx context.Context, rp1 *types.Refr attr := &authorization.Attributes{ APIName: "RefreshWorkflowTasks", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -463,7 +463,7 @@ func (a *adminHandler) RemoveTask(ctx context.Context, rp1 *types.RemoveTaskRequ attr := &authorization.Attributes{ APIName: "RemoveTask", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -479,7 +479,7 @@ func (a *adminHandler) ResendReplicationTasks(ctx context.Context, rp1 *types.Re attr := &authorization.Attributes{ APIName: "ResendReplicationTasks", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -495,7 +495,7 @@ func (a *adminHandler) ResetQueue(ctx context.Context, rp1 *types.ResetQueueRequ attr := &authorization.Attributes{ APIName: "ResetQueue", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -511,7 +511,7 @@ func (a *adminHandler) RespondCrossClusterTasksCompleted(ctx context.Context, rp attr := &authorization.Attributes{ APIName: "RespondCrossClusterTasksCompleted", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -527,7 +527,7 @@ func (a *adminHandler) RestoreDynamicConfig(ctx context.Context, rp1 *types.Rest attr := &authorization.Attributes{ APIName: "RestoreDynamicConfig", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -551,7 +551,7 @@ func (a *adminHandler) UpdateDomainAsyncWorkflowConfiguraton(ctx context.Context attr := &authorization.Attributes{ APIName: "UpdateDomainAsyncWorkflowConfiguraton", Permission: authorization.PermissionAdmin, - RequestBody: up1, + RequestBody: authorization.NewFilteredRequestBody(up1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -567,7 +567,7 @@ func (a *adminHandler) UpdateDomainIsolationGroups(ctx context.Context, request attr := &authorization.Attributes{ APIName: "UpdateDomainIsolationGroups", Permission: authorization.PermissionAdmin, - RequestBody: request, + RequestBody: authorization.NewFilteredRequestBody(request), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -583,7 +583,7 @@ func (a *adminHandler) UpdateDynamicConfig(ctx context.Context, up1 *types.Updat attr := &authorization.Attributes{ APIName: "UpdateDynamicConfig", Permission: authorization.PermissionAdmin, - RequestBody: up1, + RequestBody: authorization.NewFilteredRequestBody(up1), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { @@ -599,7 +599,7 @@ func (a *adminHandler) UpdateGlobalIsolationGroups(ctx context.Context, request attr := &authorization.Attributes{ APIName: "UpdateGlobalIsolationGroups", Permission: authorization.PermissionAdmin, - RequestBody: request, + RequestBody: authorization.NewFilteredRequestBody(request), } isAuthorized, err := a.isAuthorized(ctx, attr) if err != nil { diff --git a/service/frontend/wrappers/accesscontrolled/api_generated.go b/service/frontend/wrappers/accesscontrolled/api_generated.go index ef10754387b..9f36b36ff36 100644 --- a/service/frontend/wrappers/accesscontrolled/api_generated.go +++ b/service/frontend/wrappers/accesscontrolled/api_generated.go @@ -66,7 +66,7 @@ func (a *apiHandler) CountWorkflowExecutions(ctx context.Context, cp1 *types.Cou attr := &authorization.Attributes{ APIName: "CountWorkflowExecutions", Permission: authorization.PermissionRead, - RequestBody: cp1, + RequestBody: authorization.NewFilteredRequestBody(cp1), DomainName: cp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -84,7 +84,7 @@ func (a *apiHandler) DeprecateDomain(ctx context.Context, dp1 *types.DeprecateDo attr := &authorization.Attributes{ APIName: "DeprecateDomain", Permission: authorization.PermissionAdmin, - RequestBody: dp1, + RequestBody: authorization.NewFilteredRequestBody(dp1), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { @@ -101,7 +101,7 @@ func (a *apiHandler) DescribeDomain(ctx context.Context, dp1 *types.DescribeDoma attr := &authorization.Attributes{ APIName: "DescribeDomain", Permission: authorization.PermissionRead, - RequestBody: dp1, + RequestBody: authorization.NewFilteredRequestBody(dp1), DomainName: dp1.GetName(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -119,7 +119,7 @@ func (a *apiHandler) DescribeTaskList(ctx context.Context, dp1 *types.DescribeTa attr := &authorization.Attributes{ APIName: "DescribeTaskList", Permission: authorization.PermissionRead, - RequestBody: dp1, + RequestBody: authorization.NewFilteredRequestBody(dp1), DomainName: dp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -137,7 +137,7 @@ func (a *apiHandler) DescribeWorkflowExecution(ctx context.Context, dp1 *types.D attr := &authorization.Attributes{ APIName: "DescribeWorkflowExecution", Permission: authorization.PermissionRead, - RequestBody: dp1, + RequestBody: authorization.NewFilteredRequestBody(dp1), DomainName: dp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -167,7 +167,7 @@ func (a *apiHandler) GetTaskListsByDomain(ctx context.Context, gp1 *types.GetTas attr := &authorization.Attributes{ APIName: "GetTaskListsByDomain", Permission: authorization.PermissionRead, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), DomainName: gp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -185,7 +185,7 @@ func (a *apiHandler) GetWorkflowExecutionHistory(ctx context.Context, gp1 *types attr := &authorization.Attributes{ APIName: "GetWorkflowExecutionHistory", Permission: authorization.PermissionRead, - RequestBody: gp1, + RequestBody: authorization.NewFilteredRequestBody(gp1), DomainName: gp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -207,7 +207,7 @@ func (a *apiHandler) ListArchivedWorkflowExecutions(ctx context.Context, lp1 *ty attr := &authorization.Attributes{ APIName: "ListArchivedWorkflowExecutions", Permission: authorization.PermissionRead, - RequestBody: lp1, + RequestBody: authorization.NewFilteredRequestBody(lp1), DomainName: lp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -225,7 +225,7 @@ func (a *apiHandler) ListClosedWorkflowExecutions(ctx context.Context, lp1 *type attr := &authorization.Attributes{ APIName: "ListClosedWorkflowExecutions", Permission: authorization.PermissionRead, - RequestBody: lp1, + RequestBody: authorization.NewFilteredRequestBody(lp1), DomainName: lp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -243,7 +243,7 @@ func (a *apiHandler) ListDomains(ctx context.Context, lp1 *types.ListDomainsRequ attr := &authorization.Attributes{ APIName: "ListDomains", Permission: authorization.PermissionAdmin, - RequestBody: lp1, + RequestBody: authorization.NewFilteredRequestBody(lp1), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { @@ -260,7 +260,7 @@ func (a *apiHandler) ListOpenWorkflowExecutions(ctx context.Context, lp1 *types. attr := &authorization.Attributes{ APIName: "ListOpenWorkflowExecutions", Permission: authorization.PermissionRead, - RequestBody: lp1, + RequestBody: authorization.NewFilteredRequestBody(lp1), DomainName: lp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -278,7 +278,7 @@ func (a *apiHandler) ListTaskListPartitions(ctx context.Context, lp1 *types.List attr := &authorization.Attributes{ APIName: "ListTaskListPartitions", Permission: authorization.PermissionRead, - RequestBody: lp1, + RequestBody: authorization.NewFilteredRequestBody(lp1), DomainName: lp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -296,7 +296,7 @@ func (a *apiHandler) ListWorkflowExecutions(ctx context.Context, lp1 *types.List attr := &authorization.Attributes{ APIName: "ListWorkflowExecutions", Permission: authorization.PermissionRead, - RequestBody: lp1, + RequestBody: authorization.NewFilteredRequestBody(lp1), DomainName: lp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -314,7 +314,7 @@ func (a *apiHandler) PollForActivityTask(ctx context.Context, pp1 *types.PollFor attr := &authorization.Attributes{ APIName: "PollForActivityTask", Permission: authorization.PermissionWrite, - RequestBody: pp1, + RequestBody: authorization.NewFilteredRequestBody(pp1), DomainName: pp1.GetDomain(), TaskList: pp1.TaskList, } @@ -333,7 +333,7 @@ func (a *apiHandler) PollForDecisionTask(ctx context.Context, pp1 *types.PollFor attr := &authorization.Attributes{ APIName: "PollForDecisionTask", Permission: authorization.PermissionWrite, - RequestBody: pp1, + RequestBody: authorization.NewFilteredRequestBody(pp1), DomainName: pp1.GetDomain(), TaskList: pp1.TaskList, } @@ -352,7 +352,7 @@ func (a *apiHandler) QueryWorkflow(ctx context.Context, qp1 *types.QueryWorkflow attr := &authorization.Attributes{ APIName: "QueryWorkflow", Permission: authorization.PermissionRead, - RequestBody: qp1, + RequestBody: authorization.NewFilteredRequestBody(qp1), DomainName: qp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -378,7 +378,7 @@ func (a *apiHandler) RefreshWorkflowTasks(ctx context.Context, rp1 *types.Refres attr := &authorization.Attributes{ APIName: "RefreshWorkflowTasks", Permission: authorization.PermissionWrite, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), DomainName: rp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -396,7 +396,7 @@ func (a *apiHandler) RegisterDomain(ctx context.Context, rp1 *types.RegisterDoma attr := &authorization.Attributes{ APIName: "RegisterDomain", Permission: authorization.PermissionAdmin, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { @@ -413,7 +413,7 @@ func (a *apiHandler) RequestCancelWorkflowExecution(ctx context.Context, rp1 *ty attr := &authorization.Attributes{ APIName: "RequestCancelWorkflowExecution", Permission: authorization.PermissionWrite, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), DomainName: rp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -431,7 +431,7 @@ func (a *apiHandler) ResetStickyTaskList(ctx context.Context, rp1 *types.ResetSt attr := &authorization.Attributes{ APIName: "ResetStickyTaskList", Permission: authorization.PermissionWrite, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), DomainName: rp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -449,7 +449,7 @@ func (a *apiHandler) ResetWorkflowExecution(ctx context.Context, rp1 *types.Rese attr := &authorization.Attributes{ APIName: "ResetWorkflowExecution", Permission: authorization.PermissionWrite, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), DomainName: rp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -503,7 +503,7 @@ func (a *apiHandler) RestartWorkflowExecution(ctx context.Context, rp1 *types.Re attr := &authorization.Attributes{ APIName: "RestartWorkflowExecution", Permission: authorization.PermissionWrite, - RequestBody: rp1, + RequestBody: authorization.NewFilteredRequestBody(rp1), DomainName: rp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -521,7 +521,7 @@ func (a *apiHandler) ScanWorkflowExecutions(ctx context.Context, lp1 *types.List attr := &authorization.Attributes{ APIName: "ScanWorkflowExecutions", Permission: authorization.PermissionRead, - RequestBody: lp1, + RequestBody: authorization.NewFilteredRequestBody(lp1), DomainName: lp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -539,7 +539,7 @@ func (a *apiHandler) SignalWithStartWorkflowExecution(ctx context.Context, sp1 * attr := &authorization.Attributes{ APIName: "SignalWithStartWorkflowExecution", Permission: authorization.PermissionWrite, - RequestBody: sp1, + RequestBody: authorization.NewFilteredRequestBody(sp1), DomainName: sp1.GetDomain(), WorkflowType: sp1.WorkflowType, } @@ -558,7 +558,7 @@ func (a *apiHandler) SignalWithStartWorkflowExecutionAsync(ctx context.Context, attr := &authorization.Attributes{ APIName: "SignalWithStartWorkflowExecutionAsync", Permission: authorization.PermissionWrite, - RequestBody: sp1, + RequestBody: authorization.NewFilteredRequestBody(sp1), DomainName: sp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -576,7 +576,7 @@ func (a *apiHandler) SignalWorkflowExecution(ctx context.Context, sp1 *types.Sig attr := &authorization.Attributes{ APIName: "SignalWorkflowExecution", Permission: authorization.PermissionWrite, - RequestBody: sp1, + RequestBody: authorization.NewFilteredRequestBody(sp1), DomainName: sp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -594,7 +594,7 @@ func (a *apiHandler) StartWorkflowExecution(ctx context.Context, sp1 *types.Star attr := &authorization.Attributes{ APIName: "StartWorkflowExecution", Permission: authorization.PermissionWrite, - RequestBody: sp1, + RequestBody: authorization.NewFilteredRequestBody(sp1), DomainName: sp1.GetDomain(), WorkflowType: sp1.WorkflowType, } @@ -613,7 +613,7 @@ func (a *apiHandler) StartWorkflowExecutionAsync(ctx context.Context, sp1 *types attr := &authorization.Attributes{ APIName: "StartWorkflowExecutionAsync", Permission: authorization.PermissionWrite, - RequestBody: sp1, + RequestBody: authorization.NewFilteredRequestBody(sp1), DomainName: sp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -631,7 +631,7 @@ func (a *apiHandler) TerminateWorkflowExecution(ctx context.Context, tp1 *types. attr := &authorization.Attributes{ APIName: "TerminateWorkflowExecution", Permission: authorization.PermissionWrite, - RequestBody: tp1, + RequestBody: authorization.NewFilteredRequestBody(tp1), DomainName: tp1.GetDomain(), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) @@ -649,7 +649,7 @@ func (a *apiHandler) UpdateDomain(ctx context.Context, up1 *types.UpdateDomainRe attr := &authorization.Attributes{ APIName: "UpdateDomain", Permission: authorization.PermissionAdmin, - RequestBody: up1, + RequestBody: authorization.NewFilteredRequestBody(up1), } isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { From e30187eac3728e488b54b2553c8dcad06e34e063 Mon Sep 17 00:00:00 2001 From: Jakob Haahr Taankvist Date: Thu, 31 Oct 2024 13:56:08 +0100 Subject: [PATCH 2/5] Fixed issue with nil, and removed more tests of the old version --- common/authorization/authorizer.go | 19 +-- common/authorization/authorizer_test.go | 87 +++++++++++ common/types/replicator.go | 49 ------ common/types/replicator_test.go | 147 ------------------ common/types/shared.go | 12 -- common/types/shared_test.go | 70 +++++++++ common/types_test/shared_test.go | 188 ------------------------ 7 files changed, 167 insertions(+), 405 deletions(-) create mode 100644 common/types/shared_test.go delete mode 100644 common/types_test/shared_test.go diff --git a/common/authorization/authorizer.go b/common/authorization/authorizer.go index 31849da088c..da35a3635d7 100644 --- a/common/authorization/authorizer.go +++ b/common/authorization/authorizer.go @@ -24,8 +24,10 @@ package authorization import ( "context" + "encoding/json" "fmt" "os" + "reflect" "strings" clientworker "go.uber.org/cadence/worker" @@ -120,20 +122,19 @@ type simpleRequestLogWrapper struct { } func (f *simpleRequestLogWrapper) SerializeForLogging() (string, error) { - return types.SerializeRequest(f.request) -} + if f.request == nil || reflect.ValueOf(f.request).IsNil() { + return "", nil + } -type nullRequestLogWrapper struct{} + res, err := json.Marshal(f.request) + if err != nil { + return "", err + } -func (f *nullRequestLogWrapper) SerializeForLogging() (string, error) { - return "", nil + return string(res), nil } func NewFilteredRequestBody(request interface{}) FilteredRequestBody { - if request == nil { - return &nullRequestLogWrapper{} - } - return &simpleRequestLogWrapper{request} } diff --git a/common/authorization/authorizer_test.go b/common/authorization/authorizer_test.go index 11b059bcddc..4c79790f1ba 100644 --- a/common/authorization/authorizer_test.go +++ b/common/authorization/authorizer_test.go @@ -26,6 +26,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/uber/cadence/common/types" "github.com/uber/cadence/common" ) @@ -95,3 +96,89 @@ func Test_validatePermission(t *testing.T) { }) } } + +func TestSignalWithStartWorkflowExecutionRequestSerializeForLogging(t *testing.T) { + tests := map[string]struct { + input *types.SignalWithStartWorkflowExecutionRequest + expectedOutput string + expectedErrorOutput error + }{ + "complete request without error": { + input: createNewSignalWithStartWorkflowExecutionRequest(), + expectedOutput: "{\"domain\":\"testDomain\",\"workflowId\":\"testWorkflowID\",\"workflowType\":{\"name\":\"testWorkflowType\"},\"taskList\":{\"name\":\"testTaskList\",\"kind\":\"STICKY\"},\"executionStartToCloseTimeoutSeconds\":1,\"taskStartToCloseTimeoutSeconds\":1,\"identity\":\"testIdentity\",\"requestId\":\"DF66E35D-A5B0-425D-8731-6AAC4A4B6368\",\"workflowIdReusePolicy\":\"AllowDuplicate\",\"signalName\":\"testRequest\",\"control\":\"dGVzdENvbnRyb2w=\",\"retryPolicy\":{\"initialIntervalInSeconds\":1,\"backoffCoefficient\":1,\"maximumIntervalInSeconds\":1,\"maximumAttempts\":1,\"nonRetriableErrorReasons\":[\"testArray\"],\"expirationIntervalInSeconds\":1},\"cronSchedule\":\"testSchedule\",\"header\":{},\"delayStartSeconds\":1,\"jitterStartSeconds\":1,\"firstRunAtTimestamp\":1}", + expectedErrorOutput: nil, + }, + + "empty request without error": { + input: &types.SignalWithStartWorkflowExecutionRequest{}, + expectedOutput: "{}", + expectedErrorOutput: nil, + }, + + "nil request without error": { + input: nil, + expectedOutput: "", + expectedErrorOutput: nil, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + assert.NotPanics(t, func() { + wrappedInput := NewFilteredRequestBody(test.input) + output, err := wrappedInput.SerializeForLogging() + assert.Equal(t, test.expectedOutput, output) + assert.Equal(t, test.expectedErrorOutput, err) + assert.NotContains(t, output, "PII") + }) + }) + } +} + +func createNewSignalWithStartWorkflowExecutionRequest() *types.SignalWithStartWorkflowExecutionRequest { + testTasklistKind := types.TaskListKind(1) + testExecutionStartToCloseTimeoutSeconds := int32(1) + testTaskStartToCloseTimeoutSeconds := int32(1) + testWorkflowIDReusePolicy := types.WorkflowIDReusePolicy(1) + testDelayStartSeconds := int32(1) + testJitterStartSeconds := int32(1) + testFirstRunAtTimestamp := int64(1) + piiTestArray := []byte("testInputPII") + piiTestMap := make(map[string][]byte) + piiTestMap["PII"] = piiTestArray + + testReq := &types.SignalWithStartWorkflowExecutionRequest{ + Domain: "testDomain", + WorkflowID: "testWorkflowID", + WorkflowType: &types.WorkflowType{Name: "testWorkflowType"}, + TaskList: &types.TaskList{ + Name: "testTaskList", + Kind: &testTasklistKind, + }, + Input: piiTestArray, + ExecutionStartToCloseTimeoutSeconds: &testExecutionStartToCloseTimeoutSeconds, + TaskStartToCloseTimeoutSeconds: &testTaskStartToCloseTimeoutSeconds, + Identity: "testIdentity", + RequestID: "DF66E35D-A5B0-425D-8731-6AAC4A4B6368", + WorkflowIDReusePolicy: &testWorkflowIDReusePolicy, + SignalName: "testRequest", + SignalInput: piiTestArray, + Control: []byte("testControl"), + RetryPolicy: &types.RetryPolicy{ + InitialIntervalInSeconds: 1, + BackoffCoefficient: 1, + MaximumIntervalInSeconds: 1, + MaximumAttempts: 1, + NonRetriableErrorReasons: []string{"testArray"}, + ExpirationIntervalInSeconds: 1, + }, + CronSchedule: "testSchedule", + Memo: &types.Memo{Fields: piiTestMap}, + SearchAttributes: &types.SearchAttributes{IndexedFields: piiTestMap}, + Header: &types.Header{Fields: map[string][]byte{}}, + DelayStartSeconds: &testDelayStartSeconds, + JitterStartSeconds: &testJitterStartSeconds, + FirstRunAtTimestamp: &testFirstRunAtTimestamp, + } + return testReq +} diff --git a/common/types/replicator.go b/common/types/replicator.go index 4ca8c52b1f6..ec0d1d1963c 100644 --- a/common/types/replicator.go +++ b/common/types/replicator.go @@ -229,13 +229,6 @@ type GetDLQReplicationMessagesRequest struct { TaskInfos []*ReplicationTaskInfo `json:"taskInfos,omitempty"` } -func (v *GetDLQReplicationMessagesRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetTaskInfos is an internal getter (TBD...) func (v *GetDLQReplicationMessagesRequest) GetTaskInfos() (o []*ReplicationTaskInfo) { if v != nil && v.TaskInfos != nil { @@ -256,13 +249,6 @@ type GetDomainReplicationMessagesRequest struct { ClusterName string `json:"clusterName,omitempty"` } -func (v *GetDomainReplicationMessagesRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetLastRetrievedMessageID is an internal getter (TBD...) func (v *GetDomainReplicationMessagesRequest) GetLastRetrievedMessageID() (o int64) { if v != nil && v.LastRetrievedMessageID != nil { @@ -298,13 +284,6 @@ type GetReplicationMessagesRequest struct { ClusterName string `json:"clusterName,omitempty"` } -func (v *GetReplicationMessagesRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetClusterName is an internal getter (TBD...) func (v *GetReplicationMessagesRequest) GetClusterName() (o string) { if v != nil { @@ -390,13 +369,6 @@ type CountDLQMessagesRequest struct { ForceFetch bool } -func (v *CountDLQMessagesRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - type CountDLQMessagesResponse struct { History map[HistoryDLQCountKey]int64 Domain int64 @@ -421,13 +393,6 @@ type MergeDLQMessagesRequest struct { NextPageToken []byte `json:"nextPageToken,omitempty"` } -func (v *MergeDLQMessagesRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetType is an internal getter (TBD...) func (v *MergeDLQMessagesRequest) GetType() (o DLQType) { if v != nil && v.Type != nil { @@ -489,13 +454,6 @@ type PurgeDLQMessagesRequest struct { InclusiveEndMessageID *int64 `json:"inclusiveEndMessageID,omitempty"` } -func (v *PurgeDLQMessagesRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetType is an internal getter (TBD...) func (v *PurgeDLQMessagesRequest) GetType() (o DLQType) { if v != nil && v.Type != nil { @@ -538,13 +496,6 @@ type ReadDLQMessagesRequest struct { NextPageToken []byte `json:"nextPageToken,omitempty"` } -func (v *ReadDLQMessagesRequest) SerializeForLogging() (string, error) { - if v == nil { - return "", nil - } - return SerializeRequest(v) -} - // GetType is an internal getter (TBD...) func (v *ReadDLQMessagesRequest) GetType() (o DLQType) { if v != nil && v.Type != nil { diff --git a/common/types/replicator_test.go b/common/types/replicator_test.go index eac4e3318f6..55fa0183f1d 100644 --- a/common/types/replicator_test.go +++ b/common/types/replicator_test.go @@ -298,24 +298,6 @@ func TestMergeDLQMessagesRequest_GetInclusiveEndMessageID(t *testing.T) { assert.Equal(t, int64(0), res) } -func TestGetDLQReplicationMessagesRequest_SerializeForLogging(t *testing.T) { - // Test case where the struct is nil - var nilStruct *GetDLQReplicationMessagesRequest - res, err := nilStruct.SerializeForLogging() - assert.Equal(t, "", res) - assert.NoError(t, err) - - // Test case with a non-nil struct - taskInfos := []*ReplicationTaskInfo{{}, {}} - testStruct := GetDLQReplicationMessagesRequest{ - TaskInfos: taskInfos, - } - - res, err = testStruct.SerializeForLogging() - assert.NotEmpty(t, res) - assert.NoError(t, err) -} - func TestGetDLQReplicationMessagesRequest_GetTaskInfos(t *testing.T) { taskInfos := []*ReplicationTaskInfo{{}, {}} testStruct := GetDLQReplicationMessagesRequest{ @@ -371,25 +353,6 @@ func TestGetDomainReplicationMessagesRequest_GetClusterName(t *testing.T) { assert.Equal(t, "", res) } -func TestGetReplicationMessagesRequest_SerializeForLogging(t *testing.T) { - // Test case where the struct is nil - var nilStruct *GetReplicationMessagesRequest - res, err := nilStruct.SerializeForLogging() - assert.Equal(t, "", res) - assert.NoError(t, err) - - // Test case with a non-nil struct - tokens := []*ReplicationToken{{}, {}} - testStruct := GetReplicationMessagesRequest{ - Tokens: tokens, - ClusterName: "test-cluster", - } - - res, err = testStruct.SerializeForLogging() - assert.NotEmpty(t, res) - assert.NoError(t, err) -} - func TestGetReplicationMessagesRequest_GetClusterName(t *testing.T) { testStruct := GetReplicationMessagesRequest{ ClusterName: "test-cluster", @@ -420,23 +383,6 @@ func TestGetReplicationMessagesResponse_GetMessagesByShard(t *testing.T) { assert.Nil(t, res) } -func TestCountDLQMessagesRequest_SerializeForLogging(t *testing.T) { - // Test case where the struct is nil - var nilStruct *CountDLQMessagesRequest - res, err := nilStruct.SerializeForLogging() - assert.Equal(t, "", res) - assert.NoError(t, err) - - // Test case with a non-nil struct - testStruct := CountDLQMessagesRequest{ - ForceFetch: true, - } - - res, err = testStruct.SerializeForLogging() - assert.NotEmpty(t, res) - assert.NoError(t, err) -} - func TestCountDLQMessagesResponse(t *testing.T) { history := map[HistoryDLQCountKey]int64{ {ShardID: 1, SourceCluster: "cluster-1"}: 100, @@ -470,31 +416,6 @@ func TestHistoryCountDLQMessagesResponse(t *testing.T) { assert.Nil(t, emptyStruct.Entries) } -func TestMergeDLQMessagesRequest_SerializeForLogging(t *testing.T) { - // Test case where the struct is nil - var nilStruct *MergeDLQMessagesRequest - res, err := nilStruct.SerializeForLogging() - assert.Equal(t, "", res) - assert.NoError(t, err) - - // Test case with a non-nil struct - dlqType := DLQTypeReplication - endMessageID := int64(102) - nextPageToken := []byte("token") - testStruct := MergeDLQMessagesRequest{ - Type: &dlqType, - ShardID: 101, - SourceCluster: "cluster-1", - InclusiveEndMessageID: &endMessageID, - MaximumPageSize: 50, - NextPageToken: nextPageToken, - } - - res, err = testStruct.SerializeForLogging() - assert.NotEmpty(t, res) - assert.NoError(t, err) -} - func TestMergeDLQMessagesRequest_Getters(t *testing.T) { dlqType := DLQTypeReplication endMessageID := int64(102) @@ -606,28 +527,6 @@ func TestHistoryTaskV2Attributes_GetNewRunEvents(t *testing.T) { assert.Nil(t, res) } -func TestPurgeDLQMessagesRequest_SerializeForLogging(t *testing.T) { - // Test case where the struct is nil - var nilStruct *PurgeDLQMessagesRequest - res, err := nilStruct.SerializeForLogging() - assert.Equal(t, "", res) - assert.NoError(t, err) - - // Test case with a non-nil struct - dlqType := DLQTypeReplication - endMessageID := int64(12345) - testStruct := PurgeDLQMessagesRequest{ - Type: &dlqType, - ShardID: 101, - SourceCluster: "test-cluster", - InclusiveEndMessageID: &endMessageID, - } - - res, err = testStruct.SerializeForLogging() - assert.NotEmpty(t, res) - assert.NoError(t, err) -} - func TestPurgeDLQMessagesRequest_Getters(t *testing.T) { dlqType := DLQTypeReplication endMessageID := int64(12345) @@ -651,31 +550,6 @@ func TestPurgeDLQMessagesRequest_Getters(t *testing.T) { assert.Equal(t, int64(0), nilStruct.GetInclusiveEndMessageID()) } -func TestReadDLQMessagesRequest_SerializeForLogging(t *testing.T) { - // Test case where the struct is nil - var nilStruct *ReadDLQMessagesRequest - res, err := nilStruct.SerializeForLogging() - assert.Equal(t, "", res) - assert.NoError(t, err) - - // Test case with a non-nil struct - dlqType := DLQTypeReplication - endMessageID := int64(12345) - nextPageToken := []byte("token") - testStruct := ReadDLQMessagesRequest{ - Type: &dlqType, - ShardID: 101, - SourceCluster: "test-cluster", - InclusiveEndMessageID: &endMessageID, - MaximumPageSize: 50, - NextPageToken: nextPageToken, - } - - res, err = testStruct.SerializeForLogging() - assert.NotEmpty(t, res) - assert.NoError(t, err) -} - func TestReadDLQMessagesRequest_Getters(t *testing.T) { dlqType := DLQTypeReplication endMessageID := int64(12345) @@ -1282,24 +1156,3 @@ func TestSyncShardStatus_GetTimestamp(t *testing.T) { res = nilStruct.GetTimestamp() assert.Equal(t, int64(0), res) } - -func TestGetDomainReplicationMessagesRequest_SerializeForLogging(t *testing.T) { - // Test case where the struct is nil - var nilStruct *GetDomainReplicationMessagesRequest - res, err := nilStruct.SerializeForLogging() - assert.Equal(t, "", res) - assert.NoError(t, err) - - // Test case with a non-nil struct - lastRetrievedMessageID := int64(12345) - lastProcessedMessageID := int64(67890) - testStruct := GetDomainReplicationMessagesRequest{ - LastRetrievedMessageID: &lastRetrievedMessageID, - LastProcessedMessageID: &lastProcessedMessageID, - ClusterName: "test-cluster", - } - - res, err = testStruct.SerializeForLogging() - assert.NotEmpty(t, res) - assert.NoError(t, err) -} diff --git a/common/types/shared.go b/common/types/shared.go index b5f691a5af0..d2c9fd7597e 100644 --- a/common/types/shared.go +++ b/common/types/shared.go @@ -21,7 +21,6 @@ package types import ( - "encoding/json" "fmt" "strconv" "strings" @@ -8068,17 +8067,6 @@ type StickyWorkerUnavailableError struct { Message string `json:"message,required"` } -// SerializeRequest Serialize an arbitrary request for logging -// pass in a pointer as a parameter to save space -func SerializeRequest(request interface{}) (string, error) { - res, err := json.Marshal(request) - if err != nil { - return "", err - } - - return string(res), nil -} - // Any is an internal mirror of google.protobuf.Any, serving the same purposes, but // intentionally breaking direct compatibility because it may hold data that is not // actually protobuf encoded. diff --git a/common/types/shared_test.go b/common/types/shared_test.go new file mode 100644 index 00000000000..59abaeb3efa --- /dev/null +++ b/common/types/shared_test.go @@ -0,0 +1,70 @@ +// The MIT License (MIT) + +// Copyright (c) 2017-2020 Uber Technologies Inc. + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package types + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDataBlobDeepCopy(t *testing.T) { + tests := []struct { + name string + input *DataBlob + }{ + { + name: "nil", + input: nil, + }, + { + name: "empty", + input: &DataBlob{}, + }, + { + name: "thrift ok", + input: &DataBlob{ + EncodingType: EncodingTypeThriftRW.Ptr(), + Data: []byte("some thrift data"), + }, + }, + { + name: "json ok", + input: &DataBlob{ + EncodingType: EncodingTypeJSON.Ptr(), + Data: []byte("some json data"), + }, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + got := tc.input.DeepCopy() + assert.Equal(t, tc.input, got) + if tc.input != nil && tc.input.Data != nil && identicalByteArray(tc.input.Data, got.Data) { + t.Error("expected DeepCopy to return a new data slice") + } + }) + } +} diff --git a/common/types_test/shared_test.go b/common/types_test/shared_test.go deleted file mode 100644 index 6453c5a4a75..00000000000 --- a/common/types_test/shared_test.go +++ /dev/null @@ -1,188 +0,0 @@ -// The MIT License (MIT) - -// Copyright (c) 2017-2020 Uber Technologies Inc. - -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -package types_test - -import ( - "testing" - "unsafe" - - "github.com/stretchr/testify/assert" - - "github.com/uber/cadence/common/authorization" - "github.com/uber/cadence/common/types" -) - -func TestSignalWithStartWorkflowExecutionRequestSerializeForLogging(t *testing.T) { - tests := map[string]struct { - input *types.SignalWithStartWorkflowExecutionRequest - expectedOutput string - expectedErrorOutput error - }{ - "complete request without error": { - input: createNewSignalWithStartWorkflowExecutionRequest(), - expectedOutput: "{\"domain\":\"testDomain\",\"workflowId\":\"testWorkflowID\",\"workflowType\":{\"name\":\"testWorkflowType\"},\"taskList\":{\"name\":\"testTaskList\",\"kind\":\"STICKY\"},\"executionStartToCloseTimeoutSeconds\":1,\"taskStartToCloseTimeoutSeconds\":1,\"identity\":\"testIdentity\",\"requestId\":\"DF66E35D-A5B0-425D-8731-6AAC4A4B6368\",\"workflowIdReusePolicy\":\"AllowDuplicate\",\"signalName\":\"testRequest\",\"control\":\"dGVzdENvbnRyb2w=\",\"retryPolicy\":{\"initialIntervalInSeconds\":1,\"backoffCoefficient\":1,\"maximumIntervalInSeconds\":1,\"maximumAttempts\":1,\"nonRetriableErrorReasons\":[\"testArray\"],\"expirationIntervalInSeconds\":1},\"cronSchedule\":\"testSchedule\",\"header\":{},\"delayStartSeconds\":1,\"jitterStartSeconds\":1,\"firstRunAtTimestamp\":1}", - expectedErrorOutput: nil, - }, - - "empty request without error": { - input: &types.SignalWithStartWorkflowExecutionRequest{}, - expectedOutput: "{}", - expectedErrorOutput: nil, - }, - - "nil request without error": { - input: nil, - expectedOutput: "", - expectedErrorOutput: nil, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - assert.NotPanics(t, func() { - wrappedInput := authorization.NewFilteredRequestBody(test.input) - output, err := wrappedInput.SerializeForLogging() - assert.Equal(t, test.expectedOutput, output) - assert.Equal(t, test.expectedErrorOutput, err) - assert.NotContains(t, output, "PII") - }) - }) - } -} - -func TestSerializeRequest(t *testing.T) { - // test serializing a normal request - testReq := createNewSignalWithStartWorkflowExecutionRequest() - serializeRes, err := types.SerializeRequest(testReq) - - expectRes := "{\"domain\":\"testDomain\",\"workflowId\":\"testWorkflowID\",\"workflowType\":{\"name\":\"testWorkflowType\"}," + - "\"taskList\":{\"name\":\"testTaskList\",\"kind\":\"STICKY\"},\"executionStartToCloseTimeoutSeconds\":1," + - "\"taskStartToCloseTimeoutSeconds\":1,\"identity\":\"testIdentity\",\"requestId\":\"DF66E35D-A5B0-425D-8731-6AAC4A4B6368\"," + - "\"workflowIdReusePolicy\":\"AllowDuplicate\",\"signalName\":\"testRequest\",\"control\":\"dGVzdENvbnRyb2w=\"," + - "\"retryPolicy\":{\"initialIntervalInSeconds\":1,\"backoffCoefficient\":1,\"maximumIntervalInSeconds\":1," + - "\"maximumAttempts\":1,\"nonRetriableErrorReasons\":[\"testArray\"],\"expirationIntervalInSeconds\":1}," + - "\"cronSchedule\":\"testSchedule\",\"header\":{},\"delayStartSeconds\":1,\"jitterStartSeconds\":1,\"firstRunAtTimestamp\":1}" - expectErr := error(nil) - - assert.Equal(t, expectRes, serializeRes) - assert.Equal(t, expectErr, err) - - assert.NotPanics(t, func() { - types.SerializeRequest(nil) - }) - -} - -func createNewSignalWithStartWorkflowExecutionRequest() *types.SignalWithStartWorkflowExecutionRequest { - testTasklistKind := types.TaskListKind(1) - testExecutionStartToCloseTimeoutSeconds := int32(1) - testTaskStartToCloseTimeoutSeconds := int32(1) - testWorkflowIDReusePolicy := types.WorkflowIDReusePolicy(1) - testDelayStartSeconds := int32(1) - testJitterStartSeconds := int32(1) - testFirstRunAtTimestamp := int64(1) - piiTestArray := []byte("testInputPII") - piiTestMap := make(map[string][]byte) - piiTestMap["PII"] = piiTestArray - - testReq := &types.SignalWithStartWorkflowExecutionRequest{ - Domain: "testDomain", - WorkflowID: "testWorkflowID", - WorkflowType: &types.WorkflowType{Name: "testWorkflowType"}, - TaskList: &types.TaskList{ - Name: "testTaskList", - Kind: &testTasklistKind, - }, - Input: piiTestArray, - ExecutionStartToCloseTimeoutSeconds: &testExecutionStartToCloseTimeoutSeconds, - TaskStartToCloseTimeoutSeconds: &testTaskStartToCloseTimeoutSeconds, - Identity: "testIdentity", - RequestID: "DF66E35D-A5B0-425D-8731-6AAC4A4B6368", - WorkflowIDReusePolicy: &testWorkflowIDReusePolicy, - SignalName: "testRequest", - SignalInput: piiTestArray, - Control: []byte("testControl"), - RetryPolicy: &types.RetryPolicy{ - InitialIntervalInSeconds: 1, - BackoffCoefficient: 1, - MaximumIntervalInSeconds: 1, - MaximumAttempts: 1, - NonRetriableErrorReasons: []string{"testArray"}, - ExpirationIntervalInSeconds: 1, - }, - CronSchedule: "testSchedule", - Memo: &types.Memo{Fields: piiTestMap}, - SearchAttributes: &types.SearchAttributes{IndexedFields: piiTestMap}, - Header: &types.Header{Fields: map[string][]byte{}}, - DelayStartSeconds: &testDelayStartSeconds, - JitterStartSeconds: &testJitterStartSeconds, - FirstRunAtTimestamp: &testFirstRunAtTimestamp, - } - return testReq -} - -func TestDataBlobDeepCopy(t *testing.T) { - tests := []struct { - name string - input *types.DataBlob - }{ - { - name: "nil", - input: nil, - }, - { - name: "empty", - input: &types.DataBlob{}, - }, - { - name: "thrift ok", - input: &types.DataBlob{ - EncodingType: types.EncodingTypeThriftRW.Ptr(), - Data: []byte("some thrift data"), - }, - }, - { - name: "json ok", - input: &types.DataBlob{ - EncodingType: types.EncodingTypeJSON.Ptr(), - Data: []byte("some json data"), - }, - }, - } - - for _, tc := range tests { - tc := tc - t.Run(tc.name, func(t *testing.T) { - got := tc.input.DeepCopy() - assert.Equal(t, tc.input, got) - if tc.input != nil && tc.input.Data != nil && identicalByteArray(tc.input.Data, got.Data) { - t.Error("expected DeepCopy to return a new data slice") - } - }) - } -} - -// identicalByteArray returns true if a and b are the same slice, false otherwise. -func identicalByteArray(a, b []byte) bool { - return len(a) == len(b) && unsafe.SliceData(a) == unsafe.SliceData(b) -} From 7c171a9cf0bec0f77b534d0a6e33a3b2019f8a3b Mon Sep 17 00:00:00 2001 From: Jakob Haahr Taankvist Date: Thu, 31 Oct 2024 13:57:25 +0100 Subject: [PATCH 3/5] Moved identicalByteArray back to where it was --- common/types/admin_test.go | 6 ------ common/types/shared_test.go | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/types/admin_test.go b/common/types/admin_test.go index 535b8eb5022..6613e32bd7d 100644 --- a/common/types/admin_test.go +++ b/common/types/admin_test.go @@ -24,7 +24,6 @@ package types import ( "testing" - "unsafe" "github.com/stretchr/testify/assert" ) @@ -619,8 +618,3 @@ func TestGetWorkflowExecutionRawHistoryV2Response_GetVersionHistory(t *testing.T func ptrInt64(i int64) *int64 { return &i } - -// identicalByteArray returns true if a and b are the same slice, false otherwise. -func identicalByteArray(a, b []byte) bool { - return len(a) == len(b) && unsafe.SliceData(a) == unsafe.SliceData(b) -} diff --git a/common/types/shared_test.go b/common/types/shared_test.go index 59abaeb3efa..332a8a3afce 100644 --- a/common/types/shared_test.go +++ b/common/types/shared_test.go @@ -24,6 +24,7 @@ package types import ( "testing" + "unsafe" "github.com/stretchr/testify/assert" ) @@ -68,3 +69,8 @@ func TestDataBlobDeepCopy(t *testing.T) { }) } } + +// identicalByteArray returns true if a and b are the same slice, false otherwise. +func identicalByteArray(a, b []byte) bool { + return len(a) == len(b) && unsafe.SliceData(a) == unsafe.SliceData(b) +} From 48ed4315cffcf0426a741e2af9300477fbfd52f4 Mon Sep 17 00:00:00 2001 From: Jakob Haahr Taankvist Date: Thu, 31 Oct 2024 15:16:56 +0100 Subject: [PATCH 4/5] lint --- common/authorization/authorizer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/authorization/authorizer_test.go b/common/authorization/authorizer_test.go index 4c79790f1ba..e1ceda9fc61 100644 --- a/common/authorization/authorizer_test.go +++ b/common/authorization/authorizer_test.go @@ -26,9 +26,9 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/uber/cadence/common/types" "github.com/uber/cadence/common" + "github.com/uber/cadence/common/types" ) func Test_validatePermission(t *testing.T) { From b4a8a3cde8ad7719f96a48275dccf255f95080b0 Mon Sep 17 00:00:00 2001 From: Jakob Haahr Taankvist Date: Fri, 1 Nov 2024 09:29:55 +0100 Subject: [PATCH 5/5] better tests and added a comment --- common/authorization/authorizer.go | 3 +++ common/authorization/authorizer_test.go | 35 +++++++++++++++++-------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/common/authorization/authorizer.go b/common/authorization/authorizer.go index da35a3635d7..5dc1a0a07bd 100644 --- a/common/authorization/authorizer.go +++ b/common/authorization/authorizer.go @@ -122,6 +122,9 @@ type simpleRequestLogWrapper struct { } func (f *simpleRequestLogWrapper) SerializeForLogging() (string, error) { + // We have to check if the request is a typed nil. In the interface we have to handle typed nils. + // The reflection check is slow but this function is doing json marshalling, so performance + // shouldn't be an issue. if f.request == nil || reflect.ValueOf(f.request).IsNil() { return "", nil } diff --git a/common/authorization/authorizer_test.go b/common/authorization/authorizer_test.go index e1ceda9fc61..225ae305e8a 100644 --- a/common/authorization/authorizer_test.go +++ b/common/authorization/authorizer_test.go @@ -23,6 +23,7 @@ package authorization import ( + "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -99,26 +100,33 @@ func Test_validatePermission(t *testing.T) { func TestSignalWithStartWorkflowExecutionRequestSerializeForLogging(t *testing.T) { tests := map[string]struct { - input *types.SignalWithStartWorkflowExecutionRequest + input interface{} expectedOutput string expectedErrorOutput error }{ "complete request without error": { - input: createNewSignalWithStartWorkflowExecutionRequest(), - expectedOutput: "{\"domain\":\"testDomain\",\"workflowId\":\"testWorkflowID\",\"workflowType\":{\"name\":\"testWorkflowType\"},\"taskList\":{\"name\":\"testTaskList\",\"kind\":\"STICKY\"},\"executionStartToCloseTimeoutSeconds\":1,\"taskStartToCloseTimeoutSeconds\":1,\"identity\":\"testIdentity\",\"requestId\":\"DF66E35D-A5B0-425D-8731-6AAC4A4B6368\",\"workflowIdReusePolicy\":\"AllowDuplicate\",\"signalName\":\"testRequest\",\"control\":\"dGVzdENvbnRyb2w=\",\"retryPolicy\":{\"initialIntervalInSeconds\":1,\"backoffCoefficient\":1,\"maximumIntervalInSeconds\":1,\"maximumAttempts\":1,\"nonRetriableErrorReasons\":[\"testArray\"],\"expirationIntervalInSeconds\":1},\"cronSchedule\":\"testSchedule\",\"header\":{},\"delayStartSeconds\":1,\"jitterStartSeconds\":1,\"firstRunAtTimestamp\":1}", - expectedErrorOutput: nil, + input: createNewSignalWithStartWorkflowExecutionRequest(), + expectedOutput: "{\"domain\":\"testDomain\",\"workflowId\":\"testWorkflowID\",\"workflowType\":{\"name\":\"testWorkflowType\"},\"taskList\":{\"name\":\"testTaskList\",\"kind\":\"STICKY\"},\"executionStartToCloseTimeoutSeconds\":1,\"taskStartToCloseTimeoutSeconds\":1,\"identity\":\"testIdentity\",\"requestId\":\"DF66E35D-A5B0-425D-8731-6AAC4A4B6368\",\"workflowIdReusePolicy\":\"AllowDuplicate\",\"signalName\":\"testRequest\",\"control\":\"dGVzdENvbnRyb2w=\",\"retryPolicy\":{\"initialIntervalInSeconds\":1,\"backoffCoefficient\":1,\"maximumIntervalInSeconds\":1,\"maximumAttempts\":1,\"nonRetriableErrorReasons\":[\"testArray\"],\"expirationIntervalInSeconds\":1},\"cronSchedule\":\"testSchedule\",\"header\":{},\"delayStartSeconds\":1,\"jitterStartSeconds\":1,\"firstRunAtTimestamp\":1}", + }, + + "non marchalable struct should error": { + input: make(chan struct{}), + expectedErrorOutput: &json.UnsupportedTypeError{}, }, "empty request without error": { - input: &types.SignalWithStartWorkflowExecutionRequest{}, - expectedOutput: "{}", - expectedErrorOutput: nil, + input: &types.SignalWithStartWorkflowExecutionRequest{}, + expectedOutput: "{}", + }, + + "typed nil request without error": { + input: (*types.SignalWithStartWorkflowExecutionRequest)(nil), + expectedOutput: "", }, "nil request without error": { - input: nil, - expectedOutput: "", - expectedErrorOutput: nil, + input: nil, + expectedOutput: "", }, } @@ -128,7 +136,12 @@ func TestSignalWithStartWorkflowExecutionRequestSerializeForLogging(t *testing.T wrappedInput := NewFilteredRequestBody(test.input) output, err := wrappedInput.SerializeForLogging() assert.Equal(t, test.expectedOutput, output) - assert.Equal(t, test.expectedErrorOutput, err) + if test.expectedErrorOutput != nil { + assert.ErrorAs(t, err, &test.expectedErrorOutput) + } else { + assert.NoError(t, err) + } + assert.NotContains(t, output, "PII") }) })