From 38ac185f1da0bcd3a33242a9b23a74060da577cb Mon Sep 17 00:00:00 2001 From: "Tobias L. Maier" Date: Wed, 18 Mar 2026 16:38:44 +0000 Subject: [PATCH 1/3] Fix test setup for local development and single-adapter environments Pin minitest to ~> 5.0 in the Gemfile because minitest 6.0 removed Minitest.run_one_method which minitest-fork_executor depends on. Without the pin, bundle resolves to minitest 6.0.2 and the test suite crashes on startup. Make adapter requires in test/setup.rb graceful so the test suite can run when only one database adapter gem is installed (e.g. sqlite3 only). In CI all adapters are present, so this is a no-op there. Also add SecondaryContext (TransientRecord context for the existing SecondaryRecord class) to enable writing tests that exercise multi-database scenarios. --- Gemfile | 3 +++ test/setup.rb | 10 +++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index bb94df82..95a1c679 100644 --- a/Gemfile +++ b/Gemfile @@ -2,3 +2,6 @@ source "https://rubygems.org" gemspec + +# minitest 6.0 removed run_one_method which minitest-fork_executor depends on +gem "minitest", "~> 5.0" diff --git a/test/setup.rb b/test/setup.rb index 6be93235..f9445421 100644 --- a/test/setup.rb +++ b/test/setup.rb @@ -8,9 +8,11 @@ require "logger" require "active_record" -require "pg" -require "mysql2" -require "sqlite3" +%w[pg mysql2 sqlite3].each do |adapter_gem| + require adapter_gem +rescue LoadError + # Not all adapters need to be installed; only the one matching DATABASE_ADAPTER is required. +end adapter = ENV.fetch("DATABASE_ADAPTER") @@ -91,6 +93,8 @@ class SecondaryRecord < ApplicationRecord SecondaryRecord.establish_connection :secondary +SecondaryContext = TransientRecord.context_for SecondaryRecord + if ActiveRecord.version >= Gem::Version.new("7.1") # See https://github.com/rails/rails/pull/46522 for details. # When `false` (default in Rails 7.1) - validate presence only when the foreign key changed. From 6601df2becdfb8067c457154fa5fc994c0920863 Mon Sep 17 00:00:00 2001 From: "Tobias L. Maier" Date: Wed, 18 Mar 2026 16:38:56 +0000 Subject: [PATCH 2/3] Add tests for models connected to a non-primary database (#149) When a model uses establish_connection or connects_to to point at a different database, detectors should silently skip it. Previously they would crash (e.g. PG::UndefinedTable) because the detector queried the primary connection for a table that only exists in the secondary database. Add two test cases using SecondaryContext: - missing_non_null_constraint: a secondary-DB model with a nullable column and a presence validator must not crash the detector. - undefined_table_references: a secondary-DB model must not be reported as referencing an undefined table. --- .../detectors/missing_non_null_constraint_test.rb | 13 +++++++++++++ .../detectors/undefined_table_references_test.rb | 8 ++++++++ 2 files changed, 21 insertions(+) diff --git a/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb b/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb index 675a017f..c1977ffc 100644 --- a/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb +++ b/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb @@ -349,4 +349,17 @@ def test_created_on_updated_on_with_null_are_disallowed add `NOT NULL` to users.updated_on - timestamp columns are set automatically by Active Record and allowing NULL may lead to inconsistencies introduced by bulk operations OUTPUT end + + def test_models_with_non_primary_connection_are_skipped + # Models connected to a secondary database should be silently skipped. + # Previously this would crash with e.g. PG::UndefinedTable when the + # secondary table didn't exist in the primary database (see #149). + SecondaryContext.create_table(:users) do |t| + t.string :name, null: true + end.define_model do + validates :name, presence: true + end + + refute_problems + end end diff --git a/test/active_record_doctor/detectors/undefined_table_references_test.rb b/test/active_record_doctor/detectors/undefined_table_references_test.rb index 4176e089..e4553c06 100644 --- a/test/active_record_doctor/detectors/undefined_table_references_test.rb +++ b/test/active_record_doctor/detectors/undefined_table_references_test.rb @@ -52,4 +52,12 @@ def test_global_ignore_tables refute_problems end + + def test_models_with_non_primary_connection_are_skipped + # Models connected to a secondary database should be silently skipped, + # not reported as referencing undefined tables (see #149). + SecondaryContext.create_table(:users).define_model + + refute_problems + end end From a78e0914e8622975700e531bce8e0e333e989b99 Mon Sep 17 00:00:00 2001 From: "Tobias L. Maier" Date: Wed, 18 Mar 2026 16:39:10 +0000 Subject: [PATCH 3/3] Skip models connected to non-primary databases (#149) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All detectors use Base#models to discover ActiveRecord models, then inspect their tables via Base#connection which is hardcoded to ActiveRecord::Base.connection (the primary database). When a model uses establish_connection or connects_to for a different database, model.table_exists? returns true (checked against its own connection), but connection.columns(table) crashes because the primary database has no such table. Fix by filtering Base#models to exclude models whose connection_pool differs from ActiveRecord::Base.connection_pool. This is a single guard that protects all detectors at once — no per-detector changes are needed. Document the limitation in the README under "Multiple Databases". --- README.md | 10 ++++++++++ lib/active_record_doctor/detectors/base.rb | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 7cc55885..14154b69 100644 --- a/README.md +++ b/README.md @@ -184,6 +184,16 @@ ActiveRecordDoctor.configure do end ``` +### Multiple Databases + +`active_record_doctor` currently only analyzes models connected to the primary +database. Models that use a different database connection — whether via +`establish_connection`, `connects_to`, or by inheriting from an abstract class +with its own connection — are automatically skipped by all detectors. + +This means that if you have gems or parts of your application that connect to a +secondary database, their models won't be checked and won't cause errors. + ### Indexing Unindexed Foreign Keys Foreign keys should be indexed unless it's proven ineffective. However, Rails diff --git a/lib/active_record_doctor/detectors/base.rb b/lib/active_record_doctor/detectors/base.rb index b02630ba..df253d2e 100644 --- a/lib/active_record_doctor/detectors/base.rb +++ b/lib/active_record_doctor/detectors/base.rb @@ -156,7 +156,10 @@ def check_constraints(table_name) end def models - ActiveRecord::Base.descendants.sort_by(&:name) + base_pool = ActiveRecord::Base.connection_pool + ActiveRecord::Base.descendants + .reject { |model| model.connection_pool != base_pool } + .sort_by(&:name) end def underscored_name