Reduce the number of queries by caching schema info

lib/active_record_doctor.rb

@@ -25,6 +25,7 @@
 require "active_record_doctor/errors"
 require "active_record_doctor/help"
 require "active_record_doctor/runner"
+require "active_record_doctor/caching_schema_inspector"
 require "active_record_doctor/version"
 require "active_record_doctor/config"
 require "active_record_doctor/config/loader"

lib/active_record_doctor/caching_schema_inspector.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module ActiveRecordDoctor
+  class CachingSchemaInspector # :nodoc:
+    def initialize(connection)
+      @connection = connection
+      @primary_keys = {}
+      @indexes = {}
+      @foreign_keys = {}
+      @check_constraints = {}
+    end
+
+    def tables
+      @tables ||= @connection.tables
+    end
+
+    def data_sources
+      @data_sources ||= @connection.data_sources
+    end
+
+    def views
+      @views ||= @connection.views
+    end
+
+    def primary_key(table_name)
+      @primary_keys[table_name] ||= @connection.primary_key(table_name)
+    end
+
+    def columns(table_name)
+      @connection.schema_cache.columns(table_name)
+    end
+
+    def indexes(table_name)
+      @indexes.fetch(table_name) do
+        @indexes[table_name] = @connection.indexes(table_name)
+      end
+    end
+
+    def foreign_keys(table_name)
+      @foreign_keys[table_name] ||= @connection.foreign_keys(table_name)
+    end
+
+    def check_constraints(table_name)
+      @check_constraints[table_name] ||=
+        if @connection.supports_check_constraints?
+          @connection.check_constraints(table_name).select(&:validated?).map(&:expression)
+        else
+          # We don't support this Rails/database combination yet.
+          []
+        end
+    end
+  end
+end

lib/active_record_doctor/detectors/base.rb

@@ -38,11 +38,12 @@ def locals_and_globals
         end
       end
 
-      def initialize(config:, logger:, io:)
+      def initialize(config:, logger:, io:, schema_inspector:)
         @problems = []
         @config = config
         @logger = logger
         @io = io
+        @schema_inspector = schema_inspector
       end
 
       def run
@@ -114,18 +115,31 @@ def connection
       end
 
       def indexes(table_name)
-        connection.indexes(table_name)
+        @schema_inspector.indexes(table_name)
       end
 
-      def primary_key(table_name)
-        primary_key_name = connection.primary_key(table_name)
-        return nil if primary_key_name.nil?
+      def tables
+        @schema_inspector.tables
+      end
+
+      def data_sources
+        @schema_inspector.data_sources
+      end
 
-        column(table_name, primary_key_name)
+      def views
+        @schema_inspector.views
+      end
+
+      def primary_key(table_name)
+        @schema_inspector.primary_key(table_name)
       end
 
       def column(table_name, column_name)
-        connection.columns(table_name).find { |column| column.name == column_name }
+        columns(table_name).find { |column| column.name == column_name }
+      end
+
+      def columns(table_name)
+        @schema_inspector.columns(table_name)
       end
 
       def not_null_check_constraint_exists?(table, column)
@@ -135,24 +149,20 @@ def not_null_check_constraint_exists?(table, column)
         end
       end
 
+      def table_exists?(table_name)
+        tables.include?(table_name)
+      end
+
+      def data_source_exists?(name)
+        data_sources.include?(name)
+      end
+
       def check_constraints(table_name)
-        if connection.supports_check_constraints?
-          connection.check_constraints(table_name).select(&:validated?).map(&:expression)
-        elsif Utils.postgresql?(connection)
-          definitions =
-            connection.select_values(<<-SQL)
-              SELECT pg_get_constraintdef(oid, true)
-              FROM pg_constraint
-              WHERE contype = 'c'
-                AND convalidated
-                AND conrelid = #{connection.quote(table_name)}::regclass
-            SQL
-
-          definitions.map { |definition| definition[/CHECK \((.+)\)/m, 1] }
-        else
-          # We don't support this Rails/database combination yet.
-          []
-        end
+        @schema_inspector.check_constraints(table_name)
+      end
+
+      def foreign_keys(table_name)
+        @schema_inspector.foreign_keys(table_name)
       end
 
       def models
@@ -175,7 +185,7 @@ def each_model(except: [], abstract: nil, existing_tables_only: false)
               log("#{model.name} - non-abstract model; skipping")
             when abstract == false && model.abstract_class?
               log("#{model.name} - abstract model; skipping")
-            when existing_tables_only && (model.table_name.nil? || !model.table_exists?)
+            when existing_tables_only && (model.table_name.nil? || !table_exists?(model.table_name))
               log("#{model.name} - backed by a non-existent table #{model.table_name}; skipping")
             else
               log(model.name) do
@@ -187,7 +197,7 @@ def each_model(except: [], abstract: nil, existing_tables_only: false)
       end
 
       def each_index(table_name, except: [], multicolumn_only: false)
-        indexes = connection.indexes(table_name)
+        indexes = indexes(table_name)
 
         message =
           if multicolumn_only
@@ -214,7 +224,7 @@ def each_index(table_name, except: [], multicolumn_only: false)
 
       def each_attribute(model, except: [], type: nil)
         log("Iterating over attributes of #{model.name}") do
-          connection.columns(model.table_name).each do |column|
+          columns(model.table_name).each do |column|
             case
             when ignored?("#{model.name}.#{column.name}", except)
               log("#{model.name}.#{column.name} - ignored via the configuration; skipping")
@@ -231,7 +241,7 @@ def each_attribute(model, except: [], type: nil)
 
       def each_column(table_name, only: nil, except: [])
         log("Iterating over columns of #{table_name}") do
-          connection.columns(table_name).each do |column|
+          columns(table_name).each do |column|
             case
             when ignored?("#{table_name}.#{column.name}", except)
               log("#{column.name} - ignored via the configuration; skipping")
@@ -253,7 +263,7 @@ def looks_like_foreign_key?(column)
 
       def each_foreign_key(table_name)
         log("Iterating over foreign keys on #{table_name}") do
-          connection.foreign_keys(table_name).each do |foreign_key|
+          foreign_keys(table_name).each do |foreign_key|
             log("#{foreign_key.name} - #{foreign_key.from_table}(#{foreign_key.options[:column]}) to #{foreign_key.to_table}(#{foreign_key.options[:primary_key]})") do # rubocop:disable Layout/LineLength
               yield(foreign_key)
             end
@@ -263,7 +273,7 @@ def each_foreign_key(table_name)
 
       def each_table(except: [])
         log("Iterating over tables") do
-          connection.tables.each do |table|
+          tables.each do |table|
             case
             when ignored?(table, except)
               log("#{table} - ignored via the configuration; skipping")
@@ -278,7 +288,7 @@ def each_table(except: [])
 
       def each_data_source(except: [])
         log("Iterating over data sources") do
-          connection.data_sources.each do |data_source|
+          data_sources.each do |data_source|
             if ignored?(data_source, except)
               log("#{data_source} - ignored via the configuration; skipping")
             else

lib/active_record_doctor/detectors/extraneous_indexes.rb

@@ -61,7 +61,7 @@ def indexed_primary_keys
         log(__method__) do
           each_table(except: config(:ignore_tables)) do |table|
             each_index(table, except: config(:ignore_indexes), multicolumn_only: true) do |index|
-              primary_key = connection.primary_key(table)
+              primary_key = primary_key(table)
               if index.columns == [primary_key] && index.where.nil?
                 problem!(table: table, extraneous_index: index.name, replacement_indexes: nil)
               end

lib/active_record_doctor/detectors/incorrect_dependent_option.rb

@@ -180,7 +180,7 @@ def dependent_models(model)
       end
 
       def foreign_key(from_table, to_table)
-        connection.foreign_keys(from_table).find do |foreign_key|
+        foreign_keys(from_table).find do |foreign_key|
           foreign_key.to_table == to_table
         end
       end

lib/active_record_doctor/detectors/missing_foreign_keys.rb

@@ -24,7 +24,7 @@ def message(table:, column:)
 
       def detect
         each_model(except: config(:ignore_models), existing_tables_only: true) do |model|
-          foreign_keys = connection.foreign_keys(model.table_name)
+          foreign_keys = foreign_keys(model.table_name)
           foreign_key_columns = foreign_keys.map { |key| key.options[:column] }
 
           each_association(model, type: :belongs_to) do |association|

lib/active_record_doctor/detectors/missing_non_null_constraint.rb

@@ -34,7 +34,6 @@ def message(column:, table:)
 
       def detect
         table_models = models.select(&:table_exists?).group_by(&:table_name)
-        views = connection.views
 
         table_models.each do |table, models|
           next if ignored?(table, config(:ignore_tables))
@@ -44,7 +43,7 @@ def detect
             model.abstract_class? || sti_base_model?(model)
           end
 
-          connection.columns(table).each do |column|
+          columns(table).each do |column|
             next if ignored?("#{table}.#{column.name}", config(:ignore_columns))
             next if !column.null
             next if !concrete_models.all? { |model| non_null_needed?(model, column) }

lib/active_record_doctor/detectors/missing_presence_validation.rb

@@ -36,7 +36,7 @@ def detect
         each_model(except: config(:ignore_models), existing_tables_only: true) do |model|
           # List all columns and then remove those that don't need or don't have
           # a missing validator.
-          problematic_columns = connection.columns(model.table_name)
+          problematic_columns = columns(model.table_name)
           problematic_columns.reject! do |column|
             # The primary key, timestamps, and counter caches are special
             # columns that are automatically managed by Rails and don't need

lib/active_record_doctor/detectors/missing_unique_indexes.rb

@@ -110,7 +110,7 @@ def has_ones_without_indexes # rubocop:disable Naming/PredicateName
 
             table_name = has_one.klass.table_name
             next if unique_index?(table_name, columns)
-            next if Array(connection.primary_key(table_name)) == columns
+            next if Array(primary_key(table_name)) == columns
 
             problem!(model: model, table: table_name, columns: columns, problem: :has_ones)
           end

lib/active_record_doctor/detectors/short_primary_key_type.rb

@@ -23,7 +23,7 @@ def detect
         return if ActiveRecordDoctor::Utils.sqlite?
 
         each_table(except: config(:ignore_tables)) do |table|
-          column = primary_key(table)
+          column = column(table, primary_key(table))
           next if column.nil?
           next if !integer?(column) || bigint?(column)
 

lib/active_record_doctor/detectors/undefined_table_references.rb

@@ -21,7 +21,7 @@ def message(model:, table:)
 
       def detect
         each_model(except: config(:ignore_models), abstract: false) do |model|
-          next if connection.data_source_exists?(model.table_name)
+          next if data_source_exists?(model.table_name)
 
           problem!(model: model.name, table: model.table_name)
         end

lib/active_record_doctor/detectors/unindexed_foreign_keys.rb

@@ -30,7 +30,7 @@ def detect
             next unless looks_like_foreign_key?(column) || foreign_key?(table, column)
             next if indexed?(table, column)
             next if indexed_as_polymorphic?(table, column)
-            next if connection.primary_key(table) == column.name
+            next if primary_key(table) == column.name
 
             type_column_name = type_column_name(column)
 
@@ -47,25 +47,25 @@ def detect
       end
 
       def foreign_key?(table, column)
-        connection.foreign_keys(table).any? do |foreign_key|
+        foreign_keys(table).any? do |foreign_key|
           foreign_key.column == column.name
         end
       end
 
       def indexed?(table, column)
-        connection.indexes(table).any? do |index|
+        indexes(table).any? do |index|
           index.columns.first == column.name
         end
       end
 
       def indexed_as_polymorphic?(table, column)
-        connection.indexes(table).any? do |index|
+        indexes(table).any? do |index|
           index.columns[0, 2] == [type_column_name(column), column.name]
         end
       end
 
       def column_exists?(table, column_name)
-        connection.columns(table).any? { |column| column.name == column_name }
+        columns(table).any? { |column| column.name == column_name }
       end
 
       def type_column_name(column)

lib/active_record_doctor/rake/task.rb

@@ -64,7 +64,11 @@ def define
       private
 
       def runner
-        @runner ||= ActiveRecordDoctor::Runner.new(config: config, logger: logger)
+        @runner ||= begin
+          connection = ActiveRecord::Base.connection
+          schema_inspector = ActiveRecordDoctor::CachingSchemaInspector.new(connection)
+          ActiveRecordDoctor::Runner.new(config: config, logger: logger, schema_inspector: schema_inspector)
+        end
       end
 
       def config

lib/active_record_doctor/runner.rb

@@ -5,9 +5,10 @@ module ActiveRecordDoctor # :nodoc:
   # and an output device for use by detectors.
   class Runner
     # io is injected via constructor parameters to facilitate testing.
-    def initialize(config:, logger:, io: $stdout)
+    def initialize(config:, logger:, schema_inspector:, io: $stdout)
       @config = config
       @logger = logger
+      @schema_inspector = schema_inspector
       @io = io
     end
 
@@ -16,7 +17,8 @@ def run_one(name)
         ActiveRecordDoctor.detectors.fetch(name).run(
           config: config,
           logger: logger,
-          io: io
+          io: io,
+          schema_inspector: schema_inspector
         )
       end
     end
@@ -41,6 +43,6 @@ def help(name)
 
     private
 
-    attr_reader :config, :logger, :io
+    attr_reader :config, :logger, :io, :schema_inspector
   end
 end

test/active_record_doctor/runner_test.rb

@@ -3,10 +3,14 @@
 class ActiveRecordDoctor::RunnerTest < Minitest::Test
   def setup
     @io = StringIO.new
+    connection = ActiveRecord::Base.connection
+    schema_inspector = ActiveRecordDoctor::CachingSchemaInspector.new(connection)
+
     @runner = ActiveRecordDoctor::Runner.new(
       config: load_config,
       logger: ActiveRecordDoctor::Logger::Dummy.new,
-      io: @io
+      io: @io,
+      schema_inspector: schema_inspector
     )
   end