Clean up duplicate migration logic

2 files changed, 21 insertions, 18 deletions

diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index f699e83ab3..471c5dc05a 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -1165,33 +1165,21 @@ module ActiveRecord
 
     private
 
+    # Used for running a specific migration.
     def run_without_lock
       migration = migrations.detect { |m| m.version == @target_version }
       raise UnknownMigrationVersionError.new(@target_version) if migration.nil?
-      unless (up? && migrated.include?(migration.version.to_i)) || (down? && !migrated.include?(migration.version.to_i))
-        begin
-          execute_migration_in_transaction(migration, @direction)
-        rescue => e
-          canceled_msg = use_transaction?(migration) ? ", this migration was canceled" : ""
-          raise StandardError, "An error has occurred#{canceled_msg}:\n\n#{e}", e.backtrace
-        end
-      end
+      execute_migration_in_transaction(migration, @direction)
     end
 
+    # Used for running multiple migrations up to or down to a certain value.
     def migrate_without_lock
-      if !target && @target_version && @target_version > 0
+      if invalid_target?
         raise UnknownMigrationVersionError.new(@target_version)
       end
 
       runnable.each do |migration|
-        Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger
-
-        begin
-          execute_migration_in_transaction(migration, @direction)
-        rescue => e
-          canceled_msg = use_transaction?(migration) ? "this and " : ""
-          raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace
-        end
+        execute_migration_in_transaction(migration, @direction)
       end
     end
 
@@ -1199,11 +1187,26 @@ module ActiveRecord
       migrated.include?(migration.version.to_i)
     end
 
+    # Return true if a valid version is not provided.
+    def invalid_target?
+      !target && @target_version && @target_version > 0
+    end
+
     def execute_migration_in_transaction(migration, direction)
+      return if down? && !migrated.include?(migration.version.to_i)
+      return if up?   &&  migrated.include?(migration.version.to_i)
+
+      Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger
+
       ddl_transaction(migration) do
         migration.migrate(direction)
         record_version_state_after_migrating(migration.version)
       end
+    rescue => e
+      msg = "An error has occurred, "
+      msg << "this and " if use_transaction?(migration)
+      msg << "all later migrations canceled:\n\n#{e}"
+      raise StandardError, msg, e.backtrace
     end
 
     def target
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index cfa223f93e..4d264caffb 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -301,7 +301,7 @@ class MigrationTest < ActiveRecord::TestCase
 
       e = assert_raise(StandardError) { migrator.run }
 
-      assert_equal "An error has occurred, this migration was canceled:\n\nSomething broke", e.message
+      assert_equal "An error has occurred, this and all later migrations canceled:\n\nSomething broke", e.message
 
       assert_no_column Person, :last_name,
         "On error, the Migrator should revert schema changes but it did not."