Customizable key-value separator and quoting to be machine-readable

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/). To revert back to the default formatter, run `Marginalia::Comment.update_formatter!(:default)`.
+
+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 default formatted SQL query that looks 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
 
 In addition to the request or job-level component-based annotations,

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
 

lib/marginalia/comment.rb

@@ -1,12 +1,16 @@
 # frozen_string_literal: true
 
 require 'socket'
+require 'marginalia/formatter'
 
 module Marginalia
   module Comment
-    mattr_accessor :components, :lines_to_ignore, :prepend_comment
+    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
@@ -19,12 +23,17 @@ def self.update_adapter!(adapter)
       self.marginalia_adapter = adapter
     end
 
+    def self.update_formatter!(formatter = :default)
+      self.formatter = Marginalia::FormatterFactory.from_symbol(formatter)
+    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}#{self.formatter.key_value_separator}"\
+                 "#{self.formatter.quote_value(component_value)},"
         end
       end
       ret.chop!

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

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

test/query_comments_test.rb

@@ -363,6 +363,23 @@ def test_add_comments_to_beginning_of_query
     Marginalia::Comment.prepend_comment = nil
   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_sqlcommenter_formatting_quotes
+    Marginalia.application_name = "Joe's app"
+    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
     Marginalia.application_name = nil
     Marginalia::Comment.lines_to_ignore = nil