Merge pull request rails#53795 from fatkodima/fix-constraint-methods-be-revertible-with-invalid-options

Fix add_unique_constraint/add_check_constraint/add_foreign_key to be revertible when given invalid options

Author: kamipo

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

@@ -159,6 +159,8 @@ def export_name_on_schema_dump?
       end
 
       def defined_for?(to_table: nil, validate: nil, **options)
+        options = options.slice(*self.options.keys)
+
         (to_table.nil? || to_table.to_s == self.to_table) &&
           (validate.nil? || validate == self.options.fetch(:validate, validate)) &&
           options.all? { |k, v| Array(self.options[k]).map(&:to_s) == Array(v).map(&:to_s) }
@@ -185,6 +187,8 @@ def export_name_on_schema_dump?
       end
 
       def defined_for?(name:, expression: nil, validate: nil, **options)
+        options = options.slice(*self.options.keys)
+
         self.name == name.to_s &&
           (validate.nil? || validate == self.options.fetch(:validate, validate)) &&
           options.all? { |k, v| self.options[k].to_s == v.to_s }

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

@@ -232,6 +232,8 @@ def export_name_on_schema_dump?
         end
 
         def defined_for?(name: nil, column: nil, **options)
+          options = options.slice(*self.options.keys)
+
           (name.nil? || self.name == name.to_s) &&
             (column.nil? || Array(self.column) == Array(column).map(&:to_s)) &&
             options.all? { |k, v| self.options[k].to_s == v.to_s }

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

@@ -75,6 +75,7 @@ def remove_foreign_key(from_table, to_table = nil, **options)
               Base.pluralize_table_names ? table.pluralize : table
             end
             table = strip_table_name_prefix_and_suffix(table)
+            options = options.slice(*fk.options.keys)
             fk_to_table = strip_table_name_prefix_and_suffix(fk.to_table)
             fk_to_table == table && options.all? { |k, v| fk.options[k].to_s == v.to_s }
           end || raise(ArgumentError, "Table '#{from_table}' has no foreign key for #{to_table || options}")

activerecord/test/cases/invertible_migration_test.rb

@@ -19,6 +19,7 @@ def change
           t.column :content, :text
           t.column :remind_at, :datetime
           t.column :place_id, :integer
+          t.column :parent_id, :bigint
         end
       end
     end
@@ -220,6 +221,24 @@ def change
       end
     end
 
+    class RevertUniqueConstraintWithInvalidOption < SilentMigration
+      def change
+        add_unique_constraint :horses, :place_id, invalid: :option
+      end
+    end
+
+    class RevertForeignKeyWithInvalidOption < SilentMigration
+      def change
+        add_foreign_key :horses, :horses, column: :parent_id, invalid: :option
+      end
+    end
+
+    class RevertCheckConstraintWithInvalidOption < SilentMigration
+      def change
+        add_check_constraint :horses, "place_id > 0", invalid: :option
+      end
+    end
+
     self.use_transactional_tests = false
 
     setup do
@@ -526,5 +545,42 @@ def test_up_only
       assert_not connection.column_exists?(:horses, :oldie)
       Horse.reset_column_information
     end
+
+    if ActiveRecord::Base.lease_connection.supports_unique_constraints?
+      def test_migrate_revert_add_unique_constraint_with_invalid_option
+        InvertibleMigration.new.migrate(:up)
+        RevertUniqueConstraintWithInvalidOption.new.migrate(:up)
+
+        connection = ActiveRecord::Base.lease_connection
+        assert_equal 1, connection.unique_constraints(:horses).count
+
+        RevertUniqueConstraintWithInvalidOption.new.migrate(:down)
+        assert_empty connection.unique_constraints(:horses)
+      end
+    end
+
+    def test_migrate_revert_add_foreign_key_with_invalid_option
+      InvertibleMigration.new.migrate(:up)
+      RevertForeignKeyWithInvalidOption.new.migrate(:up)
+
+      connection = ActiveRecord::Base.lease_connection
+      assert_equal 1, connection.foreign_keys(:horses).count
+
+      RevertForeignKeyWithInvalidOption.new.migrate(:down)
+      assert_empty connection.foreign_keys(:horses)
+    end
+
+    if ActiveRecord::Base.lease_connection.supports_check_constraints?
+      def test_migrate_revert_add_check_constraint_with_invalid_option
+        InvertibleMigration.new.migrate(:up)
+        RevertCheckConstraintWithInvalidOption.new.migrate(:up)
+
+        connection = ActiveRecord::Base.lease_connection
+        assert_equal 1, connection.check_constraints(:horses).count
+
+        RevertCheckConstraintWithInvalidOption.new.migrate(:down)
+        assert_empty connection.check_constraints(:horses)
+      end
+    end
   end
 end