Enforcing the not_valid: true option when creating constraints, including foreign keys

.rubocop.yml

@@ -16,3 +16,6 @@ Metrics/BlockLength:
 Naming/FileName:
   Exclude:
     - 'lib/rubocop-sequel.rb'
+
+RSpec/ExampleLength:
+  Enabled: false

config/default.yml

@@ -11,6 +11,13 @@ Sequel/ConcurrentIndex:
   VersionAdded: 0.0.1
   VersionChanged: 0.3.3
 
+Sequel/EnforceNotValidConstraint:
+  Description: 'Ensures that new constraints are created with the `not_valid: true` option.'
+  Reference: https://www.rubydoc.info/gems/rubocop-sequel/RuboCop/Cop/Sequel/EnforceNotValidConstraint
+  Enabled: false
+  VersionAdded: '0.3.4'
+  VersionChanged: '0.3.4'
+
 Sequel/JSONColumn:
   Description: >-
     Advocates the use of the `jsonb` column type over `json` or `hstore` for performance

lib/rubocop-sequel.rb

@@ -12,3 +12,4 @@
 require 'rubocop/cop/sequel/migration_name'
 require 'rubocop/cop/sequel/save_changes'
 require 'rubocop/cop/sequel/partial_constraint'
+require 'rubocop/cop/sequel/enforce_not_valid_constraint'

lib/rubocop/cop/sequel/enforce_not_valid_constraint.rb

@@ -0,0 +1,108 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Sequel
+      # Ensures that new constraints are created with the `not_valid: true` option.
+      #
+      # This cop checks database migration files for `add_foreign_key` and
+      # `add_constraint` operations to ensure that new constraints are created
+      # with the `not_valid: true` option, which is necessary for deferred validation.
+      #
+      # This cop can be configured to exclude or include specific tables through
+      # the `ExcludeTables` and `IncludeTables` options.
+      #
+      # By using the not_valid: true option, the constraint is created but not enforced
+      # on existing data. This allows the database to continue operating normally without
+      # the overhead of immediate validation. You can then validate the existing data
+      # in a controlled manner using a separate process or during off-peak hours,
+      # minimizing the impact on your application’s performance. This approach ensures
+      # that the new constraint is applied to new data immediately while providing
+      # flexibility in handling existing data, promoting better performance and
+      # stability during schema changes. Also it is a good
+      #
+      # @example
+      #   # bad
+      #   add_foreign_key :users, :orders, column: :user_id
+      #
+      #   # good
+      #   add_foreign_key :users, :orders, column: :user_id, not_valid: true
+      #
+      #   # bad
+      #   alter_table :users do
+      #     add_constraint(:user_email_check) { "email IS NOT NULL" }
+      #   end
+      #
+      #   # good
+      #   alter_table :users do
+      #     add_constraint(:user_email_check, not_valid: true) { "email IS NOT NULL" }
+      #   end
+      #
+      # @example Configuration
+      #   Sequel/EnforceNotValidConstraint:
+      #     Enabled: true
+      #     Include:
+      #       - "db/migrations"
+      #     ExcludeTables:
+      #       - users
+      #     IncludeTables:
+      #       - comments
+      class EnforceNotValidConstraint < Base
+        MSG = "Add the 'not_valid: true' option when creating new constraints to ensure " \
+              'they are not validated immediately.'
+
+        # Matcher for alter_table blocks
+        def_node_matcher :alter_table_block?, <<-MATCHER
+          (block
+            (send _ :alter_table ({str sym} $_))
+            _
+            $...
+          )
+        MATCHER
+
+        # Matcher for constraint operations like add_foreign_key and add_constraint
+        def_node_search :constraint_operations, <<~MATCHER
+          (send _ {:add_foreign_key :add_constraint} ...)
+        MATCHER
+
+        # Matcher for check if the not_valid: true argument is present
+        def_node_search :not_valid_constraint?, <<~MATCHER
+          (pair (_ {:not_valid "not_valid"}) true)
+        MATCHER
+
+        # Check constraint operations inside the alter_table block
+        #
+        # @param node [RuboCop::AST::Node] the AST node being checked
+        def on_block(node)
+          alter_table_block?(node) do |table_name, _block_nodes|
+            next if exclude_table?(table_name)
+
+            constraint_operations(node).each do |block_node|
+              validate_node(block_node)
+            end
+          end
+        end
+
+        private
+
+        # Validate constraint operations
+        #
+        # @param node [RuboCop::AST::Node] the AST node being checked
+        def validate_node(node)
+          add_offense(node, message: MSG) unless not_valid_constraint?(node)
+        end
+
+        # Skip the table if it is excluded or not included
+        #
+        # @param table_name [String,Symbol] the name of the table
+        # @return [Boolean] whether the table should be skipped
+        def exclude_table?(table_name)
+          return true if cop_config['ExcludeTables']&.include?(table_name.to_s)
+          return false unless cop_config.key?('IncludeTables')
+
+          !cop_config['IncludeTables'].include?(table_name.to_s)
+        end
+      end
+    end
+  end
+end

spec/project_spec.rb

@@ -15,7 +15,7 @@
       expect(description.include?("\n")).to be(false)
     end
 
-    it 'stars from a verb' do # rubocop:disable RSpec/ExampleLength
+    it 'stars from a verb' do
       description = config.dig(cop_name, 'Description')
       start_with_subject = description.match(/\AThis cop (?<verb>.+?) .*/)
       suggestion = start_with_subject[:verb]&.capitalize if start_with_subject
@@ -97,7 +97,7 @@
     end
   end
 
-  it 'sorts cop names alphabetically' do # rubocop:disable RSpec/ExampleLength
+  it 'sorts cop names alphabetically' do
     previous_key = ''
     config_default = YAML.load_file('config/default.yml')
 

spec/rubocop/cop/sequel/enforce_not_valid_constraint_spec.rb

@@ -0,0 +1,162 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Sequel::EnforceNotValidConstraint do
+  subject(:cop) { described_class.new(config) }
+
+  let(:cop_config) { { 'Enabled' => true } }
+  let(:config) { RuboCop::Config.new('Sequel/EnforceNotValidConstraint' => cop_config) }
+
+  context 'when add_constraint is used' do
+    it 'registers an offense when used without `not_valid: true` argument' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :foo do
+          add_constraint :not_valid, Sequel.lit("jsonb_typeof(column) = 'object'")
+          add_constraint({name: "not_valid"}, Sequel.lit("jsonb_typeof(column) = 'object'"))
+        end
+      RUBY
+
+      expect(offenses.size).to eq(2)
+    end
+
+    it 'registers an offense when used with `not_valid: false` argument' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :foo do
+          add_constraint({name: "valid", not_valid: false}, Sequel.lit("jsonb_typeof(column) = 'object'"))
+        end
+      RUBY
+
+      expect(offenses.size).to eq(1)
+    end
+
+    it 'does not register an offense when using `not_valid: true` argument' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :foo do
+          add_constraint({name: "name", not_valid: true }, Sequel.lit("jsonb_typeof(column) = 'object'"))
+        end
+      RUBY
+
+      expect(offenses.size).to eq(0)
+    end
+
+    context 'when wrap with additional block' do
+      it 'registers an offense when used with `not_valid: false` argument' do
+        offenses = inspect_source(<<~RUBY)
+          alter_table :foo do
+            begin
+              add_constraint({name: "valid", not_valid: false}, Sequel.lit("jsonb_typeof(column) = 'object'"))
+            end
+          end
+        RUBY
+
+        expect(offenses.size).to eq(1)
+      end
+    end
+
+    context 'when hash-rockets is used to define argument' do
+      it "does not register an offense when using `'not_valid' => true` argument" do
+        offenses = inspect_source(<<~RUBY)
+          alter_table :foo do
+            add_constraint({name: "name", "not_valid" => true }, Sequel.lit("jsonb_typeof(column) = 'object'"))
+          end
+        RUBY
+
+        expect(offenses.size).to eq(0)
+      end
+    end
+  end
+
+  context 'when add_foreign_key is used' do
+    it 'registers an offense when used without `not_valid: true` argument' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :foo do
+          add_foreign_key(:not_valid, :table)
+          add_foreign_key(:not_valid, :table, name: "not_valid")
+        end
+      RUBY
+
+      expect(offenses.size).to eq(2)
+    end
+
+    it 'registers an offense when used with `not_valid: false` argument' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :foo do
+          add_foreign_key(:valid, :table, name: "not_valid", not_valid: false)
+        end
+      RUBY
+
+      expect(offenses.size).to eq(1)
+    end
+
+    it 'does not register an offense when using `not_valid: true` argument' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :foo do
+          add_foreign_key(:not_valid, :table, name: "not_valid", not_valid: true)
+        end
+      RUBY
+
+      expect(offenses.size).to eq(0)
+    end
+  end
+
+  context 'with ExcludeTables setting' do
+    let(:cop_config) do
+      {
+        'Enabled' => true,
+        'ExcludeTables' => ['excluded_table']
+      }
+    end
+
+    it 'does not register an offense when using `not_valid: true` argument inside excluded table migration' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :excluded_table do
+          add_foreign_key(:not_valid, :table)
+          add_foreign_key(:not_valid, :table, name: "not_valid")
+        end
+      RUBY
+
+      expect(offenses.size).to eq(0)
+    end
+
+    it 'register an offense when using `not_valid: true` argument for not excluded table' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :not_excluded_table do
+          add_foreign_key(:not_valid, :table)
+          add_foreign_key(:not_valid, :table, name: "not_valid")
+        end
+      RUBY
+
+      expect(offenses.size).to eq(2)
+    end
+  end
+
+  context 'with IncludeTables setting' do
+    let(:cop_config) do
+      {
+        'Enabled' => true,
+        'IncludeTables' => ['included_table']
+      }
+    end
+
+    it 'does not register an offense when using `not_valid: true` argument inside excluded table migration' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :included_table do
+          add_foreign_key(:not_valid, :table)
+          add_foreign_key(:not_valid, :table, name: "not_valid")
+        end
+      RUBY
+
+      expect(offenses.size).to eq(2)
+    end
+
+    it 'register an offense when using `not_valid: true` argument for not excluded table' do
+      offenses = inspect_source(<<~RUBY)
+        alter_table :not_included_table do
+          add_foreign_key(:not_valid, :table)
+          add_foreign_key(:not_valid, :table, name: "not_valid")
+        end
+      RUBY
+
+      expect(offenses.size).to eq(0)
+    end
+  end
+end