From 146ae32b851e0bff81d94f360fa376ce9b90dc2e Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@hawthorn.email>
Date: Tue, 23 Jul 2024 22:04:46 -0700
Subject: [PATCH] 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.
---
 actionpack/lib/action_controller/railtie.rb  |  2 +-
 activejob/lib/active_job/railtie.rb          |  4 +++-
 activerecord/lib/active_record/query_logs.rb | 10 +++++-----
 activerecord/lib/active_record/railtie.rb    |  2 +-
 activerecord/test/cases/query_logs_test.rb   |  6 +++---
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index 5ee9ae0db94..884bcc973a2 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -123,7 +123,7 @@ module ActionController
         app.config.active_record.query_log_tags |= [:action]
 
         ActiveSupport.on_load(:active_record) do
-          ActiveRecord::QueryLogs.taggings.merge!(
+          ActiveRecord::QueryLogs.taggings = ActiveRecord::QueryLogs.taggings.merge(
             controller:            ->(context) { context[:controller]&.controller_name },
             action:                ->(context) { context[:controller]&.action_name },
             namespaced_controller: ->(context) {
diff --git a/activejob/lib/active_job/railtie.rb b/activejob/lib/active_job/railtie.rb
index 1c56a0f75e7..578843aaeee 100644
--- a/activejob/lib/active_job/railtie.rb
+++ b/activejob/lib/active_job/railtie.rb
@@ -93,7 +93,9 @@ module ActiveJob
         app.config.active_record.query_log_tags |= [:job]
 
         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
diff --git a/activerecord/lib/active_record/query_logs.rb b/activerecord/lib/active_record/query_logs.rb
index 47aa6d06bb5..26d8fc006c1 100644
--- a/activerecord/lib/active_record/query_logs.rb
+++ b/activerecord/lib/active_record/query_logs.rb
@@ -102,8 +102,8 @@ module ActiveRecord
       end
     end
 
-    @taggings = {}
-    @tags = [ :application ]
+    @taggings = {}.freeze
+    @tags = [ :application ].freeze
     @prepend_comment = false
     @cache_query_log_tags = false
     @tags_formatter = false
@@ -115,17 +115,16 @@ module ActiveRecord
       attr_accessor :prepend_comment, :cache_query_log_tags # :nodoc:
 
       def taggings=(taggings) # :nodoc:
-        @taggings = taggings
+        @taggings = taggings.freeze
         @handlers = rebuild_handlers
       end
 
       def tags=(tags) # :nodoc:
-        @tags = tags
+        @tags = tags.freeze
         @handlers = rebuild_handlers
       end
 
       def tags_formatter=(format) # :nodoc:
-        @tags_formatter = format
         @formatter = case format
         when :legacy
           LegacyFormatter
@@ -134,6 +133,7 @@ module ActiveRecord
         else
           raise ArgumentError, "Formatter is unsupported: #{format}"
         end
+        @tags_formatter = format
       end
 
       def call(sql, connection) # :nodoc:
diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb
index 6a61a2483bb..cb1cabc929d 100644
--- a/activerecord/lib/active_record/railtie.rb
+++ b/activerecord/lib/active_record/railtie.rb
@@ -385,7 +385,7 @@ To keep using the current cache store, you can turn off cache versioning entirel
       config.after_initialize do
         if app.config.active_record.query_log_tags_enabled
           ActiveRecord.query_transformers << ActiveRecord::QueryLogs
-          ActiveRecord::QueryLogs.taggings.merge!(
+          ActiveRecord::QueryLogs.taggings = ActiveRecord::QueryLogs.taggings.merge(
             application:  Rails.application.class.name.split("::").first,
             pid:          -> { Process.pid.to_s },
             socket:       ->(context) { context[:connection].pool.db_config.socket },
diff --git a/activerecord/test/cases/query_logs_test.rb b/activerecord/test/cases/query_logs_test.rb
index de13e22fbda..73725678830 100644
--- a/activerecord/test/cases/query_logs_test.rb
+++ b/activerecord/test/cases/query_logs_test.rb
@@ -20,8 +20,8 @@ class QueryLogsTest < ActiveRecord::TestCase
     ActiveRecord::QueryLogs.prepend_comment = false
     ActiveRecord::QueryLogs.cache_query_log_tags = false
     ActiveRecord::QueryLogs.cached_comment = nil
-    ActiveRecord::QueryLogs.taggings[:application] = -> {
-      "active_record"
+    ActiveRecord::QueryLogs.taggings = {
+      application: -> { "active_record" }
     }
   end
 
@@ -185,7 +185,7 @@ class QueryLogsTest < ActiveRecord::TestCase
 
   def test_sql_commenter_format
     ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter
-    ActiveRecord::QueryLogs.tags = [:application, {}]
+    ActiveRecord::QueryLogs.tags = [:application]
 
     assert_queries_match(%r{/\*application='active_record'\*/}) do
       Dashboard.first