Freeze modifications to ActiveRecord::QueryLogs

Tags and taggings build a cache when they are assigned, which means we
cannot support them being mutated. This freezes tags and taggings to
ensure they aren't changed after assignment.

This may not cover all cases, and per the previous PR we intend this to
be configured mostly via application.config, but it covers a case we
were making in our test suite and so should hopefully cover a mistake
users are somewhat likely to make.
This commit is contained in:
John Hawthorn 2024-07-23 22:04:46 -07:00
parent 8551d3d805
commit 146ae32b85
5 changed files with 13 additions and 11 deletions

View File

@ -123,7 +123,7 @@ module ActionController
app.config.active_record.query_log_tags |= [:action] app.config.active_record.query_log_tags |= [:action]
ActiveSupport.on_load(:active_record) do ActiveSupport.on_load(:active_record) do
ActiveRecord::QueryLogs.taggings.merge!( ActiveRecord::QueryLogs.taggings = ActiveRecord::QueryLogs.taggings.merge(
controller: ->(context) { context[:controller]&.controller_name }, controller: ->(context) { context[:controller]&.controller_name },
action: ->(context) { context[:controller]&.action_name }, action: ->(context) { context[:controller]&.action_name },
namespaced_controller: ->(context) { namespaced_controller: ->(context) {

View File

@ -93,7 +93,9 @@ module ActiveJob
app.config.active_record.query_log_tags |= [:job] app.config.active_record.query_log_tags |= [:job]
ActiveSupport.on_load(:active_record) do ActiveSupport.on_load(:active_record) do
ActiveRecord::QueryLogs.taggings[:job] = ->(context) { context[:job].class.name if context[:job] } ActiveRecord::QueryLogs.taggings = ActiveRecord::QueryLogs.taggings.merge(
job: ->(context) { context[:job].class.name if context[:job] }
)
end end
end end
end end

View File

@ -102,8 +102,8 @@ module ActiveRecord
end end
end end
@taggings = {} @taggings = {}.freeze
@tags = [ :application ] @tags = [ :application ].freeze
@prepend_comment = false @prepend_comment = false
@cache_query_log_tags = false @cache_query_log_tags = false
@tags_formatter = false @tags_formatter = false
@ -115,17 +115,16 @@ module ActiveRecord
attr_accessor :prepend_comment, :cache_query_log_tags # :nodoc: attr_accessor :prepend_comment, :cache_query_log_tags # :nodoc:
def taggings=(taggings) # :nodoc: def taggings=(taggings) # :nodoc:
@taggings = taggings @taggings = taggings.freeze
@handlers = rebuild_handlers @handlers = rebuild_handlers
end end
def tags=(tags) # :nodoc: def tags=(tags) # :nodoc:
@tags = tags @tags = tags.freeze
@handlers = rebuild_handlers @handlers = rebuild_handlers
end end
def tags_formatter=(format) # :nodoc: def tags_formatter=(format) # :nodoc:
@tags_formatter = format
@formatter = case format @formatter = case format
when :legacy when :legacy
LegacyFormatter LegacyFormatter
@ -134,6 +133,7 @@ module ActiveRecord
else else
raise ArgumentError, "Formatter is unsupported: #{format}" raise ArgumentError, "Formatter is unsupported: #{format}"
end end
@tags_formatter = format
end end
def call(sql, connection) # :nodoc: def call(sql, connection) # :nodoc:

View File

@ -385,7 +385,7 @@ To keep using the current cache store, you can turn off cache versioning entirel
config.after_initialize do config.after_initialize do
if app.config.active_record.query_log_tags_enabled if app.config.active_record.query_log_tags_enabled
ActiveRecord.query_transformers << ActiveRecord::QueryLogs ActiveRecord.query_transformers << ActiveRecord::QueryLogs
ActiveRecord::QueryLogs.taggings.merge!( ActiveRecord::QueryLogs.taggings = ActiveRecord::QueryLogs.taggings.merge(
application: Rails.application.class.name.split("::").first, application: Rails.application.class.name.split("::").first,
pid: -> { Process.pid.to_s }, pid: -> { Process.pid.to_s },
socket: ->(context) { context[:connection].pool.db_config.socket }, socket: ->(context) { context[:connection].pool.db_config.socket },

View File

@ -20,8 +20,8 @@ class QueryLogsTest < ActiveRecord::TestCase
ActiveRecord::QueryLogs.prepend_comment = false ActiveRecord::QueryLogs.prepend_comment = false
ActiveRecord::QueryLogs.cache_query_log_tags = false ActiveRecord::QueryLogs.cache_query_log_tags = false
ActiveRecord::QueryLogs.cached_comment = nil ActiveRecord::QueryLogs.cached_comment = nil
ActiveRecord::QueryLogs.taggings[:application] = -> { ActiveRecord::QueryLogs.taggings = {
"active_record" application: -> { "active_record" }
} }
end end
@ -185,7 +185,7 @@ class QueryLogsTest < ActiveRecord::TestCase
def test_sql_commenter_format def test_sql_commenter_format
ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter
ActiveRecord::QueryLogs.tags = [:application, {}] ActiveRecord::QueryLogs.tags = [:application]
assert_queries_match(%r{/\*application='active_record'\*/}) do assert_queries_match(%r{/\*application='active_record'\*/}) do
Dashboard.first Dashboard.first