Merge pull request #30 from Betterment/no_undo_idempotent_migrate

No undo idempotent migrate

Makefile

@@ -1,6 +1,6 @@
 SHELL = /bin/sh
 
-VERSION=0.9.2
+VERSION=0.9.3
 BUILD=`git rev-parse HEAD`
 
 LDFLAGS=-ldflags "-w -s \

cmds/create_remote_kill.go

@@ -68,7 +68,7 @@ func createRemoteKill(split, reason, overrideTo, firstBadVersion, fixedVersion *
 		return err
 	}
 
-	// These validations are the difference between remote_kill and unset_remote_kill which is why they're inline
+	// These validations are the difference between create and destroy remote_kill which is why they're inline
 	err = validations.Presence("override_to", overrideTo)
 	if err != nil {
 		return err

cmds/destroy_remote_kill.go

@@ -9,12 +9,12 @@ import (
 )
 
 var destroyRemoteKillDoc = `
-Unsets a remote kill, allowing users of affected apps to see whatever variant
+Destroys a remote kill, allowing users of affected apps to see whatever variant
 of the split they would otherwise see.
 
 Example:
 
-testtrack unset_remote_kill my_fancy_experiment
+testtrack destroy remote_kill my_fancy_experiment
 `
 
 func init() {

featurecompletions/featurecompletions.go

@@ -102,26 +102,18 @@ func (f *FeatureCompletion) SameResourceAs(other migrations.IMigration) bool {
 	return false
 }
 
-// Inverse returns a logical inverse operation if possible
-func (f *FeatureCompletion) Inverse() (migrations.IMigration, error) {
-	if f.version == nil {
-		return nil, fmt.Errorf("can't invert feature_completion destroy %s for %s", *f.migrationVersion, *f.featureGate)
-	}
-	return &FeatureCompletion{
-		featureGate: f.featureGate,
-		version:     nil,
-	}, nil
-}
-
 // ApplyToSchema applies a migrations changes to in-memory schema representation
-func (f *FeatureCompletion) ApplyToSchema(schema *serializers.Schema, _ migrations.Repository) error {
+func (f *FeatureCompletion) ApplyToSchema(schema *serializers.Schema, _ migrations.Repository, idempotently bool) error {
 	if f.version == nil { // Delete
 		for i, candidate := range schema.FeatureCompletions {
 			if candidate.FeatureGate == *f.featureGate {
 				schema.FeatureCompletions = append(schema.FeatureCompletions[:i], schema.FeatureCompletions[i+1:]...)
 				return nil
 			}
 		}
+		if idempotently {
+			return nil
+		}
 		return fmt.Errorf("Couldn't locate feature_completion of %s in schema", *f.featureGate)
 	}
 	for i, candidate := range schema.FeatureCompletions { // Replace

identifiertypes/identifiertypes.go

@@ -88,15 +88,13 @@ func (i *IdentifierType) SameResourceAs(other migrations.IMigration) bool {
 	return false
 }
 
-// Inverse returns a logical inverse operation if possible
-func (i *IdentifierType) Inverse() (migrations.IMigration, error) {
-	return nil, fmt.Errorf("can't invert identifier_type creation %s %s", *i.migrationVersion, *i.name)
-}
-
 // ApplyToSchema applies a migrations changes to in-memory schema representation
-func (i *IdentifierType) ApplyToSchema(schema *serializers.Schema, _ migrations.Repository) error {
+func (i *IdentifierType) ApplyToSchema(schema *serializers.Schema, _ migrations.Repository, idempotently bool) error {
 	for _, candidate := range schema.IdentifierTypes {
 		if candidate.Name == *i.name {
+			if idempotently {
+				return nil
+			}
 			return fmt.Errorf("identifier_type %s already exists", *i.name)
 		}
 	}

migrationmanagers/migrationmanagers.go

@@ -61,7 +61,7 @@ func (m *MigrationManager) Save() error {
 	if err != nil {
 		return err
 	}
-	err = m.ApplyToSchema(migrationRepo)
+	err = m.ApplyToSchema(migrationRepo, false)
 	if err != nil {
 		return err
 	}
@@ -83,13 +83,13 @@ func (m *MigrationManager) Run(migrationRepo migrations.Repository) error {
 }
 
 // ApplyToSchema validates and applies a migration to the in-memory schema representation
-func (m *MigrationManager) ApplyToSchema(migrationRepo migrations.Repository) error {
+func (m *MigrationManager) ApplyToSchema(migrationRepo migrations.Repository, idempotently bool) error {
 	err := m.migration.Validate()
 	if err != nil {
 		return err
 	}
 
-	err = m.migration.ApplyToSchema(m.schema, migrationRepo)
+	err = m.migration.ApplyToSchema(m.schema, migrationRepo, idempotently)
 	if err != nil {
 		return err
 	}
@@ -101,15 +101,15 @@ func (m *MigrationManager) ApplyToSchema(migrationRepo migrations.Repository) er
 	return nil
 }
 
-// Apply applies a migration to the TestTrack server and in-memory schema
-// without recording the version to TestTrack server
+// Apply idempotently applies a migration to the TestTrack server and in-memory
+// schema without recording the version to TestTrack server
 func (m *MigrationManager) Apply(migrationRepo migrations.Repository) error {
 	err := m.migration.Validate()
 	if err != nil {
 		return err
 	}
 
-	err = m.migration.ApplyToSchema(m.schema, migrationRepo)
+	err = m.migration.ApplyToSchema(m.schema, migrationRepo, true)
 	if err != nil {
 		return err
 	}

migrationrunners/migrationrunners.go

@@ -1,17 +1,11 @@
 package migrationrunners
 
 import (
-	"fmt"
-	"os"
-	"path/filepath"
-
 	"github.com/Betterment/testtrack-cli/migrationloaders"
 	"github.com/Betterment/testtrack-cli/migrationmanagers"
-	"github.com/Betterment/testtrack-cli/migrations"
 	"github.com/Betterment/testtrack-cli/schema"
 	"github.com/Betterment/testtrack-cli/serializers"
 	"github.com/Betterment/testtrack-cli/servers"
-	"github.com/pkg/errors"
 )
 
 // Runner runs sets of migrations
@@ -59,74 +53,6 @@ func (r *Runner) RunOutstanding() error {
 	return nil
 }
 
-// Undo unapplies the latest migration if possible, removes it from local
-// TestTrack server, and deletes the migration file
-func (r *Runner) Undo() error {
-	migration, err := r.unapplyLatest()
-	if err != nil {
-		return err
-	}
-
-	migrationVersion := *migration.MigrationVersion()
-	filepaths, err := filepath.Glob(fmt.Sprintf("testtrack/migrate/%s_*.yml", migrationVersion))
-	if err != nil {
-		return err
-	}
-	if len(filepaths) != 1 {
-		return fmt.Errorf("Couldn't find exactly one migration %s to delete", migrationVersion)
-	}
-
-	err = schema.Write(r.schema)
-	if err != nil {
-		return err
-	}
-
-	return os.Remove(filepaths[0])
-}
-
-func (r *Runner) unapplyLatest() (migrations.IMigration, error) {
-	migrationRepo, err := migrationloaders.Load()
-	if err != nil {
-		return nil, err
-	}
-
-	versions := migrationRepo.SortedVersions()
-
-	if len(versions) == 0 {
-		return nil, errors.New("no migration to undo")
-	}
-
-	latestMigration := migrationRepo[versions[len(versions)-1]]
-
-	var previousMigration migrations.IMigration
-	for i := len(versions) - 2; i >= 0; i-- {
-		m := migrationRepo[versions[i]]
-		if m.SameResourceAs(latestMigration) {
-			previousMigration = m
-			break
-		}
-	}
-
-	if previousMigration == nil {
-		previousMigration, err = latestMigration.Inverse()
-		if err != nil {
-			return nil, errors.Wrap(err, "can't undo - you may want to `testtrack create` a new migration for this resource and then delete this migration file")
-		}
-	}
-
-	mgr := migrationmanagers.NewWithDependencies(previousMigration, r.server, r.schema)
-	err = mgr.ApplyToSchema(migrationRepo)
-	if err != nil {
-		return nil, err
-	}
-	if len(versions) > 1 {
-		r.schema.SchemaVersion = versions[len(versions)-2]
-	} else {
-		r.schema.SchemaVersion = ""
-	}
-	return latestMigration, nil
-}
-
 func (r *Runner) getAppliedMigrationVersions() ([]serializers.MigrationVersion, error) {
 	appliedMigrationVersions := make([]serializers.MigrationVersion, 0)
 

migrations/migrations.go

@@ -21,8 +21,7 @@ type IMigration interface {
 	Serializable() interface{}
 	MigrationVersion() *string
 	SameResourceAs(other IMigration) bool
-	Inverse() (IMigration, error)
-	ApplyToSchema(*serializers.Schema, Repository) error
+	ApplyToSchema(schema *serializers.Schema, migrationRepo Repository, idempotently bool) error
 }
 
 var migrationFilenameRegex = regexp.MustCompile(`^(\d{13}(?:v\d{3})?)_[a-z\d_\-\.]+.yml$`)

remotekills/remotekills.go

@@ -130,29 +130,18 @@ func (r *RemoteKill) SameResourceAs(other migrations.IMigration) bool {
 	return false
 }
 
-// Inverse returns a logical inverse operation if possible
-func (r *RemoteKill) Inverse() (migrations.IMigration, error) {
-	if r.firstBadVersion == nil {
-		return nil, fmt.Errorf("can't invert remote_kill destroy %s for %s %s", *r.migrationVersion, *r.split, *r.reason)
-	}
-	return &RemoteKill{
-		split:           r.split,
-		reason:          r.reason,
-		overrideTo:      nil,
-		firstBadVersion: nil,
-		fixedVersion:    nil,
-	}, nil
-}
-
 // ApplyToSchema applies a migrations changes to in-memory schema representation
-func (r *RemoteKill) ApplyToSchema(schema *serializers.Schema, _ migrations.Repository) error {
+func (r *RemoteKill) ApplyToSchema(schema *serializers.Schema, _ migrations.Repository, idempotently bool) error {
 	if r.firstBadVersion == nil { // Delete
 		for i, candidate := range schema.RemoteKills {
 			if candidate.Split == *r.split && candidate.Reason == *r.reason {
 				schema.RemoteKills = append(schema.RemoteKills[:i], schema.RemoteKills[i+1:]...)
 				return nil
 			}
 		}
+		if idempotently {
+			return nil
+		}
 		return fmt.Errorf("Couldn't locate remote_kill %s of %s in schema", *r.reason, *r.split)
 	}
 	for i, candidate := range schema.RemoteKills { // Replace

schema/schema.go

@@ -189,7 +189,7 @@ func applyAllMigrationsToSchema(schema *serializers.Schema) error {
 	versions := migrationRepo.SortedVersions()
 
 	for _, version := range versions {
-		err = migrationRepo[version].ApplyToSchema(schema, migrationRepo)
+		err = migrationRepo[version].ApplyToSchema(schema, migrationRepo, false)
 		if err != nil {
 			return err
 		}

splitdecisions/splitdecisions.go

@@ -93,13 +93,8 @@ func (s *SplitDecision) SameResourceAs(other migrations.IMigration) bool {
 	return false
 }
 
-// Inverse returns a logical inverse operation if possible
-func (s *SplitDecision) Inverse() (migrations.IMigration, error) {
-	return nil, fmt.Errorf("can't invert split decision %s", *s.split)
-}
-
 // ApplyToSchema applies a migrations changes to in-memory schema representation
-func (s *SplitDecision) ApplyToSchema(schema *serializers.Schema, migrationRepo migrations.Repository) error {
+func (s *SplitDecision) ApplyToSchema(schema *serializers.Schema, migrationRepo migrations.Repository, idempotently bool) error {
 	for i, candidate := range schema.Splits {
 		if candidate.Name == *s.split {
 			schema.Splits[i].Decided = true
@@ -131,5 +126,8 @@ func (s *SplitDecision) ApplyToSchema(schema *serializers.Schema, migrationRepo
 			return nil
 		}
 	}
+	if idempotently {
+		return nil
+	}
 	return fmt.Errorf("Couldn't locate split %s in schema to decide", *s.split)
 }

splitretirements/splitretirements.go

@@ -93,13 +93,8 @@ func (s *SplitRetirement) SameResourceAs(other migrations.IMigration) bool {
 	return false
 }
 
-// Inverse returns a logical inverse operation if possible
-func (s *SplitRetirement) Inverse() (migrations.IMigration, error) {
-	return nil, fmt.Errorf("can't invert split retirement %s", *s.split)
-}
-
 // ApplyToSchema applies a migrations changes to in-memory schema representation
-func (s *SplitRetirement) ApplyToSchema(schema *serializers.Schema, _ migrations.Repository) error {
+func (s *SplitRetirement) ApplyToSchema(schema *serializers.Schema, _ migrations.Repository, idempotently bool) error {
 	for i, candidate := range schema.Splits {
 		if candidate.Name == *s.split {
 			weights, err := splits.WeightsFromYAML(candidate.Weights)
@@ -114,5 +109,8 @@ func (s *SplitRetirement) ApplyToSchema(schema *serializers.Schema, _ migrations
 			return nil
 		}
 	}
+	if idempotently {
+		return nil
+	}
 	return fmt.Errorf("Couldn't locate split %s in schema to retire", *s.split)
 }

splits/splits.go

@@ -140,13 +140,8 @@ func (s *Split) SameResourceAs(other migrations.IMigration) bool {
 	return false
 }
 
-// Inverse returns a logical inverse operation if possible
-func (s *Split) Inverse() (migrations.IMigration, error) {
-	return nil, fmt.Errorf("can't invert split creation %s", *s.name)
-}
-
 // ApplyToSchema applies a migrations changes to in-memory schema representation
-func (s *Split) ApplyToSchema(schema *serializers.Schema, migrationRepo migrations.Repository) error {
+func (s *Split) ApplyToSchema(schema *serializers.Schema, migrationRepo migrations.Repository, _idempotently bool) error {
 	for i, candidate := range schema.Splits { // Replace
 		if candidate.Name == *s.name {
 			schemaWeights, err := WeightsFromYAML(candidate.Weights)