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:

351a8d9bc9/activerecord/lib/active_record/connection_adapters/pool_config.rb (L13)
351a8d9bc9/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.
This commit is contained in:
Rafael Mendonça França 2024-01-26 22:19:45 +00:00
parent 1cf7025b4d
commit 4d1d7d3d1b
No known key found for this signature in database
GPG Key ID: FC23B6D0F1EEE948
5 changed files with 47 additions and 17 deletions

View File

@ -113,8 +113,12 @@ module ActiveRecord
configuration_hash[:schema_cache_path]
end
def default_schema_cache_path
"db/schema_cache.yml"
def default_schema_cache_path(db_dir = "db")
if primary?
File.join(db_dir, "schema_cache.yml")
else
File.join(db_dir, "#{name}_schema_cache.yml")
end
end
def lazy_schema_cache_path

View File

@ -439,13 +439,10 @@ module ActiveRecord
def cache_dump_filename(db_config_or_name, schema_cache_path: nil)
if db_config_or_name.is_a?(DatabaseConfigurations::DatabaseConfig)
filename = if db_config_or_name.primary?
"schema_cache.yml"
else
"#{db_config_or_name.name}_schema_cache.yml"
end
schema_cache_path || db_config_or_name.schema_cache_path || ENV["SCHEMA_CACHE"] || File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, filename)
schema_cache_path ||
db_config_or_name.schema_cache_path ||
ENV["SCHEMA_CACHE"] ||
db_config_or_name.default_schema_cache_path(ActiveRecord::Tasks::DatabaseTasks.db_dir)
else
ActiveRecord.deprecator.deprecation_warning(<<~MSG.squish)
Passing a database name to `cache_dump_filename` is deprecated and will be removed in Rails 7.3. Pass a

View File

@ -141,11 +141,31 @@ module ActiveRecord
assert_equal "db/schema_cache.yml", config.default_schema_cache_path
end
def test_schema_cache_path_default_for_custom_name
config = HashConfig.new("default_env", "alternate", adapter: "abstract")
assert_equal "db/alternate_schema_cache.yml", config.default_schema_cache_path
end
def test_schema_cache_path_default_for_different_db_dir
config = HashConfig.new("default_env", "alternate", adapter: "abstract")
assert_equal "my_db/alternate_schema_cache.yml", config.default_schema_cache_path("my_db")
end
def test_schema_cache_path_configuration_hash
config = HashConfig.new("default_env", "primary", { schema_cache_path: "db/config_schema_cache.yml", adapter: "abstract" })
assert_equal "db/config_schema_cache.yml", config.schema_cache_path
end
def test_lazy_schema_cache_path
config = HashConfig.new("default_env", "primary", { schema_cache_path: "db/config_schema_cache.yml", adapter: "abstract" })
assert_equal "db/config_schema_cache.yml", config.lazy_schema_cache_path
end
def test_lazy_schema_cache_path_uses_default_if_config_is_not_present
config = HashConfig.new("default_env", "alternate", { adapter: "abstract" })
assert_equal "db/alternate_schema_cache.yml", config.lazy_schema_cache_path
end
def test_validate_checks_the_adapter_exists
config = HashConfig.new("default_env", "primary", adapter: "abstract")
assert config.validate!

View File

@ -325,6 +325,20 @@ module ActiveRecord
ENV["SCHEMA_CACHE"] = old_path
end
def test_cache_dump_default_filename_with_custom_db_dir
old_path = ENV["SCHEMA_CACHE"]
ENV.delete("SCHEMA_CACHE")
config = DatabaseConfigurations::HashConfig.new("development", "primary", {})
ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, "my_db") do
path = ActiveRecord::Tasks::DatabaseTasks.cache_dump_filename(config)
assert_equal "my_db/schema_cache.yml", path
end
ensure
ENV["SCHEMA_CACHE"] = old_path
end
def test_deprecated_cache_dump_default_filename
old_path = ENV["SCHEMA_CACHE"]
ENV.delete("SCHEMA_CACHE")

View File

@ -810,9 +810,7 @@ module ApplicationTests
rails "db:drop" rescue nil
end
# Note that schema cache loader depends on the connection and
# does not work for all connections.
test "schema_cache is loaded on primary db in multi-db app" do
test "schema_cache is loaded on all connection db in multi-db app if it exists for the connection" do
require "#{app_path}/config/environment"
db_migrate_and_schema_cache_dump
@ -822,11 +820,8 @@ module ApplicationTests
cache_tables_a = rails("runner", "p ActiveRecord::Base.connection.schema_cache.columns('books')").strip
assert_includes cache_tables_a, "title", "expected cache_tables_a to include a title entry"
expired_warning = capture(:stderr) do
cache_size_b = rails("runner", "p AnimalsBase.connection.schema_cache.size", stderr: true).strip
assert_equal "0", cache_size_b
end
assert_match(/Ignoring .*\.yml because it has expired/, expired_warning)
assert_equal "12", cache_size_b, "expected the cache size for animals to be valid since it was dumped"
cache_tables_b = rails("runner", "p AnimalsBase.connection.schema_cache.columns('dogs')").strip
assert_includes cache_tables_b, "name", "expected cache_tables_b to include a name entry"