Skip to content

Commit 4d1d7d3

Browse files
committed
Unify the logic to determine the default schema cache path for a database configuration
When the application has more than one database configuration, only the primary was being loaded by default on boot time. All the other connection pools were loading the schema cache lazily. This logic can be found in: https://github.com/rails/rails/blob/351a8d9bc99e69cc8d1ced86dc1d523e2dc51985/activerecord/lib/active_record/connection_adapters/pool_config.rb#L13 https://github.com/rails/rails/blob/351a8d9bc99e69cc8d1ced86dc1d523e2dc51985/activerecord/lib/active_record/railtie.rb#L149-L178 The lazy code path wasn't using the same logic to determine the name of the default schema cache file that the Railties uses, so it was loading the schema cache of the primary config to all the other configs. If no table name coincided nothing bad would happen, but if table names coincided, the schema would be completly wrong in all non-primary connections.
1 parent 1cf7025 commit 4d1d7d3

File tree

5 files changed

+47
-17
lines changed

5 files changed

+47
-17
lines changed

activerecord/lib/active_record/database_configurations/hash_config.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,12 @@ def schema_cache_path
113113
configuration_hash[:schema_cache_path]
114114
end
115115

116-
def default_schema_cache_path
117-
"db/schema_cache.yml"
116+
def default_schema_cache_path(db_dir = "db")
117+
if primary?
118+
File.join(db_dir, "schema_cache.yml")
119+
else
120+
File.join(db_dir, "#{name}_schema_cache.yml")
121+
end
118122
end
119123

120124
def lazy_schema_cache_path

activerecord/lib/active_record/tasks/database_tasks.rb

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -439,13 +439,10 @@ def schema_dump_path(db_config, format = ActiveRecord.schema_format)
439439

440440
def cache_dump_filename(db_config_or_name, schema_cache_path: nil)
441441
if db_config_or_name.is_a?(DatabaseConfigurations::DatabaseConfig)
442-
filename = if db_config_or_name.primary?
443-
"schema_cache.yml"
444-
else
445-
"#{db_config_or_name.name}_schema_cache.yml"
446-
end
447-
448-
schema_cache_path || db_config_or_name.schema_cache_path || ENV["SCHEMA_CACHE"] || File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, filename)
442+
schema_cache_path ||
443+
db_config_or_name.schema_cache_path ||
444+
ENV["SCHEMA_CACHE"] ||
445+
db_config_or_name.default_schema_cache_path(ActiveRecord::Tasks::DatabaseTasks.db_dir)
449446
else
450447
ActiveRecord.deprecator.deprecation_warning(<<~MSG.squish)
451448
Passing a database name to `cache_dump_filename` is deprecated and will be removed in Rails 7.3. Pass a

activerecord/test/cases/database_configurations/hash_config_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,31 @@ def test_schema_cache_path_default_for_primary
141141
assert_equal "db/schema_cache.yml", config.default_schema_cache_path
142142
end
143143

144+
def test_schema_cache_path_default_for_custom_name
145+
config = HashConfig.new("default_env", "alternate", adapter: "abstract")
146+
assert_equal "db/alternate_schema_cache.yml", config.default_schema_cache_path
147+
end
148+
149+
def test_schema_cache_path_default_for_different_db_dir
150+
config = HashConfig.new("default_env", "alternate", adapter: "abstract")
151+
assert_equal "my_db/alternate_schema_cache.yml", config.default_schema_cache_path("my_db")
152+
end
153+
144154
def test_schema_cache_path_configuration_hash
145155
config = HashConfig.new("default_env", "primary", { schema_cache_path: "db/config_schema_cache.yml", adapter: "abstract" })
146156
assert_equal "db/config_schema_cache.yml", config.schema_cache_path
147157
end
148158

159+
def test_lazy_schema_cache_path
160+
config = HashConfig.new("default_env", "primary", { schema_cache_path: "db/config_schema_cache.yml", adapter: "abstract" })
161+
assert_equal "db/config_schema_cache.yml", config.lazy_schema_cache_path
162+
end
163+
164+
def test_lazy_schema_cache_path_uses_default_if_config_is_not_present
165+
config = HashConfig.new("default_env", "alternate", { adapter: "abstract" })
166+
assert_equal "db/alternate_schema_cache.yml", config.lazy_schema_cache_path
167+
end
168+
149169
def test_validate_checks_the_adapter_exists
150170
config = HashConfig.new("default_env", "primary", adapter: "abstract")
151171
assert config.validate!

activerecord/test/cases/tasks/database_tasks_test.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,20 @@ def test_cache_dump_default_filename
325325
ENV["SCHEMA_CACHE"] = old_path
326326
end
327327

328+
def test_cache_dump_default_filename_with_custom_db_dir
329+
old_path = ENV["SCHEMA_CACHE"]
330+
ENV.delete("SCHEMA_CACHE")
331+
332+
config = DatabaseConfigurations::HashConfig.new("development", "primary", {})
333+
334+
ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, "my_db") do
335+
path = ActiveRecord::Tasks::DatabaseTasks.cache_dump_filename(config)
336+
assert_equal "my_db/schema_cache.yml", path
337+
end
338+
ensure
339+
ENV["SCHEMA_CACHE"] = old_path
340+
end
341+
328342
def test_deprecated_cache_dump_default_filename
329343
old_path = ENV["SCHEMA_CACHE"]
330344
ENV.delete("SCHEMA_CACHE")

railties/test/application/rake/multi_dbs_test.rb

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -810,9 +810,7 @@ class TwoMigration < ActiveRecord::Migration::Current
810810
rails "db:drop" rescue nil
811811
end
812812

813-
# Note that schema cache loader depends on the connection and
814-
# does not work for all connections.
815-
test "schema_cache is loaded on primary db in multi-db app" do
813+
test "schema_cache is loaded on all connection db in multi-db app if it exists for the connection" do
816814
require "#{app_path}/config/environment"
817815
db_migrate_and_schema_cache_dump
818816

@@ -822,11 +820,8 @@ class TwoMigration < ActiveRecord::Migration::Current
822820
cache_tables_a = rails("runner", "p ActiveRecord::Base.connection.schema_cache.columns('books')").strip
823821
assert_includes cache_tables_a, "title", "expected cache_tables_a to include a title entry"
824822

825-
expired_warning = capture(:stderr) do
826-
cache_size_b = rails("runner", "p AnimalsBase.connection.schema_cache.size", stderr: true).strip
827-
assert_equal "0", cache_size_b
828-
end
829-
assert_match(/Ignoring .*\.yml because it has expired/, expired_warning)
823+
cache_size_b = rails("runner", "p AnimalsBase.connection.schema_cache.size", stderr: true).strip
824+
assert_equal "12", cache_size_b, "expected the cache size for animals to be valid since it was dumped"
830825

831826
cache_tables_b = rails("runner", "p AnimalsBase.connection.schema_cache.columns('dogs')").strip
832827
assert_includes cache_tables_b, "name", "expected cache_tables_b to include a name entry"

0 commit comments

Comments
 (0)