Skip to content

Commit

Permalink
Fix on Ruby 3 (#283)
Browse files Browse the repository at this point in the history
  • Loading branch information
mlarraz committed Jun 4, 2021
1 parent 882207a commit b53800d
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 31 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 19 additions & 15 deletions lib/makara/connection_wrapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
33 changes: 20 additions & 13 deletions lib/makara/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 11 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b53800d

Please sign in to comment.