Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 6 & Ruby 3 #578

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ end
appraise "rails52" do
gem "activerecord", "~> 5.2.0"
end

appraise "rails60" do
gem "activerecord", "~> 6.0.6.1"
end
# vim: ft=ruby
8 changes: 4 additions & 4 deletions ar-octopus.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@ Gem::Specification.new do |s|

s.required_ruby_version = '>= 2.2.0'

s.add_dependency 'activerecord', '>= 4.2.0'
s.add_dependency 'activesupport', '>= 4.2.0'
s.add_dependency 'activerecord', "~> 6.0.6.1"
s.add_dependency 'activesupport', "~> 6.0.6.1"

s.add_development_dependency 'appraisal', '>= 0.3.8'
s.add_development_dependency 'mysql2', '>= 0.3.18', "< 0.5"
s.add_development_dependency 'mysql2', '~> 0.5'
s.add_development_dependency 'pg', '~> 0.18'
s.add_development_dependency 'rake'
s.add_development_dependency 'rspec', '>= 3'
s.add_development_dependency 'rubocop'
s.add_development_dependency 'sqlite3', '~> 1.3.6'
s.add_development_dependency 'sqlite3', '~> 1.4'
s.add_development_dependency 'pry-byebug'

s.license = 'MIT'
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails42.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
source "https://rubygems.org"

gem "activerecord", "~> 4.2.0"
gem "mysql2", "0.4.10"

gemspec path: "../"
7 changes: 7 additions & 0 deletions gemfiles/rails60.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "activerecord", "~> 6.0.6.1"

gemspec path: "../"
18 changes: 17 additions & 1 deletion lib/octopus.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def self.config
file_name = File.join(Octopus.directory, 'config/shards.yml').to_s

if File.exist?(file_name) || File.symlink?(file_name)
config ||= HashWithIndifferentAccess.new(YAML.load(ERB.new(File.read(file_name)).result))[Octopus.env]
config ||= HashWithIndifferentAccess.new(load_yaml(file_name))[Octopus.env]
else
config ||= HashWithIndifferentAccess.new
end
Expand All @@ -28,6 +28,18 @@ def self.config
end
end

# To support psych-4
# Copied from rails patch
# https://github.com/rails/rails/commit/179d0a1f474ada02e0030ac3bd062fc653765dbe
def self.load_yaml(file_name)
source = ERB.new(File.read(file_name)).result
begin
YAML.load(source, aliases: true) || {}
rescue ArgumentError
YAML.load(source) || {}
end
end

def self.load_balancer=(balancer)
@load_balancer = balancer
end
Expand Down Expand Up @@ -114,6 +126,10 @@ def self.rails52?
ActiveRecord::VERSION::MAJOR == 5 && ActiveRecord::VERSION::MINOR == 2
end

def self.rails60?
ActiveRecord::VERSION::MAJOR > 6 || ActiveRecord::VERSION::MAJOR == 6
end

def self.atleast_rails51?
ActiveRecord::VERSION::MAJOR > 5 || (ActiveRecord::VERSION::MAJOR == 5 && ActiveRecord::VERSION::MINOR >= 1)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/octopus/collection_association.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Octopus
module CollectionAssociation
def self.included(base)
if Octopus.rails51? || Octopus.rails52?
if Octopus.atleast_rails51?
base.sharded_methods :reader, :writer, :ids_reader, :ids_writer, :create, :create!,
:build, :include?,
:load_target, :reload, :size, :select
Expand Down
12 changes: 10 additions & 2 deletions lib/octopus/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ def update_attributes!(*args)
run_on_shard { super }
end

def update(*args)
run_on_shard { super }
end

def update!(*args)
run_on_shard { super }
end

def reload(*args)
run_on_shard { super }
end
Expand All @@ -32,8 +40,8 @@ def update_column(*args)
run_on_shard { super }
end

def increment!(*args)
run_on_shard { super }
def increment!(...)
run_on_shard { super(...) }
end

def decrement!(*args)
Expand Down
38 changes: 32 additions & 6 deletions lib/octopus/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,10 @@ def check_schema_migrations(shard)
def transaction(options = {}, &block)
if !sharded && current_model_replicated?
run_queries_on_shard(Octopus.master_shard) do
select_connection.transaction(options, &block)
select_connection.transaction(**options, &block)
end
else
select_connection.transaction(options, &block)
select_connection.transaction(**options, &block)
end
end

Expand Down Expand Up @@ -212,17 +212,26 @@ def initialize_metadata_table
# We are planning to migrate to a much stable logic for the Proxy that doesn't require method missing.
def legacy_method_missing_logic(method, *args, &block)
if should_clean_connection_proxy?(method)
conn = select_connection
clean_connection_proxy
conn.send(method, *args, &block)
args, preparable = handle_args(args)
if preparable.nil?
select_connection.send(method, *args, &block)
else
select_connection.send(method, *args, **preparable, &block)
end
elsif should_send_queries_to_shard_slave_group?(method)
send_queries_to_shard_slave_group(method, *args, &block)
elsif should_send_queries_to_slave_group?(method)
send_queries_to_slave_group(method, *args, &block)
elsif should_send_queries_to_replicated_databases?(method)
send_queries_to_selected_slave(method, *args, &block)
else
val = select_connection.send(method, *args, &block)
args, preparable = handle_args(args)
val = if preparable.nil?
select_connection.send(method, *args, &block)
else
select_connection.send(method, *args, **preparable, &block)
end

if val.instance_of? ActiveRecord::Result
val.current_shard = shard_name
Expand Down Expand Up @@ -309,7 +318,12 @@ def send_queries_to_balancer(balancer, method, *args, &block)
# while preserving `current_shard`
def send_queries_to_slave(slave, method, *args, &block)
using_shard(slave) do
val = select_connection.send(method, *args, &block)
args, preparable = handle_args(args)
val = if preparable.nil?
select_connection.send(method, *args, &block)
else
select_connection.send(method, *args, **preparable, &block)
end
if val.instance_of? ActiveRecord::Result
val.current_shard = slave
end
Expand Down Expand Up @@ -361,5 +375,17 @@ def using_group(group, &_block)
self.current_group = older_group
end
end

private

def handle_args(args)
return [[], nil] if args.empty?

preparable_args = args.select { |arg| arg.is_a?(Hash) && arg.key?(:preparable) }
return [args, nil] if preparable_args.empty?

args -= preparable_args
[args, preparable_args.first]
end
end
end
6 changes: 4 additions & 2 deletions lib/octopus/relation_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ def method_missing(method, *args, &block)
if !block && BATCH_METHODS.include?(method)
::Enumerator.new do |yielder|
run_on_shard do
@ar_relation.public_send(method, *args) do |batch_item|
parsed_args = args.empty? ? {} : args.first
@ar_relation.public_send(method, **parsed_args) do |batch_item|
yielder << batch_item
end
end
Expand All @@ -43,7 +44,8 @@ def method_missing(method, *args, &block)
elsif WHERE_CHAIN_METHODS.include?(method)
::Octopus::ScopeProxy.new(@current_shard, run_on_shard { @ar_relation.public_send(method, *args) } )
elsif block
@ar_relation.public_send(method, *args, &block)
parsed_args = args.empty? ? {} : args.first
@ar_relation.public_send(method, **parsed_args, &block)
else
run_on_shard do
if method == :load_records
Expand Down
2 changes: 1 addition & 1 deletion lib/octopus/scope_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def using(shard)

# Transaction Method send all queries to a specified shard.
def transaction(options = {}, &block)
run_on_shard { klass.transaction(options, &block) }
run_on_shard { klass.transaction(**options, &block) }
end

def connection
Expand Down
2 changes: 1 addition & 1 deletion lib/octopus/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module Octopus
VERSION = '0.10.2'
VERSION = '0.11.0'
end
3 changes: 2 additions & 1 deletion spec/octopus/migration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

def get_all_versions
if Octopus.atleast_rails52?
schema = ActiveRecord::SchemaMigration
migrations_root = File.expand_path(File.join(File.dirname(__FILE__), '..', 'migrations'))
ActiveRecord::MigrationContext.new(migrations_root).get_all_versions
ActiveRecord::MigrationContext.new(migrations_root, schema).get_all_versions
else
ActiveRecord::Migrator.get_all_versions
end
Expand Down
2 changes: 1 addition & 1 deletion spec/octopus/model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@
end

user = User.using(:brazil).where(:name => 'User1').first
expect(user.as_json(:except => [:created_at, :updated_at, :id])).to eq('admin' => nil, 'name' => 'User1', 'number' => nil)
expect(user.as_json(:except => [:created_at, :updated_at, :id, :current_shard])).to eq('admin' => nil, 'name' => 'User1', 'number' => nil)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is :currend_shard included in the result of using() now? That's what broke a bunch of specs when trying to upgrade our rails 5.2 to rails 6.0 using this branch

end

describe 'transaction' do
Expand Down
2 changes: 1 addition & 1 deletion spec/octopus/proxy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
expect(config[:username]).to eq("#{ENV['MYSQL_USER'] || 'root'}")
end

unless Octopus.rails50? || Octopus.rails51?|| Octopus.rails52?
unless Octopus.rails50? || Octopus.rails51?|| Octopus.rails52? || Octopus.rails60?
it 'should respond correctly to respond_to?(:pk_and_sequence_for)' do
expect(proxy.respond_to?(:pk_and_sequence_for)).to be true
end
Expand Down
5 changes: 3 additions & 2 deletions spec/support/octopus_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ def self.migrating_to_version(version, &_block)

def self.migrate_to_version(direction, root, version)
if Octopus.atleast_rails52?
migrations = ActiveRecord::MigrationContext.new(root).migrations.select {|mig| version == mig.version }
ActiveRecord::Migrator.new(direction, migrations, version).run
schema = ActiveRecord::SchemaMigration
migrations = ActiveRecord::MigrationContext.new(root, schema).migrations.select {|mig| version == mig.version }
ActiveRecord::Migrator.new(direction, migrations, schema, version).run
else
ActiveRecord::Migrator.run(direction, root, version)
end
Expand Down