Merge pull request #10217 from mirasrael/make-migrator-run-transactional-4.0

Support transactions in Migrator.run

Conflicts:
	activerecord/CHANGELOG.md

3 files changed, 42 insertions, 5 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 52300fd53f..278da322f2 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,9 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Run `rake migrate:down` & `rake migrate:up` in transaction if database supports.
+
+    *Alexander Bondarev*
+
 *   `0x` prefix must be added when assigning hexadecimal string into `bit` column in PostgreSQL.
 
     *kennyj*
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 451104106c..d2f10cf2ac 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -867,11 +867,18 @@ module ActiveRecord
     alias :current :current_migration
 
     def run
-      target = migrations.detect { |m| m.version == @target_version }
-      raise UnknownMigrationVersionError.new(@target_version) if target.nil?
-      unless (up? && migrated.include?(target.version.to_i)) || (down? && !migrated.include?(target.version.to_i))
-        target.migrate(@direction)
-        record_version_state_after_migrating(target.version)
+      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
+          ddl_transaction(migration) do
+            migration.migrate(@direction)
+            record_version_state_after_migrating(migration.version)
+          end
+        rescue => e
+          canceled_msg = use_transaction?(migration) ? ", the migration canceled" : ""
+          raise StandardError, "An error has occurred#{canceled_msg}:\n\n#{e}", e.backtrace
+        end
       end
     end
 
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index f8afb7c591..b26ffa0e79 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -258,6 +258,32 @@ class MigrationTest < ActiveRecord::TestCase
      "On error, the Migrator should revert schema changes but it did not."
   end
 
+  def test_migrator_one_up_with_exception_and_rollback_using_run
+    unless ActiveRecord::Base.connection.supports_ddl_transactions?
+      skip "not supported on #{ActiveRecord::Base.connection.class}"
+    end
+
+    assert_not Person.column_methods_hash.include?(:last_name)
+
+    migration = Class.new(ActiveRecord::Migration) {
+      def version; 100 end
+      def migrate(x)
+        add_column "people", "last_name", :string
+        raise 'Something broke'
+      end
+    }.new
+
+    migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
+
+    e = assert_raise(StandardError) { migrator.run }
+
+    assert_equal "An error has occurred, the migration canceled:\n\nSomething broke", e.message
+
+    Person.reset_column_information
+    assert_not Person.column_methods_hash.include?(:last_name),
+      "On error, the Migrator should revert schema changes but it did not."
+  end
+
   def test_migration_without_transaction
     unless ActiveRecord::Base.connection.supports_ddl_transactions?
       skip "not supported on #{ActiveRecord::Base.connection.class}"