Merge pull request rails#52773 from sobrinho/ar-drop-tables

Allow drop_table to accept an array of table names

Author: rafaelfranca

activerecord/CHANGELOG.md

@@ -1,3 +1,13 @@
+*   Allow `drop_table` to accept an array of table names.
+
+    This will let you to drop multiple tables in a single call.
+
+    ```ruby
+    ActiveRecord::Base.connection.drop_table(:users, :posts)
+    ```
+
+    *Gabriel Sobrinho*
+
 *   Add support for PostgreSQL `IF NOT EXISTS` via the `:if_not_exists` option
     on the `add_enum_value` method.
 

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -525,7 +525,7 @@ def rename_table(table_name, new_name, **)
         raise NotImplementedError, "rename_table is not implemented"
       end
 
-      # Drops a table from the database.
+      # Drops a table or tables from the database.
       #
       # [<tt>:force</tt>]
       #   Set to +:cascade+ to drop dependent objects as well.
@@ -536,10 +536,12 @@ def rename_table(table_name, new_name, **)
       #
       # Although this command ignores most +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)
-        schema_cache.clear_data_source_cache!(table_name.to_s)
-        execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}"
+      # In that case, +options+ and the block will be used by #create_table except if you provide more than one table which is not supported.
+      def drop_table(*table_names, **options)
+        table_names.each do |table_name|
+          schema_cache.clear_data_source_cache!(table_name.to_s)
+          execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}"
+        end
       end
 
       # Add a new +type+ column named +column_name+ to +table_name+.

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -332,7 +332,7 @@ def rename_table(table_name, new_name, **options)
         rename_table_indexes(table_name, new_name, **options)
       end
 
-      # Drops a table from the database.
+      # Drops a table or tables from the database.
       #
       # [<tt>:force</tt>]
       #   Set to +:cascade+ to drop dependent objects as well.
@@ -346,10 +346,10 @@ def rename_table(table_name, new_name, **options)
       #
       # Although this command ignores most +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)
-        schema_cache.clear_data_source_cache!(table_name.to_s)
-        execute "DROP#{' TEMPORARY' if options[:temporary]} TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}"
+      # In that case, +options+ and the block will be used by #create_table except if you provide more than one table which is not supported.
+      def drop_table(*table_names, **options)
+        table_names.each { |table_name| schema_cache.clear_data_source_cache!(table_name.to_s) }
+        execute "DROP#{' TEMPORARY' if options[:temporary]} TABLE#{' IF EXISTS' if options[:if_exists]} #{table_names.map { |table_name| quote_table_name(table_name) }.join(', ')}#{' CASCADE' if options[:force] == :cascade}"
       end
 
       def rename_index(table_name, old_name, new_name)

activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb

@@ -54,9 +54,9 @@ def drop_database(name) # :nodoc:
           execute "DROP DATABASE IF EXISTS #{quote_table_name(name)}"
         end
 
-        def drop_table(table_name, **options) # :nodoc:
-          schema_cache.clear_data_source_cache!(table_name.to_s)
-          execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}"
+        def drop_table(*table_names, **options) # :nodoc:
+          table_names.each { |table_name| schema_cache.clear_data_source_cache!(table_name.to_s) }
+          execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{table_names.map { |table_name| quote_table_name(table_name) }.join(', ')}#{' CASCADE' if options[:force] == :cascade}"
         end
 
         # Returns true if schema exists.

activerecord/lib/active_record/migration.rb

@@ -355,7 +355,7 @@ def initialize
   #
   # === Deletion
   #
-  # * <tt>drop_table(name)</tt>: Drops the table called +name+.
+  # * <tt>drop_table(*names)</tt>: Drops the given tables.
   # * <tt>drop_join_table(table_1, table_2, options)</tt>: Drops the join table
   #   specified by the given arguments.
   # * <tt>remove_column(table_name, column_name, type, options)</tt>: Removes the column
@@ -602,7 +602,7 @@ def create_join_table(table_1, table_2, **options)
         end
       end
 
-      def drop_table(table_name, **options)
+      def drop_table(*table_names, **options)
         if block_given?
           super { |t| yield compatible_table_definition(t) }
         else

activerecord/lib/active_record/migration/command_recorder.rb

@@ -202,13 +202,18 @@ def invert_create_table(args, &block)
         end
 
         def invert_drop_table(args, &block)
-          if args.last.is_a?(Hash)
-            args.last.delete(:if_exists)
+          options = args.extract_options!
+          options.delete(:if_exists)
+
+          if args.size > 1
+            raise ActiveRecord::IrreversibleMigration, "To avoid mistakes, drop_table is only reversible if given a single table name."
           end
-          if args.size == 1 && block == nil
+
+          if args.size == 1 && options == {} && block == nil
             raise ActiveRecord::IrreversibleMigration, "To avoid mistakes, drop_table is only reversible if given options or a block (can be empty)."
           end
-          super
+
+          super(args.push(options), &block)
         end
 
         def invert_rename_table(args)

activerecord/test/cases/adapters/abstract_mysql_adapter/active_schema_test.rb

@@ -118,6 +118,10 @@ def test_drop_table
     assert_equal "DROP TABLE `people`", drop_table(:people)
   end
 
+  def test_drop_tables
+    assert_equal "DROP TABLE `people`, `sobrinho`", drop_table(:people, :sobrinho)
+  end
+
   def test_create_mysql_database_with_encoding
     if ActiveRecord::Base.lease_connection.send(:row_format_dynamic_by_default?)
       assert_equal "CREATE DATABASE `matt` DEFAULT CHARACTER SET `utf8mb4`", create_database(:matt)
@@ -147,6 +151,10 @@ def test_drop_table_with_specific_database
     assert_equal "DROP TABLE `otherdb`.`people`", drop_table("otherdb.people")
   end
 
+  def test_drop_tables_with_specific_database
+    assert_equal "DROP TABLE `otherdb`.`people`, `otherdb`.`sobrinho`", drop_table("otherdb.people", "otherdb.sobrinho")
+  end
+
   def test_add_timestamps
     with_real_execute do
       ActiveRecord::Base.lease_connection.create_table :delete_me

activerecord/test/cases/migration/change_schema_test.rb

@@ -467,10 +467,24 @@ def test_drop_table_if_exists
         assert_not connection.table_exists?(:testings)
       end
 
+      def test_drop_tables_if_exists
+        connection.create_table(:testings)
+        connection.create_table(:sobrinho)
+        assert connection.table_exists?(:testings)
+        assert connection.table_exists?(:sobrinho)
+        connection.drop_table(:testings, :sobrinho, if_exists: true)
+        assert_not connection.table_exists?(:testings)
+        assert_not connection.table_exists?(:sobrinho)
+      end
+
       def test_drop_table_if_exists_nothing_raised
         assert_nothing_raised { connection.drop_table(:nonexistent, if_exists: true) }
       end
 
+      def test_drop_tables_if_exists_nothing_raised
+        assert_nothing_raised { connection.drop_table(:nonexistent, :nonexistent_sobrinho, if_exists: true) }
+      end
+
       private
         def testing_table_with_only_foo_attribute
           connection.create_table :testings, id: false do |t|

activerecord/test/cases/migration/command_recorder_test.rb

@@ -172,11 +172,31 @@ def test_invert_drop_table_with_if_exists
       end
 
       def test_invert_drop_table_without_a_block_nor_option
-        assert_raises(ActiveRecord::IrreversibleMigration) do
+        assert_raises(ActiveRecord::IrreversibleMigration, match: "To avoid mistakes, drop_table is only reversible if given options or a block (can be empty).") do
           @recorder.inverse_of :drop_table, [:people_reminders]
         end
       end
 
+      def test_invert_drop_table_with_multiple_tables
+        assert_raises(ActiveRecord::IrreversibleMigration, match: "To avoid mistakes, drop_table is only reversible if given a single table name.") do
+          @recorder.inverse_of :drop_table, [:musics, :artists]
+        end
+      end
+
+      def test_invert_drop_table_with_multiple_tables_and_options
+        assert_raises(ActiveRecord::IrreversibleMigration, match: "To avoid mistakes, drop_table is only reversible if given a single table name.") do
+          @recorder.inverse_of :drop_table, [:musics, :artists, id: false]
+        end
+      end
+
+      def test_invert_drop_table_with_multiple_tables_and_block
+        block = Proc.new { }
+
+        assert_raises(ActiveRecord::IrreversibleMigration, match: "To avoid mistakes, drop_table is only reversible if given a single table name.") do
+          @recorder.inverse_of :drop_table, [:musics, :artists], &block
+        end
+      end
+
       def test_invert_create_join_table
         drop_join_table = @recorder.inverse_of :create_join_table, [:musics, :artists]
         assert_equal [:drop_join_table, [:musics, :artists], nil], drop_join_table