Skip to content

Commit

Permalink
DEV: Fix reading_role/writing_role deprecations (#48)
Browse files Browse the repository at this point in the history
* DEV: Fix reading_role/writing_role deprecations

Introduced in rails/rails#42445

Example of a warning:
```
DEPRECATION WARNING: ActiveRecord::Base.writing_role is deprecated and will be removed in Rails 7.1.
Use `ActiveRecord.writing_role` instead.
 (called from block (2 levels) in <top (required)> at /__w/discourse/discourse/spec/rails_helper.rb:304)
```

* changelog

* Add rails 7.0 to the ci matrix

* reorder
  • Loading branch information
CvX committed Apr 6, 2023
1 parent 2ece801 commit cfcf68a
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 18 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ jobs:
strategy:
fail-fast: false
matrix:
ruby: ['2.7', '3.0', '3.1', '3.2']
rails: ['6.1.0']
ruby: ['3.2', '3.1', '3.0', '2.7']
rails: ['7.0.0']
include:
- ruby: '3.2'
rails: '6.1.0'
- ruby: '3.2'
rails: '6.0.0'

Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

- DEV: Remove the support for Ruby < 2.7
- DEV: Compatibility with Rails 7.1+

## [0.8.0] - 2022-01-17

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ production:
replica_port: <replica db server port>
```

The gem will automatically create an `ActiveRecord::ConnectionAdapters::ConnectionHandler` with the `ActiveRecord::Base.reading_role` as the `handler_key`.
The gem will automatically create an `ActiveRecord::ConnectionAdapters::ConnectionHandler` with the `ActiveRecord.reading_role` as the `handler_key`.

#### Failover/Fallback Hooks

Expand Down
10 changes: 10 additions & 0 deletions lib/rails_failover/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
require_relative 'active_record/middleware'
require_relative 'active_record/handler'

AR = ::ActiveRecord.respond_to?(:reading_role) ? ::ActiveRecord : ::ActiveRecord::Base

module RailsFailover
module ActiveRecord
def self.logger=(logger)
Expand Down Expand Up @@ -61,5 +63,13 @@ def self.on_fallback_callback!(key)
rescue => e
logger.warn("RailsFailover::ActiveRecord.on_fallback failed: #{e.class} #{e.message}\n#{e.backtrace.join("\n")}")
end

def self.reading_role
AR.reading_role
end

def self.writing_role
AR.writing_role
end
end
end
12 changes: 6 additions & 6 deletions lib/rails_failover/active_record/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ def initialize(app)
end

def call(env)
current_role = ::ActiveRecord::Base.current_role || ::ActiveRecord::Base.writing_role
is_writing_role = current_role.to_s.end_with?(::ActiveRecord::Base.writing_role.to_s)
current_role = ::ActiveRecord::Base.current_role || RailsFailover::ActiveRecord.writing_role
is_writing_role = current_role.to_s.end_with?(RailsFailover::ActiveRecord.writing_role.to_s)
writing_role = resolve_writing_role(current_role, is_writing_role)

role =
Expand Down Expand Up @@ -97,17 +97,17 @@ def resolve_writing_role(current_role, is_writing_role)
current_role
else
current_role.to_s.sub(
/#{::ActiveRecord::Base.reading_role}$/,
::ActiveRecord::Base.writing_role.to_s
/#{RailsFailover::ActiveRecord.reading_role}$/,
RailsFailover::ActiveRecord.writing_role.to_s
).to_sym
end
end

def resolve_reading_role(current_role, is_writing_role)
if is_writing_role
current_role.to_s.sub(
/#{::ActiveRecord::Base.writing_role}$/,
::ActiveRecord::Base.reading_role.to_s
/#{RailsFailover::ActiveRecord.writing_role}$/,
RailsFailover::ActiveRecord.reading_role.to_s
).to_sym
else
current_role
Expand Down
8 changes: 4 additions & 4 deletions lib/rails_failover/active_record/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ class Railtie < ::Rails::Railtie

# We are doing this manually for now since we're awaiting Rails 6.1 to be released which will
# have more stable ActiveRecord APIs for handling multiple databases with different roles.
::ActiveRecord::Base.connection_handlers[::ActiveRecord::Base.reading_role] =
::ActiveRecord::Base.connection_handlers[RailsFailover::ActiveRecord.reading_role] =
::ActiveRecord::ConnectionAdapters::ConnectionHandler.new

::ActiveRecord::Base.connection_handlers[::ActiveRecord::Base.writing_role].connection_pools.each do |connection_pool|
::ActiveRecord::Base.connection_handlers[RailsFailover::ActiveRecord.writing_role].connection_pools.each do |connection_pool|
if connection_pool.respond_to?(:db_config)
config = connection_pool.db_config.configuration_hash
else
config = connection_pool.spec.config
end
RailsFailover::ActiveRecord.establish_reading_connection(
::ActiveRecord::Base.connection_handlers[::ActiveRecord::Base.reading_role],
::ActiveRecord::Base.connection_handlers[RailsFailover::ActiveRecord.reading_role],
config
)
end
Expand All @@ -43,7 +43,7 @@ class Railtie < ::Rails::Railtie
rescue ::ActiveRecord::NoDatabaseError
# Do nothing since database hasn't been created
rescue ::PG::Error, ::ActiveRecord::ConnectionNotEstablished
Handler.instance.verify_primary(::ActiveRecord::Base.writing_role)
Handler.instance.verify_primary(RailsFailover::ActiveRecord.writing_role)
::ActiveRecord::Base.connection_handler = ::ActiveRecord::Base.lookup_connection_handler(:reading)
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/rails_failover/middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@
middleware = described_class.new(app)
status, headers, body = middleware.call(Rack::MockRequest.env_for("/", {}))

expect(headers[described_class::CURRENT_ROLE_HEADER]).to eq(::ActiveRecord::Base.writing_role)
expect(headers[described_class::WRITING_ROLE_HEADER]).to eq(::ActiveRecord::Base.writing_role)
expect(headers[described_class::CURRENT_ROLE_HEADER]).to eq(RailsFailover::ActiveRecord.writing_role)
expect(headers[described_class::WRITING_ROLE_HEADER]).to eq(RailsFailover::ActiveRecord.writing_role)

RailsFailover::ActiveRecord.register_force_reading_role_callback do |env|
true
end

status, headers, body = middleware.call(Rack::MockRequest.env_for("/", {}))

expect(headers[described_class::CURRENT_ROLE_HEADER]).to eq(::ActiveRecord::Base.reading_role)
expect(headers[described_class::WRITING_ROLE_HEADER]).to eq(::ActiveRecord::Base.writing_role)
expect(headers[described_class::CURRENT_ROLE_HEADER]).to eq(RailsFailover::ActiveRecord.reading_role)
expect(headers[described_class::WRITING_ROLE_HEADER]).to eq(RailsFailover::ActiveRecord.writing_role)
ensure
described_class.force_reading_role_callback = nil
end
Expand Down
2 changes: 1 addition & 1 deletion spec/support/dummy_app/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

source "https://rubygems.org"

gem "rails", "~> #{ENV["RAILS_VERSION"] || "6.1.0"}"
gem "rails", "~> #{ENV["RAILS_VERSION"] || "7.0.0"}"

# Use SCSS for stylesheets
gem "sass-rails", ">= 6"
Expand Down

0 comments on commit cfcf68a

Please sign in to comment.