From b53800d0157a67d7ac6654424ba3e4d1cbd0aff7 Mon Sep 17 00:00:00 2001 From: Matt Larraz Date: Tue, 19 Jan 2021 11:36:47 -0800 Subject: [PATCH] Fix on Ruby 3 (#283) --- .github/workflows/CI.yml | 2 +- lib/makara/connection_wrapper.rb | 34 +++++++++++-------- lib/makara/proxy.rb | 33 +++++++++++------- .../makara_mysql2_adapter_spec.rb | 7 +++- .../makara_postgresql_adapter_spec.rb | 8 ++++- spec/spec_helper.rb | 11 ++++++ 6 files changed, 64 insertions(+), 31 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index f3c5ed43..0495d44d 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -29,7 +29,7 @@ jobs: ruby: 3.0 - gemfile: ar52 ruby: ruby-head - continue-on-error: ${{ matrix.ruby == 3.0 || matrix.ruby == 'jruby' || matrix.ruby == 'ruby-head' || matrix.gemfile == 'ar-head' }} + continue-on-error: ${{ matrix.ruby == 'jruby' || matrix.ruby == 'ruby-head' || matrix.gemfile == 'ar-head' }} services: postgres: image: postgis/postgis:12-3.1 diff --git a/lib/makara/connection_wrapper.rb b/lib/makara/connection_wrapper.rb index 7cdb1885..11e5deee 100644 --- a/lib/makara/connection_wrapper.rb +++ b/lib/makara/connection_wrapper.rb @@ -103,13 +103,11 @@ def execute(*args) # we want to forward all private methods, since we could have kicked out from a private scenario def method_missing(m, *args, &block) - if _makara_connection.respond_to?(m) - _makara_connection.public_send(m, *args, &block) - else # probably private method - _makara_connection.__send__(m, *args, &block) - end + _makara_connection.send(m, *args, &block) end + ruby2_keywords :method_missing if Module.private_method_defined?(:ruby2_keywords) + def respond_to_missing?(m, include_private = false) _makara_connection.respond_to?(m, true) end @@ -120,7 +118,7 @@ def respond_to_missing?(m, include_private = false) # all extra functionality is in the format of _makara* def _makara_decorate_connection(con) - extension = %Q{ + extension = <<~RUBY # the proxy object controlling this connection def _makara @_makara @@ -144,38 +142,44 @@ def _makara_hijack def _makara_name #{@config[:name].inspect} end - } + RUBY + + args = RUBY_VERSION >= "3.0.0" ? "..." : "*args, &block" # Each method the Makara::Proxy needs to hijack should be redefined in the underlying connection. # The new definition should allow for the proxy to intercept the invocation if required. @proxy.class.hijack_methods.each do |meth| - extension << %Q{ - def #{meth}(*args, &block) + method_call = RUBY_VERSION >= "3.0.0" ? "public_send(#{meth.inspect}, ...)" : "#{meth}(*args, &block)" + + extension << <<~RUBY + def #{meth}(#{args}) _makara_hijack do |proxy| if proxy - proxy.#{meth}(*args, &block) + proxy.#{method_call} else super end end end - } + RUBY end # Control methods must always be passed to the # Makara::Proxy control object for handling (typically # related to ActiveRecord connection pool management) @proxy.class.control_methods.each do |meth| - extension << %Q{ - def #{meth}(*args, &block) + method_call = RUBY_VERSION >= "3.0.0" ? "public_send(#{meth.inspect}, ...)" : "#{meth}(*args=args, block)" + + extension << <<~RUBY + def #{meth}(#{args}) proxy = _makara if proxy - proxy.control.#{meth}(*args=args, block) + proxy.control.#{method_call} else super # Only if we are not wrapped any longer end end - } + RUBY end # extend the instance diff --git a/lib/makara/proxy.rb b/lib/makara/proxy.rb index 1ea76915..0fad69c4 100644 --- a/lib/makara/proxy.rb +++ b/lib/makara/proxy.rb @@ -24,19 +24,23 @@ def hijack_method(*method_names) self.hijack_methods |= method_names method_names.each do |method_name| - define_method method_name do |*args, &block| + define_method(method_name) do |*args, &block| appropriate_connection(method_name, args) do |con| con.send(method_name, *args, &block) end end + + ruby2_keywords method_name if Module.private_method_defined?(:ruby2_keywords) end end def send_to_all(*method_names) method_names.each do |method_name| - define_method method_name do |*args| - send_to_all method_name, *args + define_method(method_name) do |*args| + send_to_all(method_name, *args) end + + ruby2_keywords method_name if Module.private_method_defined?(:ruby2_keywords) end end @@ -45,9 +49,11 @@ def control_method(*method_names) self.control_methods |= method_names method_names.each do |method_name| - define_method method_name do |*args, &block| + define_method(method_name) do |*args, &block| control&.send(method_name, *args, &block) end + + ruby2_keywords method_name if Module.private_method_defined?(:ruby2_keywords) end end end @@ -118,20 +124,20 @@ def strategy_class_for(strategy_name) def method_missing(m, *args, &block) if METHOD_MISSING_SKIP.include?(m) - return super(m, *args, &block) + return super end any_connection do |con| - if con.respond_to?(m) - con.public_send(m, *args, &block) - elsif con.respond_to?(m, true) - con.__send__(m, *args, &block) + if con.respond_to?(m, true) + con.send(m, *args, &block) else - super(m, *args, &block) + super end end end + ruby2_keywords :method_missing if Module.private_method_defined?(:ruby2_keywords) + def respond_to_missing?(m, include_private = false) any_connection do |con| con._makara_connection.respond_to?(m, true) @@ -157,15 +163,16 @@ def disconnect! protected - def send_to_all(method_name, *args) # slave pool must run first to allow for slave-->master failover without running operations on master twice. handling_an_all_execution(method_name) do - @slave_pool.send_to_all method_name, *args - @master_pool.send_to_all method_name, *args + @slave_pool.send_to_all(method_name, *args) + @master_pool.send_to_all(method_name, *args) end end + ruby2_keywords :send_to_all if Module.private_method_defined?(:ruby2_keywords) + def any_connection if @master_pool.disabled @slave_pool.provide do |con| diff --git a/spec/active_record/connection_adapters/makara_mysql2_adapter_spec.rb b/spec/active_record/connection_adapters/makara_mysql2_adapter_spec.rb index e02ea382..e0b5356f 100644 --- a/spec/active_record/connection_adapters/makara_mysql2_adapter_spec.rb +++ b/spec/active_record/connection_adapters/makara_mysql2_adapter_spec.rb @@ -160,7 +160,12 @@ Test::User.exists? # flush other (schema) things that need to happen con = connection.slave_pool.connections.first - expect(con).to receive(:exec_query).with(/SELECT\s+1\s*(AS one)?\s+FROM .?users.?\s+LIMIT\s+.?1/, any_args).once.and_call_original + expect(con).to receive(:exec_query) do |query| + expect(query).to match(/SELECT\s+1\s*(AS one)?\s+FROM .?users.?\s+LIMIT\s+.?1/) + end.once. + # and_call_original # Switch back to this once https://github.com/rspec/rspec-mocks/pull/1385 is released + and_wrap_original { |m, *args| m.call(*args.first(3)) } + Test::User.exists? end diff --git a/spec/active_record/connection_adapters/makara_postgresql_adapter_spec.rb b/spec/active_record/connection_adapters/makara_postgresql_adapter_spec.rb index 52ee91e6..11940c16 100644 --- a/spec/active_record/connection_adapters/makara_postgresql_adapter_spec.rb +++ b/spec/active_record/connection_adapters/makara_postgresql_adapter_spec.rb @@ -73,7 +73,13 @@ Test::User.exists? # flush other (schema) things that need to happen con = connection.slave_pool.connections.first - expect(con).to receive(:exec_query).with(/SELECT\s+1\s*(AS one)?\s+FROM .?users.?\s+LIMIT\s+.?1/, any_args).once.and_call_original + + expect(con).to receive(:exec_query) do |query| + expect(query).to match(/SELECT\s+1\s*(AS one)?\s+FROM .?users.?\s+LIMIT\s+.?1/) + end.once. + # and_call_original # Switch back to this once https://github.com/rspec/rspec-mocks/pull/1385 is released + and_wrap_original { |m, *args| m.call(*args.first(3)) } + Test::User.exists? end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index bcff6afe..31a59fa3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -15,6 +15,17 @@ rescue LoadError end +if RUBY_VERSION >= "2.7.0" + Warning[:deprecated] = true +end + +# Delete once Timecop fixes Ruby 3.1 support +Time.class_eval do + class << self + ruby2_keywords :new if Module.private_method_defined?(:ruby2_keywords) + end +end + RSpec.configure do |config| config.run_all_when_everything_filtered = true config.filter_run :focus