Make validate_not_null_constraint, remove_foreign_key and `remove…

…_check_constraint` idempotent

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## master (unreleased)
 
+- Make `validate_not_null_constraint`, `remove_foreign_key` and `remove_check_constraint` idempotent
+
 - Add helpers for validating constraints in background
 
     ```ruby

lib/online_migrations/schema_statements.rb

@@ -520,7 +520,17 @@ def add_not_null_constraint(table_name, column_name, name: nil, validate: true)
     #
     def validate_not_null_constraint(table_name, column_name, name: nil)
       name ||= __not_null_constraint_name(table_name, column_name)
-      validate_check_constraint(table_name, name: name)
+      column = column_for(table_name, column_name)
+
+      if column.null == false &&
+         !__not_null_constraint_exists?(table_name, column_name, name: name)
+        Utils.say(<<~MSG.squish)
+          NOT NULL constraint was not validated: it does not exist and
+          column #{table_name}.#{column_name} is already defined as `NOT NULL`
+        MSG
+      else
+        validate_check_constraint(table_name, name: name)
+      end
     end
 
     # Removes a NOT NULL constraint from the column
@@ -844,6 +854,22 @@ def validate_foreign_key(from_table, to_table = nil, **options)
       end
     end
 
+    # Extends default method to be idempotent.
+    #
+    # @see https://edgeapi.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-remove_foreign_key
+    #
+    def remove_foreign_key(from_table, to_table = nil, **options)
+      # Do not consider validation for idempotency.
+      if foreign_key_exists?(from_table, to_table, **options.except(:validate))
+        super
+      else
+        Utils.say(<<~MSG.squish)
+          Foreign key was not removed because it does not exist (this may be due to an aborted migration or similar).
+          from_table: #{from_table}, to_table: #{to_table}, options: #{options.inspect}
+        MSG
+      end
+    end
+
     # Extends default method to be idempotent
     #
     # @see https://edgeapi.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_check_constraint
@@ -888,6 +914,21 @@ def validate_check_constraint(table_name, **options)
       end
     end
 
+    # Extends default method to be idempotent
+    #
+    # @see https://edgeapi.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-remove_check_constraint
+    #
+    def remove_check_constraint(table_name, expression = nil, **options)
+      if __check_constraint_exists?(table_name, expression: expression, **options)
+        super
+      else
+        Utils.say(<<~MSG.squish)
+          Check constraint was not removed because it does not exist (this may be due to an aborted migration or similar).
+          table_name: #{table_name}, expression: #{expression}, options: #{options.inspect}
+        MSG
+      end
+    end
+
     if Utils.ar_version >= 7.1
       def add_exclusion_constraint(table_name, expression, **options)
         if __exclusion_constraint_exists?(table_name, expression: expression, **options)

test/schema_statements/check_constraints_test.rb

@@ -92,6 +92,21 @@ def test_validate_non_existing_check_constraint
       end
     end
 
+    def test_remove_check_constraint
+      connection.add_check_constraint :milestones, "points >= 0"
+      assert_equal 1, connection.check_constraints(:milestones).size
+      connection.remove_check_constraint :milestones, "points >= 0"
+      assert_empty connection.check_constraints(:milestones)
+    end
+
+    def test_remove_check_constraint_when_not_exists
+      assert_empty connection.check_constraints(:milestones)
+
+      assert_nothing_raised do
+        connection.remove_check_constraint :milestones, "points >= 0"
+      end
+    end
+
     def test_add_not_null_constraint
       milestone = Milestone.create!(name: nil)
 
@@ -186,6 +201,14 @@ def test_validate_non_existing_not_null_constraint_raises
       end
     end
 
+    def test_validate_non_existing_not_null_constraint_and_already_not_null_column_passes
+      connection.change_column_null :milestones, :name, false
+
+      assert_nothing_raised do
+        connection.validate_not_null_constraint :milestones, :name, name: "non_existing"
+      end
+    end
+
     def test_remove_not_null_constraint
       connection.add_not_null_constraint :milestones, :name
       connection.remove_not_null_constraint :milestones, :name

test/schema_statements/foreign_keys_test.rb

@@ -36,6 +36,22 @@ def test_validate_non_existing_foreign_key
       end
     end
 
+    def test_remove_foreign_key
+      connection.add_foreign_key :milestones, :projects
+      assert connection.foreign_key_exists?(:milestones, :projects)
+
+      connection.remove_foreign_key :milestones, :projects
+      assert_not connection.foreign_key_exists?(:milestones, :projects)
+    end
+
+    def test_remove_foreign_key_when_not_exists
+      assert_empty connection.foreign_keys(:milestones)
+
+      assert_nothing_raised do
+        connection.remove_foreign_key :milestones, :projects
+      end
+    end
+
     def test_validate_foreign_key_in_background
       connection.add_foreign_key(:milestones, :projects, validate: false)