diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index ef2107ad24..b292b35ff4 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -373,22 +373,52 @@ module ActiveRecord @name = name @version = version @connection = nil - @reverting = false end # instantiate the delegate object after initialize is defined self.verbose = true self.delegate = new + # Reverses the migration commands for the given block. + # + # The following migration will remove the table 'horses' + # and create the table 'apples' on the way up, and the reverse + # on the way down. + # + # class FixTLMigration < ActiveRecord::Migration + # def change + # revert do + # create_table(:horses) do |t| + # t.text :content + # t.datetime :remind_at + # end + # end + # create_table(:apples) do |t| + # t.string :variety + # end + # end + # end + # + # This command can be nested. + # def revert - @reverting = true - yield - ensure - @reverting = false + if @connection.respond_to? :revert + @connection.revert { yield } + else + recorder = CommandRecorder.new(@connection) + @connection = recorder + suppress_messages do + @connection.revert { yield } + end + @connection = recorder.delegate + recorder.commands.each do |cmd, args, block| + send(cmd, *args, &block) + end + end end def reverting? - @reverting + @connection.respond_to?(:reverting) && @connection.reverting end def up @@ -414,29 +444,19 @@ module ActiveRecord time = nil ActiveRecord::Base.connection_pool.with_connection do |conn| - @connection = conn - if respond_to?(:change) - if direction == :down - recorder = CommandRecorder.new(@connection) - suppress_messages do - @connection = recorder + time = Benchmark.measure do + @connection = conn + if respond_to?(:change) + if direction == :down + revert { change } + else change end - @connection = conn - time = Benchmark.measure { - self.revert { - recorder.inverse.each do |cmd, args| - send(cmd, *args) - end - } - } else - time = Benchmark.measure { change } + send(direction) end - else - time = Benchmark.measure { send(direction) } + @connection = nil end - @connection = nil end case direction @@ -483,7 +503,7 @@ module ActiveRecord arg_list = arguments.map{ |a| a.inspect } * ', ' say_with_time "#{method}(#{arg_list})" do - unless reverting? + unless @connection.respond_to? :revert unless arguments.empty? || method == :execute arguments[0] = Migrator.proper_table_name(arguments.first) arguments[1] = Migrator.proper_table_name(arguments.second) if method == :rename_table diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 55c384ae41..a79428b69a 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -16,35 +16,54 @@ module ActiveRecord class CommandRecorder include JoinTable - attr_accessor :commands, :delegate + attr_accessor :commands, :delegate, :reverting def initialize(delegate = nil) @commands = [] @delegate = delegate + @reverting = false + end + + # While executing the given block, the recorded will be in reverting mode. + # All commands recorded will end up being recorded reverted + # and in reverse order. + # For example: + # + # recorder.revert{ recorder.record(:rename_table, [:old, :new]) } + # # same effect as recorder.record(:rename_table, [:new, :old]) + def revert + @reverting = !@reverting + previous = @commands + @commands = [] + yield + ensure + @commands = previous.concat(@commands.reverse) + @reverting = !@reverting end # record +command+. +command+ should be a method name and arguments. # For example: # # recorder.record(:method_name, [:arg1, :arg2]) - def record(*command) - @commands << command + def record(*command, &block) + if @reverting + @commands << inverse_of(*command, &block) + else + @commands << (command << block) + end end - # Returns a list that represents commands that are the inverse of the - # commands stored in +commands+. For example: + # Returns the inverse of the given command. For example: # - # recorder.record(:rename_table, [:old, :new]) - # recorder.inverse # => [:rename_table, [:new, :old]] + # recorder.inverse_of(:rename_table, [:old, :new]) + # # => [:rename_table, [:new, :old]] # # This method will raise an +IrreversibleMigration+ exception if it cannot - # invert the +commands+. - def inverse - @commands.reverse.map { |name, args| - method = :"invert_#{name}" - raise IrreversibleMigration unless respond_to?(method, true) - send(method, args) - } + # invert the +command+. + def inverse_of(command, args, &block) + method = :"invert_#{command}" + raise IrreversibleMigration unless respond_to?(method, true) + send(method, args, &block) end def respond_to?(*args) # :nodoc: @@ -56,9 +75,9 @@ module ActiveRecord :change_column, :change_column_default, :add_reference, :remove_reference, ].each do |method| class_eval <<-EOV, __FILE__, __LINE__ + 1 - def #{method}(*args) # def create_table(*args) - record(:"#{method}", args) # record(:create_table, args) - end # end + def #{method}(*args, &block) # def create_table(*args, &block) + record(:"#{method}", args, &block) # record(:create_table, args, &block) + end # end EOV end alias :add_belongs_to :add_reference 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 - end - - def test_inverted_commands_are_reveresed - @recorder.record :create_table, [:hello] - @recorder.record :create_table, [:world] - tables = @recorder.inverse.map(&:last) + 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_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