Fix ValidateUpdateWorkflowStateStatus() added more tests

Author: gow

common/persistence/workflow_state_status_validator.go

@@ -67,9 +67,19 @@ func ValidateUpdateWorkflowStateStatus(
 	}
 
 	// validate workflow state & status
-	if (state == enumsspb.WORKFLOW_EXECUTION_STATE_COMPLETED && status == enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING) ||
-		(state != enumsspb.WORKFLOW_EXECUTION_STATE_COMPLETED && (status != enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING && status != enumspb.WORKFLOW_EXECUTION_STATUS_PAUSED)) {
-		return serviceerror.NewInternalf("Update workflow with invalid state: %v or status: %v", state, status)
+	switch state {
+	case enumsspb.WORKFLOW_EXECUTION_STATE_RUNNING, enumsspb.WORKFLOW_EXECUTION_STATE_ZOMBIE:
+		if status != enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING && status != enumspb.WORKFLOW_EXECUTION_STATUS_PAUSED {
+			return serviceerror.NewInternalf("Update workflow with invalid state: %v or status: %v", state, status)
+		}
+	case enumsspb.WORKFLOW_EXECUTION_STATE_CREATED:
+		if status != enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING {
+			return serviceerror.NewInternalf("Update workflow with invalid state: %v or status: %v", state, status)
+		}
+	default:
+		if status == enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING || status == enumspb.WORKFLOW_EXECUTION_STATUS_PAUSED {
+			return serviceerror.NewInternalf("Update workflow with invalid state: %v or status: %v", state, status)
+		}
 	}
 	return nil
 }

common/persistence/workflow_state_status_validator_test.go

@@ -111,24 +111,26 @@ func (s *workflowStateStatusSuite) TestCreateWorkflowStateStatus_WorkflowStateZo
 }
 
 func (s *workflowStateStatusSuite) TestUpdateWorkflowStateStatus_WorkflowStateCreated() {
-	statuses := []enumspb.WorkflowExecutionStatus{
+	disallowedStatuses := []enumspb.WorkflowExecutionStatus{
 		enumspb.WORKFLOW_EXECUTION_STATUS_COMPLETED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_FAILED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_CANCELED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_TERMINATED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_CONTINUED_AS_NEW,
 		enumspb.WORKFLOW_EXECUTION_STATUS_TIMED_OUT,
+		enumspb.WORKFLOW_EXECUTION_STATUS_PAUSED,
 	}
 
+	// Updating workflow to State: CREATED and status: RUNNING is allowed
 	s.NoError(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_CREATED, enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING))
 
-	for _, status := range statuses {
+	for _, status := range disallowedStatuses {
 		s.Error(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_CREATED, status))
 	}
 }
 
 func (s *workflowStateStatusSuite) TestUpdateWorkflowStateStatus_WorkflowStateRunning() {
-	statuses := []enumspb.WorkflowExecutionStatus{
+	disallowedStatuses := []enumspb.WorkflowExecutionStatus{
 		enumspb.WORKFLOW_EXECUTION_STATUS_COMPLETED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_FAILED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_CANCELED,
@@ -137,15 +139,17 @@ func (s *workflowStateStatusSuite) TestUpdateWorkflowStateStatus_WorkflowStateRu
 		enumspb.WORKFLOW_EXECUTION_STATUS_TIMED_OUT,
 	}
 
+	// Updating workflow to State: RUNNING and status: {RUNNING, PAUSED} is allowed
 	s.NoError(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_RUNNING, enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING))
+	s.NoError(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_RUNNING, enumspb.WORKFLOW_EXECUTION_STATUS_PAUSED))
 
-	for _, status := range statuses {
+	for _, status := range disallowedStatuses {
 		s.Error(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_RUNNING, status))
 	}
 }
 
 func (s *workflowStateStatusSuite) TestUpdateWorkflowStateStatus_WorkflowStateCompleted() {
-	statuses := []enumspb.WorkflowExecutionStatus{
+	allowedStatuses := []enumspb.WorkflowExecutionStatus{
 		enumspb.WORKFLOW_EXECUTION_STATUS_COMPLETED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_FAILED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_CANCELED,
@@ -154,15 +158,17 @@ func (s *workflowStateStatusSuite) TestUpdateWorkflowStateStatus_WorkflowStateCo
 		enumspb.WORKFLOW_EXECUTION_STATUS_TIMED_OUT,
 	}
 
+	// Updating workflow to State: COMPLETED and status: {RUNNING, PAUSED} is *not* allowed
 	s.Error(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_COMPLETED, enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING))
+	s.Error(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_COMPLETED, enumspb.WORKFLOW_EXECUTION_STATUS_PAUSED))
 
-	for _, status := range statuses {
+	for _, status := range allowedStatuses {
 		s.NoError(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_COMPLETED, status))
 	}
 }
 
 func (s *workflowStateStatusSuite) TestUpdateWorkflowStateStatus_WorkflowStateZombie() {
-	statuses := []enumspb.WorkflowExecutionStatus{
+	disallowedStatuses := []enumspb.WorkflowExecutionStatus{
 		enumspb.WORKFLOW_EXECUTION_STATUS_COMPLETED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_FAILED,
 		enumspb.WORKFLOW_EXECUTION_STATUS_CANCELED,
@@ -171,9 +177,11 @@ func (s *workflowStateStatusSuite) TestUpdateWorkflowStateStatus_WorkflowStateZo
 		enumspb.WORKFLOW_EXECUTION_STATUS_TIMED_OUT,
 	}
 
+	// Updating workflow to State: ZOMBIE and status: {RUNNING, PAUSED} is allowed
 	s.NoError(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_ZOMBIE, enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING))
+	s.NoError(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_ZOMBIE, enumspb.WORKFLOW_EXECUTION_STATUS_PAUSED))
 
-	for _, status := range statuses {
+	for _, status := range disallowedStatuses {
 		s.Error(ValidateUpdateWorkflowStateStatus(enumsspb.WORKFLOW_EXECUTION_STATE_ZOMBIE, status))
 	}
 }

service/history/workflow/workflow_task_state_machine.go

@@ -1028,7 +1028,6 @@ func (m *workflowTaskStateMachine) UpdateWorkflowTask(
 
 	// NOTE:
 	// - do not update executionInfo.TaskQueue!
-	// - do not update executionInfo.WorkflowTaskStamp (only changed when rescheduling workflow task)
 
 	m.ms.logger.Debug("Workflow task updated",
 		tag.WorkflowScheduledEventID(workflowTask.ScheduledEventID),