Merge pull request rails#43541 from keeran/keeran/remove-with-tag

Remove with_tag from QueryLogs

Author: byroot

activerecord/lib/active_record/query_logs.rb

@@ -110,31 +110,15 @@ def set_context(**options)
         update_context(**previous_context)
       end
 
-      # Temporarily tag any query executed within `&block`. Can be nested.
-      def with_tag(tag, &block)
-        inline_tags.push(tag)
-        yield if block_given?
-      ensure
-        inline_tags.pop
-      end
-
       def call(sql) # :nodoc:
-        parts = self.comments
         if prepend_comment
-          parts << sql
+          "#{self.comment} #{sql}"
         else
-          parts.unshift(sql)
-        end
-        parts.join(" ")
+          "#{sql} #{self.comment}"
+        end.strip
       end
 
       private
-        # Returns an array of comments which need to be added to the query, comprised
-        # of configured and inline tags.
-        def comments
-          [ comment, inline_comment ].compact
-        end
-
         # Returns an SQL comment +String+ containing the query log tags.
         # Sets and returns a cached comment if <tt>cache_query_log_tags</tt> is +true+.
         def comment
@@ -152,21 +136,6 @@ def uncached_comment
           end
         end
 
-        # Returns a +String+ containing any inline comments from +with_tag+.
-        def inline_comment
-          return nil unless inline_tags.present?
-          "/*#{escape_sql_comment(inline_tag_content)}*/"
-        end
-
-        # Return the set of active inline tags from +with_tag+.
-        def inline_tags
-          if context[:inline_tags].nil?
-            context[:inline_tags] = []
-          else
-            context[:inline_tags]
-          end
-        end
-
         def context
           Thread.current[:active_record_query_log_tags_context] ||= Hash.new { NullObject.new }
         end
@@ -194,10 +163,6 @@ def tag_content
             "#{key}:#{val}" unless val.nil?
           end.join(",")
         end
-
-        def inline_tag_content
-          inline_tags.join
-        end
     end
   end
 end

activerecord/test/cases/query_logs_test.rb

@@ -100,10 +100,10 @@ def test_retrieves_comment_from_cache_when_enabled_and_set
     ActiveRecord::QueryLogs.cache_query_log_tags = true
     ActiveRecord::QueryLogs.tags = [ :application ]
 
-    assert_equal " /*application:active_record*/", ActiveRecord::QueryLogs.call("")
+    assert_equal "/*application:active_record*/", ActiveRecord::QueryLogs.call("")
 
     ActiveRecord::QueryLogs.stub(:cached_comment, "/*cached_comment*/") do
-      assert_equal " /*cached_comment*/", ActiveRecord::QueryLogs.call("")
+      assert_equal "/*cached_comment*/", ActiveRecord::QueryLogs.call("")
     end
   ensure
     ActiveRecord::QueryLogs.cached_comment = nil
@@ -115,12 +115,12 @@ def test_resets_cache_on_context_update
     ActiveRecord::QueryLogs.update_context(temporary: "value")
     ActiveRecord::QueryLogs.tags = [ temporary_tag: ->(context) { context[:temporary] } ]
 
-    assert_equal " /*temporary_tag:value*/", ActiveRecord::QueryLogs.call("")
+    assert_equal "/*temporary_tag:value*/", ActiveRecord::QueryLogs.call("")
 
     ActiveRecord::QueryLogs.update_context(temporary: "new_value")
 
     assert_nil ActiveRecord::QueryLogs.cached_comment
-    assert_equal " /*temporary_tag:new_value*/", ActiveRecord::QueryLogs.call("")
+    assert_equal "/*temporary_tag:new_value*/", ActiveRecord::QueryLogs.call("")
   ensure
     ActiveRecord::QueryLogs.cached_comment = nil
     ActiveRecord::QueryLogs.cache_query_log_tags = false
@@ -149,58 +149,6 @@ def test_default_tag_behavior
     end
   end
 
-  def test_inline_tags_only_affect_block
-    # disable regular comment tags
-    ActiveRecord::QueryLogs.tags = []
-
-    # confirm single inline tag
-    assert_sql(%r{/\*foo\*/$}) do
-      ActiveRecord::QueryLogs.with_tag("foo") do
-        Dashboard.first
-      end
-    end
-
-    # confirm different inline tag
-    assert_sql(%r{/\*bar\*/$}) do
-      ActiveRecord::QueryLogs.with_tag("bar") do
-        Dashboard.first
-      end
-    end
-
-    # confirm no tags are persisted
-    ActiveRecord::QueryLogs.tags = [ :application ]
-
-    assert_sql(%r{/\*application:active_record\*/$}) do
-      Dashboard.first
-    end
-  ensure
-    ActiveRecord::QueryLogs.tags = [ :application ]
-  end
-
-  def test_nested_inline_tags
-    assert_sql(%r{/\*foobar\*/$}) do
-      ActiveRecord::QueryLogs.with_tag("foo") do
-        ActiveRecord::QueryLogs.with_tag("bar") do
-          Dashboard.first
-        end
-      end
-    end
-  end
-
-  def test_bad_inline_tags
-    assert_sql(%r{/\*; DROP TABLE USERS;\*/$}) do
-      ActiveRecord::QueryLogs.with_tag("*/; DROP TABLE USERS;/*") do
-        Dashboard.first
-      end
-    end
-
-    assert_sql(%r{/\*; DROP TABLE USERS;\*/$}) do
-      ActiveRecord::QueryLogs.with_tag("**//; DROP TABLE USERS;//**") do
-        Dashboard.first
-      end
-    end
-  end
-
   def test_empty_comments_are_not_added
     original_tags = ActiveRecord::QueryLogs.tags
     ActiveRecord::QueryLogs.tags = [ empty: -> { nil } ]