Skip to content

Commit

Permalink
Fix sessions
Browse files Browse the repository at this point in the history
  • Loading branch information
johnnyshields committed Apr 20, 2024
1 parent 5a0eef9 commit a176439
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 13 deletions.
54 changes: 54 additions & 0 deletions lib/mongoid/clients/sessions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ def with_session(options = {})
end

raise e
rescue *transactions_not_supported_exceptions
raise Mongoid::Errors::TransactionsNotSupported
ensure
Threaded.clear_session(client: persistence_context.client)
end
Expand Down Expand Up @@ -89,6 +91,8 @@ def transaction(options = {}, session_options: {})
session.start_transaction(options)
yield
commit_transaction(session)
rescue *transactions_not_supported_exceptions
raise Mongoid::Errors::TransactionsNotSupported
rescue Mongoid::Errors::Rollback
abort_transaction(session)
rescue Mongoid::Errors::InvalidSessionNesting
Expand Down Expand Up @@ -148,6 +152,22 @@ def after_rollback(*args, &block)

private

# Driver version 2.20 introduced a new exception for reporting that
# transactions are not supported. Prior to that, the condition was
# discovered by the rescue clause falling through to a different
# exception.
#
# This method ensures that Mongoid continues to work with older driver
# versions, by only returning the new exception.
#
# Once support is removed for all versions prior to 2.20.0, we can
# replace this method.
def transactions_not_supported_exceptions
return nil unless defined? Mongo::Error::TransactionsNotSupported

Mongo::Error::TransactionsNotSupported
end

# @return [ Mongo::Session ] Session for the current client.
def _session
Threaded.get_session(client: persistence_context.client)
Expand Down Expand Up @@ -231,6 +251,40 @@ def transaction_include_any_action?(actions)
end
end
end

# If at least one session is active, this ensures that the
# current model's client is compatible with one of them.
#
# "Compatible" is defined to mean: the same client was used
# to open one of the active sessions.
#
# Currently emits a warning.
def ensure_client_compatibility!
# short circuit: if no sessions are active, there's nothing
# to check.
return unless Threaded.sessions.any?

# at this point, we know that at least one session is currently
# active. let's see if one of them was started with the model's
# client...
session = Threaded.get_session(client: persistence_context.client)
return unless session.nil?

# if the session is nil, then we have a case of the programmer trying to use
# a model within a transaction, where the model is not itself
# controlled by that transaction. this is potentially a bug, so
# let's tell them about it.
#
# This is hacky; we're hijacking Mongoid::Errors::MongoidError in
# order to get the spiffy error message translation. If we later
# decide to raise an error instead of just writing a message, we can
# subclass MongoidError and raise that exception here.
message = Errors::MongoidError.new.compose_message(
'client_session_mismatch',
model: self.class.name
)
logger.info(message)
end
end
end
end
1 change: 1 addition & 0 deletions lib/mongoid/persistable/creatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ def prepare_insert(options = {})
return self if performing_validations?(options) &&
invalid?(options[:context] || :create)

ensure_client_compatibility!
run_callbacks(:commit, with_children: true, skip_if: -> { in_transaction? }) do
run_callbacks(:save, with_children: false) do
run_callbacks(:create, with_children: false) do
Expand Down
1 change: 1 addition & 0 deletions lib/mongoid/persistable/updatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def prepare_update(options = {})
raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly

enforce_immutability_of_id_field!
ensure_client_compatibility!
return false if performing_validations?(options) &&
invalid?(options[:context] || :update)

Expand Down
44 changes: 31 additions & 13 deletions spec/mongoid/clients/sessions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,8 @@
require 'spec_helper'

describe Mongoid::Clients::Sessions do

before(:all) do
CONFIG[:clients][:other] = CONFIG[:clients][:default].dup
CONFIG[:clients][:other][:database] = 'other'
Mongoid::Clients.clients.each_value(&:close)
Mongoid::Config.send(:clients=, CONFIG[:clients])
Mongoid::Clients.with_name(:other).subscribe(Mongo::Monitoring::COMMAND, EventSubscriber.new)
end

after(:all) do
Mongoid::Clients.with_name(:other).close
Mongoid::Clients.clients.delete(:other)
end
let(:buffer) { StringIO.new }
let(:logger) { Logger.new(buffer, Logger::DEBUG) }

let(:subscriber) do
client = Mongoid::Clients.with_name(:other)
Expand All @@ -35,6 +24,27 @@
subscriber.started_events.select { |event| event.command_name.to_s == 'update' }
end

around do |example|
old_logger = Mongoid.logger
Mongoid.logger = logger
example.run
ensure
Mongoid.logger = old_logger
end

before(:all) do
CONFIG[:clients][:other] = CONFIG[:clients][:default].dup
CONFIG[:clients][:other][:database] = 'other'
Mongoid::Clients.clients.each_value(&:close)
Mongoid::Config.send(:clients=, CONFIG[:clients])
Mongoid::Clients.with_name(:other).subscribe(Mongo::Monitoring::COMMAND, EventSubscriber.new)
end

after(:all) do
Mongoid::Clients.with_name(:other).close
Mongoid::Clients.clients.delete(:other)
end

context 'when a session is used on a model class' do

context 'when sessions are supported' do
Expand Down Expand Up @@ -229,6 +239,10 @@
expect(insert_lsids_sent.uniq.size).to eq(1)
expect(update_lsids_sent.uniq).to eq(insert_lsids_sent.uniq)
end

it 'does not warn about a different client' do
expect(buffer.string).to_not include("used within another client's session")
end
end

context 'when the other class uses a different client' do
Expand Down Expand Up @@ -257,6 +271,10 @@
update_lsids_sent = update_events.collect { |event| event.command['lsid'] }
expect(update_lsids_sent.size).to eq(2)
end

it 'warns about a different client' do
expect(buffer.string).to include("used within another client's session")
end
end

context 'when sessions are nested' do
Expand Down

0 comments on commit a176439

Please sign in to comment.