internal: simplify ActionType: make it a string; shorter constants (#4214)

## Why
Simpler. Avoid conversion back and forth.

Author: denik

bundle/deploy/terraform/showplanfile.go

@@ -72,15 +72,15 @@ func populatePlan(ctx context.Context, plan *deployplan.Plan, changes []*tfjson.
 		var actionType deployplan.ActionType
 		switch {
 		case rc.Change.Actions.Delete():
-			actionType = deployplan.ActionTypeDelete
+			actionType = deployplan.Delete
 		case rc.Change.Actions.Replace():
-			actionType = deployplan.ActionTypeRecreate
+			actionType = deployplan.Recreate
 		case rc.Change.Actions.Create():
-			actionType = deployplan.ActionTypeCreate
+			actionType = deployplan.Create
 		case rc.Change.Actions.Update():
-			actionType = deployplan.ActionTypeUpdate
+			actionType = deployplan.Update
 		case rc.Change.Actions.NoOp():
-			actionType = deployplan.ActionTypeSkip
+			actionType = deployplan.Skip
 		default:
 			continue
 		}
@@ -105,7 +105,7 @@ func populatePlan(ctx context.Context, plan *deployplan.Plan, changes []*tfjson.
 			key = "resources." + group + "." + rc.Name
 		}
 
-		plan.Plan[key] = &deployplan.PlanEntry{Action: actionType.String()}
+		plan.Plan[key] = &deployplan.PlanEntry{Action: actionType}
 	}
 }
 

bundle/deploy/terraform/showplanfile_test.go

@@ -50,29 +50,29 @@ func TestPopulatePlan(t *testing.T) {
 	// Assert that the actions list contains all expected actions
 	expectedActions := []deployplan.Action{
 		{
-			ActionType:  deployplan.ActionTypeCreate,
+			ActionType:  deployplan.Create,
 			ResourceKey: "resources.pipelines.create pipeline",
 		},
 		{
-			ActionType:  deployplan.ActionTypeDelete,
+			ActionType:  deployplan.Delete,
 			ResourceKey: "resources.pipelines.delete pipeline",
 		},
 		{
-			ActionType:  deployplan.ActionTypeRecreate,
+			ActionType:  deployplan.Recreate,
 			ResourceKey: "resources.pipelines.recreate pipeline",
 		},
 	}
 	assert.Equal(t, expectedActions, actions)
 
 	// Also test that the plan was populated correctly with expected entries
 	assert.Contains(t, plan.Plan, "resources.pipelines.create pipeline")
-	assert.Equal(t, "create", plan.Plan["resources.pipelines.create pipeline"].Action)
+	assert.Equal(t, deployplan.Create, plan.Plan["resources.pipelines.create pipeline"].Action)
 
 	assert.Contains(t, plan.Plan, "resources.pipelines.delete pipeline")
-	assert.Equal(t, "delete", plan.Plan["resources.pipelines.delete pipeline"].Action)
+	assert.Equal(t, deployplan.Delete, plan.Plan["resources.pipelines.delete pipeline"].Action)
 
 	assert.Contains(t, plan.Plan, "resources.pipelines.recreate pipeline")
-	assert.Equal(t, "recreate", plan.Plan["resources.pipelines.recreate pipeline"].Action)
+	assert.Equal(t, deployplan.Recreate, plan.Plan["resources.pipelines.recreate pipeline"].Action)
 
 	// Unknown resource type should not be in the plan
 	assert.NotContains(t, plan.Plan, "resources.recreate whatever")

bundle/deployplan/action.go

@@ -22,57 +22,36 @@ func (a Action) IsChildResource() bool {
 	return len(items) == 4
 }
 
-type ActionType int
+type ActionType string
 
 // Actions are ordered in increasing severity.
 // If case of several options, action with highest severity wins.
 // Note, Create/Delete are handled explicitly and never compared.
 const (
-	ActionTypeUndefined ActionType = iota
-	ActionTypeSkip
-	ActionTypeResize
-	ActionTypeUpdate
-	ActionTypeUpdateWithID
-	ActionTypeCreate
-	ActionTypeRecreate
-	ActionTypeDelete
+	Undefined    ActionType = ""
+	Skip         ActionType = "skip"
+	Resize       ActionType = "resize"
+	Update       ActionType = "update"
+	UpdateWithID ActionType = "update_id"
+	Create       ActionType = "create"
+	Recreate     ActionType = "recreate"
+	Delete       ActionType = "delete"
 )
 
-const (
-	ActionTypeUndefinedString    = ""
-	ActionTypeSkipString         = "skip"
-	ActionTypeResizeString       = "resize"
-	ActionTypeUpdateString       = "update"
-	ActionTypeUpdateWithIDString = "update_id"
-	ActionTypeCreateString       = "create"
-	ActionTypeRecreateString     = "recreate"
-	ActionTypeDeleteString       = "delete"
-)
-
-var actionName = map[ActionType]string{
-	ActionTypeSkip:         ActionTypeSkipString,
-	ActionTypeResize:       ActionTypeResizeString,
-	ActionTypeUpdate:       ActionTypeUpdateString,
-	ActionTypeUpdateWithID: ActionTypeUpdateWithIDString,
-	ActionTypeCreate:       ActionTypeCreateString,
-	ActionTypeRecreate:     ActionTypeRecreateString,
-	ActionTypeDelete:       ActionTypeDeleteString,
-}
-
-var nameToAction = map[string]ActionType{}
-
-func init() {
-	for k, v := range actionName {
-		if _, ok := nameToAction[v]; ok {
-			panic("duplicate action string: " + v)
-		}
-		nameToAction[v] = k
-	}
+var actionOrder = map[ActionType]int{
+	Undefined:    0,
+	Skip:         1,
+	Resize:       2,
+	Update:       3,
+	UpdateWithID: 4,
+	Create:       5,
+	Recreate:     6,
+	Delete:       7,
 }
 
 func (a ActionType) KeepsID() bool {
 	switch a {
-	case ActionTypeCreate, ActionTypeUpdateWithID, ActionTypeRecreate, ActionTypeDelete:
+	case Create, UpdateWithID, Recreate, Delete:
 		return false
 	default:
 		return true
@@ -81,30 +60,15 @@ func (a ActionType) KeepsID() bool {
 
 // StringShort short version of action string, without suffix
 func (a ActionType) StringShort() string {
-	items := strings.SplitN(actionName[a], "_", 2)
+	items := strings.SplitN(string(a), "_", 2)
 	return items[0]
 }
 
-// String returns the string representation of the action type.
-func (a ActionType) String() string {
-	return actionName[a]
-}
-
-func ActionTypeFromString(s string) ActionType {
-	actionType, ok := nameToAction[s]
-	if !ok {
-		return ActionTypeUndefined
-	}
-	return actionType
-}
-
-// Filter returns actions that match the specified action type
-func Filter(changes []Action, actionType ActionType) []Action {
-	var result []Action
-	for _, action := range changes {
-		if action.ActionType == actionType {
-			result = append(result, action)
-		}
+// GetHigherAction returns the action with higher severity between a and b.
+// Actions are ordered by severity in actionOrder map.
+func GetHigherAction(a, b ActionType) ActionType {
+	if actionOrder[a] > actionOrder[b] {
+		return a
 	}
-	return result
+	return b
 }

bundle/deployplan/action_test.go

@@ -65,7 +65,7 @@ func LoadPlanFromFile(path string) (*Plan, error) {
 type PlanEntry struct {
 	ID          string                   `json:"id,omitempty"`
 	DependsOn   []DependsOnEntry         `json:"depends_on,omitempty"`
-	Action      string                   `json:"action,omitempty"`
+	Action      ActionType               `json:"action,omitempty"`
 	NewState    *structvar.StructVarJSON `json:"new_state,omitempty"`
 	RemoteState any                      `json:"remote_state,omitempty"`
 	Changes     Changes                  `json:"changes,omitempty"`
@@ -79,11 +79,11 @@ type DependsOnEntry struct {
 type Changes map[string]*ChangeDesc
 
 type ChangeDesc struct {
-	Action string `json:"action"`
-	Reason string `json:"reason,omitempty"`
-	Old    any    `json:"old,omitempty"`
-	New    any    `json:"new,omitempty"`
-	Remote any    `json:"remote,omitempty"`
+	Action ActionType `json:"action"`
+	Reason string     `json:"reason,omitempty"`
+	Old    any        `json:"old,omitempty"`
+	New    any        `json:"new,omitempty"`
+	Remote any        `json:"remote,omitempty"`
 }
 
 // Possible values for Reason field
@@ -118,10 +118,9 @@ func (c *Changes) HasChange(fieldPath string) bool {
 func (p *Plan) GetActions() []Action {
 	actions := make([]Action, 0, len(p.Plan))
 	for key, entry := range p.Plan {
-		at := ActionTypeFromString(entry.Action)
 		actions = append(actions, Action{
 			ResourceKey: key,
-			ActionType:  at,
+			ActionType:  entry.Action,
 		})
 	}
 

bundle/deployplan/plan.go

@@ -28,7 +28,7 @@ func (d *DeploymentUnit) Destroy(ctx context.Context, db *dstate.DeploymentState
 }
 
 func (d *DeploymentUnit) Deploy(ctx context.Context, db *dstate.DeploymentState, newState any, actionType deployplan.ActionType, changes deployplan.Changes) error {
-	if actionType == deployplan.ActionTypeCreate {
+	if actionType == deployplan.Create {
 		return d.Create(ctx, db, newState)
 	}
 
@@ -43,13 +43,13 @@ func (d *DeploymentUnit) Deploy(ctx context.Context, db *dstate.DeploymentState,
 	}
 
 	switch actionType {
-	case deployplan.ActionTypeRecreate:
+	case deployplan.Recreate:
 		return d.Recreate(ctx, db, oldID, newState)
-	case deployplan.ActionTypeUpdate:
+	case deployplan.Update:
 		return d.Update(ctx, db, oldID, newState, changes)
-	case deployplan.ActionTypeUpdateWithID:
+	case deployplan.UpdateWithID:
 		return d.UpdateWithID(ctx, db, oldID, newState)
-	case deployplan.ActionTypeResize:
+	case deployplan.Resize:
 		return d.Resize(ctx, db, oldID, newState)
 	default:
 		return fmt.Errorf("internal error: unexpected actionType: %#v", actionType)

bundle/direct/apply.go

@@ -55,15 +55,14 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
 			errorPrefix = "cannot migrate " + resourceKey
 		}
 
-		at := deployplan.ActionTypeFromString(action)
-		if at == deployplan.ActionTypeUndefined {
+		if action == deployplan.Undefined {
 			logdiag.LogError(ctx, fmt.Errorf("cannot deploy %s: unknown action %q", resourceKey, action))
 			return false
 		}
 
 		// If a dependency failed, report and skip execution for this node by returning false
 		if failedDependency != nil {
-			if at != deployplan.ActionTypeSkip {
+			if action != deployplan.Skip {
 				logdiag.LogError(ctx, fmt.Errorf("%s: dependency failed: %s", errorPrefix, *failedDependency))
 			}
 			return false
@@ -81,7 +80,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
 			DependsOn:   entry.DependsOn,
 		}
 
-		if at == deployplan.ActionTypeDelete {
+		if action == deployplan.Delete {
 			if migrateMode {
 				logdiag.LogError(ctx, fmt.Errorf("%s: Unexpected delete action during migration", errorPrefix))
 				return false
@@ -96,7 +95,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
 
 		// We don't keep NewState around for 'skip' nodes
 
-		if at != deployplan.ActionTypeSkip {
+		if action != deployplan.Skip {
 			if !b.resolveReferences(ctx, resourceKey, entry, errorPrefix, false) {
 				return false
 			}
@@ -123,7 +122,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
 				err = b.StateDB.SaveState(resourceKey, dbentry.ID, sv.Value, entry.DependsOn)
 			} else {
 				// TODO: redo calcDiff to downgrade planned action if possible (?)
-				err = d.Deploy(ctx, &b.StateDB, sv.Value, at, entry.Changes)
+				err = d.Deploy(ctx, &b.StateDB, sv.Value, action, entry.Changes)
 			}
 
 			if err != nil {
@@ -178,8 +177,8 @@ func (b *DeploymentBundle) LookupReferenceRemote(ctx context.Context, path *stru
 
 	defer b.Plan.ReadUnlockEntry(targetResourceKey)
 
-	targetAction := deployplan.ActionTypeFromString(targetEntry.Action)
-	if targetAction == deployplan.ActionTypeUndefined {
+	targetAction := targetEntry.Action
+	if targetAction == deployplan.Undefined {
 		return nil, fmt.Errorf("internal error: %s: missing action in the plan", targetResourceKey)
 	}