Skip to content

Commit

Permalink
Remove complexity from the custom query logs
Browse files Browse the repository at this point in the history
We only have two specific implementations, so we don't need to
create generic formatter implementations for this feature.
  • Loading branch information
rafaelfranca committed Oct 5, 2022
1 parent cc8658a commit b48abc0
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 21 deletions.
7 changes: 4 additions & 3 deletions activerecord/lib/active_record/query_logs.rb
Expand Up @@ -94,9 +94,9 @@ def clear_cache # :nodoc:
def update_formatter(format)
self.tags_formatter = case format
when :legacy
Formatter.new(key_value_separator: ":")
LegacyFormatter.new
when :sqlcommenter
QuotingFormatter.new(key_value_separator: "=")
SQLCommenter.new
else
raise ArgumentError, "Formatter is unsupported: #{formatter}"
end
Expand Down Expand Up @@ -148,7 +148,8 @@ def tag_content
else
handler
end
"#{key}#{self.formatter.key_value_separator}#{self.formatter.format_value(val)}" unless val.nil?

self.formatter.format(key, val) unless val.nil?
end.join(",")
end
end
Expand Down
39 changes: 21 additions & 18 deletions activerecord/lib/active_record/query_logs_formatter.rb
Expand Up @@ -2,30 +2,33 @@

module ActiveRecord
module QueryLogs
class Formatter # :nodoc:
attr_reader :key_value_separator

# @param [String] key_value_separator: indicates the string used for
# separating keys and values.
#
# @param [Symbol] quote_values: indicates how values will be formatted (eg:
# in single quotes, not quoted at all, etc)
def initialize(key_value_separator:)
@key_value_separator = key_value_separator
class LegacyFormatter # :nodoc:
def initialize
@key_value_separator = ":"
end

# @param [String-coercible] value
# @return [String] The formatted value that will be used in our key-value
# pairs.
def format_value(value)
value
# Formats the key value pairs into a string.
def format(key, value)
"#{key}#{key_value_separator}#{format_value(value)}"
end

private
attr_reader :key_value_separator

def format_value(value)
value
end
end

class QuotingFormatter < Formatter # :nodoc:
def format_value(value)
"'#{value.to_s.gsub("'", "\\\\'")}'"
class SQLCommenter < LegacyFormatter # :nodoc:
def initialize
@key_value_separator = "="
end

private
def format_value(value)
"'#{value.to_s.gsub("'", "\\\\'")}'"
end
end
end
end

1 comment on commit b48abc0

@modulitos
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate you taking the time to clean this up!
I think I went a little overboard with the factory pattern and "composition over inheritance", which is unnecessary with only 2 formatters. Thanks for simplifying this.

I'm looking forward to seeing how sqlcommenter is adopted in 7.1.x. Hopefully we can remove the legacy formatter in the next release, and simplify this code even further :)

Please sign in to comment.