Migration revert #7280

Closed
wants to merge 1 commit into
from
View
23 activerecord/CHANGELOG.md
@@ -1,5 +1,28 @@
## Rails 4.0.0 (unreleased) ##
+* Add Migration#revert
+
+ Can be used to revert migration commands or a whole migration.
+
+ For example, to create a revertible migration removing the table 'horses' and
+ creating the table 'apples':
+
+ class FixTenderloveMigration < 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
+
+ *Marc-André Lafortune*
+
* Fix AR#create to return an unsaved record when AR::RecordInvalid is
raised. Fixes #3217.
View
121 activerecord/lib/active_record/migration.rb
@@ -368,22 +368,88 @@ def initialize(name = self.class.name, version = nil)
@name = name
@version = version
@connection = nil
- @reverting = false
end
# instantiate the delegate object after initialize is defined
self.verbose = true
self.delegate = new
- def revert
- @reverting = true
- yield
- ensure
- @reverting = false
+ # Reverses the migration commands for the given block and
+ # the given migrations.
+ #
+ # 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
+ #
+ # Or equivalently, if +TenderloveMigration+ is defined as in the
+ # documentation for Migration:
+ #
+ # class FixupTLMigration < ActiveRecord::Migration
+ # def change
+ # revert TenderloveMigration
+ #
+ # create_table(:apples) do |t|
+ # t.string :variety
+ # end
+ # end
+ # end
+ #
+ # This command can be nested.
+ #
+ def revert(*migration_classes)
+ run(*migration_classes.reverse, :revert => true) unless migration_classes.empty?
+ if block_given?
+ 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
end
def reverting?
- @reverting
+ @connection.respond_to?(:reverting) && @connection.reverting
+ end
+
+ # Runs the given migration classes.
+ # Last argument can specify options:
+ # - :direction (default is :up)
+ # - :revert (default is false)
+ #
+ def run(*migration_classes)
+ opts = migration_classes.extract_options!
+ dir = opts[:direction] || :up
+ dir = (dir == :down ? :up : :down) if opts[:revert]
+ if reverting?
+ # If in revert and going :up, say, we want to execute :down without reverting, so
+ revert { run(*migration_classes, direction: dir, revert: true) }
+ else
+ migration_classes.each do |migration_class|
+ migration_class.new.exec_migration(@connection, dir)
+ end
+ end
end
def up
@@ -409,29 +475,9 @@ def migrate(direction)
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
- change
- end
- @connection = conn
- time = Benchmark.measure {
- self.revert {
- recorder.inverse.each do |cmd, args|
- send(cmd, *args)
- end
- }
- }
- else
- time = Benchmark.measure { change }
- end
- else
- time = Benchmark.measure { send(direction) }
+ time = Benchmark.measure do
+ exec_migration(conn, direction)
end
- @connection = nil
end
case direction
@@ -440,6 +486,21 @@ def migrate(direction)
end
end
+ def exec_migration(conn, direction)
+ @connection = conn
+ if respond_to?(:change)
+ if direction == :down
+ revert { change }
+ else
+ change
+ end
+ else
+ send(direction)
+ end
+ ensure
+ @connection = nil
+ end
+
def write(text="")
puts(text) if verbose
end
@@ -478,7 +539,7 @@ def method_missing(method, *arguments, &block)
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
View
48 activerecord/lib/active_record/migration/command_recorder.rb
@@ -16,35 +16,49 @@ class Migration
class CommandRecorder
include JoinTable
- attr_accessor :commands, :delegate
+ attr_accessor :commands, :delegate, :reverting
def initialize(delegate = nil)
@commands = []
@delegate = delegate
+ @reverting = false
+ end
+
+ 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)
+ 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
#
- # 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)
+ method = :"invert_#{command}"
+ raise IrreversibleMigration unless respond_to?(method, true)
+ send(method, args)
end
def respond_to?(*args) # :nodoc:
@@ -53,9 +67,9 @@ def respond_to?(*args) # :nodoc:
[:create_table, :create_join_table, :change_table, :rename_table, :add_column, :remove_column, :rename_index, :rename_column, :add_index, :remove_index, :add_timestamps, :remove_timestamps, :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
View
86 activerecord/test/cases/invertible_migration_test.rb
@@ -17,6 +17,17 @@ def change
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|
@@ -40,6 +51,23 @@ def self.down
end
end
+ class RevertWholeMigration < SilentMigration
+ def initialize(name = self.class.name, version = nil, migration)
+ @migration = migration
+ super(name, version)
+ end
+
+ def change
+ revert @migration
+ end
+ end
+
+ class NestedRevertWholeMigration < RevertWholeMigration
+ def change
+ revert { super }
+ end
+ end
+
def teardown
if ActiveRecord::Base.connection.table_exists?("horses")
ActiveRecord::Base.connection.drop_table("horses")
@@ -67,6 +95,64 @@ def test_migrate_down
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_migrate_revert_whole_migration
+ migration = InvertibleMigration.new
+ [LegacyMigration, InvertibleMigration].each do |klass|
+ revert = RevertWholeMigration.new(klass)
+ 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
+ end
+
+ def test_migrate_nested_revert_whole_migration
+ revert = NestedRevertWholeMigration.new(InvertibleRevertMigration)
+ revert.migrate :down
+ assert revert.connection.table_exists?("horses")
+ revert.migrate :up
+ assert !revert.connection.table_exists?("horses")
+ end
+
+ def test_revert_order
+ block = Proc.new{|t| t.string :name }
+ recorder = ActiveRecord::Migration::CommandRecorder.new(ActiveRecord::Base.connection)
+ 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_legacy_up
LegacyMigration.migrate :up
assert ActiveRecord::Base.connection.table_exists?("horses"), "horses should exist"
View
99 activerecord/test/cases/migration/command_recorder_test.rb
@@ -26,7 +26,7 @@ def create_table(name); end
}.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 @@ def test_unknown_commands_delegate
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 @@ def test_record
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