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

Cache the result of is_sharded? #311

Merged
merged 2 commits into from
Apr 6, 2023
Merged
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
14 changes: 8 additions & 6 deletions lib/active_record_shards.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ class << self
end

def self.app_env
env = Rails.env if defined?(Rails.env)
env ||= RAILS_ENV if Object.const_defined?(:RAILS_ENV)
env ||= ENV['RAILS_ENV']
env ||= APP_ENV if Object.const_defined?(:APP_ENV)
env ||= ENV['APP_ENV']
env || 'development'
@app_env ||= begin
env = Rails.env if defined?(Rails.env)
env ||= RAILS_ENV if Object.const_defined?(:RAILS_ENV)
env ||= ENV['RAILS_ENV']
env ||= APP_ENV if Object.const_defined?(:APP_ENV)
env ||= ENV['APP_ENV']
env || 'development'
end
end
end

Expand Down
31 changes: 20 additions & 11 deletions lib/active_record_shards/model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,26 @@ def not_sharded
end

def is_sharded? # rubocop:disable Naming/PredicateName
if self == ActiveRecord::Base
sharded != false && supports_sharding?
elsif self == base_class
if sharded.nil?
ActiveRecord::Base.is_sharded?
else
sharded != false
end
else
base_class.is_sharded?
end
# "sharded" here means self.sharded, but actually writing "self.sharded"
# doesn't work until Ruby 2.7 (and this gem currently supports 2.6) because
# the sharded attr_accessor is private. Private methods must be called without
# a receiver, but Ruby 2.7+ does allow an explicit "self" as a receiver.
return sharded unless sharded.nil?

# Despite self.sharded not working, self.sharded= _DOES_ work. That's an exception
# to the "private methods must be called with no receiver" rule (presumably
# because it would otherwise be ambiguous with local variable assignment).
self.sharded = if self == ActiveRecord::Base
sharded != false && supports_sharding?
elsif self == base_class
if sharded.nil?
ActiveRecord::Base.is_sharded?
else
sharded != false
end
else
base_class.is_sharded?
end
end

def on_replica_by_default?
Expand Down
6 changes: 5 additions & 1 deletion test/active_record_shards_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,13 @@
end

Object.send(:remove_const, 'RAILS_ENV')
ActiveRecordShards.instance_eval { @app_env = nil }
end

after { Object.const_set('RAILS_ENV', 'test') }
after do
Object.const_set('RAILS_ENV', 'test')
ActiveRecordShards.instance_eval { @app_env = nil }
end

describe 'Rails.env' do
before do
Expand Down
2 changes: 2 additions & 0 deletions test/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,12 @@ module RailsEnvSwitch
def switch_app_env(env)
before do
silence_warnings { Object.const_set("RAILS_ENV", env) }
ActiveRecordShards.instance_eval { @app_env = nil }
ActiveRecord::Base.establish_connection(::RAILS_ENV.to_sym)
end
after do
silence_warnings { Object.const_set("RAILS_ENV", 'test') }
ActiveRecordShards.instance_eval { @app_env = nil }
ActiveRecord::Base.establish_connection(::RAILS_ENV.to_sym)
tmp_sharded_model = Class.new(ActiveRecord::Base)
assert_equal('ars_test', tmp_sharded_model.connection.current_database)
Expand Down