diff --git a/lib/sequent/migrations/errors.rb b/lib/sequent/migrations/errors.rb new file mode 100644 index 00000000..e868b68b --- /dev/null +++ b/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 diff --git a/lib/sequent/migrations/planner.rb b/lib/sequent/migrations/planner.rb index 0a916d5a..f0f6aa71 100644 --- a/lib/sequent/migrations/planner.rb +++ b/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 diff --git a/lib/sequent/migrations/view_schema.rb b/lib/sequent/migrations/view_schema.rb index a1627552..297078bb 100644 --- a/lib/sequent/migrations/view_schema.rb +++ b/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 diff --git a/spec/lib/sequent/migrations/view_schema_spec.rb b/spec/lib/sequent/migrations/view_schema_spec.rb index 9200c816..668d4a01 100644 --- a/spec/lib/sequent/migrations/view_schema_spec.rb +++ b/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