Allow reverting of migration commands with Migration#revert [#8267]

2 files changed, 74 insertions, 48 deletions

diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb
index 8f1cdd47ea..6d285e23d7 100644
--- a/activerecord/test/cases/invertible_migration_test.rb
+++ b/activerecord/test/cases/invertible_migration_test.rb
@@ -17,6 +17,17 @@ module ActiveRecord
       end
     end
 
+    class InvertibleRevertMigration < SilentMigration
+      def change
+        revert do
+          create_table("horses") do |t|
+            t.column :content, :text
+            t.column :remind_at, :datetime
+          end
+        end
+      end
+    end
+
     class NonInvertibleMigration < SilentMigration
       def change
         create_table("horses") do |t|
@@ -67,6 +78,18 @@ module ActiveRecord
       assert !migration.connection.table_exists?("horses")
     end
 
+    def test_migrate_revert
+      migration = InvertibleMigration.new
+      revert = InvertibleRevertMigration.new
+      migration.migrate :up
+      revert.migrate :up
+      assert !migration.connection.table_exists?("horses")
+      revert.migrate :down
+      assert migration.connection.table_exists?("horses")
+      migration.migrate :down
+      assert !migration.connection.table_exists?("horses")
+    end
+
     def test_legacy_up
       LegacyMigration.migrate :up
       assert ActiveRecord::Base.connection.table_exists?("horses"), "horses should exist"
diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb
index f2213ee6aa..b222b5b75a 100644
--- a/activerecord/test/cases/migration/command_recorder_test.rb
+++ b/activerecord/test/cases/migration/command_recorder_test.rb
@@ -26,7 +26,7 @@ module ActiveRecord
         }.new)
         assert recorder.respond_to?(:create_table), 'respond_to? create_table'
         recorder.send(:create_table, :horses)
-        assert_equal [[:create_table, [:horses]]], recorder.commands
+        assert_equal [[:create_table, [:horses], nil]], recorder.commands
       end
 
       def test_unknown_commands_delegate
@@ -35,9 +35,8 @@ module ActiveRecord
       end
 
       def test_unknown_commands_raise_exception_if_they_cannot_delegate
-        @recorder.record :execute, ['some sql']
         assert_raises(ActiveRecord::IrreversibleMigration) do
-          @recorder.inverse
+          @recorder.inverse_of :execute, ['some sql']
         end
       end
 
@@ -46,120 +45,124 @@ module ActiveRecord
         assert_equal 1, @recorder.commands.length
       end
 
-      def test_inverse
-        @recorder.record :create_table, [:system_settings]
-        assert_equal 1, @recorder.inverse.length
-
-        @recorder.record :rename_table, [:old, :new]
-        assert_equal 2, @recorder.inverse.length
+      def test_inverted_commands_are_reversed
+        @recorder.revert do
+          @recorder.record :create_table, [:hello]
+          @recorder.record :create_table, [:world]
+        end
+        tables = @recorder.commands.map(&:last)
+        assert_equal [[:world], [:hello]], tables
       end
 
-      def test_inverted_commands_are_reveresed
-        @recorder.record :create_table, [:hello]
-        @recorder.record :create_table, [:world]
-        tables = @recorder.inverse.map(&:last)
-        assert_equal [[:world], [:hello]], tables
+      def test_revert_order
+        block = Proc.new{|t| t.string :name }
+        @recorder.instance_eval do
+          create_table("apples", &block)
+          revert do
+            create_table("bananas", &block)
+            revert do
+              create_table("clementines")
+              create_table("dates")
+            end
+            create_table("elderberries")
+          end
+          revert do
+            create_table("figs")
+            create_table("grapes")
+          end
+        end
+        assert_equal [[:create_table, ["apples"], block], [:drop_table, ["elderberries"]],
+                      [:create_table, ["clementines"], nil], [:create_table, ["dates"], nil],
+                      [:drop_table, ["bananas"]], [:drop_table, ["grapes"]],
+                      [:drop_table, ["figs"]]], @recorder.commands
       end
 
+
       def test_invert_create_table
-        @recorder.record :create_table, [:system_settings]
-        drop_table = @recorder.inverse.first
+        @recorder.revert do
+          @recorder.record :create_table, [:system_settings]
+        end
+        drop_table = @recorder.commands.first
         assert_equal [:drop_table, [:system_settings]], drop_table
       end
 
       def test_invert_create_table_with_options
-        @recorder.record :create_table, [:people_reminders, {:id => false}]
-        drop_table = @recorder.inverse.first
+        drop_table = @recorder.inverse_of :create_table, [:people_reminders, id: false]
         assert_equal [:drop_table, [:people_reminders]], drop_table
       end
 
       def test_invert_create_join_table
-        @recorder.record :create_join_table, [:musics, :artists]
-        drop_table = @recorder.inverse.first
+        drop_table = @recorder.inverse_of :create_join_table, [:musics, :artists]
         assert_equal [:drop_table, [:artists_musics]], drop_table
       end
 
       def test_invert_create_join_table_with_table_name
-        @recorder.record :create_join_table, [:musics, :artists, {:table_name => :catalog}]
-        drop_table = @recorder.inverse.first
+        drop_table = @recorder.inverse_of :create_join_table, [:musics, :artists, table_name: :catalog]
         assert_equal [:drop_table, [:catalog]], drop_table
       end
 
       def test_invert_rename_table
-        @recorder.record :rename_table, [:old, :new]
-        rename = @recorder.inverse.first
+        rename = @recorder.inverse_of :rename_table, [:old, :new]
         assert_equal [:rename_table, [:new, :old]], rename
       end
 
       def test_invert_add_column
-        @recorder.record :add_column, [:table, :column, :type, {}]
-        remove = @recorder.inverse.first
+        remove = @recorder.inverse_of :add_column, [:table, :column, :type, {}]
         assert_equal [:remove_column, [:table, :column]], remove
       end
 
       def test_invert_rename_column
-        @recorder.record :rename_column, [:table, :old, :new]
-        rename = @recorder.inverse.first
+        rename = @recorder.inverse_of :rename_column, [:table, :old, :new]
         assert_equal [:rename_column, [:table, :new, :old]], rename
       end
 
       def test_invert_add_index
-        @recorder.record :add_index, [:table, [:one, :two], {:options => true}]
-        remove = @recorder.inverse.first
+        remove = @recorder.inverse_of :add_index, [:table, [:one, :two], options: true]
         assert_equal [:remove_index, [:table, {:column => [:one, :two]}]], remove
       end
 
       def test_invert_add_index_with_name
-        @recorder.record :add_index, [:table, [:one, :two], {:name => "new_index"}]
-        remove = @recorder.inverse.first
+        remove = @recorder.inverse_of :add_index, [:table, [:one, :two], name: "new_index"]
         assert_equal [:remove_index, [:table, {:name => "new_index"}]], remove
       end
 
       def test_invert_add_index_with_no_options
-        @recorder.record :add_index, [:table, [:one, :two]]
-        remove = @recorder.inverse.first
+        remove = @recorder.inverse_of :add_index, [:table, [:one, :two]]
         assert_equal [:remove_index, [:table, {:column => [:one, :two]}]], remove
       end
 
       def test_invert_rename_index
-        @recorder.record :rename_index, [:table, :old, :new]
-        rename = @recorder.inverse.first
+        rename = @recorder.inverse_of :rename_index, [:table, :old, :new]
         assert_equal [:rename_index, [:table, :new, :old]], rename
       end
 
       def test_invert_add_timestamps
-        @recorder.record :add_timestamps, [:table]
-        remove = @recorder.inverse.first
+        remove = @recorder.inverse_of :add_timestamps, [:table]
         assert_equal [:remove_timestamps, [:table]], remove
       end
 
       def test_invert_remove_timestamps
-        @recorder.record :remove_timestamps, [:table]
-        add = @recorder.inverse.first
+        add = @recorder.inverse_of :remove_timestamps, [:table]
         assert_equal [:add_timestamps, [:table]], add
       end
 
       def test_invert_add_reference
-        @recorder.record :add_reference, [:table, :taggable, { polymorphic: true }]
-        remove = @recorder.inverse.first
+        remove = @recorder.inverse_of :add_reference, [:table, :taggable, { polymorphic: true }]
         assert_equal [:remove_reference, [:table, :taggable, { polymorphic: true }]], remove
       end
 
       def test_invert_add_belongs_to_alias
-        @recorder.record :add_belongs_to, [:table, :user]
-        remove = @recorder.inverse.first
+        remove = @recorder.inverse_of :add_belongs_to, [:table, :user]
         assert_equal [:remove_reference, [:table, :user]], remove
       end
 
       def test_invert_remove_reference
-        @recorder.record :remove_reference, [:table, :taggable, { polymorphic: true }]
-        add = @recorder.inverse.first
+        add = @recorder.inverse_of :remove_reference, [:table, :taggable, { polymorphic: true }]
         assert_equal [:add_reference, [:table, :taggable, { polymorphic: true }]], add
       end
 
       def test_invert_remove_belongs_to_alias
-        @recorder.record :remove_belongs_to, [:table, :user]
-        add = @recorder.inverse.first
+        add = @recorder.inverse_of :remove_belongs_to, [:table, :user]
         assert_equal [:add_reference, [:table, :user]], add
       end
     end