From 743b65bf379e2c311532c32369a81aff8ed969e1 Mon Sep 17 00:00:00 2001 From: modulitos Date: Sat, 5 Feb 2022 20:29:46 -0800 Subject: [PATCH 1/4] Customizable key-value separator and quoting --- README.md | 6 ++++++ lib/marginalia.rb | 8 ++++++++ lib/marginalia/comment.rb | 20 +++++++++++++++++++- test/query_comments_test.rb | 15 +++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index db2e04c..66d915b 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,12 @@ a new prepared statement for each potentially exhausting system resources. [Disable prepared statements](https://guides.rubyonrails.org/configuring.html#configuring-a-postgresql-database) if you wish to use components with high cardinality values. +#### Customizing key-value formatting + +To change the default key-value separator (from `,`), set `Marginalia::Comment.key_value_separator`. +To surround all values with single quotes (`'`) and escape internal quotes as `\'`, +set `Marginalia::Comment.quote_values = :single`. + ## Contributing Start by bundling and creating the test database: diff --git a/lib/marginalia.rb b/lib/marginalia.rb index 205c45f..12c678d 100644 --- a/lib/marginalia.rb +++ b/lib/marginalia.rb @@ -112,6 +112,14 @@ def record_query_comment end end + def self.with_comment_settings(settings) + prev = settings.keys.map { |key| [key, Marginalia::Comment.send(key)] } + settings.each { |key, value| Marginalia::Comment.send(:"#{key}=", value) } + yield + ensure + prev.each { |(key, value)| Marginalia::Comment.send(:"#{key}=", value) } + end + def self.with_annotation(comment, &block) Marginalia::Comment.inline_annotations.push(comment) block.call if block.present? diff --git a/lib/marginalia/comment.rb b/lib/marginalia/comment.rb index f4a77de..22f12ba 100644 --- a/lib/marginalia/comment.rb +++ b/lib/marginalia/comment.rb @@ -5,6 +5,17 @@ module Marginalia module Comment mattr_accessor :components, :lines_to_ignore, :prepend_comment + + + # @return [String] String used for separating keys and values. + mattr_accessor :key_value_separator + self.key_value_separator = ':' + + # @return [false, :single] If `:single`, surrounds values with single quotes + # (') and escapes internal single quotes as \'. + mattr_accessor :quote_values + self.quote_values = false + Marginalia::Comment.components ||= [:application, :controller, :action] def self.update!(controller = nil) @@ -19,12 +30,19 @@ def self.update_adapter!(adapter) self.marginalia_adapter = adapter end + # @param [String] value + # @return [String] + def self.quote_value(value) + quote_values ? "'#{value.gsub("'", "\\\\'")}'" : value + end + def self.construct_comment ret = String.new self.components.each do |c| component_value = self.send(c) if component_value.present? - ret << "#{c}:#{component_value}," + ret << "#{c}#{key_value_separator}"\ + "#{quote_value(component_value)}," end end ret.chop! diff --git a/test/query_comments_test.rb b/test/query_comments_test.rb index a5cb9c9..3e17786 100644 --- a/test/query_comments_test.rb +++ b/test/query_comments_test.rb @@ -363,6 +363,21 @@ def test_add_comments_to_beginning_of_query Marginalia::Comment.prepend_comment = nil end + def test_comment_key_value_separator + Marginalia.with_comment_settings(key_value_separator: '=') do + ActiveRecord::Base.connection.execute "select id from posts" + assert_equal 'select id from posts /*application=rails*/', @queries.first + end + end + + def test_comment_quote_values_single + Marginalia.application_name = "Joe's app" + Marginalia.with_comment_settings(quote_values: :single) do + ActiveRecord::Base.connection.execute "select id from posts" + assert_equal "select id from posts /*application:'Joe\\'s app'*/", @queries.first + end + end + def teardown Marginalia.application_name = nil Marginalia::Comment.lines_to_ignore = nil From 35d510401efd9317a999c3d745710a0a7143a475 Mon Sep 17 00:00:00 2001 From: modulitos Date: Sun, 13 Mar 2022 17:13:30 -0700 Subject: [PATCH 2/4] docs(README): update typo from ',' to ':' --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 66d915b..8d91b4a 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,7 @@ if you wish to use components with high cardinality values. #### Customizing key-value formatting -To change the default key-value separator (from `,`), set `Marginalia::Comment.key_value_separator`. +To change the default key-value separator (from `:`), set `Marginalia::Comment.key_value_separator`. To surround all values with single quotes (`'`) and escape internal quotes as `\'`, set `Marginalia::Comment.quote_values = :single`. From 0c55ab893fe6e2d0887d5478ff4ec52eaad25fae Mon Sep 17 00:00:00 2001 From: modulitos Date: Thu, 28 Apr 2022 23:06:30 -0700 Subject: [PATCH 3/4] refactor low-level configs into :default and :sqlcommenter shorthands --- README.md | 20 +++++++++---- Rakefile | 11 ++++++-- lib/marginalia.rb | 8 ------ lib/marginalia/comment.rb | 25 ++++++----------- lib/marginalia/formatter.rb | 56 +++++++++++++++++++++++++++++++++++++ test/formatters_test.rb | 30 ++++++++++++++++++++ test/query_comments_test.rb | 22 ++++++++------- 7 files changed, 128 insertions(+), 44 deletions(-) create mode 100644 lib/marginalia/formatter.rb create mode 100644 test/formatters_test.rb diff --git a/README.md b/README.md index 8d91b4a..17bc200 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,20 @@ In order to not lose the marginalia comments from your logs, you can prepend the Marginalia::Comment.prepend_comment = true +#### Comment formatting + +Comment formatting can be configured to be more machine-readable by adding this command: `Marginalia::Comment.update_formatter!(:sqlcommenter)`. This will format Marginalia comments to be consistent with other SQL commenting projects, like [Google's sqlcommenter](https://google.github.io/sqlcommenter/). + +This format makes the following changes: +1. The key-value separator will use an equals sign (`=`) instead of a colon (`:`). +2. Values will be surrounded with single quotes (`'`) and escape internal quotes as `\'`. + +For example, a SQL query like this: +`"select id from posts /*application:Joe's app,controller:my_controller*/"` + +will be formatted like this: +`"select id from posts /*application='Joe\\'s app',controller='my_controller*/"` + #### Inline query annotations In addition to the request or job-level component-based annotations, @@ -124,12 +138,6 @@ a new prepared statement for each potentially exhausting system resources. [Disable prepared statements](https://guides.rubyonrails.org/configuring.html#configuring-a-postgresql-database) if you wish to use components with high cardinality values. -#### Customizing key-value formatting - -To change the default key-value separator (from `:`), set `Marginalia::Comment.key_value_separator`. -To surround all values with single quotes (`'`) and escape internal quotes as `\'`, -set `Marginalia::Comment.quote_values = :single`. - ## Contributing Start by bundling and creating the test database: diff --git a/Rakefile b/Rakefile index 069e7c1..0bd9523 100755 --- a/Rakefile +++ b/Rakefile @@ -5,7 +5,7 @@ task :default => ['test:all'] namespace :test do desc "test all drivers" - task :all => [:mysql2, :postgresql, :sqlite] + task :all => [:mysql2, :postgresql, :sqlite, :formatters] desc "test mysql2 driver" task :mysql2 do @@ -14,12 +14,17 @@ namespace :test do desc "test PostgreSQL driver" task :postgresql do - sh "DRIVER=postgresql DB_USERNAME=postgres bundle exec ruby -Ilib -Itest test/*_test.rb" + sh "DRIVER=postgresql DB_USERNAME=postgres bundle exec ruby -Ilib -Itest test/query_comments_test.rb" end desc "test sqlite3 driver" task :sqlite do - sh "DRIVER=sqlite3 bundle exec ruby -Ilib -Itest test/*_test.rb" + sh "DRIVER=sqlite3 bundle exec ruby -Ilib -Itest test/query_comments_test.rb" + end + + desc "test formatters" + task :formatters do + sh "bundle exec ruby -Ilib -Itest test/formatters_test.rb" end end diff --git a/lib/marginalia.rb b/lib/marginalia.rb index 12c678d..205c45f 100644 --- a/lib/marginalia.rb +++ b/lib/marginalia.rb @@ -112,14 +112,6 @@ def record_query_comment end end - def self.with_comment_settings(settings) - prev = settings.keys.map { |key| [key, Marginalia::Comment.send(key)] } - settings.each { |key, value| Marginalia::Comment.send(:"#{key}=", value) } - yield - ensure - prev.each { |(key, value)| Marginalia::Comment.send(:"#{key}=", value) } - end - def self.with_annotation(comment, &block) Marginalia::Comment.inline_annotations.push(comment) block.call if block.present? diff --git a/lib/marginalia/comment.rb b/lib/marginalia/comment.rb index 22f12ba..b2ff359 100644 --- a/lib/marginalia/comment.rb +++ b/lib/marginalia/comment.rb @@ -1,23 +1,16 @@ # frozen_string_literal: true require 'socket' +require 'marginalia/formatter' module Marginalia module Comment - mattr_accessor :components, :lines_to_ignore, :prepend_comment - - - # @return [String] String used for separating keys and values. - mattr_accessor :key_value_separator - self.key_value_separator = ':' - - # @return [false, :single] If `:single`, surrounds values with single quotes - # (') and escapes internal single quotes as \'. - mattr_accessor :quote_values - self.quote_values = false + mattr_accessor :components, :lines_to_ignore, :prepend_comment, :formatter Marginalia::Comment.components ||= [:application, :controller, :action] + Marginalia::Comment.formatter ||= Marginalia::FormatterFactory.from_symbol(:default) + def self.update!(controller = nil) self.marginalia_controller = controller end @@ -30,10 +23,8 @@ def self.update_adapter!(adapter) self.marginalia_adapter = adapter end - # @param [String] value - # @return [String] - def self.quote_value(value) - quote_values ? "'#{value.gsub("'", "\\\\'")}'" : value + def self.update_formatter!(formatter = :default) + self.formatter = Marginalia::FormatterFactory.from_symbol(formatter) end def self.construct_comment @@ -41,8 +32,8 @@ def self.construct_comment self.components.each do |c| component_value = self.send(c) if component_value.present? - ret << "#{c}#{key_value_separator}"\ - "#{quote_value(component_value)}," + ret << "#{c}#{self.formatter.key_value_separator}"\ + "#{self.formatter.quote_value(component_value)}," end end ret.chop! diff --git a/lib/marginalia/formatter.rb b/lib/marginalia/formatter.rb new file mode 100644 index 0000000..cdf0851 --- /dev/null +++ b/lib/marginalia/formatter.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true +module Marginalia + class Formatter + attr_reader :key_value_separator + + SUPPORTED_QUOTE_VALUES = %i[ + none + single + ].freeze + + # @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:, quote_values:) + unless SUPPORTED_QUOTE_VALUES.include?(quote_values) + raise ArgumentError, "Quote_values arg is unsupported: #{quote_values}" + end + @key_value_separator = key_value_separator + @quote_values = quote_values + end + + # @param [string] value + # @return [String] The formatted value that will be used in our key-value + # pairs. + def quote_value(value) + if @quote_values == :none + value + else + "'#{value.gsub("'", "\\\\'")}'" + end + end + end + + class FormatterFactory + SUPPORTED_FORMATTERS = %i[ + default + sqlcommenter + ].freeze + + # @param [Symbol] formatter: the kind of formatter we're building. + # @return [Formatter] + def self.from_symbol(formatter) + unless SUPPORTED_FORMATTERS.include?(formatter) + raise ArgumentError, "Formatter is unsupported: #{formatter}" + end + + if formatter == :default + Formatter.new(key_value_separator: ':', quote_values: :none) + else + Formatter.new(key_value_separator: '=', quote_values: :single) + end + end + end +end diff --git a/test/formatters_test.rb b/test/formatters_test.rb new file mode 100644 index 0000000..db91539 --- /dev/null +++ b/test/formatters_test.rb @@ -0,0 +1,30 @@ +require "minitest/autorun" + +# Shim for compatibility with older versions of MiniTest +MiniTest::Test = MiniTest::Unit::TestCase unless defined?(MiniTest::Test) + +require 'marginalia/formatter' + +class FormatterTest < MiniTest::Test + def test_factory_invalid_formatter + assert_raises(ArgumentError) do + Marginalia::FormatterFactory.from_symbol(:non_existing_formatter) + end + end + + def test_factory_invalid_quote_values + assert_raises(ArgumentError) do + Marginalia::Formatter.new(key_value_separator: ':', quote_values: :does_not_exist) + end + end + + def test_sqlcommenter_key_value_separator + formatter = Marginalia::FormatterFactory.from_symbol(:sqlcommenter) + assert_equal('=', formatter.key_value_separator) + end + + def test_sqlcommenter_quote_value + formatter = Marginalia::FormatterFactory.from_symbol(:sqlcommenter) + assert_equal("'Joe\\'s Crab Shack'", formatter.quote_value("Joe's Crab Shack")) + end +end diff --git a/test/query_comments_test.rb b/test/query_comments_test.rb index 3e17786..caf804c 100644 --- a/test/query_comments_test.rb +++ b/test/query_comments_test.rb @@ -363,19 +363,21 @@ def test_add_comments_to_beginning_of_query Marginalia::Comment.prepend_comment = nil end - def test_comment_key_value_separator - Marginalia.with_comment_settings(key_value_separator: '=') do - ActiveRecord::Base.connection.execute "select id from posts" - assert_equal 'select id from posts /*application=rails*/', @queries.first - end + def test_sqlcommenter_formatting + Marginalia::Comment.update_formatter!(:sqlcommenter) + ActiveRecord::Base.connection.execute "select id from posts" + assert_equal "select id from posts /*application='rails'*/", @queries.first + ensure + Marginalia::Comment.update_formatter!(:default) end - def test_comment_quote_values_single + def test_sqlcommenter_formatting_quotes Marginalia.application_name = "Joe's app" - Marginalia.with_comment_settings(quote_values: :single) do - ActiveRecord::Base.connection.execute "select id from posts" - assert_equal "select id from posts /*application:'Joe\\'s app'*/", @queries.first - end + Marginalia::Comment.update_formatter!(:sqlcommenter) + ActiveRecord::Base.connection.execute "select id from posts" + assert_equal "select id from posts /*application='Joe\\'s app'*/", @queries.first + ensure + Marginalia::Comment.update_formatter!(:default) end def teardown From b4d7863c2b9aeaa2ffca9c64e603ff793c925dc1 Mon Sep 17 00:00:00 2001 From: modulitos Date: Thu, 12 May 2022 23:50:11 -0700 Subject: [PATCH 4/4] update readme docs --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 17bc200..1e4ebd9 100644 --- a/README.md +++ b/README.md @@ -95,17 +95,17 @@ In order to not lose the marginalia comments from your logs, you can prepend the #### Comment formatting -Comment formatting can be configured to be more machine-readable by adding this command: `Marginalia::Comment.update_formatter!(:sqlcommenter)`. This will format Marginalia comments to be consistent with other SQL commenting projects, like [Google's sqlcommenter](https://google.github.io/sqlcommenter/). +Comment formatting can be configured to be more machine-readable by adding this command: `Marginalia::Comment.update_formatter!(:sqlcommenter)`. This will format Marginalia comments to be consistent with other SQL commenting projects, like [Google's sqlcommenter](https://google.github.io/sqlcommenter/). To revert back to the default formatter, run `Marginalia::Comment.update_formatter!(:default)`. -This format makes the following changes: +The `:sqlcommenter` formatter makes the following changes: 1. The key-value separator will use an equals sign (`=`) instead of a colon (`:`). 2. Values will be surrounded with single quotes (`'`) and escape internal quotes as `\'`. -For example, a SQL query like this: +For example, a default formatted SQL query that looks like this: `"select id from posts /*application:Joe's app,controller:my_controller*/"` -will be formatted like this: -`"select id from posts /*application='Joe\\'s app',controller='my_controller*/"` +will be formatted like this using `:sqlcommenter`: +`"select id from posts /*application='Joe\\'s app',controller='my_controller'*/"` #### Inline query annotations