From f737b5ec1380963d2e354d25550e27ee3930831a Mon Sep 17 00:00:00 2001
From: Alex Ghiculescu <alex@tanda.co>
Date: Wed, 19 Oct 2022 15:33:51 -0500
Subject: [PATCH] Fix double logging in `ActiveRecord::QueryLog`

Fixes https://github.com/rails/rails/issues/46103

An issue exists if you set `config.active_record.query_log_tags` to an array that includes `:controller`, `:action`, or `:job`; the relevant item will get duplicated in the log line. This occured because the relevant railties would add the item to `config.active_record.query_log_tags` again during setup. This PR fixes that by only adding those items to the config if they aren't already set.

The issue proposed more documentation to work around this, but I think it's a bug and should be fixed directly.
---
 actionpack/CHANGELOG.md                      |  7 ++++++
 actionpack/lib/action_controller/railtie.rb  |  2 +-
 activejob/CHANGELOG.md                       |  7 ++++++
 activejob/lib/active_job/railtie.rb          |  2 +-
 railties/test/application/query_logs_test.rb | 23 ++++++++++++++++++++
 5 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index c03eefc810..24b05fad67 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Don't double log the `controller` or `action` when using `ActiveRecord::QueryLog`
+
+    Previously if you set `config.active_record.query_log_tags` to an array that included
+    `:controller` or `:action`, that item would get logged twice. This bug has been fixed.
+
+    *Alex Ghiculescu*
+
 *   Add the following permissions policy directives: `hid`, `idle-detection`, `screen-wake-lock`,
     `serial`, `sync-xhr`, `web-share`.
 
diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index 282e756d92..36d90b2ab9 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -112,7 +112,7 @@ module ActionController
         app.config.action_controller.log_query_tags_around_actions
 
       if query_logs_tags_enabled
-        app.config.active_record.query_log_tags += [:controller, :action]
+        app.config.active_record.query_log_tags |= [:controller, :action]
 
         ActiveSupport.on_load(:active_record) do
           ActiveRecord::QueryLogs.taggings.merge!(
diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md
index ca5cba9ee6..226284f10d 100644
--- a/activejob/CHANGELOG.md
+++ b/activejob/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Don't double log the `job` when using `ActiveRecord::QueryLog`
+
+    Previously if you set `config.active_record.query_log_tags` to an array that included
+    `:job`, the job name would get logged twice. This bug has been fixed.
+
+    *Alex Ghiculescu*
+
 *   Add support for Sidekiq's transaction-aware client
 
     *Jonathan del Strother*
diff --git a/activejob/lib/active_job/railtie.rb b/activejob/lib/active_job/railtie.rb
index b03125b042..f2c66fb798 100644
--- a/activejob/lib/active_job/railtie.rb
+++ b/activejob/lib/active_job/railtie.rb
@@ -72,7 +72,7 @@ module ActiveJob
         app.config.active_job.log_query_tags_around_perform
 
       if query_logs_tags_enabled
-        app.config.active_record.query_log_tags << :job
+        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] }
diff --git a/railties/test/application/query_logs_test.rb b/railties/test/application/query_logs_test.rb
index b96d26a45b..f000f1a637 100644
--- a/railties/test/application/query_logs_test.rb
+++ b/railties/test/application/query_logs_test.rb
@@ -115,6 +115,18 @@ module ApplicationTests
       assert_not_includes comment, "controller:users"
     end
 
+    test "controller tags are not doubled up if already configured" do
+      add_to_config "config.active_record.query_log_tags_enabled = true"
+      add_to_config "config.active_record.query_log_tags = [ :action, :job, :controller, :pid ]"
+
+      boot_app
+
+      get "/"
+      comment = last_response.body.strip
+
+      assert_match(/\/\*action='index',controller='users',pid='\d+'\*\//, comment)
+    end
+
     test "job perform method has tagging filters enabled by default" do
       add_to_config "config.active_record.query_log_tags_enabled = true"
 
@@ -136,6 +148,17 @@ module ApplicationTests
       assert_not_includes comment, "UserJob"
     end
 
+    test "job tags are not doubled up if already configured" do
+      add_to_config "config.active_record.query_log_tags_enabled = true"
+      add_to_config "config.active_record.query_log_tags = [ :action, :job, :controller, :pid ]"
+
+      boot_app
+
+      comment = UserJob.new.perform_now
+
+      assert_match(/\/\*job='UserJob',pid='\d+'\*\//, comment)
+    end
+
     test "query cache is cleared between requests" do
       add_to_config "config.active_record.query_log_tags_enabled = true"
       add_to_config "config.active_record.cache_query_log_tags = true"