From af871a0623740f53a4dca5858b78efb35f0e32e0 Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Mon, 19 Nov 2012 01:12:36 -0500 Subject: [PATCH] Make drop_table reversible [#8267] --- .../abstract/schema_statements.rb | 4 +++ .../migration/command_recorder.rb | 13 +++++-- .../test/cases/invertible_migration_test.rb | 6 ++-- .../cases/migration/command_recorder_test.rb | 35 +++++++++++++------ 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index bf3155e4e2..a2feb04b77 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -305,6 +305,10 @@ module ActiveRecord end # Drops a table from the database. + # + # Although this command ignores +options+ and the block if one is given, it can be helpful + # to provide these in a migration's +change+ method so it can be reverted. + # In that case, +options+ and the block will be used by create_table. def drop_table(table_name, options = {}) execute "DROP TABLE #{quote_table_name(table_name)}" end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 8dad1b123f..f37ec1feaa 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -73,7 +73,7 @@ module ActiveRecord [: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, :transaction, - :drop_join_table, + :drop_join_table, :drop_table ].each do |method| class_eval <<-EOV, __FILE__, __LINE__ + 1 def #{method}(*args, &block) # def create_table(*args, &block) @@ -90,8 +90,15 @@ module ActiveRecord [:transaction, args, block] end - def invert_create_table(args) - [:drop_table, [args.first]] + def invert_create_table(args, &block) + [:drop_table, args, block] + end + + def invert_drop_table(args, &block) + if args.size == 1 && block == nil + raise ActiveRecord::IrreversibleMigration, "To avoid mistakes, drop_table is only reversible if given options or a block (can be empty)." + end + [:create_table, args, block] end def invert_create_join_table(args, &block) diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb index 484b4c0fea..cf4e39c4ef 100644 --- a/activerecord/test/cases/invertible_migration_test.rb +++ b/activerecord/test/cases/invertible_migration_test.rb @@ -186,10 +186,10 @@ module ActiveRecord create_table("grapes") end end - assert_equal [[:create_table, ["apples"], block], [:drop_table, ["elderberries"]], + assert_equal [[:create_table, ["apples"], block], [:drop_table, ["elderberries"], nil], [:create_table, ["clementines"], nil], [:create_table, ["dates"], nil], - [:drop_table, ["bananas"]], [:drop_table, ["grapes"]], - [:drop_table, ["figs"]]], recorder.commands + [:drop_table, ["bananas"], block], [:drop_table, ["grapes"], nil], + [:drop_table, ["figs"], nil]], recorder.commands end def test_legacy_up diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index af033f11d9..66be0df4cd 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -50,7 +50,7 @@ module ActiveRecord @recorder.record :create_table, [:hello] @recorder.record :create_table, [:world] end - tables = @recorder.commands.map(&:last) + tables = @recorder.commands.map{|_cmd, args, _block| args} assert_equal [[:world], [:hello]], tables end @@ -61,20 +61,20 @@ module ActiveRecord revert do create_table("bananas", &block) revert do - create_table("clementines") + create_table("clementines", &block) create_table("dates") end create_table("elderberries") end revert do - create_table("figs") + create_table("figs", &block) 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 + assert_equal [[:create_table, ["apples"], block], [:drop_table, ["elderberries"], nil], + [:create_table, ["clementines"], block], [:create_table, ["dates"], nil], + [:drop_table, ["bananas"], block], [:drop_table, ["grapes"], nil], + [:drop_table, ["figs"], block]], @recorder.commands end @@ -83,12 +83,25 @@ module ActiveRecord @recorder.record :create_table, [:system_settings] end drop_table = @recorder.commands.first - assert_equal [:drop_table, [:system_settings]], drop_table + assert_equal [:drop_table, [:system_settings], nil], drop_table end - def test_invert_create_table_with_options - drop_table = @recorder.inverse_of :create_table, [:people_reminders, id: false] - assert_equal [:drop_table, [:people_reminders]], drop_table + def test_invert_create_table_with_options_and_block + block = Proc.new{} + drop_table = @recorder.inverse_of :create_table, [:people_reminders, id: false], &block + assert_equal [:drop_table, [:people_reminders, id: false], block], drop_table + end + + def test_invert_drop_table + block = Proc.new{} + create_table = @recorder.inverse_of :drop_table, [:people_reminders, id: false], &block + assert_equal [:create_table, [:people_reminders, id: false], block], create_table + end + + def test_invert_drop_table_without_a_block_nor_option + assert_raises(ActiveRecord::IrreversibleMigration) do + @recorder.inverse_of :drop_table, [:people_reminders] + end end def test_invert_create_join_table