Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validating the migration ID exist in the list of migrations #21

Merged
merged 2 commits into from Dec 2, 2018
Merged

Validating the migration ID exist in the list of migrations #21

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6932ef
add migration id validation to ensure migration id exists in list of …
Nov 22, 2018
88cddeb
remove allowing empty string as wildcare to migrate/rollback complete…
Nov 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions gormigrate.go
View file
Open in desktop
Expand Up @@ -79,6 +79,10 @@ var (
// ErrNoRunnedMigration is returned when any runned migration was found while
// running RollbackLast
ErrNoRunnedMigration = errors.New("gormigrate: Could not find last runned migration")

// ErrMigrationIDDoesNotExist is returned when migrating or rolling back to a migration ID that
// does not exist in the list of migrations
ErrMigrationIDDoesNotExist = errors.New("gormigrate: Tried to migrate to an ID that doesn't exist")
)

// New returns a new Gormigrate.
Expand Down Expand Up @@ -126,6 +130,10 @@ func (g *Gormigrate) migrate(migrationID string) error {
return err
}

if err := g.checkIDExist(migrationID); err != nil {
return err
}

if err := g.createMigrationTableIfNotExists(); err != nil {
return err
}
Expand Down Expand Up @@ -164,6 +172,18 @@ func (g *Gormigrate) checkDuplicatedID() error {
return nil
}

func (g *Gormigrate) checkIDExist(migrationID string) error {
if migrationID == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... maybe we should remove this if and just return the error if migrationID is ""?

I don't think rollbackTo should rollback all migration in any circumstance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the check to allow empty migration ID. This also meant changes to Migrate so it doesn't migrate to an empty string, but to the last migration in the list, which I think is same semantics as before.

return nil
}
for _, migrate := range g.migrations {
if migrate.ID == migrationID {
return nil
}
}
return ErrMigrationIDDoesNotExist
}

// RollbackLast undo the last migration
func (g *Gormigrate) RollbackLast() error {
if len(g.migrations) == 0 {
Expand All @@ -188,6 +208,10 @@ func (g *Gormigrate) RollbackTo(migrationID string) error {
return ErrNoMigrationDefined
}

if err := g.checkIDExist(migrationID); err != nil {
return err
}

g.begin()

for i := len(g.migrations) - 1; i >= 0; i-- {
Expand Down