From 0273dbb7b4d7d7c10f04ff9e8bc44a0fcb422b55 Mon Sep 17 00:00:00 2001
From: fatkodima <fatkodima123@gmail.com>
Date: Thu, 16 Jun 2022 03:40:56 +0300
Subject: [PATCH] Reduce the number of queries by caching schema info

---
 lib/active_record_doctor.rb                   |  1 +
 .../caching_schema_inspector.rb               | 70 +++++++++++++++++++
 lib/active_record_doctor/detectors/base.rb    | 53 +++++---------
 .../detectors/extraneous_indexes.rb           |  2 +-
 .../incorrect_boolean_presence_validation.rb  |  2 +-
 .../detectors/incorrect_dependent_option.rb   |  2 +-
 .../detectors/incorrect_length_validation.rb  |  2 +-
 .../detectors/mismatched_foreign_key_type.rb  |  2 +-
 .../detectors/missing_foreign_keys.rb         |  6 +-
 .../detectors/missing_non_null_constraint.rb  |  2 +-
 .../detectors/missing_presence_validation.rb  |  2 +-
 .../detectors/undefined_table_references.rb   |  2 +-
 .../detectors/unindexed_deleted_at.rb         |  2 +-
 .../detectors/unindexed_foreign_keys.rb       |  6 +-
 lib/active_record_doctor/rake/task.rb         |  6 +-
 lib/active_record_doctor/runner.rb            |  7 +-
 test/active_record_doctor/runner_test.rb      |  9 ++-
 test/setup.rb                                 |  8 ++-
 18 files changed, 123 insertions(+), 61 deletions(-)
 create mode 100644 lib/active_record_doctor/caching_schema_inspector.rb

diff --git a/lib/active_record_doctor.rb b/lib/active_record_doctor.rb
index 65d1da4..e634277 100644
--- a/lib/active_record_doctor.rb
+++ b/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"
diff --git a/lib/active_record_doctor/caching_schema_inspector.rb b/lib/active_record_doctor/caching_schema_inspector.rb
new file mode 100644
index 0000000..93b8ce1
--- /dev/null
+++ b/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
diff --git a/lib/active_record_doctor/detectors/base.rb b/lib/active_record_doctor/detectors/base.rb
index d683b9a..a5a9179 100644
--- a/lib/active_record_doctor/detectors/base.rb
+++ b/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
diff --git a/lib/active_record_doctor/detectors/extraneous_indexes.rb b/lib/active_record_doctor/detectors/extraneous_indexes.rb
index 9aa1f60..7966276 100644
--- a/lib/active_record_doctor/detectors/extraneous_indexes.rb
+++ b/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)
diff --git a/lib/active_record_doctor/detectors/incorrect_boolean_presence_validation.rb b/lib/active_record_doctor/detectors/incorrect_boolean_presence_validation.rb
index abea5dd..a6dce74 100644
--- a/lib/active_record_doctor/detectors/incorrect_boolean_presence_validation.rb
+++ b/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)
diff --git a/lib/active_record_doctor/detectors/incorrect_dependent_option.rb b/lib/active_record_doctor/detectors/incorrect_dependent_option.rb
index 9795b67..0f53135 100644
--- a/lib/active_record_doctor/detectors/incorrect_dependent_option.rb
+++ b/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
diff --git a/lib/active_record_doctor/detectors/incorrect_length_validation.rb b/lib/active_record_doctor/detectors/incorrect_length_validation.rb
index 69a45ae..855bb95 100644
--- a/lib/active_record_doctor/detectors/incorrect_length_validation.rb
+++ b/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)
 
diff --git a/lib/active_record_doctor/detectors/mismatched_foreign_key_type.rb b/lib/active_record_doctor/detectors/mismatched_foreign_key_type.rb
index 2f684d8..b35665e 100644
--- a/lib/active_record_doctor/detectors/mismatched_foreign_key_type.rb
+++ b/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}")
diff --git a/lib/active_record_doctor/detectors/missing_foreign_keys.rb b/lib/active_record_doctor/detectors/missing_foreign_keys.rb
index d6fa847..aa7b3da 100644
--- a/lib/active_record_doctor/detectors/missing_foreign_keys.rb
+++ b/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
diff --git a/lib/active_record_doctor/detectors/missing_non_null_constraint.rb b/lib/active_record_doctor/detectors/missing_non_null_constraint.rb
index 5f4283f..f791579 100644
--- a/lib/active_record_doctor/detectors/missing_non_null_constraint.rb
+++ b/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) }
diff --git a/lib/active_record_doctor/detectors/missing_presence_validation.rb b/lib/active_record_doctor/detectors/missing_presence_validation.rb
index d258efd..7930c51 100644
--- a/lib/active_record_doctor/detectors/missing_presence_validation.rb
+++ b/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}")
diff --git a/lib/active_record_doctor/detectors/undefined_table_references.rb b/lib/active_record_doctor/detectors/undefined_table_references.rb
index b2392a4..7180f12 100644
--- a/lib/active_record_doctor/detectors/undefined_table_references.rb
+++ b/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
diff --git a/lib/active_record_doctor/detectors/unindexed_deleted_at.rb b/lib/active_record_doctor/detectors/unindexed_deleted_at.rb
index 9e6852e..8e881b9 100644
--- a/lib/active_record_doctor/detectors/unindexed_deleted_at.rb
+++ b/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)
diff --git a/lib/active_record_doctor/detectors/unindexed_foreign_keys.rb b/lib/active_record_doctor/detectors/unindexed_foreign_keys.rb
index 469053d..ba8aedc 100644
--- a/lib/active_record_doctor/detectors/unindexed_foreign_keys.rb
+++ b/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
diff --git a/lib/active_record_doctor/rake/task.rb b/lib/active_record_doctor/rake/task.rb
index d92e1f6..7c1d4a5 100644
--- a/lib/active_record_doctor/rake/task.rb
+++ b/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
diff --git a/lib/active_record_doctor/runner.rb b/lib/active_record_doctor/runner.rb
index c2e2623..7cbe43f 100644
--- a/lib/active_record_doctor/runner.rb
+++ b/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
diff --git a/test/active_record_doctor/runner_test.rb b/test/active_record_doctor/runner_test.rb
index 5efba67..58feb70 100644
--- a/test/active_record_doctor/runner_test.rb
+++ b/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)
 
diff --git a/test/setup.rb b/test/setup.rb
index 29f0048..ea45841 100644
--- a/test/setup.rb
+++ b/test/setup.rb
@@ -93,11 +93,17 @@ def detector_name
 
   def run_detector
     io = StringIO.new
-    runner = ActiveRecordDoctor::Runner.new(load_config, io)
+    runner = create_runner(io: io)
     success = runner.run_one(detector_name)
     [success, io.string]
   end
 
+  def create_runner(io: StringIO.new)
+    connection = ActiveRecord::Base.connection
+    schema_inspector = ActiveRecordDoctor::CachingSchemaInspector.new(connection)
+    ActiveRecordDoctor::Runner.new(load_config, schema_inspector, io)
+  end
+
   def load_config
     ActiveRecordDoctor.load_config_with_defaults(@config_path)
   end