From fa86f1df60310dec03855eb1bd03b4c57de27224 Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Sun, 5 Jan 2025 10:17:09 +0900 Subject: [PATCH 01/10] rename legacy classes --- lib/arproxy.rb | 6 +++--- lib/arproxy/{chain_tail.rb => legacy_chain_tail.rb} | 2 +- ...dapter_patch.rb => legacy_connection_adapter_patch.rb} | 2 +- lib/arproxy/{proxy_chain.rb => legacy_proxy_chain.rb} | 8 ++++---- spec/unit/arproxy_spec.rb | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) rename lib/arproxy/{chain_tail.rb => legacy_chain_tail.rb} (83%) rename lib/arproxy/{connection_adapter_patch.rb => legacy_connection_adapter_patch.rb} (99%) rename lib/arproxy/{proxy_chain.rb => legacy_proxy_chain.rb} (83%) diff --git a/lib/arproxy.rb b/lib/arproxy.rb index a430ad5..937136e 100644 --- a/lib/arproxy.rb +++ b/lib/arproxy.rb @@ -1,7 +1,7 @@ require 'logger' require 'arproxy/base' require 'arproxy/config' -require 'arproxy/proxy_chain' +require 'arproxy/legacy_proxy_chain' require 'arproxy/error' require 'arproxy/plugin' @@ -31,8 +31,8 @@ def enable! raise Arproxy::Error, 'Arproxy has not been configured' end - @patch = ConnectionAdapterPatch.new(@config.adapter_class) - @proxy_chain = ProxyChain.new(@config, @patch) + @patch = LegacyConnectionAdapterPatch.new(@config.adapter_class) + @proxy_chain = LegacyProxyChain.new(@config, @patch) @proxy_chain.enable! @enabled = true diff --git a/lib/arproxy/chain_tail.rb b/lib/arproxy/legacy_chain_tail.rb similarity index 83% rename from lib/arproxy/chain_tail.rb rename to lib/arproxy/legacy_chain_tail.rb index 593ff5a..61db7e9 100644 --- a/lib/arproxy/chain_tail.rb +++ b/lib/arproxy/legacy_chain_tail.rb @@ -1,5 +1,5 @@ module Arproxy - class ChainTail < Base + class LegacyChainTail < Base def initialize(proxy_chain) self.proxy_chain = proxy_chain end diff --git a/lib/arproxy/connection_adapter_patch.rb b/lib/arproxy/legacy_connection_adapter_patch.rb similarity index 99% rename from lib/arproxy/connection_adapter_patch.rb rename to lib/arproxy/legacy_connection_adapter_patch.rb index f0d6270..276a46e 100644 --- a/lib/arproxy/connection_adapter_patch.rb +++ b/lib/arproxy/legacy_connection_adapter_patch.rb @@ -1,5 +1,5 @@ module Arproxy - class ConnectionAdapterPatch + class LegacyConnectionAdapterPatch attr_reader :adapter_class def initialize(adapter_class) diff --git a/lib/arproxy/proxy_chain.rb b/lib/arproxy/legacy_proxy_chain.rb similarity index 83% rename from lib/arproxy/proxy_chain.rb rename to lib/arproxy/legacy_proxy_chain.rb index a7b0462..55af852 100644 --- a/lib/arproxy/proxy_chain.rb +++ b/lib/arproxy/legacy_proxy_chain.rb @@ -1,8 +1,8 @@ -require_relative './chain_tail' -require_relative './connection_adapter_patch' +require_relative './legacy_chain_tail' +require_relative './legacy_connection_adapter_patch' module Arproxy - class ProxyChain + class LegacyProxyChain attr_reader :head, :tail, :patch def initialize(config, patch) @@ -12,7 +12,7 @@ def initialize(config, patch) end def setup - @tail = ChainTail.new self + @tail = LegacyChainTail.new self @head = @config.proxies.reverse.inject(@tail) do |next_proxy, proxy_config| cls, options = proxy_config proxy = cls.new(*options) diff --git a/spec/unit/arproxy_spec.rb b/spec/unit/arproxy_spec.rb index 62910d4..8916824 100644 --- a/spec/unit/arproxy_spec.rb +++ b/spec/unit/arproxy_spec.rb @@ -34,7 +34,7 @@ def execute2(sql, name = nil, binds = [], **kwargs) { sql: sql, name: name, binds: binds, kwargs: kwargs } end end - Arproxy::ConnectionAdapterPatch.register_patches('Dummy', patches: [:execute1], binds_patches: [:execute2]) + Arproxy::LegacyConnectionAdapterPatch.register_patches('Dummy', patches: [:execute1], binds_patches: [:execute2]) end end From b49fbee1b21e3267237cb10804983528276a635a Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Sun, 5 Jan 2025 11:11:31 +0900 Subject: [PATCH 02/10] Implement new proxy interface --- lib/arproxy/proxy.rb | 13 +++++++++++++ lib/arproxy/proxy_chain_head.rb | 27 ++++++++++++++++++++++++++ lib/arproxy/proxy_chain_tail.rb | 14 ++++++++++++++ lib/arproxy/query_context.rb | 17 +++++++++++++++++ spec/unit/v2_proxy_spec.rb | 34 +++++++++++++++++++++++++++++++++ 5 files changed, 105 insertions(+) create mode 100644 lib/arproxy/proxy.rb create mode 100644 lib/arproxy/proxy_chain_head.rb create mode 100644 lib/arproxy/proxy_chain_tail.rb create mode 100644 lib/arproxy/query_context.rb create mode 100644 spec/unit/v2_proxy_spec.rb diff --git a/lib/arproxy/proxy.rb b/lib/arproxy/proxy.rb new file mode 100644 index 0000000..0e13197 --- /dev/null +++ b/lib/arproxy/proxy.rb @@ -0,0 +1,13 @@ +module Arproxy + class Proxy + attr_reader :context, :next_proxy + + def initialize(next_proxy) + @next_proxy = next_proxy + end + + def execute(sql, context) + next_proxy.execute(sql, context) + end + end +end diff --git a/lib/arproxy/proxy_chain_head.rb b/lib/arproxy/proxy_chain_head.rb new file mode 100644 index 0000000..13eda25 --- /dev/null +++ b/lib/arproxy/proxy_chain_head.rb @@ -0,0 +1,27 @@ +require_relative './query_context' +require_relative './proxy' + +module Arproxy + class ProxyChainHead < Proxy + def execute_head_with_binds(execute_method_name, sql, name = nil, binds = [], **kwargs) + context = QueryContext.new( + execute_method_name: execute_method_name, + with_binds: true, + name: name, + binds: binds, + kwargs: kwargs, + ) + execute(sql, context) + end + + def execute_head(execute_method_name, sql, name = nil, **kwargs) + context = QueryContext.new( + execute_method_name: execute_method_name, + with_binds: false, + name: name, + kwargs: kwargs, + ) + execute(sql, context) + end + end +end diff --git a/lib/arproxy/proxy_chain_tail.rb b/lib/arproxy/proxy_chain_tail.rb new file mode 100644 index 0000000..ea78a65 --- /dev/null +++ b/lib/arproxy/proxy_chain_tail.rb @@ -0,0 +1,14 @@ +require_relative './proxy' + +module Arproxy + class ProxyChainTail < Proxy + def execute(sql, context) + raw_connection = next_proxy + if context.with_binds? + raw_connection.send(context.execute_method_name, sql, context.name, context.binds, **context.kwargs) + else + raw_connection.send(context.execute_method_name, sql, context.name, **context.kwargs) + end + end + end +end diff --git a/lib/arproxy/query_context.rb b/lib/arproxy/query_context.rb new file mode 100644 index 0000000..e52a1ee --- /dev/null +++ b/lib/arproxy/query_context.rb @@ -0,0 +1,17 @@ +module Arproxy + class QueryContext + attr_accessor :execute_method_name, :with_binds, :name, :binds, :kwargs + + def initialize(execute_method_name:, with_binds:, name: nil, binds: [], kwargs: {}) + @execute_method_name = execute_method_name + @with_binds = with_binds + @name = name + @binds = binds + @kwargs = kwargs + end + + def with_binds? + !!@with_binds + end + end +end diff --git a/spec/unit/v2_proxy_spec.rb b/spec/unit/v2_proxy_spec.rb new file mode 100644 index 0000000..121d858 --- /dev/null +++ b/spec/unit/v2_proxy_spec.rb @@ -0,0 +1,34 @@ +require_relative './spec_helper' +require 'arproxy/proxy_chain_head' +require 'arproxy/proxy_chain_tail' +require 'arproxy/proxy' + +describe Arproxy::Proxy do + it do + class DummyConnectionAdapter + def execute(sql, name = nil, binds = [], **kwargs) + "#{sql}" + end + end + + class Proxy1 < Arproxy::Proxy + def execute(sql, context) + super("#{sql} /* Proxy1 */", context) + end + end + + class Proxy2 < Arproxy::Proxy + def execute(sql, context) + super("#{sql} /* Proxy2 */", context) + end + end + + conn = DummyConnectionAdapter.new + tail = Arproxy::ProxyChainTail.new(conn) + p2 = Proxy2.new(tail) + p1 = Proxy1.new(p2) + head = Arproxy::ProxyChainHead.new(p1) + + expect(head.execute_head_with_binds('execute', 'SELECT 1', 'test', [1])).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + end +end From b7942d5e990f18b3a207ee00e0c2cdd8c2ff339c Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Sun, 5 Jan 2025 15:06:31 +0900 Subject: [PATCH 03/10] Remove legacy interface --- lib/arproxy.rb | 7 ++- lib/arproxy/base.rb | 6 +- lib/arproxy/config.rb | 13 +++++ ...r_patch.rb => connection_adapter_patch.rb} | 39 +++++-------- lib/arproxy/legacy_chain_tail.rb | 11 ---- lib/arproxy/proxy.rb | 6 +- .../{legacy_proxy_chain.rb => proxy_chain.rb} | 22 +++----- lib/arproxy/proxy_chain_head.rb | 6 +- lib/arproxy/proxy_chain_tail.rb | 5 +- lib/arproxy/query_context.rb | 5 +- .../shared_examples/custom_proxies.rb | 10 +++- spec/lib/arproxy/plugin/query_logger.rb | 6 +- spec/lib/arproxy/plugin/test_plugin.rb | 7 ++- spec/unit/arproxy_spec.rb | 55 ++++++++----------- spec/unit/{v2_proxy_spec.rb => proxy_spec.rb} | 15 +++-- 15 files changed, 99 insertions(+), 114 deletions(-) rename lib/arproxy/{legacy_connection_adapter_patch.rb => connection_adapter_patch.rb} (71%) delete mode 100644 lib/arproxy/legacy_chain_tail.rb rename lib/arproxy/{legacy_proxy_chain.rb => proxy_chain.rb} (54%) rename spec/unit/{v2_proxy_spec.rb => proxy_spec.rb} (65%) diff --git a/lib/arproxy.rb b/lib/arproxy.rb index 937136e..09c6aaf 100644 --- a/lib/arproxy.rb +++ b/lib/arproxy.rb @@ -1,7 +1,8 @@ require 'logger' require 'arproxy/base' require 'arproxy/config' -require 'arproxy/legacy_proxy_chain' +require 'arproxy/connection_adapter_patch' +require 'arproxy/proxy_chain' require 'arproxy/error' require 'arproxy/plugin' @@ -31,8 +32,8 @@ def enable! raise Arproxy::Error, 'Arproxy has not been configured' end - @patch = LegacyConnectionAdapterPatch.new(@config.adapter_class) - @proxy_chain = LegacyProxyChain.new(@config, @patch) + @patch = ConnectionAdapterPatch.new(@config.adapter_class) + @proxy_chain = ProxyChain.new(@config, @patch) @proxy_chain.enable! @enabled = true diff --git a/lib/arproxy/base.rb b/lib/arproxy/base.rb index d9336e1..a4d6439 100644 --- a/lib/arproxy/base.rb +++ b/lib/arproxy/base.rb @@ -1,9 +1,5 @@ module Arproxy + # This class is no longer used since Arproxy v2. class Base - attr_accessor :proxy_chain, :next_proxy - - def execute(sql, name=nil) - next_proxy.execute sql, name - end end end diff --git a/lib/arproxy/config.rb b/lib/arproxy/config.rb index cc0cee2..ca6d934 100644 --- a/lib/arproxy/config.rb +++ b/lib/arproxy/config.rb @@ -1,5 +1,7 @@ require 'active_record' require 'active_record/base' +require_relative './base' +require_relative './error' module Arproxy class Config @@ -14,12 +16,23 @@ def initialize end def use(proxy_class, *options) + if proxy_class.is_a?(Class) && proxy_class.ancestors.include?(Arproxy::Base) + # TODO: Add a migration guide URL + raise Arproxy::Error, "Error on loading a proxy `#{proxy_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v2. Use `Arproxy::Proxy` instead." + end + ::Arproxy.logger.debug("Arproxy: Mounting #{proxy_class.inspect} (#{options.inspect})") @proxies << [proxy_class, options] end def plugin(name, *options) plugin_class = Plugin.get(name) + + if plugin_class.is_a?(Class) && plugin_class.ancestors.include?(Arproxy::Base) + # TODO: Add a migration guide URL + raise Arproxy::Error, "Error on loading a plugin `#{plugin_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v2. Use `Arproxy::Proxy` instead." + end + use(plugin_class, *options) end diff --git a/lib/arproxy/legacy_connection_adapter_patch.rb b/lib/arproxy/connection_adapter_patch.rb similarity index 71% rename from lib/arproxy/legacy_connection_adapter_patch.rb rename to lib/arproxy/connection_adapter_patch.rb index 276a46e..dff09f5 100644 --- a/lib/arproxy/legacy_connection_adapter_patch.rb +++ b/lib/arproxy/connection_adapter_patch.rb @@ -1,5 +1,5 @@ module Arproxy - class LegacyConnectionAdapterPatch + class ConnectionAdapterPatch attr_reader :adapter_class def initialize(adapter_class) @@ -73,19 +73,14 @@ def disable! def apply_patch(target_method) return if @applied_patches.include?(target_method) adapter_class.class_eval do - break if instance_methods.include?(:"#{target_method}_with_arproxy") - define_method("#{target_method}_with_arproxy") do |sql, name=nil, **kwargs| - ::Arproxy.proxy_chain.connection = self - proxy_chain_result = ::Arproxy.proxy_chain.head.execute(sql, name) - if proxy_chain_result && proxy_chain_result.is_a?(Array) - _sql, _name = proxy_chain_result - self.send("#{target_method}_without_arproxy", _sql, _name, **kwargs) - else - nil - end + raw_execute_method_name = :"#{target_method}_without_arproxy" + patched_execute_method_name = :"#{target_method}_with_arproxy" + break if instance_methods.include?(patched_execute_method_name) + define_method(patched_execute_method_name) do |sql, name=nil, **kwargs| + ::Arproxy.proxy_chain.head.execute_head(self, raw_execute_method_name, sql, name, **kwargs) end - alias_method :"#{target_method}_without_arproxy", target_method - alias_method target_method, :"#{target_method}_with_arproxy" + alias_method raw_execute_method_name, target_method + alias_method target_method, patched_execute_method_name end @applied_patches << target_method end @@ -93,18 +88,14 @@ def apply_patch(target_method) def apply_patch_binds(target_method) return if @applied_patches.include?(target_method) adapter_class.class_eval do - define_method("#{target_method}_with_arproxy") do |sql, name=nil, binds=[], **kwargs| - ::Arproxy.proxy_chain.connection = self - proxy_chain_result = ::Arproxy.proxy_chain.head.execute(sql, name) - if proxy_chain_result && proxy_chain_result.is_a?(Array) - _sql, _name = proxy_chain_result - self.send("#{target_method}_without_arproxy", _sql, _name, binds, **kwargs) - else - nil - end + raw_execute_method_name = :"#{target_method}_without_arproxy" + patched_execute_method_name = :"#{target_method}_with_arproxy" + break if instance_methods.include?(patched_execute_method_name) + define_method(patched_execute_method_name) do |sql, name=nil, binds=[], **kwargs| + ::Arproxy.proxy_chain.head.execute_head_with_binds(self, raw_execute_method_name, sql, name, binds, **kwargs) end - alias_method :"#{target_method}_without_arproxy", target_method - alias_method target_method, :"#{target_method}_with_arproxy" + alias_method raw_execute_method_name, target_method + alias_method target_method, patched_execute_method_name end @applied_patches << target_method end diff --git a/lib/arproxy/legacy_chain_tail.rb b/lib/arproxy/legacy_chain_tail.rb deleted file mode 100644 index 61db7e9..0000000 --- a/lib/arproxy/legacy_chain_tail.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Arproxy - class LegacyChainTail < Base - def initialize(proxy_chain) - self.proxy_chain = proxy_chain - end - - def execute(sql, name=nil) - [sql, name] - end - end -end diff --git a/lib/arproxy/proxy.rb b/lib/arproxy/proxy.rb index 0e13197..f2310e5 100644 --- a/lib/arproxy/proxy.rb +++ b/lib/arproxy/proxy.rb @@ -1,10 +1,6 @@ module Arproxy class Proxy - attr_reader :context, :next_proxy - - def initialize(next_proxy) - @next_proxy = next_proxy - end + attr_accessor :context, :next_proxy def execute(sql, context) next_proxy.execute(sql, context) diff --git a/lib/arproxy/legacy_proxy_chain.rb b/lib/arproxy/proxy_chain.rb similarity index 54% rename from lib/arproxy/legacy_proxy_chain.rb rename to lib/arproxy/proxy_chain.rb index 55af852..de1f0ea 100644 --- a/lib/arproxy/legacy_proxy_chain.rb +++ b/lib/arproxy/proxy_chain.rb @@ -1,8 +1,9 @@ -require_relative './legacy_chain_tail' -require_relative './legacy_connection_adapter_patch' +require_relative './proxy_chain_head' +require_relative './proxy_chain_tail' +require_relative './connection_adapter_patch' module Arproxy - class LegacyProxyChain + class ProxyChain attr_reader :head, :tail, :patch def initialize(config, patch) @@ -12,14 +13,15 @@ def initialize(config, patch) end def setup - @tail = LegacyChainTail.new self - @head = @config.proxies.reverse.inject(@tail) do |next_proxy, proxy_config| + @tail = ProxyChainTail.new + head = @config.proxies.reverse.inject(@tail) do |next_proxy, proxy_config| cls, options = proxy_config proxy = cls.new(*options) - proxy.proxy_chain = self proxy.next_proxy = next_proxy proxy end + @head = ProxyChainHead.new + @head.next_proxy = head end private :setup @@ -36,13 +38,5 @@ def enable! def disable! @patch.disable! end - - def connection - Thread.current[:arproxy_connection] - end - - def connection=(val) - Thread.current[:arproxy_connection] = val - end end end diff --git a/lib/arproxy/proxy_chain_head.rb b/lib/arproxy/proxy_chain_head.rb index 13eda25..6cca25d 100644 --- a/lib/arproxy/proxy_chain_head.rb +++ b/lib/arproxy/proxy_chain_head.rb @@ -3,8 +3,9 @@ module Arproxy class ProxyChainHead < Proxy - def execute_head_with_binds(execute_method_name, sql, name = nil, binds = [], **kwargs) + def execute_head_with_binds(raw_connection, execute_method_name, sql, name = nil, binds = [], **kwargs) context = QueryContext.new( + raw_connection: raw_connection, execute_method_name: execute_method_name, with_binds: true, name: name, @@ -14,8 +15,9 @@ def execute_head_with_binds(execute_method_name, sql, name = nil, binds = [], ** execute(sql, context) end - def execute_head(execute_method_name, sql, name = nil, **kwargs) + def execute_head(raw_connection, execute_method_name, sql, name = nil, **kwargs) context = QueryContext.new( + raw_connection: raw_connection, execute_method_name: execute_method_name, with_binds: false, name: name, diff --git a/lib/arproxy/proxy_chain_tail.rb b/lib/arproxy/proxy_chain_tail.rb index ea78a65..3baeeac 100644 --- a/lib/arproxy/proxy_chain_tail.rb +++ b/lib/arproxy/proxy_chain_tail.rb @@ -3,11 +3,10 @@ module Arproxy class ProxyChainTail < Proxy def execute(sql, context) - raw_connection = next_proxy if context.with_binds? - raw_connection.send(context.execute_method_name, sql, context.name, context.binds, **context.kwargs) + context.raw_connection.send(context.execute_method_name, sql, context.name, context.binds, **context.kwargs) else - raw_connection.send(context.execute_method_name, sql, context.name, **context.kwargs) + context.raw_connection.send(context.execute_method_name, sql, context.name, **context.kwargs) end end end diff --git a/lib/arproxy/query_context.rb b/lib/arproxy/query_context.rb index e52a1ee..bf4b8a8 100644 --- a/lib/arproxy/query_context.rb +++ b/lib/arproxy/query_context.rb @@ -1,8 +1,9 @@ module Arproxy class QueryContext - attr_accessor :execute_method_name, :with_binds, :name, :binds, :kwargs + attr_accessor :raw_connection, :execute_method_name, :with_binds, :name, :binds, :kwargs - def initialize(execute_method_name:, with_binds:, name: nil, binds: [], kwargs: {}) + def initialize(raw_connection:, execute_method_name:, with_binds:, name: nil, binds: [], kwargs: {}) + @raw_connection = raw_connection @execute_method_name = execute_method_name @with_binds = with_binds @name = name diff --git a/spec/integration/shared_examples/custom_proxies.rb b/spec/integration/shared_examples/custom_proxies.rb index 91e33ae..9869df3 100644 --- a/spec/integration/shared_examples/custom_proxies.rb +++ b/spec/integration/shared_examples/custom_proxies.rb @@ -1,9 +1,15 @@ class Product < ActiveRecord::Base end -class HelloProxy < Arproxy::Base +class HelloLegacyProxy < Arproxy::Base def execute(sql, name = nil) - super("#{sql} -- Hello Arproxy!", name) + super("#{sql} -- Hello Legacy Arproxy!", name) + end +end + +class HelloProxy < Arproxy::Proxy + def execute(sql, context) + super("#{sql} -- Hello Arproxy!", context) end end diff --git a/spec/lib/arproxy/plugin/query_logger.rb b/spec/lib/arproxy/plugin/query_logger.rb index 11d4a07..4526549 100644 --- a/spec/lib/arproxy/plugin/query_logger.rb +++ b/spec/lib/arproxy/plugin/query_logger.rb @@ -1,13 +1,13 @@ require 'arproxy/plugin' -class QueryLogger < Arproxy::Base +class QueryLogger < Arproxy::Proxy Arproxy::Plugin.register(:query_logger, self) - def execute(sql, name = nil) + def execute(sql, context) @@log ||= [] @@log << sql if ENV['DEBUG'] - puts "QueryLogger: [#{name}] #{sql}" + puts "QueryLogger: [#{context.name}] #{sql}" end super end diff --git a/spec/lib/arproxy/plugin/test_plugin.rb b/spec/lib/arproxy/plugin/test_plugin.rb index 7cef06e..538d0a3 100644 --- a/spec/lib/arproxy/plugin/test_plugin.rb +++ b/spec/lib/arproxy/plugin/test_plugin.rb @@ -1,13 +1,14 @@ module Arproxy::Plugin - class TestPlugin < Arproxy::Base + class TestPlugin < Arproxy::Proxy Arproxy::Plugin.register(:test_plugin, self) def initialize(*options) @options = options end - def execute(sql, name=nil) - super("#{sql} /* options: #{@options.inspect} */", "#{name}_PLUGIN") + def execute(sql, context) + context.name = "#{context.name}_PLUGIN" + super("#{sql} /* options: #{@options.inspect} */", context) end end end diff --git a/spec/unit/arproxy_spec.rb b/spec/unit/arproxy_spec.rb index 8916824..aaac781 100644 --- a/spec/unit/arproxy_spec.rb +++ b/spec/unit/arproxy_spec.rb @@ -5,13 +5,13 @@ allow(Arproxy).to receive(:logger) { Logger.new('/dev/null') } end - class ProxyA < Arproxy::Base + class LegacyProxyA < Arproxy::Base def execute(sql, name) super "#{sql}_A", "#{name}_A" end end - class ProxyB < Arproxy::Base + class LegacyProxyB < Arproxy::Base def initialize(opt=nil) @opt = opt end @@ -21,6 +21,24 @@ def execute(sql, name) end end + class ProxyA < Arproxy::Proxy + def execute(sql, context) + context.name = "#{context.name}_A" + super "#{sql}_A", context + end + end + + class ProxyB < Arproxy::Proxy + def initialize(opt=nil) + @opt = opt + end + + def execute(sql, context) + context.name = "#{context.name}_B#{@opt}" + super "#{sql}_B#{@opt}", context + end + end + module ::ActiveRecord module ConnectionAdapters class DummyAdapter @@ -34,7 +52,7 @@ def execute2(sql, name = nil, binds = [], **kwargs) { sql: sql, name: name, binds: binds, kwargs: kwargs } end end - Arproxy::LegacyConnectionAdapterPatch.register_patches('Dummy', patches: [:execute1], binds_patches: [:execute2]) + Arproxy::ConnectionAdapterPatch.register_patches('Dummy', patches: [:execute1], binds_patches: [:execute2]) end end @@ -96,10 +114,10 @@ def execute2(sql, name = nil, binds = [], **kwargs) end context 'with a proxy that returns nil' do - class ReadonlyAccess < Arproxy::Base - def execute(sql, name) + class ReadonlyAccess < Arproxy::Proxy + def execute(sql, context) if sql =~ /^(SELECT)\b/ - super sql, name + super sql, context else nil end @@ -204,29 +222,4 @@ def execute(sql, name) ) end end - - context 'ProxyChain thread-safety' do - class ProxyWithConnectionId < Arproxy::Base - def execute(sql, name) - sleep 0.1 - super "#{sql} /* connection_id=#{self.proxy_chain.connection.object_id} */", name - end - end - - before do - Arproxy.clear_configuration - Arproxy.configure do |config| - config.adapter = 'dummy' - config.use ProxyWithConnectionId - end - Arproxy.enable! - end - - context 'with two threads' do - let!(:thr1) { Thread.new { connection.dup.execute1 'SELECT 1' } } - let!(:thr2) { Thread.new { connection.dup.execute1 'SELECT 1' } } - - it { expect(thr1.value).not_to eq(thr2.value) } - end - end end diff --git a/spec/unit/v2_proxy_spec.rb b/spec/unit/proxy_spec.rb similarity index 65% rename from spec/unit/v2_proxy_spec.rb rename to spec/unit/proxy_spec.rb index 121d858..6725826 100644 --- a/spec/unit/v2_proxy_spec.rb +++ b/spec/unit/proxy_spec.rb @@ -23,12 +23,15 @@ def execute(sql, context) end end - conn = DummyConnectionAdapter.new - tail = Arproxy::ProxyChainTail.new(conn) - p2 = Proxy2.new(tail) - p1 = Proxy1.new(p2) - head = Arproxy::ProxyChainHead.new(p1) + tail = Arproxy::ProxyChainTail.new + p2 = Proxy2.new + p2.next_proxy = tail + p1 = Proxy1.new + p1.next_proxy = p2 + head = Arproxy::ProxyChainHead.new + head.next_proxy = p1 - expect(head.execute_head_with_binds('execute', 'SELECT 1', 'test', [1])).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + conn = DummyConnectionAdapter.new + expect(head.execute_head_with_binds(conn, 'execute', 'SELECT 1', 'test', [1])).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') end end From 323c58990bf659656604388db9181201c8100aa0 Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Sun, 5 Jan 2025 15:52:48 +0900 Subject: [PATCH 04/10] Add test cases for depracation errors --- lib/arproxy/proxy.rb | 6 ++ lib/arproxy/proxy_chain_tail.rb | 5 ++ spec/lib/arproxy/plugin/legacy_plugin.rb | 9 +++ spec/unit/arproxy_spec.rb | 59 +++++++++++++++++++ ...proxy_spec.rb => proxy_chain_head_spec.rb} | 23 ++++++-- 5 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 spec/lib/arproxy/plugin/legacy_plugin.rb rename spec/unit/{proxy_spec.rb => proxy_chain_head_spec.rb} (55%) diff --git a/lib/arproxy/proxy.rb b/lib/arproxy/proxy.rb index f2310e5..0c70e01 100644 --- a/lib/arproxy/proxy.rb +++ b/lib/arproxy/proxy.rb @@ -1,8 +1,14 @@ +require 'arproxy/query_context' + module Arproxy class Proxy attr_accessor :context, :next_proxy def execute(sql, context) + unless context.instance_of?(QueryContext) + raise Arproxy::Error, "`context` is expected a `Arproxy::QueryContext` but got `#{context.class}`" + end + next_proxy.execute(sql, context) end end diff --git a/lib/arproxy/proxy_chain_tail.rb b/lib/arproxy/proxy_chain_tail.rb index 3baeeac..31a5c6a 100644 --- a/lib/arproxy/proxy_chain_tail.rb +++ b/lib/arproxy/proxy_chain_tail.rb @@ -1,8 +1,13 @@ require_relative './proxy' +require_relative './query_context' module Arproxy class ProxyChainTail < Proxy def execute(sql, context) + unless context.instance_of?(QueryContext) + raise Arproxy::Error, "`context` is expected a `Arproxy::QueryContext` but got `#{context.class}`" + end + if context.with_binds? context.raw_connection.send(context.execute_method_name, sql, context.name, context.binds, **context.kwargs) else diff --git a/spec/lib/arproxy/plugin/legacy_plugin.rb b/spec/lib/arproxy/plugin/legacy_plugin.rb new file mode 100644 index 0000000..02f769f --- /dev/null +++ b/spec/lib/arproxy/plugin/legacy_plugin.rb @@ -0,0 +1,9 @@ +module Arproxy::Plugin + class LegacyPlugin < Arproxy::Base + Arproxy::Plugin.register(:legacy_plugin, self) + + def execute(sql, name) + super("#{sql} /* legacy_plugin */", name) + end + end +end diff --git a/spec/unit/arproxy_spec.rb b/spec/unit/arproxy_spec.rb index aaac781..bb2db20 100644 --- a/spec/unit/arproxy_spec.rb +++ b/spec/unit/arproxy_spec.rb @@ -137,6 +137,50 @@ def execute(sql, context) it { expect(connection.execute1('UPDATE foo SET bar = 1', 'NAME')).to eq(nil) } end + context 'with a legacy proxy' do + class LegacyProxy < Arproxy::Base + def execute(sql, name) + super("#{sql} /* legacy_proxy */", name) + end + end + + before do + Arproxy.clear_configuration + end + + it 'raises an error' do + expect { + Arproxy.configure do |config| + config.adapter = 'dummy' + config.use LegacyProxy + end + }.to raise_error(Arproxy::Error, /Use `Arproxy::Proxy` instead/) + end + end + + context 'calls #execute with an String argument instead of `context`' do + class WrongProxy < Arproxy::Proxy + def execute(sql, context) + super("#{sql} /* my_proxy */", "name=#{context.name}") + end + end + + before do + Arproxy.clear_configuration + Arproxy.configure do |config| + config.adapter = 'dummy' + config.use WrongProxy + end + Arproxy.enable! + end + + it do + expect { + connection.execute1('SQL', 'NAME') + }.to raise_error(Arproxy::Error, /expected a `Arproxy::QueryContext`/) + end + end + context do before do Arproxy.clear_configuration @@ -222,4 +266,19 @@ def execute(sql, context) ) end end + + context 'use a legacy plugin' do + before do + Arproxy.clear_configuration + end + + it 'raises an error' do + expect { + Arproxy.configure do |config| + config.adapter = 'dummy' + config.plugin :legacy_plugin + end + }.to raise_error(Arproxy::Error, /Use `Arproxy::Proxy` instead/) + end + end end diff --git a/spec/unit/proxy_spec.rb b/spec/unit/proxy_chain_head_spec.rb similarity index 55% rename from spec/unit/proxy_spec.rb rename to spec/unit/proxy_chain_head_spec.rb index 6725826..329ff6e 100644 --- a/spec/unit/proxy_spec.rb +++ b/spec/unit/proxy_chain_head_spec.rb @@ -3,8 +3,8 @@ require 'arproxy/proxy_chain_tail' require 'arproxy/proxy' -describe Arproxy::Proxy do - it do +describe Arproxy::ProxyChainHead do + before(:all) do class DummyConnectionAdapter def execute(sql, name = nil, binds = [], **kwargs) "#{sql}" @@ -28,10 +28,21 @@ def execute(sql, context) p2.next_proxy = tail p1 = Proxy1.new p1.next_proxy = p2 - head = Arproxy::ProxyChainHead.new - head.next_proxy = p1 + @head = Arproxy::ProxyChainHead.new + @head.next_proxy = p1 - conn = DummyConnectionAdapter.new - expect(head.execute_head_with_binds(conn, 'execute', 'SELECT 1', 'test', [1])).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + @conn = DummyConnectionAdapter.new + end + + describe '#execute_head_with_binds' do + it do + expect(@head.execute_head_with_binds(@conn, 'execute', 'SELECT 1', 'test', [1])).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + end + end + + describe '#execute_head' do + it do + expect(@head.execute_head(@conn, 'execute', 'SELECT 1', 'test')).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + end end end From 6f81bfa06053665dd147322cf50ae15098146e07 Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Sun, 5 Jan 2025 16:30:33 +0900 Subject: [PATCH 05/10] s/v2/v1/ --- lib/arproxy/base.rb | 2 +- lib/arproxy/config.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/arproxy/base.rb b/lib/arproxy/base.rb index a4d6439..556162a 100644 --- a/lib/arproxy/base.rb +++ b/lib/arproxy/base.rb @@ -1,5 +1,5 @@ module Arproxy - # This class is no longer used since Arproxy v2. + # This class is no longer used since Arproxy v1. class Base end end diff --git a/lib/arproxy/config.rb b/lib/arproxy/config.rb index ca6d934..b81ca3e 100644 --- a/lib/arproxy/config.rb +++ b/lib/arproxy/config.rb @@ -18,7 +18,7 @@ def initialize def use(proxy_class, *options) if proxy_class.is_a?(Class) && proxy_class.ancestors.include?(Arproxy::Base) # TODO: Add a migration guide URL - raise Arproxy::Error, "Error on loading a proxy `#{proxy_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v2. Use `Arproxy::Proxy` instead." + raise Arproxy::Error, "Error on loading a proxy `#{proxy_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v1. Use `Arproxy::Proxy` instead." end ::Arproxy.logger.debug("Arproxy: Mounting #{proxy_class.inspect} (#{options.inspect})") @@ -30,7 +30,7 @@ def plugin(name, *options) if plugin_class.is_a?(Class) && plugin_class.ancestors.include?(Arproxy::Base) # TODO: Add a migration guide URL - raise Arproxy::Error, "Error on loading a plugin `#{plugin_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v2. Use `Arproxy::Proxy` instead." + raise Arproxy::Error, "Error on loading a plugin `#{plugin_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v1. Use `Arproxy::Proxy` instead." end use(plugin_class, *options) From f7873f4aabd45079b249bf20229bf38da420cca8 Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Sun, 5 Jan 2025 16:31:06 +0900 Subject: [PATCH 06/10] Update README --- README.md | 57 +++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index a7d401d..814b59a 100644 --- a/README.md +++ b/README.md @@ -9,11 +9,11 @@ Arproxy is a library that can intercept SQL queries executed by ActiveRecord to Create your custom proxy and add its configuration in your Rails' `config/initializers/` directory: ```ruby -class QueryTracer < Arproxy::Base - def execute(sql, name=nil) +class QueryTracer < Arproxy::Proxy + def execute(sql, context) Rails.logger.debug sql Rails.logger.debug caller(1).join("\n") - super(sql, name) + super(sql, context) end end @@ -93,12 +93,48 @@ We have tested with the following versions of Ruby, ActiveRecord, and databases: # Examples +## Slow Query Logger +```ruby +class SlowQueryLogger < Arproxy::Proxy + def initialize(slow_ms) + @slow_ms = slow_ms + end + + def execute(sql, context) + result = nil + ms = Benchmark.ms { result = super(sql, context) } + if ms >= @slow_ms + Rails.logger.info "Slow(#{ms.to_i}ms): #{sql}" + end + result + end +end + +Arproxy.configure do |config| + config.use SlowQueryLogger, 1000 +end +``` + ## Adding Comments to SQLs ```ruby -class CommentAdder < Arproxy::Base - def execute(sql, name=nil) +class CommentAdder < Arproxy::Proxy + def execute(sql, context) sql += ' /*this_is_comment*/' - super(sql, name) + super(sql, context) + end +end +``` + +## Readonly Access +```ruby +class Readonly < Arproxy::Proxy + def execute(sql, context) + if sql =~ /^(SELECT|SET|SHOW|DESCRIBE)\b/ + super(sql, context) + else + Rails.logger.warn "#{context.name} (BLOCKED) #{sql}" + nil # return nil to block the query + end end end ``` @@ -108,11 +144,12 @@ end ```ruby # any_gem/lib/arproxy/plugin/my_plugin module Arproxy::Plugin - class MyPlugin < Arproxy::Base + class MyPlugin < Arproxy::Proxy Arproxy::Plugin.register(:my_plugin, self) - def execute(sql, name=nil) + def execute(sql, context) # Any processing + super(sql, context) end end end @@ -145,8 +182,8 @@ class MyProxy < Arproxy::Base end # >= v1.0.0 -class MyProxy < Arproxy::Base - def execute(sql, name=nil) +class MyProxy < Arproxy::Proxy + def execute(sql, context) super end end From 62e7e9e8ea2f68963e2f0f6dfaf5986a6f10d052 Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Sun, 5 Jan 2025 17:35:57 +0900 Subject: [PATCH 07/10] Update README --- README.md | 137 ++++++++++++++++++++---------------------------------- 1 file changed, 51 insertions(+), 86 deletions(-) diff --git a/README.md b/README.md index 814b59a..f6eb841 100644 --- a/README.md +++ b/README.md @@ -31,12 +31,40 @@ Then you can see the backtrace of SQLs in the Rails' log. MyTable.where(id: id).limit(1) # => The SQL and the backtrace appear in the log ``` -## What the `name' argument is +## What the `context` argument is + +`context` is an instance of `Arproxy::QueryContext` and contains values that are passed from Arproxy to the Database Adapter. +`context` is a set of values used when calling Database Adapter methods, and you don't need to use the `context` values directly. +However, you must always pass `context` to `super` like `super(sql, context)`. + +For example, let's look at the Mysql2Adapter implementation. When executing a query in Mysql2Adapter, the `Mysql2Adapter#internal_exec_query` method is called internally. + +``` +# https://github.com/rails/rails/blob/v7.1.0/activerecord/lib/active_record/connection_adapters/mysql2/database_statements.rb#L21 +def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc: + # ... +end +``` + +In Arproxy, this method is called at the end of the `Arproxy::Proxy#execute` method chain, and at this time `context` contains the arguments to be passed to `#internal_exec_query`: + +| member | example value | +|------------------|------------------------------------| +| `context.name` | `"SQL"` | +| `context.binds` | `[]` | +| `context.kwargs` | `{ prepare: false, async: false }` | + +You can modify the values of `context` in the proxy, but do so after understanding the implementation of the Database Adapter. + +### `context.name` + In the Rails' log you may see queries like this: + ``` User Load (22.6ms) SELECT `users`.* FROM `users` WHERE `users`.`name` = 'Issei Naruta' ``` -Then `"User Load"` is the `name`. + +Then `"User Load"` is the `context.name`. # Architecture Without Arproxy: @@ -93,7 +121,19 @@ We have tested with the following versions of Ruby, ActiveRecord, and databases: # Examples +## Adding Comments to SQLs + +```ruby +class CommentAdder < Arproxy::Proxy + def execute(sql, context) + sql += ' /*this_is_comment*/' + super(sql, context) + end +end +``` + ## Slow Query Logger + ```ruby class SlowQueryLogger < Arproxy::Proxy def initialize(slow_ms) @@ -115,17 +155,10 @@ Arproxy.configure do |config| end ``` -## Adding Comments to SQLs -```ruby -class CommentAdder < Arproxy::Proxy - def execute(sql, context) - sql += ' /*this_is_comment*/' - super(sql, context) - end -end -``` - ## Readonly Access + +If you don't call `super` in the proxy, you can block the query execution. + ```ruby class Readonly < Arproxy::Proxy def execute(sql, context) @@ -133,7 +166,7 @@ class Readonly < Arproxy::Proxy super(sql, context) else Rails.logger.warn "#{context.name} (BLOCKED) #{sql}" - nil # return nil to block the query + nil end end end @@ -149,6 +182,7 @@ module Arproxy::Plugin def execute(sql, context) # Any processing + # ... super(sql, context) end end @@ -163,15 +197,9 @@ end # Upgrading guide from v0.x to v1 -There are several incompatible changes from Arproxy v0.x to v1. -In most cases, existing configurations can be used as-is in v1, but there are some exceptions. -The specification of custom proxies (classes inheriting from Arproxy::Base) has changed as follows: - -## 1. Removal of keyword arguments (kwargs) - -In v0.2.9, `**kwargs` was added to the arguments of the `#execute` method ([#21](https://github.com/cookpad/arproxy/pull/21)), but this argument has been removed in v1. - -These `kwargs` were removed in v1 because their specifications differed depending on the Connection Adapter of each database. +The proxy specification has changed from v0.x to v1 and is not backward compatible. +The base class for proxies has changed from `Arproxy::Base` to `Arproxy::Proxy`. +Also, the arguments to `#execute` have changed from `sql, name=nil, **kwargs` to `sql, context`. ```ruby # ~> v0.2.9 @@ -189,70 +217,7 @@ class MyProxy < Arproxy::Proxy end ``` -## 2. `Arproxy::Base#execute` (`super`) no longer executes queries - -In v0.x, the `Arproxy::Base#execute` method was a method to execute a query on the Database Adapter. -That is, when `super` is called in the `#execute` method of a custom proxy, a query is executed on the Database Adapter at the end of the proxy chain. - -In v1, the `Arproxy::Base#execute` method does not execute a query. The query is executed outside the `#execute` method after the proxy chain of `#execute` is complete. - -This change was necessary to support various Database Adapters while maintaining backward compatibility with custom proxies as much as possible. - -However, this change has the following incompatibilities: - -- The return value of `super` cannot be used. -- The query execution time cannot be measured. - -### 2.1. The return value of `super` cannot be used - -In v0.x, the return value of `super` was the result of actually executing a query on the Database Adapter. -For example, if you are using the `mysql2` Adapter, the return value of `super` was a `Mysql2::Result` object. - -In v1, the return value of `super` is a value used internally by Arproxy's proxy chain instead of the result of actually executing a query on the Database Adapter. -You still need to return the return value of `super` in the `#execute` method of your custom proxy. -However, the `Arproxy::Base` in v1 does not expect to use this return value in the custom proxy. - -If your custom proxy expects the return value of `super` to be an object representing the query result, you need to be careful because it is not available in v1. - -```ruby -class MyProxy < Arproxy::Base - def execute(sql, name=nil) - result = super(sql, name) - # ... - # In v0.x, the return value of the Database Adapter such as Mysql2::Result was stored, - # but in v1, the value used internally by Arproxy's proxy chain is stored. - result - end -end -``` - -### 2.2. The query execution time cannot be measured - -For example, even if you write code to measure the execution time of `super`, it no longer means the query execution time. - -```ruby -class MyProxy < Arproxy::Base - def execute(sql, name=nil) - t = Time.now - result = super(sql, name) - # This code no longer means the query execution time - Rails.logger.info "Slow(#{Time.now - t}ms): #{sql}" - result - end -end -``` - -# Discussion - -The specification changes in v1 have allowed more Database Adapters to be supported and made Arproxy more resistant to changes in ActiveRecord's internal structure. -However, as described in the previous section, there are cases where the custom proxies written in v0.x will no longer work. - -We do not know the use cases of Arproxy users other than ourselves very well, so we are soliciting opinions on the changes in this time. -If there are many requests, we will prepare a new base class for custom proxies with a different interface from `Arproxy::Base`, so that custom proxy writing similar to that in v0.x can be done. - -For this issue, we are collecting opinions on the following discussion: - -[Calling for opinions: incompatibility between v0.x and v1 · cookpad/arproxy · Discussion #33](https://github.com/cookpad/arproxy/discussions/33) +There are no other backward incompatible changes besides the above changes in proxy base class and arguments. # Development From 31d8af176ef69482fd25bc0dd58277ddcf5232f8 Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Wed, 22 Jan 2025 19:09:58 +0900 Subject: [PATCH 08/10] Add UPGRADING.md --- README.md | 22 +--------------------- UPGRADING.md | 23 +++++++++++++++++++++++ lib/arproxy/config.rb | 6 ++---- 3 files changed, 26 insertions(+), 25 deletions(-) create mode 100644 UPGRADING.md diff --git a/README.md b/README.md index f6eb841..9d69c57 100644 --- a/README.md +++ b/README.md @@ -197,27 +197,7 @@ end # Upgrading guide from v0.x to v1 -The proxy specification has changed from v0.x to v1 and is not backward compatible. -The base class for proxies has changed from `Arproxy::Base` to `Arproxy::Proxy`. -Also, the arguments to `#execute` have changed from `sql, name=nil, **kwargs` to `sql, context`. - -```ruby -# ~> v0.2.9 -class MyProxy < Arproxy::Base - def execute(sql, name=nil, **kwargs) - super - end -end - -# >= v1.0.0 -class MyProxy < Arproxy::Proxy - def execute(sql, context) - super - end -end -``` - -There are no other backward incompatible changes besides the above changes in proxy base class and arguments. +See [UPGRADING.md](https://github.com/cookpad/arproxy/main/) # Development diff --git a/UPGRADING.md b/UPGRADING.md new file mode 100644 index 0000000..d05cf04 --- /dev/null +++ b/UPGRADING.md @@ -0,0 +1,23 @@ +# Upgrading guide from v0.x to v1 + +The proxy specification has changed from v0.x to v1 and is not backward compatible. +The base class for proxies has changed from `Arproxy::Base` to `Arproxy::Proxy`. +Also, the arguments to `#execute` have changed from `sql, name=nil, **kwargs` to `sql, context`. + +```ruby +# ~> v0.2.9 +class MyProxy < Arproxy::Base + def execute(sql, name=nil, **kwargs) + super + end +end + +# >= v1.0.0 +class MyProxy < Arproxy::Proxy + def execute(sql, context) + super + end +end +``` + +There are no other backward incompatible changes besides the above changes in proxy base class and arguments. diff --git a/lib/arproxy/config.rb b/lib/arproxy/config.rb index b81ca3e..681ba7a 100644 --- a/lib/arproxy/config.rb +++ b/lib/arproxy/config.rb @@ -17,8 +17,7 @@ def initialize def use(proxy_class, *options) if proxy_class.is_a?(Class) && proxy_class.ancestors.include?(Arproxy::Base) - # TODO: Add a migration guide URL - raise Arproxy::Error, "Error on loading a proxy `#{proxy_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v1. Use `Arproxy::Proxy` instead." + raise Arproxy::Error, "Error on loading a proxy `#{proxy_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v1. Use `Arproxy::Proxy` instead. See: https://github.com/cookpad/arproxy/blob/main/UPGRADING.md" end ::Arproxy.logger.debug("Arproxy: Mounting #{proxy_class.inspect} (#{options.inspect})") @@ -29,8 +28,7 @@ def plugin(name, *options) plugin_class = Plugin.get(name) if plugin_class.is_a?(Class) && plugin_class.ancestors.include?(Arproxy::Base) - # TODO: Add a migration guide URL - raise Arproxy::Error, "Error on loading a plugin `#{plugin_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v1. Use `Arproxy::Proxy` instead." + raise Arproxy::Error, "Error on loading a plugin `#{plugin_class.inspect}`: the superclass `Arproxy::Base` is no longer supported since Arproxy v1. Use `Arproxy::Proxy` instead. See: https://github.com/cookpad/arproxy/blob/main/UPGRADING.md" end use(plugin_class, *options) From 7330e4778230e0532fb5bf2b992c7e2c869b426d Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Wed, 22 Jan 2025 19:15:19 +0900 Subject: [PATCH 09/10] Use `require 'arproxy/*'` instead of require_relative --- lib/arproxy/config.rb | 4 ++-- lib/arproxy/proxy_chain.rb | 6 +++--- lib/arproxy/proxy_chain_head.rb | 4 ++-- lib/arproxy/proxy_chain_tail.rb | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/arproxy/config.rb b/lib/arproxy/config.rb index 681ba7a..1445079 100644 --- a/lib/arproxy/config.rb +++ b/lib/arproxy/config.rb @@ -1,7 +1,7 @@ require 'active_record' require 'active_record/base' -require_relative './base' -require_relative './error' +require 'arproxy/base' +require 'arproxy/error' module Arproxy class Config diff --git a/lib/arproxy/proxy_chain.rb b/lib/arproxy/proxy_chain.rb index de1f0ea..04d728b 100644 --- a/lib/arproxy/proxy_chain.rb +++ b/lib/arproxy/proxy_chain.rb @@ -1,6 +1,6 @@ -require_relative './proxy_chain_head' -require_relative './proxy_chain_tail' -require_relative './connection_adapter_patch' +require 'arproxy/proxy_chain_head' +require 'arproxy/proxy_chain_tail' +require 'arproxy/connection_adapter_patch' module Arproxy class ProxyChain diff --git a/lib/arproxy/proxy_chain_head.rb b/lib/arproxy/proxy_chain_head.rb index 6cca25d..93e420d 100644 --- a/lib/arproxy/proxy_chain_head.rb +++ b/lib/arproxy/proxy_chain_head.rb @@ -1,5 +1,5 @@ -require_relative './query_context' -require_relative './proxy' +require 'arproxy/query_context' +require 'arproxy/proxy' module Arproxy class ProxyChainHead < Proxy diff --git a/lib/arproxy/proxy_chain_tail.rb b/lib/arproxy/proxy_chain_tail.rb index 31a5c6a..1541f99 100644 --- a/lib/arproxy/proxy_chain_tail.rb +++ b/lib/arproxy/proxy_chain_tail.rb @@ -1,5 +1,5 @@ -require_relative './proxy' -require_relative './query_context' +require 'arproxy/proxy' +require 'arproxy/query_context' module Arproxy class ProxyChainTail < Proxy From 2476d5fa3029cd928fdecd27181edca7fdc81800 Mon Sep 17 00:00:00 2001 From: Issei Naruta Date: Wed, 22 Jan 2025 19:29:28 +0900 Subject: [PATCH 10/10] Remove ProxyChainHead class --- lib/arproxy/connection_adapter_patch.rb | 19 ++++++++++-- lib/arproxy/proxy_chain.rb | 5 +--- lib/arproxy/proxy_chain_head.rb | 29 ------------------- ...proxy_chain_head_spec.rb => proxy_spec.rb} | 25 +++++++++------- 4 files changed, 33 insertions(+), 45 deletions(-) delete mode 100644 lib/arproxy/proxy_chain_head.rb rename spec/unit/{proxy_chain_head_spec.rb => proxy_spec.rb} (50%) diff --git a/lib/arproxy/connection_adapter_patch.rb b/lib/arproxy/connection_adapter_patch.rb index dff09f5..8aab9fa 100644 --- a/lib/arproxy/connection_adapter_patch.rb +++ b/lib/arproxy/connection_adapter_patch.rb @@ -77,7 +77,14 @@ def apply_patch(target_method) patched_execute_method_name = :"#{target_method}_with_arproxy" break if instance_methods.include?(patched_execute_method_name) define_method(patched_execute_method_name) do |sql, name=nil, **kwargs| - ::Arproxy.proxy_chain.head.execute_head(self, raw_execute_method_name, sql, name, **kwargs) + context = QueryContext.new( + raw_connection: self, + execute_method_name: raw_execute_method_name, + with_binds: false, + name: name, + kwargs: kwargs, + ) + ::Arproxy.proxy_chain.head.execute(sql, context) end alias_method raw_execute_method_name, target_method alias_method target_method, patched_execute_method_name @@ -92,7 +99,15 @@ def apply_patch_binds(target_method) patched_execute_method_name = :"#{target_method}_with_arproxy" break if instance_methods.include?(patched_execute_method_name) define_method(patched_execute_method_name) do |sql, name=nil, binds=[], **kwargs| - ::Arproxy.proxy_chain.head.execute_head_with_binds(self, raw_execute_method_name, sql, name, binds, **kwargs) + context = QueryContext.new( + raw_connection: self, + execute_method_name: raw_execute_method_name, + with_binds: true, + name: name, + binds: binds, + kwargs: kwargs, + ) + ::Arproxy.proxy_chain.head.execute(sql, context) end alias_method raw_execute_method_name, target_method alias_method target_method, patched_execute_method_name diff --git a/lib/arproxy/proxy_chain.rb b/lib/arproxy/proxy_chain.rb index 04d728b..4d65d0c 100644 --- a/lib/arproxy/proxy_chain.rb +++ b/lib/arproxy/proxy_chain.rb @@ -1,4 +1,3 @@ -require 'arproxy/proxy_chain_head' require 'arproxy/proxy_chain_tail' require 'arproxy/connection_adapter_patch' @@ -14,14 +13,12 @@ def initialize(config, patch) def setup @tail = ProxyChainTail.new - head = @config.proxies.reverse.inject(@tail) do |next_proxy, proxy_config| + @head = @config.proxies.reverse.inject(@tail) do |next_proxy, proxy_config| cls, options = proxy_config proxy = cls.new(*options) proxy.next_proxy = next_proxy proxy end - @head = ProxyChainHead.new - @head.next_proxy = head end private :setup diff --git a/lib/arproxy/proxy_chain_head.rb b/lib/arproxy/proxy_chain_head.rb deleted file mode 100644 index 93e420d..0000000 --- a/lib/arproxy/proxy_chain_head.rb +++ /dev/null @@ -1,29 +0,0 @@ -require 'arproxy/query_context' -require 'arproxy/proxy' - -module Arproxy - class ProxyChainHead < Proxy - def execute_head_with_binds(raw_connection, execute_method_name, sql, name = nil, binds = [], **kwargs) - context = QueryContext.new( - raw_connection: raw_connection, - execute_method_name: execute_method_name, - with_binds: true, - name: name, - binds: binds, - kwargs: kwargs, - ) - execute(sql, context) - end - - def execute_head(raw_connection, execute_method_name, sql, name = nil, **kwargs) - context = QueryContext.new( - raw_connection: raw_connection, - execute_method_name: execute_method_name, - with_binds: false, - name: name, - kwargs: kwargs, - ) - execute(sql, context) - end - end -end diff --git a/spec/unit/proxy_chain_head_spec.rb b/spec/unit/proxy_spec.rb similarity index 50% rename from spec/unit/proxy_chain_head_spec.rb rename to spec/unit/proxy_spec.rb index 329ff6e..e4e2c62 100644 --- a/spec/unit/proxy_chain_head_spec.rb +++ b/spec/unit/proxy_spec.rb @@ -1,9 +1,9 @@ require_relative './spec_helper' -require 'arproxy/proxy_chain_head' require 'arproxy/proxy_chain_tail' require 'arproxy/proxy' +require 'arproxy/query_context' -describe Arproxy::ProxyChainHead do +describe Arproxy::Proxy do before(:all) do class DummyConnectionAdapter def execute(sql, name = nil, binds = [], **kwargs) @@ -28,21 +28,26 @@ def execute(sql, context) p2.next_proxy = tail p1 = Proxy1.new p1.next_proxy = p2 - @head = Arproxy::ProxyChainHead.new - @head.next_proxy = p1 + @head = p1 @conn = DummyConnectionAdapter.new end - describe '#execute_head_with_binds' do - it do - expect(@head.execute_head_with_binds(@conn, 'execute', 'SELECT 1', 'test', [1])).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + context 'with binds' do + let(:context) { Arproxy::QueryContext.new(raw_connection: @conn, execute_method_name: 'execute', with_binds: true, name: 'test', binds: [1]) } + describe '#execute' do + it do + expect(@head.execute('SELECT 1', context)).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + end end end - describe '#execute_head' do - it do - expect(@head.execute_head(@conn, 'execute', 'SELECT 1', 'test')).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + context 'without binds' do + let(:context) { Arproxy::QueryContext.new(raw_connection: @conn, execute_method_name: 'execute', with_binds: false, name: 'test') } + describe '#execute' do + it do + expect(@head.execute('SELECT 1', context)).to eq('SELECT 1 /* Proxy1 */ /* Proxy2 */') + end end end end