From 3bb39206703bb1f17ff1893ae9567a75c394d9d6 Mon Sep 17 00:00:00 2001 From: lwolczynski Date: Thu, 19 Sep 2024 17:49:49 -0500 Subject: [PATCH 1/3] IWF-103: Require every commandId to exist for ANY_COMMAND_COMBINATION_COMPLETED --- service/interpreter/activityImpl.go | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/service/interpreter/activityImpl.go b/service/interpreter/activityImpl.go index 2fa1855b..6930500f 100644 --- a/service/interpreter/activityImpl.go +++ b/service/interpreter/activityImpl.go @@ -13,6 +13,7 @@ import ( "io" "net/http" "os" + "slices" ) // StateStart is Deprecated, will be removed in next release @@ -224,12 +225,42 @@ func checkCommandRequestFromWaitUntilResponse(resp *iwfidl.WorkflowStateStartRes return err } } + // Check if each command in the combinations has a matching command in one of the lists + if !areAllCommandCombinationsIdsValid(commandReq) { + return fmt.Errorf("ANY_COMMAND_COMBINATION_COMPLETED can only be used when every command has an commandId that is found in TimerCommands, SignalCommands or InterStateChannelCommands") + } } } // NOTE: we don't require decider trigger type when there is no commands return nil } +func areAllCommandCombinationsIdsValid(commandReq *iwfidl.CommandRequest) bool { + timerSignalInterStateChannelCmdIds := listTimerSignalInterStateChannelCommandIds(commandReq) + for _, commandCombo := range commandReq.GetCommandCombinations() { + for _, cmdId := range commandCombo.GetCommandIds() { + if !slices.Contains(timerSignalInterStateChannelCmdIds, cmdId) { + return false + } + } + } + return true +} + +func listTimerSignalInterStateChannelCommandIds(commandReq *iwfidl.CommandRequest) []string { + var ids []string + for _, timerCmd := range commandReq.GetTimerCommands() { + ids = append(ids, timerCmd.GetCommandId()) + } + for _, signalCmd := range commandReq.GetSignalCommands() { + ids = append(ids, signalCmd.GetCommandId()) + } + for _, interStateChannelCmd := range commandReq.GetInterStateChannelCommands() { + ids = append(ids, interStateChannelCmd.GetCommandId()) + } + return ids +} + func DumpWorkflowInternal( ctx context.Context, backendType service.BackendType, req iwfidl.WorkflowDumpRequest, ) (*iwfidl.WorkflowDumpResponse, error) { From 68c13a38a61375e7369694efa5054008b9c7217c Mon Sep 17 00:00:00 2001 From: lwolczynski Date: Fri, 20 Sep 2024 10:43:40 -0500 Subject: [PATCH 2/3] IWF-103: Rename variables --- service/interpreter/activityImpl.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/service/interpreter/activityImpl.go b/service/interpreter/activityImpl.go index 6930500f..bd7a38e7 100644 --- a/service/interpreter/activityImpl.go +++ b/service/interpreter/activityImpl.go @@ -227,7 +227,7 @@ func checkCommandRequestFromWaitUntilResponse(resp *iwfidl.WorkflowStateStartRes } // Check if each command in the combinations has a matching command in one of the lists if !areAllCommandCombinationsIdsValid(commandReq) { - return fmt.Errorf("ANY_COMMAND_COMBINATION_COMPLETED can only be used when every command has an commandId that is found in TimerCommands, SignalCommands or InterStateChannelCommands") + return fmt.Errorf("ANY_COMMAND_COMBINATION_COMPLETED can only be used when every command has an commandId that is found in TimerCommands, SignalCommands or InternalChannelCommand") } } } @@ -236,10 +236,10 @@ func checkCommandRequestFromWaitUntilResponse(resp *iwfidl.WorkflowStateStartRes } func areAllCommandCombinationsIdsValid(commandReq *iwfidl.CommandRequest) bool { - timerSignalInterStateChannelCmdIds := listTimerSignalInterStateChannelCommandIds(commandReq) + timerSignalInternalChannelCmdIds := listTimerSignalInternalChannelCommandIds(commandReq) for _, commandCombo := range commandReq.GetCommandCombinations() { for _, cmdId := range commandCombo.GetCommandIds() { - if !slices.Contains(timerSignalInterStateChannelCmdIds, cmdId) { + if !slices.Contains(timerSignalInternalChannelCmdIds, cmdId) { return false } } @@ -247,7 +247,7 @@ func areAllCommandCombinationsIdsValid(commandReq *iwfidl.CommandRequest) bool { return true } -func listTimerSignalInterStateChannelCommandIds(commandReq *iwfidl.CommandRequest) []string { +func listTimerSignalInternalChannelCommandIds(commandReq *iwfidl.CommandRequest) []string { var ids []string for _, timerCmd := range commandReq.GetTimerCommands() { ids = append(ids, timerCmd.GetCommandId()) @@ -255,8 +255,8 @@ func listTimerSignalInterStateChannelCommandIds(commandReq *iwfidl.CommandReques for _, signalCmd := range commandReq.GetSignalCommands() { ids = append(ids, signalCmd.GetCommandId()) } - for _, interStateChannelCmd := range commandReq.GetInterStateChannelCommands() { - ids = append(ids, interStateChannelCmd.GetCommandId()) + for _, internalChannelCmd := range commandReq.GetInterStateChannelCommands() { + ids = append(ids, internalChannelCmd.GetCommandId()) } return ids } From 2a0f9779b44067298e4028f9307e7be74ed5705e Mon Sep 17 00:00:00 2001 From: lwolczynski Date: Fri, 20 Sep 2024 15:14:46 -0500 Subject: [PATCH 3/3] IWF-103: Add unit tests --- service/interpreter/activityImpl_test.go | 90 ++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 service/interpreter/activityImpl_test.go diff --git a/service/interpreter/activityImpl_test.go b/service/interpreter/activityImpl_test.go new file mode 100644 index 00000000..e0451b04 --- /dev/null +++ b/service/interpreter/activityImpl_test.go @@ -0,0 +1,90 @@ +package interpreter + +import ( + "github.com/indeedeng/iwf/gen/iwfidl" + "github.com/indeedeng/iwf/service/common/ptr" + "github.com/stretchr/testify/assert" + "testing" + "time" +) + +func TestInvalidAnyCommandCombination(t *testing.T) { + validTimerCommands, validSignalCommands, internalCommands := createCommands() + + resp := iwfidl.WorkflowStateStartResponse{ + CommandRequest: &iwfidl.CommandRequest{ + SignalCommands: validSignalCommands, + TimerCommands: validTimerCommands, + InterStateChannelCommands: internalCommands, + DeciderTriggerType: iwfidl.ANY_COMMAND_COMBINATION_COMPLETED.Ptr(), + CommandCombinations: []iwfidl.CommandCombination{ + { + CommandIds: []string{ + "timer-cmd1", "signal-cmd1", + }, + }, + { + CommandIds: []string{ + "timer-cmd1", "invalid", + }, + }, + }, + }, + } + + err := checkCommandRequestFromWaitUntilResponse(&resp) + + assert.Error(t, err) + assert.Equal(t, err.Error(), "ANY_COMMAND_COMBINATION_COMPLETED can only be used when every command has an commandId that is found in TimerCommands, SignalCommands or InternalChannelCommand") +} + +func TestValidAnyCommandCombination(t *testing.T) { + validTimerCommands, validSignalCommands, internalCommands := createCommands() + + resp := iwfidl.WorkflowStateStartResponse{ + CommandRequest: &iwfidl.CommandRequest{ + SignalCommands: validSignalCommands, + TimerCommands: validTimerCommands, + InterStateChannelCommands: internalCommands, + DeciderTriggerType: iwfidl.ANY_COMMAND_COMBINATION_COMPLETED.Ptr(), + CommandCombinations: []iwfidl.CommandCombination{ + { + CommandIds: []string{ + "timer-cmd1", "signal-cmd1", + }, + }, + { + CommandIds: []string{ + "timer-cmd1", "internal-cmd1", + }, + }, + }, + }, + } + + err := checkCommandRequestFromWaitUntilResponse(&resp) + + assert.NoError(t, err) +} + +func createCommands() ([]iwfidl.TimerCommand, []iwfidl.SignalCommand, []iwfidl.InterStateChannelCommand) { + validTimerCommands := []iwfidl.TimerCommand{ + { + CommandId: ptr.Any("timer-cmd1"), + FiringUnixTimestampSeconds: iwfidl.PtrInt64(time.Now().Unix() + 86400*365), // one year later + }, + } + validSignalCommands := []iwfidl.SignalCommand{ + { + CommandId: ptr.Any("signal-cmd1"), + SignalChannelName: "test-signal-name1", + }, + } + internalCommands := []iwfidl.InterStateChannelCommand{ + { + CommandId: ptr.Any("internal-cmd1"), + ChannelName: "test-internal-name1", + }, + } + return validTimerCommands, validSignalCommands, internalCommands +}