database_consistency - rubygems Package Compare versions

Comparing version 2.1.3 to 3.0.0

lib/database_consi...eckers/missing_index_find_by_checker.rb

+# frozen_string_literal: true
+
+begin
+  require 'prism'
+rescue LoadError
+  # Prism is not available; this checker will be disabled on Ruby < 3.3
+end
+
+module DatabaseConsistency
+  module Checkers
+    # This class checks for columns used in find_by queries that are missing a database index.
+    # It uses the Prism parser (Ruby stdlib since 3.3) to traverse the AST of all project
+    # source files (found by iterating loaded constants and excluding gem paths) and detect
+    # calls such as find_by_<column>, find_by(column: ...) and find_by("column" => ...).
+    # The checker is automatically skipped on Ruby versions where Prism is not available.
+    class MissingIndexFindByChecker < ColumnChecker
+      Report = ReportBuilder.define(
+        DatabaseConsistency::Report,
+        :source_location,
+        :total_findings_count
+      )
+
+      private
+
+      # We skip check when:
+      #  - Prism is not available (Ruby < 3.3)
+      #  - column is the primary key (always indexed)
+      #  - column name does not appear in any find_by call across project source files
+      def preconditions
+        defined?(Prism) && !primary_key_column? && find_by_used?
+      end
+
+      # Table of possible statuses
+      # | index    | status |
+      # | -------- | ------ |
+      # | present  | ok     |
+      # | missing  | fail   |
+      def check
+        if indexed?
+          report_template(:ok)
+        else
+          report_template(:fail, error_slug: :missing_index_find_by)
+        end
+      end
+
+      def report_template(status, error_slug: nil)
+        Report.new(
+          status: status,
+          error_slug: error_slug,
+          error_message: nil,
+          source_location: (status == :fail ? @find_by_location : nil),
+          total_findings_count: (status == :fail ? @find_by_count : nil),
+          **report_attributes
+        )
+      end
+
+      def find_by_used?
+        entry = PrismHelper.find_by_calls_index.dig(model.name.to_s, column.name.to_s)
+        return false unless entry
+
+        @find_by_location = entry[:first_location]
+        @find_by_count = entry[:total_findings_count]
+        true
+      end
+
+      def indexed?
+        model.connection.indexes(model.table_name).any? do |index|
+          Helper.extract_index_columns(index.columns).first == column.name.to_s
+        end
+      end
+
+      def primary_key_column?
+        column.name.to_s == model.primary_key.to_s
+      end
+
+      if defined?(Prism)
+        # Prism AST visitor that collects ALL find_by calls from a source file into a results hash.
+        # Key: [model_name, column_name] -- model_name is derived from the explicit receiver or the
+        # lexical class/module scope for bare calls. Bare calls outside any class are ignored.
+        # Value: "file:line" location of the first matching call.
+        #
+        # Handles:
+        #  - find_by_<col>(<value>) / Model.find_by_<col>!  (dynamic finder)
+        #  - find_by(col: <value>) / Model.find_by col:     (symbol-key hash)
+        #  - find_by("col" => <value>)                      (string-key hash)
+        #
+        # Defined only when Prism is available (Ruby 3.3+).
+        class FindByCollector < Prism::Visitor
+          # Matches the full column name from a dynamic finder method name.
+          # e.g. find_by_email -> "email", find_by_first_name -> "first_name"
+          # Multi-column patterns like find_by_name_and_email extract "name_and_email"
+          # which won't match any single-column name, so there are no false positives.
+          DYNAMIC_FINDER_RE = /\Afind_by_(.+?)!?\z/.freeze
+
+          attr_reader :results
+
+          def initialize(file)
+            super()
+            @file = file
+            @results = {}
+            @scope_stack = []
+          end
+
+          def visit_class_node(node)
+            @scope_stack.push(constant_path_name(node.constant_path))
+            super
+          ensure
+            @scope_stack.pop
+          end
+
+          def visit_module_node(node)
+            @scope_stack.push(constant_path_name(node.constant_path))
+            super
+          ensure
+            @scope_stack.pop
+          end
+
+          def visit_call_node(node)
+            name = node.name.to_s
+            if (match = DYNAMIC_FINDER_RE.match(name))
+              model_key = receiver_to_model_key(node.receiver)
+              store(model_key, match[1], node) unless model_key == :skip
+            elsif name == 'find_by' && node.arguments
+              col = single_hash_column(node.arguments)
+              model_key = receiver_to_model_key(node.receiver)
+              store(model_key, col, node) if col && model_key != :skip
+            end
+            super
+          end
+
+          private
+
+          def current_scope
+            @scope_stack.empty? ? nil : @scope_stack.join('::')
+          end
+
+          def store(model_key, col, node)
+            key = [model_key, col]
+            @results[key] ||= []
+            @results[key] << "#{@file}:#{node.location.start_line}"
+          end
+
+          def receiver_to_model_key(receiver)
+            case receiver
+            when nil then current_scope || :skip
+            when Prism::ConstantReadNode, Prism::ConstantPathNode
+              constant_path_name(receiver)
+            when Prism::CallNode
+              scoped_receiver_model(receiver)
+            else
+              :skip
+            end
+          end
+
+          def scoped_receiver_model(call_node)
+            return :skip unless %w[unscoped includes].include?(call_node.name.to_s)
+
+            rec = call_node.receiver
+            return :skip unless rec.is_a?(Prism::ConstantReadNode) || rec.is_a?(Prism::ConstantPathNode)
+
+            constant_path_name(rec)
+          end
+
+          def constant_path_name(node)
+            case node
+            when Prism::ConstantReadNode then node.name.to_s
+            when Prism::ConstantPathNode then "#{constant_path_name(node.parent)}::#{node.name}"
+            end
+          end
+
+          def single_hash_column(arguments_node)
+            arguments_node.arguments.each do |arg|
+              next unless arg.is_a?(Prism::KeywordHashNode) && arg.elements.size == 1
+
+              assoc = arg.elements.first
+              next unless assoc.is_a?(Prism::AssocNode)
+
+              key = assoc.key
+              return key.unescaped if key.is_a?(Prism::SymbolNode) || key.is_a?(Prism::StringNode)
+            end
+            nil
+          end
+        end
+      end
+    end
+  end
+end

lib/database_consi...el_checkers/view_primary_key_checker.rb

+# frozen_string_literal: true
+
+module DatabaseConsistency
+  module Checkers
+    # This class checks that a model pointing to a view has a primary_key set and that column exists
+    class ViewPrimaryKeyChecker < ModelChecker
+      private
+
+      def preconditions
+        ActiveRecord::VERSION::MAJOR >= 5 &&
+          !model.abstract_class? &&
+          model.connection.view_exists?(model.table_name)
+      end
+
+      # Table of possible statuses
+      # | primary_key set | column exists | status |
+      # | --------------- | ------------- | ------ |
+      # | no              | -             | fail   |
+      # | yes             | no            | fail   |
+      # | yes             | yes           | ok     |
+      def check
+        if model.primary_key.blank?
+          report_template(:fail, error_slug: :view_missing_primary_key)
+        elsif !primary_key_column_exists?
+          report_template(:fail, error_slug: :view_primary_key_column_missing)
+        else
+          report_template(:ok)
+        end
+      end
+
+      def primary_key_column_exists?
+        Array(model.primary_key).all? { |key| model.column_names.include?(key.to_s) }
+      end
+    end
+  end
+end

lib/database_consistency/files_helper.rb

+# frozen_string_literal: true
+
+module DatabaseConsistency
+  # The module contains file system helper methods for locating project source files.
+  module FilesHelper
+    module_function
+
+    # Returns all unique project source file paths (non-gem Ruby files from loaded constants).
+    # Memoized so the file system walk happens once per database_consistency run.
+    def project_source_files
+      @project_source_files ||=
+        if Module.respond_to?(:const_source_location)
+          collect_source_files
+        else
+          []
+        end
+    end
+
+    def collect_source_files
+      files = []
+      ObjectSpace.each_object(Module) { |mod| files << source_file_path(mod) }
+      files.compact.uniq
+    end
+
+    def source_file_path(mod)
+      return unless (name = mod.name)
+
+      file, = Module.const_source_location(name)
+      return unless file && File.exist?(file)
+      return if excluded_source_file?(file)
+
+      file
+    rescue NameError, ArgumentError
+      nil
+    end
+
+    def excluded_source_file?(file)
+      return true if defined?(Bundler) && file.include?(Bundler.bundle_path.to_s)
+      return true if defined?(Gem) && file.include?(Gem::RUBYGEMS_DIR)
+
+      excluded_by_ruby_stdlib?(file)
+    end
+
+    def excluded_by_ruby_stdlib?(file)
+      return false unless defined?(RbConfig)
+
+      file.include?(RbConfig::CONFIG['rubylibdir']) ||
+        file.include?(RbConfig::CONFIG['bindir']) ||
+        file.include?(RbConfig::CONFIG['sbindir'])
+    end
+  end
+end

lib/database_consistency/prism_helper.rb

+# frozen_string_literal: true
+
+module DatabaseConsistency
+  # The module contains Prism AST helper methods for scanning project source files.
+  module PrismHelper
+    module_function
+
+    # Returns a memoized index: {model_name => {column_name => "file:line"}}.
+    # Built once per run by scanning all project source files with Prism (Ruby 3.3+).
+    # Bare find_by calls are resolved to their lexical class/module scope.
+    def find_by_calls_index
+      return {} unless defined?(Prism)
+
+      @find_by_calls_index ||= build_find_by_calls_index
+    end
+
+    def build_find_by_calls_index
+      FilesHelper.project_source_files.each_with_object({}) do |file, index|
+        collector = Checkers::MissingIndexFindByChecker::FindByCollector.new(file)
+        collector.visit(Prism.parse_file(file).value)
+        merge_collector_results(collector.results, index)
+      rescue StandardError
+        nil
+      end
+    end
+
+    def merge_collector_results(results, index)
+      results.each do |(model_key, col), locations|
+        index[model_key] ||= {}
+        if (entry = index[model_key][col])
+          entry[:total_findings_count] += locations.size
+        else
+          index[model_key][col] = { first_location: locations.first, total_findings_count: locations.size }
+        end
+      end
+    end
+  end
+end

lib/database_consi...writers/simple/missing_index_find_by.rb

+# frozen_string_literal: true
+
+module DatabaseConsistency
+  module Writers
+    module Simple
+      class MissingIndexFindBy < Base # :nodoc:
+        private
+
+        def template
+          'column is used in find_by but is missing an index%<source_location>s'
+        end
+
+        def attributes
+          if report.source_location
+            count = report.total_findings_count || 1
+            count_str = count > 1 ? ", and #{count - 1} more occurrences" : ''
+            { source_location: " (found at #{report.source_location}#{count_str})" }
+          else
+            { source_location: '' }
+          end
+        end
+
+        def unique_attributes
+          {
+            table_or_model_name: report.table_or_model_name,
+            column_or_attribute_name: report.column_or_attribute_name
+          }
+        end
+      end
+    end
+  end
+end

lib/database_consi...ters/simple/view_missing_primary_key.rb

+# frozen_string_literal: true
+
+module DatabaseConsistency
+  module Writers
+    module Simple
+      class ViewMissingPrimaryKey < Base # :nodoc:
+        private
+
+        def template
+          'model pointing to a view should have primary_key set'
+        end
+
+        def unique_attributes
+          {
+            table_or_model_name: report.table_or_model_name
+          }
+        end
+      end
+    end
+  end
+end

lib/database_consi...mple/view_primary_key_column_missing.rb

+# frozen_string_literal: true
+
+module DatabaseConsistency
+  module Writers
+    module Simple
+      class ViewPrimaryKeyColumnMissing < Base # :nodoc:
+        private
+
+        def template
+          'model pointing to a view has a non-existent primary_key column set'
+        end
+
+        def unique_attributes
+          {
+            table_or_model_name: report.table_or_model_name
+          }
+        end
+      end
+    end
+  end
+end

lib/database_consistency.rb

@@ -7,2 +7,4 @@ # frozen_string_literal: true
 require 'database_consistency/helper'
+require 'database_consistency/files_helper'
+require 'database_consistency/prism_helper'
 require 'database_consistency/configuration'
@@ -48,2 +50,5 @@ require 'database_consistency/rescue_error'
 require 'database_consistency/writers/simple/missing_dependent_destroy'
+require 'database_consistency/writers/simple/missing_index_find_by'
+require 'database_consistency/writers/simple/view_missing_primary_key'
+require 'database_consistency/writers/simple/view_primary_key_column_missing'
 require 'database_consistency/writers/simple_writer'
@@ -82,2 +87,3 @@
 require 'database_consistency/checkers/model_checkers/missing_table_checker'
+require 'database_consistency/checkers/model_checkers/view_primary_key_checker'
 
@@ -99,2 +105,3 @@ require 'database_consistency/checkers/association_checkers/association_checker'
 require 'database_consistency/checkers/column_checkers/implicit_ordering_checker'
+require 'database_consistency/checkers/column_checkers/missing_index_find_by_checker'
 
@@ -101,0 +108,0 @@ require 'database_consistency/checkers/validator_checkers/validator_checker'

lib/database_consi...checkers/foreign_key_cascade_checker.rb

@@ -31,2 +31,3 @@ # frozen_string_literal: true
           !association.belongs_to? &&
+          association.association_primary_key.present? &&
           foreign_key &&
@@ -72,6 +73,6 @@ DEPENDENT_OPTIONS.include?(dependent_option)
                      .foreign_keys(association.klass.table_name)
-                     .find { |fk| fk.column == association.foreign_key.to_s }
+                     .find { |fk| (Helper.extract_columns(association.foreign_key) - Array.wrap(fk.column)).empty? }
       end
 
-      def report_template(status, error_slug: nil)
+      def report_template(status, error_slug: nil) # rubocop:disable Metrics/AbcSize
         Report.new(
@@ -82,5 +83,5 @@ status: status,
           primary_table: association.table_name.to_s,
-          primary_key: association.association_primary_key.to_s,
+          primary_key: Helper.extract_columns(association.association_primary_key).join('+'),
           foreign_table: association.active_record.table_name.to_s,
-          foreign_key: association.foreign_key.to_s,
+          foreign_key: Helper.extract_columns(association.foreign_key).join('+'),
           cascade_option: required_foreign_key_cascade,
@@ -87,0 +88,0 @@ **report_attributes

lib/database_consi...rs/missing_dependent_destroy_checker.rb

@@ -18,3 +18,3 @@ # frozen_string_literal: true
       def preconditions
-        association.belongs_to? && foreign_key
+        association.belongs_to? && !association.polymorphic? && foreign_key
       end
@@ -21,0 +21,0 @@

lib/database_consi.../index_checkers/unique_index_checker.rb

@@ -32,6 +32,9 @@ # frozen_string_literal: true
 
-        uniqueness_validators.any? do |validator|
-          validator.attributes.any? do |attribute|
-            sorted_index_columns == Helper.sorted_uniqueness_validator_columns(attribute, validator, model)
-          end
+        uniqueness_validators.any? { |validator| validator_matches?(validator) }
+      end
+
+      def validator_matches?(validator)
+        validator.attributes.any? do |attribute|
+          sorted_index_columns == Helper.sorted_uniqueness_validator_columns(attribute, validator, model) &&
+            Helper.conditions_match_index?(model, validator.options[:conditions], index.where)
         end
@@ -38,0 +41,0 @@ end

lib/database_consi...heckers/missing_unique_index_checker.rb

@@ -50,7 +50,11 @@ # frozen_string_literal: true
       def unique_index
-        @unique_index ||= model.connection.indexes(model.table_name).find do |index|
-          index.unique && Helper.extract_index_columns(index.columns).sort == sorted_uniqueness_validator_columns
-        end
+        @unique_index ||= model.connection.indexes(model.table_name).find { |index| index_matches?(index) }
       end
 
+      def index_matches?(index)
+        index.unique &&
+          Helper.extract_index_columns(index.columns).sort == sorted_uniqueness_validator_columns &&
+          Helper.conditions_match_index?(model, validator.options[:conditions], index.where)
+      end
+
       def primary_key_covers_validation?
@@ -57,0 +61,0 @@ primary_key = model.connection.primary_key(model.table_name)

lib/database_consistency/helper.rb

@@ -147,2 +147,34 @@ # frozen_string_literal: true
 
+    # Returns the normalized WHERE SQL produced by a conditions proc, or nil if
+    # it cannot be determined (complex proc, unsupported AR version, etc.).
+    def conditions_where_sql(model, conditions)
+      sql = model.unscoped.instance_exec(&conditions).to_sql
+      where_part = sql.split(/\bWHERE\b/i, 2).last
+      return nil unless where_part
+
+      normalize_sql(where_part.gsub("#{model.quoted_table_name}.", '').gsub('"', ''))
+    rescue StandardError
+      nil
+    end
+
+    # Returns true when validator conditions and index WHERE clause are a valid
+    # pairing: both absent means a match; exactly one present means no match;
+    # when both present the normalized SQL is compared.
+    def conditions_match_index?(model, conditions, index_where)
+      return true if conditions.nil? && index_where.blank?
+      return false if conditions.nil? || index_where.blank?
+
+      conditions_sql = conditions_where_sql(model, conditions)
+      # Strip one level of outer parentheses that some databases (e.g. PostgreSQL)
+      # add when storing/returning the index WHERE clause.
+      normalized_where = normalize_sql(index_where.sub(/\A\s*\((.+)\)\s*\z/m, '\1'))
+      conditions_sql&.casecmp?(normalized_where)
+    end
+
+    def normalize_sql(sql)
+      sql.gsub(/\bTRUE\b/i, '1').gsub(/\bFALSE\b/i, '0')
+         .gsub(/ = 't'/, ' = 1').gsub(/ = 'f'/, ' = 0')
+         .strip
+    end
+
     # @return [String]
@@ -149,0 +181,0 @@ def wrapped_attribute_name(attribute, validator, model)

lib/database_consistency/version.rb

 # frozen_string_literal: true
 
 module DatabaseConsistency
-  VERSION = '2.1.3'
+  VERSION = '3.0.0'
 end

No alert changes

No metric changes