Merge pull request #400 from zilverline/handle-invalid-migration-defi…

…nition-gracefully

handle invalid migration definition gracefully

lib/sequent/migrations/errors.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Sequent
+  module Migrations
+    class MigrationError < RuntimeError; end
+    class MigrationNotStarted < MigrationError; end
+    class MigrationDone < MigrationError; end
+    class ConcurrentMigration < MigrationError; end
+
+    class InvalidMigrationDefinition < MigrationError; end
+  end
+end

lib/sequent/migrations/planner.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'errors'
+
 module Sequent
   module Migrations
     class Planner
@@ -100,7 +102,8 @@ def create_migrations(migrations)
       def map_to_migrations(migrations)
         migrations.reduce({}) do |memo, (version, ms)|
           unless ms.is_a?(Array)
-            fail "Declared migrations for version #{version} must be an Array. For example: {'3' => [FooProjector]}"
+            fail InvalidMigrationDefinition,
+                 "Declared migrations for version #{version} must be an Array. For example: {'3' => [FooProjector]}"
           end
 
           memo[version] = ms.flat_map do |migration|
@@ -109,14 +112,15 @@ def map_to_migrations(migrations)
                 #{Sequent.configuration.migration_sql_files_directory}/#{migration.table_name}_#{version}.sql
               EOS
               unless File.exist?(alter_table_sql_file_name)
-                fail "Missing file #{alter_table_sql_file_name} to apply for version #{version}"
+                fail InvalidMigrationDefinition,
+                     "Missing file #{alter_table_sql_file_name} to apply for version #{version}"
               end
 
               migration.copy(version)
             elsif migration < Sequent::Projector
               migration.managed_tables.map { |table| ReplayTable.create(table, version) }
             else
-              fail "Unknown Migration #{migration}"
+              fail InvalidMigrationDefinition, "Unknown Migration #{migration}"
             end
           end
 

lib/sequent/migrations/view_schema.rb

@@ -3,6 +3,7 @@
 require 'parallel'
 require 'postgresql_cursor'
 
+require_relative 'errors'
 require_relative '../support/database'
 require_relative '../sequent'
 require_relative '../util/timer'
@@ -16,10 +17,6 @@
 
 module Sequent
   module Migrations
-    class MigrationError < RuntimeError; end
-    class MigrationNotStarted < MigrationError; end
-    class MigrationDone < MigrationError; end
-    class ConcurrentMigration < MigrationError; end
 
     ##
     # ViewSchema is used for migration of you view_schema. For instance
@@ -191,6 +188,7 @@ def executor
       #
       # @raise ConcurrentMigrationError if migration is already running
       def migrate_online
+        ensure_valid_plan!
         migrate_metadata_tables
 
         return if Sequent.new_version == current_version
@@ -219,6 +217,9 @@ def migrate_online
       rescue ConcurrentMigration
         # Do not rollback the migration when this is a concurrent migration as the other one is running
         raise
+      rescue InvalidMigrationDefinition
+        # Do not rollback the migration when since there is nothing to rollback
+        raise
       rescue Exception => e # rubocop:disable Lint/RescueException
         rollback_migration
         raise e
@@ -284,6 +285,10 @@ def migrate_offline
 
       private
 
+      def ensure_valid_plan!
+        plan
+      end
+
       def migrate_metadata_tables
         Sequent::ApplicationRecord.transaction do
           in_view_schema do

spec/lib/sequent/migrations/view_schema_spec.rb

@@ -593,6 +593,18 @@
             expect(AccountRecord).to have_column('foobar')
           end
         end
+
+        it 'missing the correct alter table file' do
+          expect(MessageRecord).to_not have_column('foobar')
+          Sequent.configuration.migration_sql_files_directory = 'spec/fixtures/db/2'
+          SpecMigrations.copy_and_add('3', [Sequent::Migrations.alter_table(MessageRecord)])
+          SpecMigrations.version = 3
+          expect(next_migration).to_not receive(:replay!)
+
+          expect { next_migration.migrate_online }.to raise_error(Sequent::Migrations::InvalidMigrationDefinition)
+
+          expect(Sequent::Migrations::Versions.where(version: 3).first).to be_nil
+        end
       end
     end
   end