Facilitate use of any regular ERB in database.yml

Commit 37d1429ab1 introduced the DummyERB to avoid loading the environment when
running `rake -T`.

The DummyCompiler simply replaced all output from `<%=` with a fixed string and
removed everything else. This worked okay when it was used for YAML values.
When using `<%=` within a YAML key, it caused an error in the YAML parser,
making it impossible to use ERB as you would expect. For example a
`database.yml` file containing the following should be possible:

  development:
    <% 5.times do |i| %>
    shard_<%= i %>:
      database: db/development_shard_<%= i %>.sqlite3
      adapter: sqlite3
    <% end %>

Instead of using a broken ERB compiler we can temporarily use a
`Rails.application.config` that does not raise an error when configurations are
accessed which have not been set as described in #35468.

This change removes the `DummyCompiler` and uses the standard `ERB::Compiler`.
It introduces the `DummyConfig` which delegates all known configurations to the
real `Rails::Application::Configuration` instance and returns a dummy string for
everything else. This restores the full ERB capabilities without compromising on
speed when generating the rake tasks for multiple databases.

Deprecates `config.active_record.suppress_multiple_database_warning`.
This commit is contained in:
Eike Send 2022-09-26 15:08:31 +02:00
parent 876977d0ad
commit 5ba8aa5854
No known key found for this signature in database
GPG Key ID: 97198D892AB00327
11 changed files with 111 additions and 72 deletions

View File

@ -46,10 +46,6 @@ Rails.application.configure do
# Highlight code that triggered database queries in logs.
config.active_record.verbose_query_logs = true
# Show a warning when Rails couldn't parse your database.yml
# for multiple databases.
config.active_record.suppress_multiple_database_warning = false
# Debug mode disables concatenation and preprocessing of assets.
# This option may cause significant delays in view rendering with a large
# number of complex assets.

View File

@ -46,10 +46,6 @@ Rails.application.configure do
# Highlight code that triggered database queries in logs.
config.active_record.verbose_query_logs = true
# Show a warning when Rails couldn't parse your database.yml
# for multiple databases.
config.active_record.suppress_multiple_database_warning = false
# Debug mode disables concatenation and preprocessing of assets.
# This option may cause significant delays in view rendering with a large
# number of complex assets.

View File

@ -1,3 +1,12 @@
* Allow any ERB in the database.yml when creating rake tasks.
Any ERB can be used in `database.yml` even if it accesses environment
configurations.
Deprecates `config.active_record.suppress_multiple_database_warning`.
*Eike Send*
* Add table to error for duplicate column definitions.
If a migration defines duplicate columns for a table, the error message

View File

@ -336,12 +336,19 @@ module ActiveRecord
singleton_class.attr_accessor :dump_schemas
self.dump_schemas = :schema_search_path
##
# :singleton-method:
# Show a warning when Rails couldn't parse your database.yml
# for multiple databases.
singleton_class.attr_accessor :suppress_multiple_database_warning
self.suppress_multiple_database_warning = false
def self.suppress_multiple_database_warning
ActiveSupport::Deprecation.warn(<<-MSG.squish)
config.active_record.suppress_multiple_database_warning is deprecated and will be removed in Rails 7.2.
It no longer has any effect and should be removed from the configuration file.
MSG
end
def self.suppress_multiple_database_warning=(value)
ActiveSupport::Deprecation.warn(<<-MSG.squish)
config.active_record.suppress_multiple_database_warning= is deprecated and will be removed in Rails 7.2.
It no longer has any effect and should be removed from the configuration file.
MSG
end
##
# :singleton-method:

View File

@ -140,15 +140,7 @@ module ActiveRecord
def setup_initial_database_yaml
return {} unless defined?(Rails)
begin
Rails.application.config.load_database_yaml
rescue
unless ActiveRecord.suppress_multiple_database_warning
$stderr.puts "Rails couldn't infer whether you are using multiple databases from your database.yml and can't generate the tasks for the non-primary databases. If you'd like to use this feature, please simplify your ERB."
end
{}
end
Rails.application.config.load_database_yaml
end
def for_each(databases)

View File

@ -376,24 +376,34 @@ module Rails
end
end
# Load the database YAML without evaluating ERB. This allows us to
# create the rake tasks for multiple databases without filling in the
# configuration values or loading the environment. Do not use this
# method.
# Load the <tt>config/database.yml</tt> to create the Rake tasks for
# multiple databases without loading the environment and filling in the
# environment specific configuration values.
#
# This uses a DummyERB custom compiler so YAML can ignore the ERB
# tags and load the database.yml for the rake tasks.
# Do not use this method, use #database_configuration instead.
def load_database_yaml # :nodoc:
if path = paths["config/database"].existent.first
require "rails/application/dummy_erb_compiler"
require "rails/application/dummy_config"
yaml = DummyERB.new(Pathname.new(path).read).result
original_rails_config = Rails.application.config
dummy_config = DummyConfig.new(original_rails_config)
database_config = {}
if YAML.respond_to?(:unsafe_load)
YAML.unsafe_load(yaml) || {}
else
YAML.load(yaml) || {}
begin
Rails.application.config = dummy_config
yaml = ERB.new(Pathname.new(path).read).result
if YAML.respond_to?(:unsafe_load)
database_config = YAML.unsafe_load(yaml) || {}
else
database_config = YAML.load(yaml) || {}
end
ensure
Rails.application.config = original_rails_config
end
database_config
else
{}
end

View File

@ -0,0 +1,19 @@
# frozen_string_literal: true
class DummyConfig # :nodoc:
def initialize(config)
@config = config
end
def to_s
"DummyConfig"
end
def method_missing(selector, *args, &blk)
if @config.respond_to?(selector)
@config.send(selector, *args, &blk)
else
self
end
end
end

View File

@ -1,18 +0,0 @@
# frozen_string_literal: true
# These classes are used to strip out the ERB configuration
# values so we can evaluate the database.yml without evaluating
# the ERB values.
class DummyERB < ERB # :nodoc:
def make_compiler(trim_mode)
DummyCompiler.new trim_mode
end
end
class DummyCompiler < ERB::Compiler # :nodoc:
def compile_content(stag, out)
if stag == "<%="
out.push "_erbout << ''"
end
end
end

View File

@ -1741,9 +1741,16 @@ module ApplicationTests
assert_not ActiveRecord.verbose_query_logs
end
test "config.active_record.suppress_multiple_database_warning is false by default in development" do
test "config.active_record.suppress_multiple_database_warning getter is deprecated" do
app "development"
assert_not ActiveRecord.suppress_multiple_database_warning
assert_deprecated { ActiveRecord.suppress_multiple_database_warning }
end
test "config.active_record.suppress_multiple_database_warning setter is deprecated" do
app "development"
assert_deprecated { ActiveRecord.suppress_multiple_database_warning = true }
end
test "config.active_record.use_yaml_unsafe_load is false by default" do

View File

@ -40,15 +40,6 @@ module ApplicationTests
end
end
def db_create_with_warning(expected_database)
Dir.chdir(app_path) do
output = rails("db:create")
assert_match(/Rails couldn't infer whether you are using multiple databases/, output)
assert_match(/Created database/, output)
assert File.exist?(expected_database)
end
end
test "db:create and db:drop without database URL" do
require "#{app_path}/config/environment"
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, name: "primary")
@ -100,6 +91,25 @@ module ApplicationTests
db_create_and_drop("db/development.sqlite3", environment_loaded: false)
end
test "db:create and db:drop don't raise errors when loading YAML with alias ERB" do
app_file "config/database.yml", <<-YAML
sqlite: &sqlite
adapter: sqlite3
database: db/development.sqlite3
development:
<<: *<%= ENV["DB"] || "sqlite" %>
YAML
app_file "config/environments/development.rb", <<-RUBY
Rails.application.configure do
config.database = "db/development.sqlite3"
end
RUBY
db_create_and_drop("db/development.sqlite3", environment_loaded: false)
end
test "db:create and db:drop don't raise errors when loading YAML with multiline ERB" do
app_file "config/database.yml", <<-YAML
development:
@ -118,23 +128,21 @@ module ApplicationTests
db_create_and_drop("db/development.sqlite3", environment_loaded: false)
end
test "db:create and db:drop show warning but doesn't raise errors when loading YAML with alias ERB" do
test "db:create and db:drop don't raise errors when loading ERB accessing nested configurations" do
app_file "config/database.yml", <<-YAML
sqlite: &sqlite
adapter: sqlite3
database: db/development.sqlite3
development:
<<: *<%= ENV["DB"] || "sqlite" %>
database: db/development.sqlite3
adapter: sqlite3
other: <%= Rails.application.config.other.value %>
YAML
app_file "config/environments/development.rb", <<-RUBY
Rails.application.configure do
config.database = "db/development.sqlite3"
config.other = OpenStruct.new(value: 123)
end
RUBY
db_create_with_warning("db/development.sqlite3")
db_create_and_drop("db/development.sqlite3", environment_loaded: false)
end
test "db:create and db:drop don't raise errors when loading YAML containing conditional statements in ERB" do

View File

@ -958,6 +958,19 @@ module ApplicationTests
db_create_and_drop_namespace("primary", "db/development.sqlite3")
end
test "db:create and db:drop don't raise errors when loading YAML containing ERB in database keys" do
app_file "config/database.yml", <<-YAML
development:
<% 5.times do |i| %>
shard_<%= i %>:
database: db/development_shard_<%= i %>.sqlite3
adapter: sqlite3
<% end %>
YAML
db_create_and_drop_namespace("shard_3", "db/development_shard_3.sqlite3")
end
test "schema generation when dump_schema_after_migration is true schema_dump is false" do
app_file "config/database.yml", <<~EOS
development: