Reduce the number of queries by caching schema info

lib/active_record_doctor.rb

@@ -19,6 +19,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,70 @@
+# frozen_string_literal: true
+
+module ActiveRecordDoctor
+  class CachingSchemaInspector # :nodoc:
+    def initialize(connection)
+      @connection = connection
+      @indexes = {}
+      @foreign_keys = {}
+      @check_constraints = {}
+    end
+
+    def tables
+      @tables ||=
+        if ActiveRecord::VERSION::STRING >= "5.1"
+          @connection.tables
+        else
+          @connection.data_sources
+        end
+    end
+
+    def primary_key(table_name)
+      @connection.schema_cache.primary_keys(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.fetch(table_name) do
+        @foreign_keys[table_name] = @connection.foreign_keys(table_name)
+      end
+    end
+
+    def check_constraints(table_name)
+      @check_constraints.fetch(table_name) do
+        # ActiveRecord 6.1+
+        if @connection.respond_to?(:supports_check_constraints?) && @connection.supports_check_constraints?
+          @connection.check_constraints(table_name).select(&:validated?).map(&:expression)
+        elsif postgresql?
+          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
+      end
+    end
+
+    private
+
+    def postgresql?
+      ["PostgreSQL", "PostGIS"].include?(@connection.adapter_name)
+    end
+  end
+end

lib/active_record_doctor/detectors/base.rb

@@ -13,8 +13,8 @@ class Base
       class << self
         attr_reader :description
 
-        def run(config, io)
-          new(config, io).run
+        def run(config, schema_inspector, io)
+          new(config, schema_inspector, io).run
         end
 
         def underscored_name
@@ -38,9 +38,10 @@ def locals_and_globals
         end
       end
 
-      def initialize(config, io)
+      def initialize(config, schema_inspector, io)
         @problems = []
         @config = config
+        @schema_inspector = schema_inspector
         @io = io
       end
 
@@ -92,33 +93,30 @@ def connection
       end
 
       def indexes(table_name, except: [])
-        connection.indexes(table_name).reject do |index|
+        @schema_inspector.indexes(table_name).reject do |index|
           except.include?(index.name)
         end
       end
 
       def tables(except: [])
-        tables =
-          if ActiveRecord::VERSION::STRING >= "5.1"
-            connection.tables
-          else
-            connection.data_sources
-          end
-
-        tables.reject do |table|
+        @schema_inspector.tables.reject do |table|
           except.include?(table)
         end
       end
 
       def primary_key(table_name)
-        primary_key_name = connection.primary_key(table_name)
+        primary_key_name = @schema_inspector.primary_key(table_name)
         return nil if primary_key_name.nil?
 
         column(table_name, primary_key_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)
@@ -128,25 +126,12 @@ def not_null_check_constraint_exists?(table, column)
         end
       end
 
+      def foreign_keys(table_name)
+        @schema_inspector.foreign_keys(table_name)
+      end
+
       def check_constraints(table_name)
-        # ActiveRecord 6.1+
-        if connection.respond_to?(:supports_check_constraints?) && connection.supports_check_constraints?
-          connection.check_constraints(table_name).select(&:validated?).map(&:expression)
-        elsif postgresql?
-          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 models(except: [])
@@ -158,10 +143,6 @@ def models(except: [])
       def underscored_name
         self.class.underscored_name
       end
-
-      def postgresql?
-        ["PostgreSQL", "PostGIS"].include?(connection.adapter_name)
-      end
     end
   end
 end

lib/active_record_doctor/detectors/extraneous_indexes.rb

@@ -58,7 +58,7 @@ def indexed_primary_keys
           indexes(table).each do |index|
             next if config(:ignore_indexes).include?(index.name)
 
-            primary_key = connection.primary_key(table)
+            primary_key = @schema_inspector.primary_key(table)
             next if index.columns != [primary_key] || index.where
 
             problem!(extraneous_index: index.name, replacement_indexes: nil)

lib/active_record_doctor/detectors/incorrect_boolean_presence_validation.rb

@@ -28,7 +28,7 @@ def detect
         models(except: config(:ignore_models)).each do |model|
           next unless model.table_exists?
 
-          connection.columns(model.table_name).each do |column|
+          columns(model.table_name).each do |column|
             next if config(:ignore_attributes).include?("#{model.name}.#{column.name}")
             next unless column.type == :boolean
             next unless has_presence_validator?(model, column)

lib/active_record_doctor/detectors/incorrect_dependent_option.rb

@@ -129,7 +129,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/incorrect_length_validation.rb

@@ -34,7 +34,7 @@ def detect
         models(except: config(:ignore_models)).each do |model|
           next unless model.table_exists?
 
-          connection.columns(model.table_name).each do |column|
+          columns(model.table_name).each do |column|
             next if config(:ignore_attributes).include?("#{model.name}.#{column.name}")
             next if ![:string, :text].include?(column.type)
 

lib/active_record_doctor/detectors/mismatched_foreign_key_type.rb

@@ -26,7 +26,7 @@ def message(table:, column:)
 
       def detect
         tables(except: config(:ignore_tables)).each do |table|
-          connection.foreign_keys(table).each do |foreign_key|
+          foreign_keys(table).each do |foreign_key|
             from_column = column(table, foreign_key.column)
 
             next if config(:ignore_columns).include?("#{table}.#{from_column.name}")

lib/active_record_doctor/detectors/missing_foreign_keys.rb

@@ -24,7 +24,7 @@ def message(table:, column:)
 
       def detect
         tables(except: config(:ignore_tables)).each do |table|
-          connection.columns(table).each do |column|
+          columns(table).each do |column|
             next if config(:ignore_columns).include?("#{table}.#{column.name}")
 
             # We need to skip polymorphic associations as they can reference
@@ -44,14 +44,14 @@ def named_like_foreign_key?(column)
       end
 
       def foreign_key?(table, column)
-        connection.foreign_keys(table).any? do |foreign_key|
+        foreign_keys(table).any? do |foreign_key|
           foreign_key.options[:column] == column.name
         end
       end
 
       def polymorphic_foreign_key?(table, column)
         type_column_name = column.name.sub(/_id\Z/, "_type")
-        connection.columns(table).any? do |another_column|
+        columns(table).any? do |another_column|
           another_column.name == type_column_name
         end
       end

lib/active_record_doctor/detectors/missing_non_null_constraint.rb

@@ -32,7 +32,7 @@ def detect
             model.abstract_class? || sti_base_model?(model)
           end
 
-          connection.columns(table).each do |column|
+          columns(table).each do |column|
             next if config(:ignore_columns).include?("#{table}.#{column.name}")
             next if !column.null
             next if !concrete_models.all? { |model| non_null_needed?(model, column) }

lib/active_record_doctor/detectors/missing_presence_validation.rb

@@ -26,7 +26,7 @@ def detect
         models(except: config(:ignore_models)).each do |model|
           next unless model.table_exists?
 
-          connection.columns(model.table_name).each do |column|
+          columns(model.table_name).each do |column|
             next unless validator_needed?(model, column)
             next if validator_present?(model, column)
             next if config(:ignore_attributes).include?("#{model}.#{column.name}")

lib/active_record_doctor/detectors/undefined_table_references.rb

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

lib/active_record_doctor/detectors/unindexed_deleted_at.rb

@@ -32,7 +32,7 @@ def message(index:, column_name:)
 
       def detect
         tables(except: config(:ignore_tables)).each do |table|
-          timestamp_columns = connection.columns(table).reject do |column|
+          timestamp_columns = columns(table).reject do |column|
             config(:ignore_columns).include?("#{table}.#{column.name}")
           end.select do |column|
             config(:column_names).include?(column.name)

lib/active_record_doctor/detectors/unindexed_foreign_keys.rb

@@ -26,7 +26,7 @@ def message(table:, column:)
 
       def detect
         tables(except: config(:ignore_tables)).each do |table|
-          connection.columns(table).each do |column|
+          columns(table).each do |column|
             next if config(:ignore_columns).include?("#{table}.#{column.name}")
 
             next unless foreign_key?(column)
@@ -43,14 +43,14 @@ def foreign_key?(column)
       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)
         type_column_name = column.name.sub(/_id\Z/, "_type")
-        connection.indexes(table).any? do |index|
+        indexes(table).any? do |index|
           index.columns == [type_column_name, column.name]
         end
       end

lib/active_record_doctor/rake/task.rb

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

lib/active_record_doctor/runner.rb

@@ -5,14 +5,15 @@ 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, io = $stdout)
+    def initialize(config, schema_inspector, io = $stdout)
       @config = config
+      @schema_inspector = schema_inspector
       @io = io
     end
 
     def run_one(name)
       ActiveRecordDoctor.handle_exception do
-        ActiveRecordDoctor.detectors.fetch(name).run(config, io)
+        ActiveRecordDoctor.detectors.fetch(name).run(config, schema_inspector, io)
       end
     end
 
@@ -36,6 +37,6 @@ def help(name)
 
     private
 
-    attr_reader :config, :io
+    attr_reader :config, :schema_inspector, :io
   end
 end

test/active_record_doctor/runner_test.rb

@@ -2,8 +2,7 @@
 
 class ActiveRecordDoctor::RunnerTest < Minitest::Test
   def test_run_one_raises_on_unknown_detectors
-    io = StringIO.new
-    runner = ActiveRecordDoctor::Runner.new(load_config, io)
+    runner = create_runner
 
     assert_raises(KeyError) do
       runner.run_one(:performance_issues)
@@ -12,7 +11,7 @@ def test_run_one_raises_on_unknown_detectors
 
   def test_run_all_returns_true_when_no_errors
     io = StringIO.new
-    runner = ActiveRecordDoctor::Runner.new(load_config, io)
+    runner = create_runner(io: io)
 
     assert(runner.run_all)
     assert(io.string.blank?)
@@ -23,7 +22,7 @@ def test_run_all_returns_false_when_errors
     create_model(:User)
 
     io = StringIO.new
-    runner = ActiveRecordDoctor::Runner.new(load_config, io)
+    runner = create_runner(io: io)
 
     refute(runner.run_all)
     refute(io.string.blank?)
@@ -32,7 +31,7 @@ def test_run_all_returns_false_when_errors
   def test_help_prints_help
     ActiveRecordDoctor.detectors.each do |name, _|
       io = StringIO.new
-      runner = ActiveRecordDoctor::Runner.new(load_config, io)
+      runner = create_runner(io: io)
 
       runner.help(name)