Skip to content

Commit

Permalink
Nit: map<struct> instead of map<bool> (#6639)
Browse files Browse the repository at this point in the history
## What changed?
replacing `map[enumspb.ResetReapplyExcludeType]bool` with
`map[enumspb.ResetReapplyExcludeType]struct{}`

## Why?
- Small space saving
- More conventional Go.

## How did you test it?
Existing tests.

## Potential risks
N/A

## Documentation
N/A

## Is hotfix candidate?
No
  • Loading branch information
gow authored Oct 10, 2024
1 parent 4976109 commit c8720d3
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 47 deletions.
24 changes: 12 additions & 12 deletions components/nexusoperations/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (d ScheduledEventDefinition) Apply(root *hsm.Node, event *historypb.History
return err
}

func (d ScheduledEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, _ map[enumspb.ResetReapplyExcludeType]bool) error {
func (d ScheduledEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, _ map[enumspb.ResetReapplyExcludeType]struct{}) error {
// We never cherry pick command events, and instead allow user logic to reschedule those commands.
return hsm.ErrNotCherryPickable
}
Expand All @@ -72,7 +72,7 @@ func (d CancelRequestedEventDefinition) Apply(root *hsm.Node, event *historypb.H
})
}

func (d CancelRequestedEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, _ map[enumspb.ResetReapplyExcludeType]bool) error {
func (d CancelRequestedEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, _ map[enumspb.ResetReapplyExcludeType]struct{}) error {
// We never cherry pick command events, and instead allow user logic to reschedule those commands.
return hsm.ErrNotCherryPickable
}
Expand All @@ -97,8 +97,8 @@ func (d StartedEventDefinition) Apply(root *hsm.Node, event *historypb.HistoryEv
})
}

func (d StartedEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]bool) error {
if excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS] {
func (d StartedEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]struct{}) error {
if _, ok := excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS]; ok {
return hsm.ErrNotCherryPickable
}
return d.Apply(root, event)
Expand All @@ -123,8 +123,8 @@ func (d CompletedEventDefinition) Type() enumspb.EventType {
return enumspb.EVENT_TYPE_NEXUS_OPERATION_COMPLETED
}

func (d CompletedEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]bool) error {
if excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS] {
func (d CompletedEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]struct{}) error {
if _, ok := excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS]; ok {
return hsm.ErrNotCherryPickable
}
return d.Apply(root, event)
Expand All @@ -150,8 +150,8 @@ func (d FailedEventDefinition) Apply(root *hsm.Node, event *historypb.HistoryEve
})
}

func (d FailedEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]bool) error {
if excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS] {
func (d FailedEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]struct{}) error {
if _, ok := excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS]; ok {
return hsm.ErrNotCherryPickable
}
return d.Apply(root, event)
Expand All @@ -176,8 +176,8 @@ func (d CanceledEventDefinition) Apply(root *hsm.Node, event *historypb.HistoryE
})
}

func (d CanceledEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]bool) error {
if excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS] {
func (d CanceledEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]struct{}) error {
if _, ok := excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS]; ok {
return hsm.ErrNotCherryPickable
}
return d.Apply(root, event)
Expand All @@ -201,8 +201,8 @@ func (d TimedOutEventDefinition) Apply(root *hsm.Node, event *historypb.HistoryE
})
}

func (d TimedOutEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]bool) error {
if excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS] {
func (d TimedOutEventDefinition) CherryPick(root *hsm.Node, event *historypb.HistoryEvent, excludeTypes map[enumspb.ResetReapplyExcludeType]struct{}) error {
if _, ok := excludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS]; ok {
return hsm.ErrNotCherryPickable
}
return d.Apply(root, event)
Expand Down
2 changes: 1 addition & 1 deletion components/nexusoperations/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func TestCherryPick(t *testing.T) {
nexusoperations.TimedOutEventDefinition{},
}

excludeNexusOperation := map[enumspb.ResetReapplyExcludeType]bool{enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS: true}
excludeNexusOperation := map[enumspb.ResetReapplyExcludeType]struct{}{enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS: {}}
for _, nexusOperation := range nexusOperations {
err := nexusOperation.CherryPick(node.Parent, &historypb.HistoryEvent{}, excludeNexusOperation)
require.ErrorIs(t, err, hsm.ErrNotCherryPickable, "%T should not be cherrypickable when shouldExcludeNexusEvent=true", nexusOperation)
Expand Down
12 changes: 6 additions & 6 deletions service/history/api/resetworkflow/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,23 +177,23 @@ func Invoke(
func GetResetReapplyExcludeTypes(
excludeTypes []enumspb.ResetReapplyExcludeType,
includeType enumspb.ResetReapplyType,
) map[enumspb.ResetReapplyExcludeType]bool {
) map[enumspb.ResetReapplyExcludeType]struct{} {
// A client who wishes to have reapplication of all supported event types should omit the deprecated
// reset_reapply_type field (since its default value is RESET_REAPPLY_TYPE_ALL_ELIGIBLE).
exclude := map[enumspb.ResetReapplyExcludeType]bool{}
exclude := map[enumspb.ResetReapplyExcludeType]struct{}{}
switch includeType {
case enumspb.RESET_REAPPLY_TYPE_SIGNAL:
// A client sending this value of the deprecated reset_reapply_type field will not have any events other than
// signal reapplied.
exclude[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE] = true
exclude[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE] = struct{}{}
case enumspb.RESET_REAPPLY_TYPE_NONE:
exclude[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL] = true
exclude[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE] = true
exclude[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL] = struct{}{}
exclude[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE] = struct{}{}
case enumspb.RESET_REAPPLY_TYPE_UNSPECIFIED, enumspb.RESET_REAPPLY_TYPE_ALL_ELIGIBLE:
// Do nothing.
}
for _, e := range excludeTypes {
exclude[e] = true
exclude[e] = struct{}{}
}
return exclude
}
24 changes: 12 additions & 12 deletions service/history/api/resetworkflow/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ func TestResetWorkflowSuite(t *testing.T) {
func (s *resetWorkflowSuite) TestGetResetReapplyExcludeTypes() {
// Include all with no exclusions => no exclusions
s.Equal(
map[enums.ResetReapplyExcludeType]bool{},
map[enums.ResetReapplyExcludeType]struct{}{},
GetResetReapplyExcludeTypes(
[]enums.ResetReapplyExcludeType{},
enums.RESET_REAPPLY_TYPE_ALL_ELIGIBLE,
),
)
// Include all with one exclusion => one exclusion (honor exclude in presence of default value of deprecated option)
s.Equal(
map[enums.ResetReapplyExcludeType]bool{enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: true},
map[enums.ResetReapplyExcludeType]struct{}{enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: {}},
GetResetReapplyExcludeTypes(
[]enums.ResetReapplyExcludeType{enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL},
enums.RESET_REAPPLY_TYPE_ALL_ELIGIBLE,
Expand All @@ -60,7 +60,7 @@ func (s *resetWorkflowSuite) TestGetResetReapplyExcludeTypes() {
// Include signal with no exclusions => exclude updates
// (honor non-default value of deprecated option in presence of default value of non-deprecated option)
s.Equal(
map[enums.ResetReapplyExcludeType]bool{enums.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: true},
map[enums.ResetReapplyExcludeType]struct{}{enums.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: {}},
GetResetReapplyExcludeTypes(
[]enums.ResetReapplyExcludeType{},
enums.RESET_REAPPLY_TYPE_SIGNAL,
Expand All @@ -69,9 +69,9 @@ func (s *resetWorkflowSuite) TestGetResetReapplyExcludeTypes() {
// Include signal with exclude signal => include signal means they want to exclude updates, and then the explicit
// exclusion of signal trumps the deprecated inclusion
s.Equal(
map[enums.ResetReapplyExcludeType]bool{
enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: true,
enums.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: true,
map[enums.ResetReapplyExcludeType]struct{}{
enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: {},
enums.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: {},
},
GetResetReapplyExcludeTypes(
[]enums.ResetReapplyExcludeType{enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL},
Expand All @@ -81,9 +81,9 @@ func (s *resetWorkflowSuite) TestGetResetReapplyExcludeTypes() {
// Include none with no exclusions => all excluded
// (honor non-default value of deprecated option in presence of default value of non-deprecated option)
s.Equal(
map[enums.ResetReapplyExcludeType]bool{
enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: true,
enums.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: true,
map[enums.ResetReapplyExcludeType]struct{}{
enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: {},
enums.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: {},
},
GetResetReapplyExcludeTypes(
[]enums.ResetReapplyExcludeType{},
Expand All @@ -92,9 +92,9 @@ func (s *resetWorkflowSuite) TestGetResetReapplyExcludeTypes() {
)
// Include none with exclude signal is all excluded
s.Equal(
map[enums.ResetReapplyExcludeType]bool{
enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: true,
enums.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: true,
map[enums.ResetReapplyExcludeType]struct{}{
enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: {},
enums.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: {},
},
GetResetReapplyExcludeTypes(
[]enums.ResetReapplyExcludeType{enums.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL},
Expand Down
2 changes: 1 addition & 1 deletion service/history/hsm/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ type EventDefinition interface {
// Command events should never be cherry picked as we rely on the workflow to reschedule them.
// Return [ErrNotCherryPickable], [ErrStateMachineNotFound], or [ErrInvalidTransition] to skip cherry picking. Any
// other error is considered fatal and will abort the cherry pick process.
CherryPick(root *Node, event *historypb.HistoryEvent, resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]bool) error
CherryPick(root *Node, event *historypb.HistoryEvent, resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]struct{}) error
}
16 changes: 8 additions & 8 deletions service/history/ndc/workflow_resetter.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ type (
currentWorkflow Workflow,
resetReason string,
additionalReapplyEvents []*historypb.HistoryEvent,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]bool,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]struct{},
) error
}

Expand Down Expand Up @@ -123,7 +123,7 @@ func (r *workflowResetterImpl) ResetWorkflow(
currentWorkflow Workflow,
resetReason string,
additionalReapplyEvents []*historypb.HistoryEvent,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]bool,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]struct{},
) (retError error) {

namespaceEntry, err := r.namespaceRegistry.GetNamespaceByID(namespaceID)
Expand Down Expand Up @@ -580,7 +580,7 @@ func (r *workflowResetterImpl) reapplyContinueAsNewWorkflowEvents(
baseBranchToken []byte,
baseRebuildNextEventID int64,
baseNextEventID int64,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]bool,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]struct{},
) (string, error) {

// TODO change this logic to fetching all workflow [baseWorkflow, currentWorkflow]
Expand Down Expand Up @@ -682,7 +682,7 @@ func (r *workflowResetterImpl) reapplyEventsFromBranch(
firstEventID int64,
nextEventID int64,
branchToken []byte,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]bool,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]struct{},
) (string, error) {

// TODO change this logic to fetching all workflow [baseWorkflow, currentWorkflow]
Expand Down Expand Up @@ -723,7 +723,7 @@ func (r *workflowResetterImpl) reapplyEvents(
ctx context.Context,
mutableState workflow.MutableState,
events []*historypb.HistoryEvent,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]bool,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]struct{},
) ([]*historypb.HistoryEvent, error) {
// When reapplying events during WorkflowReset, we do not check for conflicting update IDs (they are not possible,
// since the workflow was in a consistent state before reset), and we do not perform deduplication (because we never
Expand All @@ -737,7 +737,7 @@ func reapplyEvents(
targetBranchUpdateRegistry update.Registry,
stateMachineRegistry *hsm.Registry,
events []*historypb.HistoryEvent,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]bool,
resetReapplyExcludeTypes map[enumspb.ResetReapplyExcludeType]struct{},
runIdForDeduplication string,
) ([]*historypb.HistoryEvent, error) {
// TODO (dan): This implementation is the result of unifying two previous implementations, one of which did
Expand All @@ -749,8 +749,8 @@ func reapplyEvents(
resource := definition.NewEventReappliedID(runIdForDeduplication, event.GetEventId(), event.GetVersion())
return mutableState.IsResourceDuplicated(resource)
}
excludeSignal := resetReapplyExcludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL]
excludeUpdate := resetReapplyExcludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE]
_, excludeSignal := resetReapplyExcludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL]
_, excludeUpdate := resetReapplyExcludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE]
var reappliedEvents []*historypb.HistoryEvent
for _, event := range events {
switch event.GetEventType() {
Expand Down
2 changes: 1 addition & 1 deletion service/history/ndc/workflow_resetter_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions service/history/ndc/workflow_resetter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -976,10 +976,10 @@ func (s *workflowResetterSuite) TestReapplyEvents_Excludes() {
s.NoError(err)
ms.EXPECT().HSM().Return(root).AnyTimes()

excludes := map[enumspb.ResetReapplyExcludeType]bool{
enumspb.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: true,
enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: true,
enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS: true,
excludes := map[enumspb.ResetReapplyExcludeType]struct{}{
enumspb.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL: {},
enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE: {},
enumspb.RESET_REAPPLY_EXCLUDE_TYPE_NEXUS: {},
}
reappliedEvents, err := reapplyEvents(context.Background(), ms, nil, smReg, events, excludes, "")
s.Empty(reappliedEvents)
Expand Down
4 changes: 2 additions & 2 deletions tests/reset_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,8 @@ func (t *resetTest) run() {
events := t.GetHistory(t.Namespace(), t.tv.WorkflowExecution())

resetReapplyExcludeTypes := resetworkflow.GetResetReapplyExcludeTypes(t.reapplyExcludeTypes, t.reapplyType)
signals := !resetReapplyExcludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL]
updates := !resetReapplyExcludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE]
_, signals := resetReapplyExcludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_SIGNAL]
_, updates := resetReapplyExcludeTypes[enumspb.RESET_REAPPLY_EXCLUDE_TYPE_UPDATE]

if !signals && !updates {
t.EqualHistoryEvents(`
Expand Down

0 comments on commit c8720d3

Please sign in to comment.