diff --git a/AGENTS.md b/AGENTS.md index 05b640f6d2..4eaa6d8b31 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,10 @@ Mongoid is a Ruby Object-Document Mapper (ODM) framework for MongoDB. Mongoid al # Development Workflow +## Planning and Design + +When producing a design document or specification for a new feature or significant change, leave the document unstaged. Do not add it to the repository. + ## Running tests Tests require a running MongoDB instance. Set the URI via the `MONGODB_URI` environment variable: diff --git a/lib/config/locales/en.yml b/lib/config/locales/en.yml index b6f6884ca1..7710088fec 100644 --- a/lib/config/locales/en.yml +++ b/lib/config/locales/en.yml @@ -413,6 +413,10 @@ en: message: "A transaction was started while another transaction was being used on this client." summary: "Transactions cannot be nested. Only one transaction can be used in a thread per client." resolution: "Only use one transaction per client at a time; transactions cannot be nested." + transaction_in_changeset: + message: "Cannot start a transaction inside a changeset." + summary: "A changeset defers all writes and flushes them as a batch when the outermost block exits. Starting a transaction inside a changeset would have no effect, because the operations inside the transaction block are queued rather than executed immediately." + resolution: "Do not nest a transaction inside a changeset." inverse_not_found: message: "When adding a(n) %{klass} to %{base}#%{name}, Mongoid could not determine the inverse foreign key to set. The attempted key was diff --git a/lib/mongoid.rb b/lib/mongoid.rb index 1c721cfadf..9294716c7e 100644 --- a/lib/mongoid.rb +++ b/lib/mongoid.rb @@ -28,6 +28,7 @@ require 'mongoid/tasks/encryption' require 'mongoid/warnings' require 'mongoid/utils' +require 'mongoid/changeset' # If we are using Rails then we will include the Mongoid railtie. # This configures initializers required to integrate Mongoid with Rails. @@ -107,6 +108,23 @@ def reconnect_clients Clients.reconnect end + # Creates or reuses the current changeset, yields it to the block, and + # flushes when the outermost scope exits. When nested inside an existing + # changeset block, the inner call accumulates without flushing. + # + # @example Explicit outer scope — flush happens once at block exit. + # Mongoid.changeset do |cs| + # parent.save + # child.save + # end + def changeset + outermost = Threaded.current_changeset.nil? + cs = Threaded.current_changeset || (Threaded.current_changeset = Changeset.new) + cs.run { yield cs } + ensure + Threaded.current_changeset = nil if outermost + end + # Convenience method for getting a named client. # # @example Get a named client. diff --git a/lib/mongoid/association/embedded/batchable.rb b/lib/mongoid/association/embedded/batchable.rb index fa2aa7aabd..f0ef3f54fd 100644 --- a/lib/mongoid/association/embedded/batchable.rb +++ b/lib/mongoid/association/embedded/batchable.rb @@ -33,10 +33,16 @@ def batch_insert(docs) def batch_clear(docs) pre_process_batch_remove(docs, :delete) unless docs.empty? - collection.find(selector).update_one( - positionally(selector, '$unset' => { path => true }), - session: _session - ) + Mongoid.changeset do |cs| + cs.add( + type: :update, + collection: collection, + selector: selector, + payload: positionally(selector, '$unset' => { path => true }), + document: nil, + session: _session + ) + end # This solves the case in which a user sets, clears and resets an # embedded document. Previously, since the embedded document was # already marked not a "new_record", it wouldn't be persisted to @@ -67,22 +73,38 @@ def batch_remove(docs, method = :delete) end if docs.empty? - collection.find(selector).update_one( - positionally(selector, '$set' => { path => [] }), - session: _session - ) - else - unless pulls.empty? - collection.find(selector).update_one( - positionally(selector, '$pull' => { path => { '_id' => { '$in' => pulls.pluck('_id') } } }), + Mongoid.changeset do |cs| + cs.add( + type: :update, + collection: collection, + selector: selector, + payload: positionally(selector, '$set' => { path => [] }), + document: nil, session: _session ) end - unless pull_alls.empty? - collection.find(selector).update_one( - positionally(selector, '$pullAll' => { path => pull_alls }), - session: _session - ) + else + Mongoid.changeset do |cs| + unless pulls.empty? + cs.add( + type: :update, + collection: collection, + selector: selector, + payload: positionally(selector, '$pull' => { path => { '_id' => { '$in' => pulls.pluck('_id') } } }), + document: nil, + session: _session + ) + end + unless pull_alls.empty? + cs.add( + type: :update, + collection: collection, + selector: selector, + payload: positionally(selector, '$pullAll' => { path => pull_alls }), + document: nil, + session: _session + ) + end end post_process_batch_remove(docs, method) end @@ -154,10 +176,16 @@ def execute_batch_set(docs) self.inserts_valid = true inserts = pre_process_batch_insert(docs) if insertable? - collection.find(selector).update_one( - positionally(selector, '$set' => { path => inserts }), - session: _session - ) + Mongoid.changeset do |cs| + cs.add( + type: :update, + collection: collection, + selector: selector, + payload: positionally(selector, '$set' => { path => inserts }), + document: nil, + session: _session + ) + end post_process_batch_insert(docs) end inserts @@ -177,10 +205,16 @@ def execute_batch_push(docs) self.inserts_valid = true pushes = pre_process_batch_insert(docs) if insertable? - collection.find(selector).update_one( - positionally(selector, '$push' => { path => { '$each' => pushes } }), - session: _session - ) + Mongoid.changeset do |cs| + cs.add( + type: :update, + collection: collection, + selector: selector, + payload: positionally(selector, '$push' => { path => { '$each' => pushes } }), + document: nil, + session: _session + ) + end post_process_batch_insert(docs) end pushes diff --git a/lib/mongoid/association/nested/many.rb b/lib/mongoid/association/nested/many.rb index 8ed6cd25b2..99488df237 100644 --- a/lib/mongoid/association/nested/many.rb +++ b/lib/mongoid/association/nested/many.rb @@ -160,11 +160,7 @@ def destroy_document(relation, doc) # @param [ Hash ] attrs The attributes. def update_document(doc, attrs) delete_id(attrs) - if association.embedded? - doc.assign_attributes(attrs) - else - doc.update_attributes(attrs) - end + doc.assign_attributes(attrs) end # Update nested association. diff --git a/lib/mongoid/association/referenced/has_and_belongs_to_many/proxy.rb b/lib/mongoid/association/referenced/has_and_belongs_to_many/proxy.rb index c9b07fdab1..ecfbb2bbd1 100644 --- a/lib/mongoid/association/referenced/has_and_belongs_to_many/proxy.rb +++ b/lib/mongoid/association/referenced/has_and_belongs_to_many/proxy.rb @@ -79,8 +79,10 @@ def <<(*args) # the changed_attributes hash. # See MONGOID-4843 for a longer discussion about this. reset_foreign_key_changes do - _base.add_to_set(foreign_key => doc.public_send(_association.primary_key)) - doc.save if child_persistable?(doc) + Mongoid.changeset do + _base.add_to_set(foreign_key => doc.public_send(_association.primary_key)) + doc.save! if child_persistable?(doc) + end reset_unloaded end end diff --git a/lib/mongoid/association/referenced/has_many/proxy.rb b/lib/mongoid/association/referenced/has_many/proxy.rb index fc1f3efc58..2d2adf2b2f 100644 --- a/lib/mongoid/association/referenced/has_many/proxy.rb +++ b/lib/mongoid/association/referenced/has_many/proxy.rb @@ -73,7 +73,7 @@ def <<(*args) if (doc = docs.first) append(doc) - doc.save if persistable? && !_assigning? && !doc.validated? + doc.save! if persistable? && !_assigning? && !doc.validated? end self end @@ -597,7 +597,7 @@ def save_or_delay(doc, docs, inserts) docs.push(doc) inserts.push(doc.send(:as_attributes)) else - doc.save + doc.save! end end end diff --git a/lib/mongoid/association/referenced/has_one/proxy.rb b/lib/mongoid/association/referenced/has_one/proxy.rb index 5cd775f3d3..f67fd853eb 100644 --- a/lib/mongoid/association/referenced/has_one/proxy.rb +++ b/lib/mongoid/association/referenced/has_one/proxy.rb @@ -45,7 +45,7 @@ def initialize(base, target, association) raise_mixed if klass.embedded? && !klass.cyclic? characterize_one(_target) bind_one - _target.save if persistable? + _target.save! if persistable? end end diff --git a/lib/mongoid/changeset.rb b/lib/mongoid/changeset.rb new file mode 100644 index 0000000000..8aab0414b2 --- /dev/null +++ b/lib/mongoid/changeset.rb @@ -0,0 +1,234 @@ +# frozen_string_literal: true + +require 'mongoid/changeset/entry' + +module Mongoid + class Changeset + attr_reader :entries, :depth + + def atomically_context? + @atomically_context + end + + def enter_atomically_context + @atomically_context = true + end + + def exit_atomically_context + @atomically_context = false + end + + def initialize + @entries = [] + @depth = 0 + @terminated = false + @atomically_context = false + end + + # Manages nesting depth. Inner calls accumulate without flushing. + def build + @depth += 1 + yield + ensure + @depth -= 1 + end + + # Outer entry point — increments depth, yields, decrements depth. + # When depth returns to zero (outermost scope), flushes. On error at + # the outermost scope, discards. + def run(&block) + raise Errors::InvalidChangesetOperation.new('Changeset is terminated') if terminated? + + result = build(&block) + flush if @depth.zero? + result + rescue StandardError + discard if @depth.zero? && !terminated? + raise + end + + # Constructs an Entry from the given keyword arguments and appends it. + def add(**kwargs) + add_entry(Entry.new(**kwargs)) + end + + # Appends a pre-built Entry to the list. + def add_entry(entry) + raise Errors::InvalidChangesetOperation.new('Changeset is terminated') if terminated? + + @entries << entry + entry + end + + # Executes all entries against the driver in registration order, then + # marks the changeset terminated. Terminates even if the flush is aborted + # by an exception — a partially-flushed changeset is not resumable. + def flush + raise Errors::InvalidChangesetOperation.new('Changeset is terminated') if terminated? + + _flush_entries + ensure + @terminated = true + end + + # Clears all staged entries without executing them, then marks terminated. + def discard + raise Errors::InvalidChangesetOperation.new('Changeset is terminated') if terminated? + + @entries.clear + @terminated = true + end + + # Returns true if the changeset has been flushed or discarded. + def terminated? + @terminated + end + + private + + def _flush_entries + @entries.reject(&:skip_callbacks) + .filter_map(&:document) + .uniq { |d| d.object_id } + .each { |doc| doc.run_before_callbacks(:flush) } + + _build_batches(@entries).each do |batch| + (batch.size == 1) ? _execute_single(batch.first) : _execute_bulk(batch) + _finalize_batch(batch) + end + + _dispatch_commits + end + + def _finalize_batch(batch) + per_doc = {}.compare_by_identity + batch.each do |entry| + next unless entry.document + + per_doc[entry.document] ||= { entries: [], callbacks: false, dirty_fields: [] } + per_doc[entry.document][:entries] << entry + per_doc[entry.document][:callbacks] ||= !entry.skip_callbacks + per_doc[entry.document][:dirty_fields].concat(entry.dirty_fields) if entry.dirty_fields + end + + per_doc.each do |doc, data| + data[:entries].each { |e| _update_document_state(e) } + data[:dirty_fields].each { |f| doc.remove_change(f) } + doc.run_after_callbacks(:flush) if data[:callbacks] + end + end + + def _dispatch_commits + seen = {}.compare_by_identity + @entries.each do |entry| + next unless entry.document + + rec = (seen[entry.document] ||= { session: nil, callbacks: false }) + rec[:session] ||= entry.session + rec[:callbacks] ||= !entry.skip_callbacks + end + + seen.each do |doc, data| + if data[:session]&.in_transaction? + Mongoid::Threaded.add_modified_document(data[:session], doc) + elsif data[:callbacks] + doc.run_callbacks(:commit) + end + end + end + + def _build_batches(entries) + batches = [] + entries.each do |entry| + last = batches.last&.first + if last&.collection == entry.collection && last&.session.equal?(entry.session) + batches.last << entry + else + batches << [ entry ] + end + end + batches + end + + def _execute_single(entry) + session_opts = entry.session ? { session: entry.session } : {} + driver_opts = entry.opts ? session_opts.merge(entry.opts) : session_opts + opt_args = _opt_args(driver_opts) + case entry.type + when :insert + entry.collection.insert_one(entry.payload, *opt_args) + when :update, :embedded_insert, :embedded_delete + entry.collection.find(entry.selector).update_one(entry.payload, *opt_args) + when :update_many + entry.collection.find(entry.selector).update_many(entry.payload, *opt_args) + when :delete + entry.collection.find(entry.selector).delete_one(*opt_args) + when :delete_many + entry.result = entry.collection.find(entry.selector).delete_many(*opt_args) + when :upsert + entry.collection.find(entry.selector).update_one(entry.payload, { upsert: true }.merge(driver_opts)) + when :upsert_replace + entry.collection.find(entry.selector).replace_one(entry.payload, { upsert: true }.merge(driver_opts)) + end + end + + def _execute_bulk(batch) + collection = batch.first.collection + session = batch.map(&:session).find { |s| s } + opts = session ? { session: session } : {} + ops = batch.map { |entry| _bulk_op_for(entry) } + collection.bulk_write(ops, *_opt_args(opts)) + end + + # The mongo 2.x driver takes positional opts hashes. Passing **{} in Ruby 2.7 + # is converted to a positional {} argument, causing RSpec argument mismatches + # in tests. Omit the opts argument entirely when there is nothing to pass. + def _opt_args(opts) + opts.empty? ? [] : [ opts ] + end + + def _bulk_op_for(entry) + inner_opts = entry.opts&.reject { |k, _| k == :session } || {} + case entry.type + when :insert + { insert_one: entry.payload } + when :update, :embedded_insert, :embedded_delete + { update_one: { filter: entry.selector, update: entry.payload }.merge(inner_opts) } + when :update_many + { update_many: { filter: entry.selector, update: entry.payload }.merge(inner_opts) } + when :delete + { delete_one: { filter: entry.selector }.merge(inner_opts) } + when :delete_many + { delete_many: { filter: entry.selector }.merge(inner_opts) } + else + _bulk_op_for_upsert(entry, inner_opts) + end + end + + def _bulk_op_for_upsert(entry, inner_opts) + case entry.type + when :upsert + { update_one: { filter: entry.selector, update: entry.payload, upsert: true }.merge(inner_opts) } + when :upsert_replace + { replace_one: { filter: entry.selector, replacement: entry.payload, upsert: true }.merge(inner_opts) } + end + end + + def _update_document_state(entry) + doc = entry.document + return unless doc + + case entry.type + when :insert, :embedded_insert, :upsert, :upsert_replace + doc.new_record = false + doc.remember_storage_options! + doc.flag_descendants_persisted + doc._reset_memoized_descendants! + when :update, :update_many + # no per-document state change needed for updates + when :embedded_delete, :delete + doc.destroyed = true + end + end + end +end diff --git a/lib/mongoid/changeset/entry.rb b/lib/mongoid/changeset/entry.rb new file mode 100644 index 0000000000..be9e0687b6 --- /dev/null +++ b/lib/mongoid/changeset/entry.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Mongoid + class Changeset + # Immutable value object representing a single staged database operation. + # Created at the moment an operation is staged (when save/destroy is called + # and validation passes, or when a criteria-level op is invoked). + # + # For :insert entries, payload is the full serialized document. + # For :update entries, payload is the result of atomic_updates() at save time. + # For :delete entries, payload is nil. + # For :update_many/:delete_many entries, document is nil. + Entry = Struct.new( + :type, # Symbol: :insert | :embedded_insert | :update | :embedded_delete | :delete | :update_many | :delete_many | :upsert | :upsert_replace + :collection, # Mongo::Collection + :selector, # Hash - MongoDB filter + :payload, # Hash | nil + :document, # Mongoid::Document | nil (nil for criteria-level entries) + :session, # Mongo::Session | nil + :opts, # Hash | nil - driver-level options (e.g. array_filters) + :result, # driver result object, set after execution (for bulk ops that return counts) + :skip_callbacks, # Boolean | nil - when true, suppresses :flush and :commit callbacks + :dirty_fields, # Array | nil - fields to remove from dirty tracking after flush + keyword_init: true + ) + end +end diff --git a/lib/mongoid/clients/sessions.rb b/lib/mongoid/clients/sessions.rb index 9476de2b59..a8bd786a47 100644 --- a/lib/mongoid/clients/sessions.rb +++ b/lib/mongoid/clients/sessions.rb @@ -81,6 +81,8 @@ def with_session(options = {}) # # @yield Provided block will be executed inside a transaction. def transaction(options = {}, session_options: {}, &block) + raise Errors::TransactionInChangeset.new if Threaded.current_changeset + with_session(session_options) do |session| session.with_transaction(options, &block).tap { run_commit_callbacks(session) } rescue *transactions_not_supported_exceptions diff --git a/lib/mongoid/contextual/memory.rb b/lib/mongoid/contextual/memory.rb index b256157bf5..f37caf78eb 100644 --- a/lib/mongoid/contextual/memory.rb +++ b/lib/mongoid/contextual/memory.rb @@ -49,10 +49,16 @@ def delete doc.send(:as_attributes) end unless removed.empty? - collection.find(selector).update_one( - positionally(selector, '$pullAll' => { path => removed }), - session: _session - ) + Mongoid.changeset do |cs| + cs.add( + type: :update, + collection: collection, + selector: selector, + payload: positionally(selector, '$pullAll' => { path => removed }), + document: nil, + session: _session + ) + end end deleted end @@ -564,7 +570,18 @@ def update_documents(attributes, docs) updates['$set'].merge!(doc.atomic_updates['$set'] || {}) doc.move_changes end - collection.find(selector).update_one(updates, session: _session) unless updates['$set'].empty? + return if updates['$set'].empty? + + Mongoid.changeset do |cs| + cs.add( + type: :update, + collection: collection, + selector: selector, + payload: updates, + document: nil, + session: _session + ) + end end # Get the limiting value. diff --git a/lib/mongoid/contextual/mongo.rb b/lib/mongoid/contextual/mongo.rb index 64bbfb3081..2c6d73ed3b 100644 --- a/lib/mongoid/contextual/mongo.rb +++ b/lib/mongoid/contextual/mongo.rb @@ -108,9 +108,24 @@ def estimated_count(options = {}) # @example Delete all the documents. # context.delete # - # @return [ nil ] Nil. + # @return [ Integer | nil ] The number of documents deleted, or nil if the + # delete was performed within a changeset (and was thus deferred). def delete - view.delete_many.deleted_count + entry_opts = _view_opts + entry = Mongoid.changeset do |cs| + cs.add( + type: :delete_many, + collection: collection, + selector: view.filter, + payload: nil, + document: nil, + session: _session, + opts: entry_opts.empty? ? nil : entry_opts + ) + end + return nil unless entry.is_a?(Changeset::Entry) && entry.result + + acknowledged_write? ? entry.result.deleted_count : 0 end alias delete_all delete @@ -515,7 +530,22 @@ def update(attributes = nil, opts = {}) # # @return [ nil | false ] False if no attributes were provided. def update_all(attributes = nil, opts = {}) - update_documents(attributes, :update_many, opts) + return false unless attributes + + prepared = AtomicUpdatePreparer.prepare(attributes, klass) + entry_opts = _view_opts.merge(opts) + Mongoid.changeset do |cs| + cs.add( + type: :update_many, + collection: collection, + selector: view.filter, + payload: prepared, + document: nil, + session: _session, + opts: entry_opts.empty? ? nil : entry_opts + ) + end + nil end # Get the first document in the database for the criteria's selector. @@ -894,6 +924,17 @@ def _session @criteria.send(:_session) end + # Extract driver-level options (e.g. collation) from the current view, + # so they can be forwarded through a changeset entry. + # + # @api private + def _view_opts + opts = {} + opts[:collation] = view.options[:collation] if view.options[:collation] + opts[:comment] = view.options[:comment] if view.options[:comment] + opts + end + def acknowledged_write? collection.write_concern.nil? || collection.write_concern.acknowledged? end diff --git a/lib/mongoid/errors.rb b/lib/mongoid/errors.rb index c3e7391a48..994f5df985 100644 --- a/lib/mongoid/errors.rb +++ b/lib/mongoid/errors.rb @@ -25,6 +25,7 @@ require 'mongoid/errors/invalid_global_executor_concurrency' require 'mongoid/errors/invalid_includes' require 'mongoid/errors/invalid_index' +require 'mongoid/errors/invalid_changeset_operation' require 'mongoid/errors/invalid_options' require 'mongoid/errors/invalid_path' require 'mongoid/errors/invalid_persistence_option' @@ -45,6 +46,7 @@ require 'mongoid/errors/invalid_storage_options' require 'mongoid/errors/invalid_time' require 'mongoid/errors/invalid_transaction_nesting' +require 'mongoid/errors/transaction_in_changeset' require 'mongoid/errors/inverse_not_found' require 'mongoid/errors/mixed_relations' require 'mongoid/errors/mixed_client_configuration' diff --git a/lib/mongoid/errors/invalid_changeset_operation.rb b/lib/mongoid/errors/invalid_changeset_operation.rb new file mode 100644 index 0000000000..54514ae176 --- /dev/null +++ b/lib/mongoid/errors/invalid_changeset_operation.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Mongoid + module Errors + # Raised when an operation is attempted on a changeset that has already + # been flushed or discarded. + class InvalidChangesetOperation < MongoidError + end + end +end diff --git a/lib/mongoid/errors/transaction_in_changeset.rb b/lib/mongoid/errors/transaction_in_changeset.rb new file mode 100644 index 0000000000..fe0fba6145 --- /dev/null +++ b/lib/mongoid/errors/transaction_in_changeset.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Mongoid + module Errors + # Raised when transaction is called from inside a changeset block. + class TransactionInChangeset < MongoidError + def initialize + super(compose_message('transaction_in_changeset')) + end + end + end +end diff --git a/lib/mongoid/interceptable.rb b/lib/mongoid/interceptable.rb index b9c723f8c8..e4d8606891 100644 --- a/lib/mongoid/interceptable.rb +++ b/lib/mongoid/interceptable.rb @@ -10,6 +10,7 @@ module Interceptable after_create after_destroy after_find + after_flush after_initialize after_save after_touch @@ -23,12 +24,25 @@ module Interceptable around_upsert before_create before_destroy + before_flush before_save before_update before_upsert before_validation ].freeze + module ClassMethods + # Register a callback to run before flush (as part of a batch write). + def before_flush(*args, &block) + set_callback(:flush, :before, *args, &block) + end + + # Register a callback to run after flush (as part of a batch write). + def after_flush(*args, &block) + set_callback(:flush, :after, *args, &block) + end + end + included do extend ActiveModel::Callbacks include ActiveModel::Validations::Callbacks @@ -46,6 +60,9 @@ module Interceptable only: :after, scope: %i[kind name] + define_callbacks :flush, + scope: %i[kind name] + attr_accessor :before_callback_halted end diff --git a/lib/mongoid/persistable.rb b/lib/mongoid/persistable.rb index d51ed38620..6d8dc2f3a1 100644 --- a/lib/mongoid/persistable.rb +++ b/lib/mongoid/persistable.rb @@ -45,79 +45,54 @@ module Persistable LIST_OPERATIONS = [ '$addToSet', '$push', '$pull', '$pullAll' ].freeze # Execute operations atomically (in a single database call) for everything - # that would happen inside the block. This method supports nesting further - # calls to atomically, which will behave according to the options described - # below. + # that would happen inside the block. When nested, inner calls merge into + # the outermost changeset and flush together when that outermost block exits. # - # An option join_context can be given which, when true, will merge the - # operations declared by the given block with the atomically block wrapping - # the current invocation for the same document, if one exists. If this - # block or any other block sharing the same context raises before - # persisting, then all the operations of that context will not be - # persisted, and will also be reset in memory. - # - # When join_context is false, the given block of operations will be - # persisted independently of other contexts. Failures in other contexts will - # not affect this one, so long as this block was able to run and persist - # changes. - # - # The default value of join_context is set by the global configuration - # option join_contexts, whose own default is false. + # Passing join_context: false persists operations independently — they are + # not affected by a failure in the enclosing block. This usage is deprecated; + # instead call save outside any enclosing changeset scope. # # @example Execute the operations atomically. # document.atomically do # document.set(name: "Tool").inc(likes: 10) # end # - # @example Execute some inner operations atomically, but independently from the outer operations. - # + # @example Execute some inner operations independently (deprecated). # document.atomically do # document.inc likes: 10 - # document.atomically join_context: false do - # # The following is persisted to the database independently. + # document.atomically(join_context: false) do + # # Persisted immediately, unaffected by outer failure. # document.unset :origin # end - # document.atomically join_context: true do - # # The following is persisted along with the other outer operations. - # document.inc member_count: 3 - # end - # document.set name: "Tool" # end # - # @param [ true | false ] join_context Join the context (i.e. merge - # declared atomic operations) of the atomically block wrapping this one - # for the same document, if one exists. + # @param [ true | false | nil ] join_context When false, operations persist + # independently of any enclosing changeset (deprecated). Any other value + # joins the enclosing changeset (the default). # - # @return [ true | false ] If the operation succeeded. + # @return [ true ] Always true. def atomically(join_context: nil) - join_context = Mongoid.join_contexts if join_context.nil? - call_depth = @atomic_depth ||= 0 - has_own_context = call_depth.zero? || !join_context - @atomic_updates_to_execute_stack ||= [] - _mongoid_push_atomic_context if has_own_context - - if block_given? - @atomic_depth += 1 - yield(self) - @atomic_depth -= 1 - end - - if has_own_context - persist_atomic_operations @atomic_context - _mongoid_remove_atomic_context_changes + if join_context == false + Mongoid::Warnings.warn_join_context_false_deprecated + if block_given? + doc = self + _atomically_independent do |cs| + cs.enter_atomically_context + yield doc + end + end + elsif block_given? + Mongoid.changeset do |cs| + was_atomic = cs.atomically_context? + cs.enter_atomically_context unless was_atomic + begin + yield self + ensure + cs.exit_atomically_context unless was_atomic + end + end end - true - rescue StandardError => e - _mongoid_reset_atomic_context_changes! if has_own_context - raise e - ensure - _mongoid_pop_atomic_context if has_own_context - - if call_depth.zero? - @atomic_depth = nil - @atomic_updates_to_execute_stack = nil - end end # Raise an error if validation failed. @@ -144,18 +119,6 @@ def fail_due_to_callback!(method) private - # Are we executing an atomically block on the current document? - # - # @api private - # - # @example Are we executing atomically? - # document.executing_atomically? - # - # @return [ true | false ] If we are current executing atomically. - def executing_atomically? - !@atomic_updates_to_execute_stack.nil? - end - # Post process the persistence operation. # # @api private @@ -172,148 +135,104 @@ def executing_atomically? def post_process_persist(result, options = {}) post_persist unless result == false errors.clear unless performing_validations?(options) - Threaded.add_modified_document(_session, self) if in_transaction? + Threaded.add_modified_document(_session, self) if in_transaction? && !Threaded.current_changeset true end - # Prepare an atomic persistence operation. Yields an empty hash to be sent - # to the update. - # - # @api private - # - # @example Prepare the atomic operation. - # document.prepare_atomic_operation do |coll, selector, opts| - # ... - # end - # - # @return [ Object ] The result of the operation. - def prepare_atomic_operation - raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly - - operations = yield({}) - persist_or_delay_atomic_operation(operations) - self - end - - # Process the atomic operations - this handles the common behavior of - # iterating through each op, getting the aliased field name, and removing - # appropriate dirty changes. + # Persist the atomic operations by staging an update entry in the current + # changeset. Called by touchable on the root document. # # @api private # - # @example Process the atomic operations. - # document.process_atomic_operations(pulls) do |field, value| - # ... - # end + # @example Persist the atomic operations. + # persist_atomic_operations(ops) # # @param [ Hash ] operations The atomic operations. - # - # @return [ Hash ] The operations. - def process_atomic_operations(operations) - operations.each do |field, value| - access = database_field_name(field) - yield(access, value) - remove_change(access) unless executing_atomically? - end - end - - # Remove the dirty changes for all fields changed in the current atomic - # context. - # - # @api private - # - # @example Remove the current atomic context's dirty changes. - # document._mongoid_remove_atomic_context_changes - def _mongoid_remove_atomic_context_changes - return unless executing_atomically? - - _mongoid_atomic_context_changed_fields.each { |f| remove_change f } - end - - # Reset the attributes for all fields changed in the current atomic - # context. - # - # @api private - # - # @example Reset the current atomic context's changed attributes. - # document._mongoid_reset_atomic_context_changes! - def _mongoid_reset_atomic_context_changes! - return unless executing_atomically? + def persist_atomic_operations(operations) + return unless persisted? && operations && !operations.empty? - _mongoid_atomic_context_changed_fields.each { |f| reset_attribute! f } + selector = atomic_selector + Mongoid.changeset do |cs| + cs.add( + type: :update, + collection: collection(_root), + selector: selector, + payload: positionally(selector, operations), + document: self, + session: _session + ) + end end - # Push a new atomic context onto the stack. + # Run the given block in an independent changeset, temporarily hiding any + # enclosing changeset so that the inner block flushes immediately on exit. # # @api private - # - # @example Push a new atomic context onto the stack. - # document._mongoid_push_atomic_context - def _mongoid_push_atomic_context - return unless executing_atomically? - - @atomic_context = {} - @atomic_updates_to_execute_stack << @atomic_context + def _atomically_independent(&block) + outer = Threaded.current_changeset + Threaded.current_changeset = nil + begin + Mongoid.changeset(&block) + ensure + Threaded.current_changeset = outer + end end - # Pop an atomic context off the stack. + # Returns an empty array when currently inside an #atomically block (to + # accumulate dirty field names for cleanup after flush), or nil otherwise. # # @api private - # - # @example Pop an atomic context off the stack. - # document._mongoid_pop_atomic_context - def _mongoid_pop_atomic_context - return unless executing_atomically? - - @atomic_updates_to_execute_stack.pop - @atomic_context = @atomic_updates_to_execute_stack.last + def _atomic_dirty_fields_init + [] if Threaded.current_changeset&.atomically_context? end - # Return the current atomic context's changed fields. + # Stores the original value of a field in changed_attributes before an + # atomic operation mutates it directly, so dirty tracking reflects the + # change during the #atomically block. # # @api private - # - # @example Return the current atomic context's changed fields. - # document._mongoid_atomic_context_changed_fields - # - # @return [ Array ] The changed fields. - def _mongoid_atomic_context_changed_fields - @atomic_context.values.flat_map(&:keys) + def _mark_dirty_field(dirty, access, original) + changed_attributes[access] ||= original if dirty end - # If we are in an atomically block, add the operations to the delayed group, - # otherwise persist immediately. + # Appends the field to the dirty accumulator when inside #atomically, or + # removes the change when not (the field was already persisted atomically). # # @api private - # - # @example Persist immediately or delay the operations. - # document.persist_or_delay_atomic_operation(ops) - # - # @param [ Hash ] operation The operation. - def persist_or_delay_atomic_operation(operation) - if executing_atomically? - operation.each do |(name, hash)| - @atomic_context[name] ||= {} - @atomic_context[name].merge!(hash) - end - else - persist_atomic_operations(operation) - end + def _track_dirty_field(dirty, access) + dirty ? dirty << access : remove_change(access) end - # Persist the atomic operations. + # Stage an atomic :update entry in the current changeset. + # Used by the atomic operation modules (inc, bit, set, unset, etc.) + # after they have computed their MongoDB operator payload. # # @api private # - # @example Persist the atomic operations. - # persist_atomic_operations(ops) + # @param [ String ] operator The MongoDB update operator, e.g. '$inc'. + # @param [ Hash ] ops The field/value pairs for the operator. + # @param [ Array | nil ] dirty Accumulated dirty field names from an + # #atomically context, or nil when not inside one. # - # @param [ Hash ] operations The atomic operations. - def persist_atomic_operations(operations) - return unless persisted? && operations && !operations.empty? + # @return [ Document ] self + def _stage_atomic_update(operator, ops, dirty: nil) + return self if ops.empty? selector = atomic_selector - _root.collection.find(selector).update_one(positionally(selector, operations), session: _session) + Mongoid.changeset do |cs| + entry = { + type: :update, + collection: collection(_root), + selector: selector, + payload: positionally(selector, { operator => ops }), + document: self, + session: _session, + skip_callbacks: true + } + entry[:dirty_fields] = dirty unless dirty.nil? + cs.add(**entry) + end + self end end end diff --git a/lib/mongoid/persistable/creatable.rb b/lib/mongoid/persistable/creatable.rb index 6f09191e56..0bf81a3dbc 100644 --- a/lib/mongoid/persistable/creatable.rb +++ b/lib/mongoid/persistable/creatable.rb @@ -16,13 +16,7 @@ module Creatable # # @return [ Document ] The persisted document. def insert(options = {}) - prepare_insert(options) do - if embedded? - insert_as_embedded - else - insert_as_root - end - end + prepare_insert(options) end private @@ -39,70 +33,82 @@ def atomic_inserts { atomic_insert_modifier => { atomic_position => as_attributes } } end - # Insert the embedded document. + # Stage an insert entry on the current changeset. # - # When the parent association is touchable (which is the default for - # +embedded_in+), the touch timestamp updates are merged into the - # same +update_one+ call that performs the insert. This avoids a - # second round-trip that the +after_save+ touch callback would - # otherwise issue. + # For root documents, adds an :insert entry. For embedded documents, + # adds an :embedded_insert entry against the root document's collection. + # If the parent is itself a new record, inserts the parent first. # # @api private - # - # @example Insert the document as embedded. - # document.insert_as_embedded - # - # @return [ Document ] The document. - def insert_as_embedded - raise Errors::NoParent.new(self.class.name) unless _parent - - if _parent.new_record? - _parent.insert + def _stage_insert + if embedded? + _stage_insert_as_embedded else - selector = _parent.atomic_selector - operations = atomic_inserts - - if _touchable_parent? - touches = _parent._gather_touch_updates(Time.current) - if touches.present? - operations['$set'] = touches - Threaded.begin_touch_merged(self) - end - end - - _root.collection.find(selector).update_one( - positionally(selector, operations), - session: _session - ) + _stage_insert_as_root end end - # Insert the root document. + # Stage an insert entry for a root document. # # @api private - # - # @example Insert the document as root. - # document.insert_as_root - # - # @return [ Document ] The document. - def insert_as_root - collection.insert_one(as_attributes, session: _session) + def _stage_insert_as_root + Threaded.current_changeset.add( + type: :insert, + collection: collection, + selector: { '_id' => _id }, + payload: as_attributes, + document: self, + session: _session + ) + # State transitions (new_record, dirty tracking) are applied at stage time, + # before the driver write, by design. If the flush raises, the document + # should be reloaded from the database — in-memory state cannot be reliably + # restored after a partial write. + # Mirrors Changeset#_update_document_state for :insert entries. + self.new_record = false + remember_storage_options! + flag_descendants_persisted + _reset_memoized_descendants! end - # Post process an insert, which sets the new record attribute to false - # and flags all the children as persisted. + # Stage an update entry for an embedded document. # # @api private - # - # @example Post process the insert. - # document.post_process_insert - # - # @return [ true ] true. - def post_process_insert + def _stage_insert_as_embedded + raise Errors::NoParent.new(self.class.name) unless _parent + + if _parent.new_record? + _parent.insert + return + end + + operations = atomic_inserts + + if _touchable_parent? + touches = _parent._gather_touch_updates(Time.current) + if touches.present? + operations['$set'] = touches + Threaded.begin_touch_merged(self) + end + end + + Threaded.current_changeset.add( + type: :embedded_insert, + collection: _root.collection, + selector: _parent.atomic_selector, + payload: positionally(_parent.atomic_selector, operations), + document: self, + session: _session + ) + # State transitions (new_record, dirty tracking) are applied at stage time, + # before the driver write, by design. If the flush raises, the document + # should be reloaded from the database — in-memory state cannot be reliably + # restored after a partial write. + # Mirrors Changeset#_update_document_state for :embedded_insert entries. self.new_record = false remember_storage_options! flag_descendants_persisted - true + _reset_memoized_descendants! end # Prepare the insert for execution. Validates and runs callbacks, etc. @@ -110,9 +116,7 @@ def post_process_insert # @api private # # @example Prepare for insertion. - # document.prepare_insert do - # collection.insert(as_document) - # end + # document.prepare_insert # # @param [ Hash ] options The options. # @@ -123,17 +127,14 @@ def prepare_insert(options = {}) invalid?(options[:context] || :create) ensure_client_compatibility! - run_callbacks(:commit, with_children: true, skip_if: -> { in_transaction? }) do + Mongoid.changeset do run_callbacks(:save, with_children: false) do run_callbacks(:create, with_children: false) do run_callbacks(:persist_parent, with_children: false) do _mongoid_run_child_callbacks(:save) do _mongoid_run_child_callbacks(:create) do - result = yield(self) - if !result.is_a?(Document) || result.errors.empty? - post_process_insert - post_process_persist(result, options) - end + _stage_insert + post_process_persist(true, options) end end end diff --git a/lib/mongoid/persistable/deletable.rb b/lib/mongoid/persistable/deletable.rb index 2f7d9bb73c..9ddf305153 100644 --- a/lib/mongoid/persistable/deletable.rb +++ b/lib/mongoid/persistable/deletable.rb @@ -59,10 +59,17 @@ def delete_as_embedded(options = {}) _parent.remove_child(self) if notifying_parent?(options) if _parent.persisted? selector = _parent.atomic_selector - _root.collection.find(selector).update_one( - positionally(selector, atomic_deletes), - session: _session - ) + Mongoid.changeset do |cs| + cs.add( + type: :embedded_delete, + collection: _root.collection, + selector: selector, + payload: positionally(selector, atomic_deletes), + document: self, + session: _session, + skip_callbacks: !flagged_for_destroy? + ) + end end true end @@ -76,7 +83,17 @@ def delete_as_embedded(options = {}) # # @return [ true ] If the document was removed. def delete_as_root - collection.find(atomic_selector).delete_one(session: _session) + Mongoid.changeset do |cs| + cs.add( + type: :delete, + collection: collection, + selector: atomic_selector, + payload: nil, + document: self, + session: _session, + skip_callbacks: !flagged_for_destroy? + ) + end true end diff --git a/lib/mongoid/persistable/destroyable.rb b/lib/mongoid/persistable/destroyable.rb index 935dc790b8..3cc9ee3c73 100644 --- a/lib/mongoid/persistable/destroyable.rb +++ b/lib/mongoid/persistable/destroyable.rb @@ -22,12 +22,10 @@ def destroy(options = nil) raise Errors::ReadonlyDocument.new(self.class) if readonly? self.flagged_for_destroy = true - result = run_callbacks(:commit, skip_if: -> { in_transaction? }) do + result = Mongoid.changeset do run_callbacks(:destroy) do if catch(:abort) { apply_destroy_dependencies! } - delete(options || {}).tap do |res| - Threaded.add_modified_document(_session, self) if res && in_transaction? - end + delete(options || {}) else false end diff --git a/lib/mongoid/persistable/incrementable.rb b/lib/mongoid/persistable/incrementable.rb index ce119b4f2c..bd1b775d73 100644 --- a/lib/mongoid/persistable/incrementable.rb +++ b/lib/mongoid/persistable/incrementable.rb @@ -17,17 +17,24 @@ module Incrementable # # @return [ Document ] The document. def inc(increments) - prepare_atomic_operation do |ops| - process_atomic_operations(increments) do |field, value| - increment = value.is_a?(BigDecimal) ? value.to_f : value - current = attributes[field] - new_value = (current || 0) + increment - process_attribute field, new_value if executing_atomically? - attributes[field] = new_value - ops[atomic_attribute_name(field)] = increment - end - { '$inc' => ops } unless ops.empty? + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + + dirty = _atomic_dirty_fields_init + ops = {} + + increments.each do |field, value| + access = database_field_name(field) + increment = value.is_a?(BigDecimal) ? value.to_f : value + current = attributes[access] + _mark_dirty_field(dirty, access, current) + attributes[access] = (current || 0) + increment + _track_dirty_field(dirty, access) + ops[atomic_attribute_name(access)] = increment end + + return self unless persisted? + + _stage_atomic_update('$inc', ops, dirty: dirty) end end end diff --git a/lib/mongoid/persistable/logical.rb b/lib/mongoid/persistable/logical.rb index 72bb693064..32b43537bf 100644 --- a/lib/mongoid/persistable/logical.rb +++ b/lib/mongoid/persistable/logical.rb @@ -16,19 +16,27 @@ module Logical # # @return [ Document ] The document. def bit(operations) - prepare_atomic_operation do |ops| - process_atomic_operations(operations) do |field, values| - value = attributes[field] - values.each do |op, val| - value &= val if op.to_s == 'and' - value |= val if op.to_s == 'or' - end - process_attribute field, value if executing_atomically? - attributes[field] = value - ops[atomic_attribute_name(field)] = values + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + dirty = _atomic_dirty_fields_init + ops = {} + + operations.each do |field, values| + access = database_field_name(field) + current = attributes[access] + _mark_dirty_field(dirty, access, current) + value = current + values.each do |op, val| + value &= val if op.to_s == 'and' + value |= val if op.to_s == 'or' end - { '$bit' => ops } unless ops.empty? + attributes[access] = value + _track_dirty_field(dirty, access) + ops[atomic_attribute_name(access)] = values end + + _stage_atomic_update('$bit', ops, dirty: dirty) end end end diff --git a/lib/mongoid/persistable/maxable.rb b/lib/mongoid/persistable/maxable.rb index 8732d5266e..c1ebec5fa0 100644 --- a/lib/mongoid/persistable/maxable.rb +++ b/lib/mongoid/persistable/maxable.rb @@ -18,16 +18,22 @@ module Maxable # # @return [ Document ] The document. def set_max(fields) - prepare_atomic_operation do |ops| - process_atomic_operations(fields) do |field, value| - current_value = attributes[field] - if value > current_value - process_attribute field, value - ops[atomic_attribute_name(field)] = value - end - end - { '$max' => ops } unless ops.empty? + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + dirty = _atomic_dirty_fields_init + ops = {} + + fields.each do |field, value| + access = database_field_name(field) + next unless value > attributes[access] + + process_attribute access, value + _track_dirty_field(dirty, access) + ops[atomic_attribute_name(access)] = value end + + _stage_atomic_update('$max', ops, dirty: dirty) end alias clamp_lower_bound set_max end diff --git a/lib/mongoid/persistable/minable.rb b/lib/mongoid/persistable/minable.rb index 6f7f099c16..6509732579 100644 --- a/lib/mongoid/persistable/minable.rb +++ b/lib/mongoid/persistable/minable.rb @@ -18,16 +18,22 @@ module Minable # # @return [ Document ] The document. def set_min(fields) - prepare_atomic_operation do |ops| - process_atomic_operations(fields) do |field, value| - current_value = attributes[field] - if value < current_value - process_attribute field, value - ops[atomic_attribute_name(field)] = value - end - end - { '$min' => ops } unless ops.empty? + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + dirty = _atomic_dirty_fields_init + ops = {} + + fields.each do |field, value| + access = database_field_name(field) + next unless value < attributes[access] + + process_attribute access, value + _track_dirty_field(dirty, access) + ops[atomic_attribute_name(access)] = value end + + _stage_atomic_update('$min', ops, dirty: dirty) end alias clamp_upper_bound set_min end diff --git a/lib/mongoid/persistable/multipliable.rb b/lib/mongoid/persistable/multipliable.rb index c1e723ea6d..85ff0059cd 100644 --- a/lib/mongoid/persistable/multipliable.rb +++ b/lib/mongoid/persistable/multipliable.rb @@ -17,17 +17,23 @@ module Multipliable # # @return [ Document ] The document. def mul(factors) - prepare_atomic_operation do |ops| - process_atomic_operations(factors) do |field, value| - factor = value.is_a?(BigDecimal) ? value.to_f : value - current = attributes[field] - new_value = (current || 0) * factor - process_attribute field, new_value if executing_atomically? - attributes[field] = new_value - ops[atomic_attribute_name(field)] = factor - end - { '$mul' => ops } unless ops.empty? + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + dirty = _atomic_dirty_fields_init + ops = {} + + factors.each do |field, value| + access = database_field_name(field) + factor = value.is_a?(BigDecimal) ? value.to_f : value + current = attributes[access] + _mark_dirty_field(dirty, access, current) + attributes[access] = (current || 0) * factor + _track_dirty_field(dirty, access) + ops[atomic_attribute_name(access)] = factor end + + _stage_atomic_update('$mul', ops, dirty: dirty) end end end diff --git a/lib/mongoid/persistable/poppable.rb b/lib/mongoid/persistable/poppable.rb index 3958f4efef..2a0beb4691 100644 --- a/lib/mongoid/persistable/poppable.rb +++ b/lib/mongoid/persistable/poppable.rb @@ -21,14 +21,19 @@ module Poppable # # @return [ Document ] The document. def pop(pops) - prepare_atomic_operation do |ops| - process_atomic_operations(pops) do |field, value| - values = send(field) - (value > 0) ? values.pop : values.shift - ops[atomic_attribute_name(field)] = value - end - { '$pop' => ops } + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + ops = {} + pops.each do |field, value| + access = database_field_name(field) + values = send(access) + (value > 0) ? values.pop : values.shift + remove_change(access) + ops[atomic_attribute_name(access)] = value end + + _stage_atomic_update('$pop', ops) end end end diff --git a/lib/mongoid/persistable/pullable.rb b/lib/mongoid/persistable/pullable.rb index f4fbda0eb4..bec1ab911b 100644 --- a/lib/mongoid/persistable/pullable.rb +++ b/lib/mongoid/persistable/pullable.rb @@ -17,13 +17,18 @@ module Pullable # # @return [ Document ] The document. def pull(pulls) - prepare_atomic_operation do |ops| - process_atomic_operations(pulls) do |field, value| - (send(field) || []).delete(value) - ops[atomic_attribute_name(field)] = value - end - { '$pull' => ops } + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + ops = {} + pulls.each do |field, value| + access = database_field_name(field) + (send(access) || []).delete(value) + remove_change(access) + ops[atomic_attribute_name(access)] = value end + + _stage_atomic_update('$pull', ops) end # Pull multiple values from the provided array fields. @@ -35,14 +40,19 @@ def pull(pulls) # # @return [ Document ] The document. def pull_all(pulls) - prepare_atomic_operation do |ops| - process_atomic_operations(pulls) do |field, value| - existing = send(field) || [] - value.each { |val| existing.delete(val) } - ops[atomic_attribute_name(field)] = value - end - { '$pullAll' => ops } + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + ops = {} + pulls.each do |field, value| + access = database_field_name(field) + existing = send(access) || [] + value.each { |val| existing.delete(val) } + remove_change(access) + ops[atomic_attribute_name(access)] = value end + + _stage_atomic_update('$pullAll', ops) end end end diff --git a/lib/mongoid/persistable/pushable.rb b/lib/mongoid/persistable/pushable.rb index 5ad01b167c..6327c1780a 100644 --- a/lib/mongoid/persistable/pushable.rb +++ b/lib/mongoid/persistable/pushable.rb @@ -16,23 +16,29 @@ module Pushable # # @return [ Document ] The document. def add_to_set(adds) - prepare_atomic_operation do |ops| - process_atomic_operations(adds) do |field, value| - existing = send(field) || attributes[field] - if existing.nil? - attributes[field] = [] - # Read the value out of attributes: - # https://jira.mongodb.org/browse/MONGOID-4874 - existing = attributes[field] - end - values = [ value ].flatten(1) - values.each do |val| - existing.push(val) unless existing.include?(val) - end - ops[atomic_attribute_name(field)] = { '$each' => values } + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + + ops = {} + adds.each do |field, value| + access = database_field_name(field) + existing = send(access) || attributes[access] + if existing.nil? + attributes[access] = [] + # Read the value out of attributes: + # https://jira.mongodb.org/browse/MONGOID-4874 + existing = attributes[access] + end + values = [ value ].flatten(1) + values.each do |val| + existing.push(val) unless existing.include?(val) end - { '$addToSet' => ops } + remove_change(access) + ops[atomic_attribute_name(access)] = { '$each' => values } end + + return self unless persisted? + + _stage_atomic_update('$addToSet', ops) end # Push a single value or multiple values onto arrays. @@ -47,18 +53,24 @@ def add_to_set(adds) # # @return [ Document ] The document. def push(pushes) - prepare_atomic_operation do |ops| - process_atomic_operations(pushes) do |field, value| - existing = send(field) || begin - attributes[field] ||= [] - attributes[field] - end - values = [ value ].flatten(1) - values.each { |val| existing.push(val) } - ops[atomic_attribute_name(field)] = { '$each' => values } + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + + ops = {} + pushes.each do |field, value| + access = database_field_name(field) + existing = send(access) || begin + attributes[access] ||= [] + attributes[access] end - { '$push' => ops } + values = [ value ].flatten(1) + values.each { |val| existing.push(val) } + remove_change(access) + ops[atomic_attribute_name(access)] = { '$each' => values } end + + return self unless persisted? + + _stage_atomic_update('$push', ops) end end end diff --git a/lib/mongoid/persistable/renamable.rb b/lib/mongoid/persistable/renamable.rb index a30c9b5ef3..042fad5b89 100644 --- a/lib/mongoid/persistable/renamable.rb +++ b/lib/mongoid/persistable/renamable.rb @@ -17,19 +17,24 @@ module Renamable # # @return [ Document ] The document. def rename(renames) - prepare_atomic_operation do |ops| - process_atomic_operations(renames) do |old_field, new_field| - new_name = new_field.to_s - if executing_atomically? - process_attribute new_name, attributes[old_field] - process_attribute old_field, nil - else - attributes[new_name] = attributes.delete(old_field) - end - ops[atomic_attribute_name(old_field)] = atomic_attribute_name(new_name) - end - { '$rename' => ops } + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + dirty = _atomic_dirty_fields_init + ops = {} + + renames.each do |old_field, new_field| + old_access = database_field_name(old_field) + new_name = new_field.to_s + _mark_dirty_field(dirty, old_access, attributes[old_access]) + _mark_dirty_field(dirty, new_name, nil) + attributes[new_name] = attributes.delete(old_access) + _track_dirty_field(dirty, old_access) + _track_dirty_field(dirty, new_name) + ops[atomic_attribute_name(old_access)] = atomic_attribute_name(new_name) end + + _stage_atomic_update('$rename', ops, dirty: dirty) end end end diff --git a/lib/mongoid/persistable/savable.rb b/lib/mongoid/persistable/savable.rb index 243debd3ef..e4ec20b880 100644 --- a/lib/mongoid/persistable/savable.rb +++ b/lib/mongoid/persistable/savable.rb @@ -21,7 +21,9 @@ module Savable # @return [ true | false ] True if success, false if not. def save(options = {}) if new_record? - !insert(options).new_record? + result = insert(options) + # staged? handles the case where the insert was deferred via Mongoid.changeset{} + !result.new_record? || result.staged? else update_document(options) end diff --git a/lib/mongoid/persistable/settable.rb b/lib/mongoid/persistable/settable.rb index 7a4928e78c..e1e208f227 100644 --- a/lib/mongoid/persistable/settable.rb +++ b/lib/mongoid/persistable/settable.rb @@ -44,41 +44,51 @@ module Settable # # @return [ Document ] The document. def set(setters) - prepare_atomic_operation do |ops| - process_atomic_operations(setters) do |field, value| - field_seq = field.to_s.split('.') - field = field_seq.shift - if field_seq.length > 0 - # nested hash path - old_value = attributes[field] + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? - # if the old value is not a hash, clobber it - old_value = {} unless old_value.is_a?(Hash) + dirty = _atomic_dirty_fields_init + ops = {} - # descend into the hash, creating intermediate keys as needed - cur_value = old_value - while field_seq.length > 1 - cur_key = field_seq.shift - # clobber on each level if type is not a hash - cur_value[cur_key] = {} unless cur_value[cur_key].is_a?(Hash) - cur_value = cur_value[cur_key] - end - - # now we are on the leaf level, perform the set - # and overwrite whatever was on this level before - cur_value[field_seq.shift] = value + setters.each do |field, value| + access = database_field_name(field) + field_seq = access.to_s.split('.') + top_field = field_seq.shift + value = _set_nested(top_field, field_seq, value) if field_seq.length > 0 + process_attribute(top_field, value) + _track_dirty_field(dirty, top_field) + ops[atomic_attribute_name(top_field)] = attributes[top_field] unless relations.include?(top_field) + end - # and set value to the value of the top level field - # because this is what we pass to $set - value = old_value - end + _stage_atomic_update('$set', ops, dirty: dirty) + end - process_attribute(field, value) + private - ops[atomic_attribute_name(field)] = attributes[field] unless relations.include?(field.to_s) - end - { '$set' => ops } unless ops.empty? + # Build the nested-hash value for a dotted field path. + # + # Descends into the top-level field's current hash value (creating + # intermediate keys as needed), sets the leaf, and returns the updated + # top-level hash to be written back via process_attribute. + # + # @api private + # + # @param [ String ] top_field The top-level attribute name. + # @param [ Array ] field_seq Remaining path segments (without top). + # @param [ Object ] value The leaf value to assign. + # + # @return [ Hash ] The updated top-level hash value. + def _set_nested(top_field, field_seq, value) + old_value = attributes[top_field] + old_value = {} unless old_value.is_a?(Hash) + cur_value = old_value + while field_seq.length > 1 + cur_key = field_seq.shift + cur_value[cur_key] = {} unless cur_value[cur_key].is_a?(Hash) + cur_value = cur_value[cur_key] end + cur_value[field_seq.shift] = value + old_value end end end diff --git a/lib/mongoid/persistable/unsettable.rb b/lib/mongoid/persistable/unsettable.rb index f6be80e465..9634bd8fe3 100644 --- a/lib/mongoid/persistable/unsettable.rb +++ b/lib/mongoid/persistable/unsettable.rb @@ -17,18 +17,20 @@ module Unsettable # # @return [ Document ] The document. def unset(*fields) - prepare_atomic_operation do |ops| - fields.flatten.each do |field| - normalized = database_field_name(field) - if executing_atomically? - process_attribute normalized, nil - else - attributes.delete(normalized) - end - ops[atomic_attribute_name(normalized)] = true - end - { '$unset' => ops } + raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly + return self unless persisted? + + dirty = _atomic_dirty_fields_init + ops = {} + + fields.flatten.each do |field| + normalized = database_field_name(field) + process_attribute normalized, nil + _track_dirty_field(dirty, normalized) + ops[atomic_attribute_name(normalized)] = true end + + _stage_atomic_update('$unset', ops, dirty: dirty) end end end diff --git a/lib/mongoid/persistable/updatable.rb b/lib/mongoid/persistable/updatable.rb index b9a7b65c68..d0a69ef259 100644 --- a/lib/mongoid/persistable/updatable.rb +++ b/lib/mongoid/persistable/updatable.rb @@ -104,11 +104,13 @@ def prepare_update(options = {}) process_flagged_destroys update_children = cascadable_children(:update) process_touch_option(options, update_children) do - run_all_callbacks_for_update(update_children) do - result = yield(self) - self.previously_new_record = false - post_process_persist(result, options) - true + Mongoid.changeset do + run_all_callbacks_for_update(update_children) do + result = yield(self) + self.previously_new_record = false + post_process_persist(result, options) + true + end end end end @@ -127,31 +129,31 @@ def update_document(options = {}) prepare_update(options) do updates, conflicts = init_atomic_updates unless updates.empty? - coll = collection(_root) selector = atomic_selector - - # TODO: DRIVERS-716: If a new "Bulk Write" API is introduced, it may - # become possible to handle the writes for conflicts in the following call. - coll.find(selector).update_one(positionally(selector, updates), session: _session) - - # The following code applies updates which would cause - # path conflicts in MongoDB, for example when changing attributes - # of foo.0.bars while adding another foo. Each conflicting update - # is applied using its own write. + payload = positionally(selector, updates) + + Threaded.current_changeset.add( + type: :update, + collection: collection(_root), + selector: selector, + payload: payload, + document: self, + session: _session + ) + + # Conflict updates are applied in separate entries to avoid MongoDB + # path conflicts (e.g. writing foo.0.bars while also adding a new foo). + # Changes are grouped by root key and popped round-robin so each + # batch contains at most one change per conflicting path. See MONGOID-4982. conflicts.each_pair do |modifier, changes| - # Group the changes according to their root key which is - # the top-level association name. - # This handles at least the cases described in MONGOID-4982. - conflicting_change_groups = changes.group_by do |key, _| - key.split('.', 2).first - end.values - - # Apply changes in batches. Pop one change from each - # field-conflict group round-robin until all changes - # have been applied. - while batched_changes = conflicting_change_groups.map(&:pop).compact.to_h.presence - coll.find(selector).update_one( - positionally(selector, modifier => batched_changes), + conflicting_change_groups = changes.group_by { |key, _| key.split('.', 2).first }.values + while batched = conflicting_change_groups.map(&:pop).compact.to_h.presence + Threaded.current_changeset.add( + type: :update, + collection: collection(_root), + selector: selector, + payload: positionally(selector, modifier => batched), + document: self, session: _session ) end @@ -209,17 +211,15 @@ def enforce_immutability_of_id_field! # @param [ Array ] update_children The children that the # :update callbacks will be executed on. def run_all_callbacks_for_update(update_children, &block) - run_callbacks(:commit, with_children: true, skip_if: -> { in_transaction? }) do - run_callbacks(:save, with_children: false) do - run_callbacks(:update, with_children: false) do - run_callbacks(:persist_parent, with_children: false) do - _mongoid_run_child_callbacks(:save) do - _mongoid_run_child_callbacks(:update, children: update_children, &block) # _mongoid_run_child_callbacks :update - end # _mongoid_run_child_callbacks :save - end # :persist_parent - end # :update - end # :save - end # :commit + run_callbacks(:save, with_children: false) do + run_callbacks(:update, with_children: false) do + run_callbacks(:persist_parent, with_children: false) do + _mongoid_run_child_callbacks(:save) do + _mongoid_run_child_callbacks(:update, children: update_children, &block) + end + end + end + end end end end diff --git a/lib/mongoid/persistable/upsertable.rb b/lib/mongoid/persistable/upsertable.rb index e80315cf5c..95280682c9 100644 --- a/lib/mongoid/persistable/upsertable.rb +++ b/lib/mongoid/persistable/upsertable.rb @@ -32,25 +32,41 @@ module Upsertable # @return [ true ] True. def upsert(options = {}) prepare_upsert(options) do - if options[:replace] - raise ArgumentError, 'cannot specify :set_on_insert with `replace: true`' if options[:set_on_insert] + raise ArgumentError, 'cannot specify :set_on_insert with `replace: true`' if options[:replace] && options[:set_on_insert] - collection.find(atomic_selector).replace_one( - as_attributes, upsert: true, session: _session - ) - else - attrs = { '$set' => as_attributes } - attrs['$setOnInsert'] = options[:set_on_insert] if options[:set_on_insert] - - collection.find(atomic_selector).update_one( - attrs, upsert: true, session: _session - ) - end + _stage_upsert(options) end end private + # Stage the upsert entry on the current changeset. + # + # @api private + def _stage_upsert(options) + if options[:replace] + Threaded.current_changeset.add( + type: :upsert_replace, + collection: collection, + selector: atomic_selector, + payload: as_attributes, + document: self, + session: _session + ) + else + attrs = { '$set' => as_attributes } + attrs['$setOnInsert'] = options[:set_on_insert] if options[:set_on_insert] + Threaded.current_changeset.add( + type: :upsert, + collection: collection, + selector: atomic_selector, + payload: attrs, + document: self, + session: _session + ) + end + end + # Prepare the upsert for execution. # # @api private @@ -69,12 +85,13 @@ def prepare_upsert(options = {}) raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly return false if performing_validations?(options) && invalid?(:upsert) - result = run_callbacks(:upsert) do - yield(self) - true + Mongoid.changeset do + run_callbacks(:upsert) do + yield(self) + post_process_persist(true, options) + true + end end - self.new_record = false - post_process_persist(result, options) and result end end end diff --git a/lib/mongoid/stateful.rb b/lib/mongoid/stateful.rb index ed3d681a86..b1a016a906 100644 --- a/lib/mongoid/stateful.rb +++ b/lib/mongoid/stateful.rb @@ -147,6 +147,32 @@ def updateable? persisted? && changed? end + # Returns true if the document has at least one staged Entry in the current + # active changeset. Returns false when no changeset is active or after the + # changeset has flushed. + # + # @return [ true | false ] Whether this document has staged entries. + def staged? + cs = Threaded.current_changeset + return false unless cs && !cs.terminated? + + cs.entries.any? { |e| e.document.equal?(self) } + end + + # Returns all Entry objects currently staged for this document in the active + # changeset, in registration order. Returns an empty array when no changeset + # is active or after it has flushed. + # + # The returned entries are live objects from the changeset — do not mutate them. + # + # @return [ Array ] The staged entries. + def staged + cs = Threaded.current_changeset + return [] unless cs && !cs.terminated? + + cs.entries.select { |e| e.document.equal?(self) } + end + private def reset_readonly diff --git a/lib/mongoid/threaded.rb b/lib/mongoid/threaded.rb index e391f8258f..c1c34447ab 100644 --- a/lib/mongoid/threaded.rb +++ b/lib/mongoid/threaded.rb @@ -28,6 +28,8 @@ module Threaded TOUCH_MERGED_KEY = 'touch-merged' + CURRENT_CHANGESET_KEY = 'current-changeset' + STACK_KEYS = Hash.new do |hash, key| hash[key] = "#{key}-stack" end @@ -296,6 +298,28 @@ def client_override=(name) set(CLIENT_OVERRIDE_KEY, name) end + # Get the current changeset for this thread or fiber. + # + # @example Get the current changeset. + # Threaded.current_changeset + # + # @return [ Mongoid::Changeset | nil ] The active changeset, or nil. + def current_changeset + get(CURRENT_CHANGESET_KEY) + end + + # Set (or clear) the current changeset for this thread or fiber. + # + # @example Set the current changeset. + # Threaded.current_changeset = changeset + # + # @param [ Mongoid::Changeset | nil ] changeset The changeset to set. + # + # @return [ Mongoid::Changeset | nil ] The changeset. + def current_changeset=(changeset) + set(CURRENT_CHANGESET_KEY, changeset) + end + # Get the current Mongoid scope. # # @example Get the scope. diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 7b9d739667..2114f7bea9 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -212,7 +212,7 @@ def define_relation_touch_method(name, association) define_method(method_name) do without_autobuild do # If the touch updates were already merged into an atomic - # insert (see Persistable::Creatable#insert_as_embedded), + # insert (see Persistable::Creatable#_stage_insert_as_embedded), # skip the redundant persistence but still run :touch # callbacks and clean up dirty tracking. if Threaded.touch_merged?(self) diff --git a/lib/mongoid/traversable.rb b/lib/mongoid/traversable.rb index cbdcdf00d8..7588195faa 100644 --- a/lib/mongoid/traversable.rb +++ b/lib/mongoid/traversable.rb @@ -211,7 +211,7 @@ class << self # @api private def self.add_discriminator_mapping(value, klass = self) self.discriminator_mapping ||= {} - self.discriminator_mapping[value] = klass + discriminator_mapping[value] = klass superclass.add_discriminator_mapping(value, klass) if hereditary? end @@ -225,7 +225,7 @@ def self.add_discriminator_mapping(value, klass = self) # # @api private def self.get_discriminator_mapping(value) - self.discriminator_mapping[value] if self.discriminator_mapping + discriminator_mapping[value] if discriminator_mapping end end diff --git a/lib/mongoid/warnings.rb b/lib/mongoid/warnings.rb index afa88a5801..115327b6e0 100644 --- a/lib/mongoid/warnings.rb +++ b/lib/mongoid/warnings.rb @@ -36,5 +36,8 @@ def warning(id, message) warning :autosave_saves_unchanged_documents, "Autosave associations are currently configured to save documents even if they haven't changed. " \ 'This legacy behavior is deprecated. Set Mongoid.autosave_saves_unchanged_documents to false to ' \ 'skip saving unchanged documents in autosave associations.' + warning :join_context_false_deprecated, + 'atomically(join_context: false) is deprecated. ' \ + 'Use save outside any enclosing changeset scope instead.' end end diff --git a/spec/integration/associations/embeds_many_spec.rb b/spec/integration/associations/embeds_many_spec.rb index 46d8238f05..11b30b8f34 100644 --- a/spec/integration/associations/embeds_many_spec.rb +++ b/spec/integration/associations/embeds_many_spec.rb @@ -329,6 +329,56 @@ class Comment end end + context 'inside a Mongoid.changeset block' do + let!(:person) { Person.create! } + + context 'batch_insert (via push)' do + it 'defers the insert until the block exits' do + Mongoid.changeset do + person.addresses.push(Address.new(street: 'Main St')) + expect(person.reload.addresses.count).to eq(0) + end + expect(person.reload.addresses.count).to eq(1) + end + end + + context 'batch_clear (via clear)' do + before { person.addresses.create!(street: 'Main St') } + + it 'defers the clear until the block exits' do + Mongoid.changeset do + person.addresses.clear + expect(person.reload.addresses.count).to eq(1) + end + expect(person.reload.addresses.count).to eq(0) + end + end + + context 'batch_remove (via delete_all)' do + before { person.addresses.create!(street: 'Main St') } + + it 'defers the removal until the block exits' do + Mongoid.changeset do + person.addresses.delete_all + expect(person.reload.addresses.count).to eq(1) + end + expect(person.reload.addresses.count).to eq(0) + end + end + + context 'batch_replace (via assignment)' do + before { person.addresses.create!(street: 'Old St') } + + it 'defers the replace until the block exits' do + Mongoid.changeset do + person.addresses = [ Address.new(street: 'New St') ] + expect(person.reload.addresses.map(&:street)).to eq([ 'Old St' ]) + end + expect(person.reload.addresses.map(&:street)).to eq([ 'New St' ]) + end + end + end + context 'when a hash is provided instead of an array for an embeds_many association' do let(:post) { EmbedsManySpec::Post.new(title: 'Broken post', comments: { content: 'Comment' }) } diff --git a/spec/mongoid/association/referenced/has_and_belongs_to_many/proxy_spec.rb b/spec/mongoid/association/referenced/has_and_belongs_to_many/proxy_spec.rb index 5bfef18b28..470ed31781 100644 --- a/spec/mongoid/association/referenced/has_and_belongs_to_many/proxy_spec.rb +++ b/spec/mongoid/association/referenced/has_and_belongs_to_many/proxy_spec.rb @@ -3542,4 +3542,28 @@ class Distributor expect(contract.signature_ids).to eq([ signature.id ]) end end + + describe 'HABTM changeset staging' do + let!(:person) { Person.create! } + let!(:pref1) { Preference.create!(name: 'Dark Mode') } + let(:pref2) { Preference.create!(name: 'Notifications') } + + it 'persists the foreign key on the base document' do + person.preferences << pref1 + expect(person.reload.preference_ids).to include(pref1.id) + end + + it 'stages the base document when inside a changeset' do + Mongoid.changeset do + person.preferences << pref1 + expect(person).to be_staged + end + expect(person.reload.preference_ids).to include(pref1.id) + end + + it 'raises when appending an invalid document' do + invalid_pref = Preference.new(name: 'x') # length 1, fails validates_length_of minimum: 2 + expect { person.preferences << invalid_pref }.to raise_error(Mongoid::Errors::Validations) + end + end end diff --git a/spec/mongoid/association/referenced/has_many_spec.rb b/spec/mongoid/association/referenced/has_many_spec.rb index 2ae3b7c551..b24f10ef16 100644 --- a/spec/mongoid/association/referenced/has_many_spec.rb +++ b/spec/mongoid/association/referenced/has_many_spec.rb @@ -1127,4 +1127,34 @@ class OtherBelongingObject; end expect(student.updated_at).to eq(update_time) end end + + describe 'validation on append (<<)', :integration do + let!(:person) { Person.create! } + + context 'when appending an invalid document' do + let(:post) { Post.new(title: '$$$invalid') } + + it 'raises Errors::Validations' do + expect { person.posts << post }.to raise_error(Mongoid::Errors::Validations) + end + + it 'does not persist the invalid document' do + begin + person.posts << post + rescue Mongoid::Errors::Validations + nil + end + expect(Post.where(person_id: person.id).first).to be_nil + end + end + + context 'when appending a valid document' do + let(:post) { Post.new(title: 'valid title') } + + it 'persists the document' do + person.posts << post + expect(Post.where(person_id: person.id).first).not_to be_nil + end + end + end end diff --git a/spec/mongoid/association/referenced/has_one_spec.rb b/spec/mongoid/association/referenced/has_one_spec.rb index 8b25445ab2..a7fa697af5 100644 --- a/spec/mongoid/association/referenced/has_one_spec.rb +++ b/spec/mongoid/association/referenced/has_one_spec.rb @@ -1233,4 +1233,34 @@ class OwnerClass expect(association.create_relation(owner, target)).to be_a(BelongingObject) end end + + describe 'validation on assignment', :integration do + let!(:person) { Person.create! } + + context 'when the target fails validation' do + let(:game) { Game.new(name: '$$$invalid') } + + it 'raises Errors::Validations when assigning an invalid target' do + expect { person.game = game }.to raise_error(Mongoid::Errors::Validations) + end + + it 'does not persist the invalid document' do + begin + person.game = game + rescue Mongoid::Errors::Validations + nil + end + expect(Game.where(person_id: person.id).first).to be_nil + end + end + + context 'when the target is valid' do + let(:game) { Game.new } + + it 'persists the target' do + person.game = game + expect(Game.where(person_id: person.id).first).not_to be_nil + end + end + end end diff --git a/spec/mongoid/attributes/nested_spec.rb b/spec/mongoid/attributes/nested_spec.rb index bf07231822..864c64137b 100644 --- a/spec/mongoid/attributes/nested_spec.rb +++ b/spec/mongoid/attributes/nested_spec.rb @@ -2717,6 +2717,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => post_one.id, 'title' => 'First' }, '1' => { 'id' => post_two.id, 'title' => 'Second' } } + person.save end context 'when reloading the document' do @@ -2802,6 +2803,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => post_one.id, '_destroy' => truth }, '1' => { 'id' => post_two.id, 'title' => 'My Blog' } } + person.save end context 'when reloading the documents' do @@ -2824,6 +2826,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => post_one.id, '_destroy' => falsehood }, '1' => { 'id' => post_two.id, 'title' => 'My Blog' } } + person.save end context 'when reloading the document' do @@ -2860,6 +2863,7 @@ class BandWithAllowDestroyedRecords < Band }, '1' => { 'id' => post_two.id, 'title' => 'New Title' } } + person.save end context 'when reloading the document' do @@ -2886,6 +2890,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => post_one.id, '_destroy' => falsehood }, '1' => { 'id' => post_two.id, 'title' => 'New Title' } } + person.save end context 'when reloading the documents' do @@ -2924,6 +2929,7 @@ class BandWithAllowDestroyedRecords < Band }, '1' => { 'id' => post_two.id, 'title' => 'New Title' } } + person.save end context 'when reloading' do @@ -2950,6 +2956,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => post_one.id, '_destroy' => falsehood }, '1' => { 'id' => post_two.id, 'title' => 'New Title' } } + person.save end context 'when reloading' do @@ -3408,6 +3415,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => preference_one.id, 'name' => 'First' }, '1' => { 'id' => preference_two.id, 'name' => 'Second' } } + person.save end context 'when reloading the document' do @@ -3457,6 +3465,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => preference_one.id, '_destroy' => truth }, '1' => { 'id' => preference_two.id, 'name' => 'My Blog' } } + person.save end context 'when reloading the documents' do @@ -3479,6 +3488,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => preference_one.id, '_destroy' => falsehood }, '1' => { 'id' => preference_two.id, 'name' => 'My Blog' } } + person.save end context 'when reloading the document' do @@ -3515,6 +3525,7 @@ class BandWithAllowDestroyedRecords < Band }, '1' => { 'id' => preference_two.id, 'name' => 'New Title' } } + person.save end context 'when reloading the document' do @@ -3541,6 +3552,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => preference_one.id, '_destroy' => falsehood }, '1' => { 'id' => preference_two.id, 'name' => 'New Title' } } + person.save end context 'when reloading the documents' do @@ -3572,6 +3584,7 @@ class BandWithAllowDestroyedRecords < Band }, '1' => { 'id' => preference_two.id, 'name' => 'New Title' } } + person.save end context 'when reloading' do @@ -3598,6 +3611,7 @@ class BandWithAllowDestroyedRecords < Band '0' => { 'id' => preference_one.id, '_destroy' => falsehood }, '1' => { 'id' => preference_two.id, 'name' => 'New Title' } } + person.save end context 'when reloading' do @@ -4552,4 +4566,78 @@ class BandWithAllowDestroyedRecords < Band end end end + + # MONGOID-5911: nested attribute updates on referenced has_many associations + # must not write to the database prematurely. All writes must be deferred + # until the parent is saved, so that a parent validation failure prevents + # any child writes from reaching the database. + describe 'persistence of referenced has_many nested attribute updates', :integration do + let!(:node) { Node.create! } + let!(:server) { node.servers.create!(name: 'original') } + + describe 'direct setter (servers_attributes=)' do + before { node.servers_attributes = [ { _id: server.id, name: 'updated' } ] } + + it 'does not immediately persist the child update' do + expect(Server.find(server.id).name).to eq('original') + end + + it 'persists the child update when the parent is subsequently saved' do + node.save! + expect(Server.find(server.id).name).to eq('updated') + end + end + + describe 'parent#update with nested attributes' do + context 'when the child attributes are valid' do + it 'returns true' do + result = node.update(servers_attributes: [ { _id: server.id, name: 'updated' } ]) + expect(result).to be true + end + + it 'persists the child update' do + node.update(servers_attributes: [ { _id: server.id, name: 'updated' } ]) + expect(Server.find(server.id).name).to eq('updated') + end + end + + context 'when the child attributes are invalid' do + it 'returns false' do + result = node.update(servers_attributes: [ { _id: server.id, name: '' } ]) + expect(result).to be false + end + + it 'does not persist the invalid child state' do + node.update(servers_attributes: [ { _id: server.id, name: '' } ]) + expect(Server.find(server.id).name).to eq('original') + end + + it 'records validation errors on the parent' do + node.update(servers_attributes: [ { _id: server.id, name: '' } ]) + expect(node.errors).not_to be_empty + end + end + + context 'when the parent is invalid but the child attributes would be valid' do + let!(:parent) { NestedValidatedParent.create!(status: 'active') } + let!(:item) { parent.labeled_items.create!(label: 'original') } + + it 'does not persist the child update' do + parent.update( + status: 'invalid_status', + labeled_items_attributes: [ { _id: item.id, label: 'updated' } ] + ) + expect(NestedLabeledItem.find(item.id).label).to eq('original') + end + + it 'returns false' do + result = parent.update( + status: 'invalid_status', + labeled_items_attributes: [ { _id: item.id, label: 'updated' } ] + ) + expect(result).to be false + end + end + end + end end diff --git a/spec/mongoid/attributes/nested_spec_models.rb b/spec/mongoid/attributes/nested_spec_models.rb index 94b7b91f99..472d3434dc 100644 --- a/spec/mongoid/attributes/nested_spec_models.rb +++ b/spec/mongoid/attributes/nested_spec_models.rb @@ -46,3 +46,21 @@ class NestedPage field :number, type: Integer embedded_in :book, class_name: 'NestedBook' end + +# Models for testing MONGOID-5911: no partial or premature writes via nested +# attributes on referenced associations. +class NestedValidatedParent + include Mongoid::Document + + field :status, type: String + has_many :labeled_items, class_name: 'NestedLabeledItem' + accepts_nested_attributes_for :labeled_items + validates :status, inclusion: { in: %w[active inactive] } +end + +class NestedLabeledItem + include Mongoid::Document + + field :label, type: String + belongs_to :nested_validated_parent +end diff --git a/spec/mongoid/changeset/entry_spec.rb b/spec/mongoid/changeset/entry_spec.rb new file mode 100644 index 0000000000..afdb309f7d --- /dev/null +++ b/spec/mongoid/changeset/entry_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Mongoid::Changeset::Entry do + subject(:entry) do + described_class.new( + type: :update, + collection: collection, + selector: selector, + payload: payload, + document: document, + session: nil + ) + end + + let(:collection) { instance_double(Mongo::Collection) } + let(:selector) { { '_id' => BSON::ObjectId.new } } + let(:payload) { { '$set' => { 'name' => 'Tool' } } } + let(:document) { instance_double(Mongoid::Document) } + + it 'exposes type' do + expect(entry.type).to eq(:update) + end + + it 'exposes collection' do + expect(entry.collection).to eq(collection) + end + + it 'exposes selector' do + expect(entry.selector).to eq(selector) + end + + it 'exposes payload' do + expect(entry.payload).to eq(payload) + end + + it 'exposes document' do + expect(entry.document).to eq(document) + end + + it 'exposes session' do + expect(entry.session).to be_nil + end + + context 'when document is nil (criteria-level entry)' do + subject(:entry) do + described_class.new( + type: :update_many, + collection: collection, + selector: { 'active' => true }, + payload: { '$set' => { 'archived' => true } }, + document: nil, + session: nil + ) + end + + it 'allows nil document' do + expect(entry.document).to be_nil + end + end +end diff --git a/spec/mongoid/changeset_spec.rb b/spec/mongoid/changeset_spec.rb new file mode 100644 index 0000000000..3a39864b72 --- /dev/null +++ b/spec/mongoid/changeset_spec.rb @@ -0,0 +1,556 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Mongoid::Changeset do + subject(:cs) { described_class.new } + + describe '#initialize' do + it 'starts with no entries' do + expect(cs.entries).to be_empty + end + + it 'starts at depth zero' do + expect(cs.depth).to eq(0) + end + + it 'is not terminated' do + expect(cs).not_to be_terminated + end + + it 'is not in atomically context' do + expect(cs).not_to be_atomically_context + end + end + + describe '#enter_atomically_context / #exit_atomically_context' do + it 'sets atomically_context when entered' do + cs.enter_atomically_context + expect(cs).to be_atomically_context + end + + it 'clears atomically_context when exited' do + cs.enter_atomically_context + cs.exit_atomically_context + expect(cs).not_to be_atomically_context + end + end + + describe '#add_entry' do + let(:entry) do + Mongoid::Changeset::Entry.new( + type: :update, collection: instance_double(Mongo::Collection), selector: {}, payload: {}, document: nil, session: nil + ) + end + + it 'appends the entry' do + cs.add_entry(entry) + expect(cs.entries).to eq([ entry ]) + end + + context 'when terminated' do + before { cs.discard } + + it 'raises' do + expect { cs.add_entry(entry) }.to raise_error(Mongoid::Errors::InvalidChangesetOperation) + end + end + end + + describe '#add' do + let(:coll) { instance_double(Mongo::Collection) } + + it 'constructs an Entry from keyword arguments and appends it' do + cs.add(type: :update, collection: coll, selector: {}, payload: {}, document: nil, session: nil) + expect(cs.entries.size).to eq(1) + expect(cs.entries.first).to be_a(Mongoid::Changeset::Entry) + expect(cs.entries.first.type).to eq(:update) + end + + it 'raises when terminated' do + cs.discard + expect do + cs.add(type: :update, collection: coll, selector: {}, payload: {}, document: nil, session: nil) + end.to raise_error(Mongoid::Errors::InvalidChangesetOperation) + end + end + + describe '#build (nesting)' do + it 'increments depth inside the block' do + cs.build { expect(cs.depth).to eq(1) } + end + + it 'returns depth to zero after the block' do + cs.build { nil } + expect(cs.depth).to eq(0) + end + + it 'nests correctly' do + cs.build do + cs.build { expect(cs.depth).to eq(2) } + expect(cs.depth).to eq(1) + end + expect(cs.depth).to eq(0) + end + + it 'restores depth on exception' do + begin + cs.build { raise 'oops' } + rescue StandardError + nil + end + expect(cs.depth).to eq(0) + end + end + + describe '#discard' do + before do + entry = Mongoid::Changeset::Entry.new( + type: :insert, collection: instance_double(Mongo::Collection), selector: {}, payload: {}, document: nil, session: nil + ) + cs.add_entry(entry) + end + + it 'clears entries' do + cs.discard + expect(cs.entries).to be_empty + end + + it 'marks as terminated' do + cs.discard + expect(cs).to be_terminated + end + + context 'when already terminated' do + before { cs.discard } + + it 'raises' do + expect { cs.discard }.to raise_error(Mongoid::Errors::InvalidChangesetOperation) + end + end + end + + describe '#terminated?' do + it 'is false initially' do + expect(cs).not_to be_terminated + end + + it 'is true after discard' do + cs.discard + expect(cs).to be_terminated + end + end + + describe '#run' do + it 'raises when terminated' do + cs.discard + expect { cs.run { nil } }.to raise_error(Mongoid::Errors::InvalidChangesetOperation) + end + + it 'discards on error at outermost depth' do + expect { cs.run { raise 'boom' } }.to raise_error(RuntimeError, 'boom') + expect(cs).to be_terminated + end + + it 'does not discard on error when nested (inner scope)' do + cs.run do + cs.build do + raise 'inner error' + rescue RuntimeError + # swallowed + end + expect(cs).not_to be_terminated + end + end + + it 'returns the block return value' do + val = cs.run { 42 } + expect(val).to eq(42) + end + end + + describe 'Mongoid.changeset' do + after { Mongoid::Threaded.current_changeset = nil } + + it 'creates a new changeset if none is active' do + cs = nil + Mongoid.changeset { |c| cs = c } + expect(cs).to be_a(Mongoid::Changeset) + end + + it 'reuses an existing changeset when nested' do + outer_cs = nil + inner_cs = nil + Mongoid.changeset do |c| + outer_cs = c + Mongoid.changeset { |ic| inner_cs = ic } + end + expect(inner_cs).to equal(outer_cs) + end + + it 'clears the changeset after the block exits' do + Mongoid.changeset { nil } + expect(Mongoid::Threaded.current_changeset).to be_nil + end + + it 'clears the changeset after a block error' do + begin + Mongoid.changeset { raise 'boom' } + rescue StandardError + nil + end + expect(Mongoid::Threaded.current_changeset).to be_nil + end + + it 'yields the changeset to the block' do + captured = nil + Mongoid.changeset { |cs| captured = cs } + expect(captured).to be_a(Mongoid::Changeset) + end + end + + describe '#flush (unit)' do + let(:klass) do + Class.new do + include Mongoid::Document + + store_in collection: 'changeset_flush_unit_test' + field :name, type: String + end + end + + let(:coll) { instance_double(Mongo::Collection) } + let(:selector) { { '_id' => BSON::ObjectId.new } } + let(:payload) { { 'name' => 'Alice' } } + + def make_entry(type:, **opts) + Mongoid::Changeset::Entry.new( + type: type, + collection: opts.fetch(:collection, coll), + selector: opts.fetch(:sel, selector), + payload: opts.fetch(:pay, payload), + document: opts[:doc], + session: opts[:session], + skip_callbacks: opts[:skip_callbacks], + dirty_fields: opts[:dirty_fields] + ) + end + + context 'with a single :insert entry' do + it 'calls insert_one on the collection' do + allow(coll).to receive(:insert_one) + cs.add_entry(make_entry(type: :insert)) + cs.flush + expect(coll).to have_received(:insert_one).with(payload) + end + end + + context 'with a single :update entry' do + it 'calls find(selector).update_one(payload)' do + view = instance_double(Mongo::Collection::View) + allow(coll).to receive(:find).with(selector).and_return(view) + allow(view).to receive(:update_one) + cs.add_entry(make_entry(type: :update)) + cs.flush + expect(view).to have_received(:update_one).with(payload) + end + end + + context 'with a single :delete entry' do + it 'calls find(selector).delete_one' do + view = instance_double(Mongo::Collection::View) + allow(coll).to receive(:find).with(selector).and_return(view) + allow(view).to receive(:delete_one) + cs.add_entry(make_entry(type: :delete, pay: nil)) + cs.flush + expect(view).to have_received(:delete_one) + end + end + + context 'with two consecutive same-collection entries' do + # Both entries use the same `coll` object; a single Ruby object is equal + # to itself by both `==` and `equal?`. The implementation uses `==` + # (value equality), which is what matters in production where + # `client[name]` returns a fresh Collection object on every call. + it 'calls bulk_write once' do + allow(coll).to receive(:bulk_write) + cs.add_entry(make_entry(type: :insert, pay: { 'name' => 'Alice' })) + cs.add_entry(make_entry(type: :insert, pay: { 'name' => 'Bob' })) + cs.flush + expect(coll).to have_received(:bulk_write).once + end + + it 'does not call insert_one' do + allow(coll).to receive(:bulk_write) + allow(coll).to receive(:insert_one) + cs.add_entry(make_entry(type: :insert, pay: { 'name' => 'Alice' })) + cs.add_entry(make_entry(type: :insert, pay: { 'name' => 'Bob' })) + cs.flush + expect(coll).not_to have_received(:insert_one) + end + end + + context 'with entries from two different collections' do + let(:coll2) { instance_double(Mongo::Collection) } + + it 'makes two separate driver calls, not a cross-collection bulk_write' do + allow(coll).to receive(:insert_one) + allow(coll2).to receive(:insert_one) + cs.add_entry(make_entry(type: :insert, collection: coll)) + cs.add_entry(make_entry(type: :insert, collection: coll2)) + cs.flush + expect(coll).to have_received(:insert_one).once + expect(coll2).to have_received(:insert_one).once + end + + it 'does not call bulk_write' do + allow(coll).to receive(:insert_one) + allow(coll).to receive(:bulk_write) + allow(coll2).to receive(:insert_one) + allow(coll2).to receive(:bulk_write) + cs.add_entry(make_entry(type: :insert, collection: coll)) + cs.add_entry(make_entry(type: :insert, collection: coll2)) + cs.flush + expect(coll).not_to have_received(:bulk_write) + expect(coll2).not_to have_received(:bulk_write) + end + end + + context 'with same-collection entries carrying different sessions' do + let(:session1) { instance_double(Mongo::Session) } + let(:session2) { instance_double(Mongo::Session) } + + it 'makes two separate driver calls rather than one bulk_write' do + allow(coll).to receive(:insert_one) + cs.add_entry(make_entry(type: :insert, pay: { 'name' => 'Alice' }, session: session1)) + cs.add_entry(make_entry(type: :insert, pay: { 'name' => 'Bob' }, session: session2)) + cs.flush + expect(coll).to have_received(:insert_one).twice + end + + it 'does not bulk_write across the session boundary' do + allow(coll).to receive(:insert_one) + allow(coll).to receive(:bulk_write) + cs.add_entry(make_entry(type: :insert, pay: { 'name' => 'Alice' }, session: session1)) + cs.add_entry(make_entry(type: :insert, pay: { 'name' => 'Bob' }, session: session2)) + cs.flush + expect(coll).not_to have_received(:bulk_write) + end + end + + context 'before_flush callback' do + it 'fires before the driver call' do + log = [] + doc = klass.new(name: 'Alice') + klass.before_flush { log << :callback } + allow(coll).to receive(:insert_one) { log << :driver } + cs.add_entry(make_entry(type: :insert, doc: doc)) + cs.flush + expect(log).to eq(%i[callback driver]) + end + end + + context 'after_flush callback' do + it 'fires after the driver call' do + log = [] + doc = klass.new(name: 'Alice') + klass.after_flush { log << :callback } + allow(coll).to receive(:insert_one) { log << :driver } + cs.add_entry(make_entry(type: :insert, doc: doc)) + cs.flush + expect(log).to eq(%i[driver callback]) + end + end + + context 'on driver error' do + it 'propagates the exception' do + allow(coll).to receive(:insert_one).and_raise(Mongo::Error::OperationFailure.new('write failed')) + cs.add_entry(make_entry(type: :insert)) + expect { cs.flush }.to raise_error(Mongo::Error::OperationFailure) + end + + it 'marks the changeset terminated even when the flush is aborted by an error' do + allow(coll).to receive(:insert_one).and_raise(Mongo::Error::OperationFailure.new('write failed')) + cs.add_entry(make_entry(type: :insert)) + begin + cs.flush + rescue Mongo::Error::OperationFailure + nil + end + expect(cs).to be_terminated + end + end + + context 'skip_callbacks flag' do + let(:doc) { klass.new(name: 'Alice') } + let(:view) { instance_double(Mongo::Collection::View) } + + before do + allow(coll).to receive(:find).and_return(view) + allow(view).to receive(:update_one) + allow(coll).to receive(:bulk_write) + end + + context 'when a single entry has skip_callbacks: true' do + before { cs.add_entry(make_entry(type: :update, doc: doc, skip_callbacks: true)) } + + it 'does not fire before_flush' do + fired = false + klass.before_flush { fired = true } + cs.flush + expect(fired).to be(false) + end + + it 'does not fire after_flush' do + fired = false + klass.after_flush { fired = true } + cs.flush + expect(fired).to be(false) + end + + it 'does not fire after_commit' do + fired = false + klass.after_commit { fired = true } + cs.flush + expect(fired).to be(false) + end + + context 'when the entry has a session that is in a transaction' do + let(:session) { instance_double(Mongo::Session, in_transaction?: true) } + + before do + cs.entries.clear + cs.add_entry(make_entry(type: :update, doc: doc, session: session, skip_callbacks: true)) + allow(Mongoid::Threaded).to receive(:add_modified_document) + end + + it 'still calls add_modified_document for transaction tracking' do + cs.flush + expect(Mongoid::Threaded).to have_received(:add_modified_document).with(session, doc) + end + end + end + + context 'when a skip_callbacks entry precedes a non-skip entry for the same document' do + before do + cs.add_entry(make_entry(type: :update, doc: doc, skip_callbacks: true)) + cs.add_entry(make_entry(type: :update, doc: doc)) + end + + it 'fires before_flush exactly once' do + count = 0 + klass.before_flush { count += 1 } + cs.flush + expect(count).to eq(1) + end + + it 'fires after_flush exactly once' do + count = 0 + klass.after_flush { count += 1 } + cs.flush + expect(count).to eq(1) + end + + it 'fires after_commit exactly once' do + count = 0 + klass.after_commit { count += 1 } + cs.flush + expect(count).to eq(1) + end + end + + context 'when a non-skip entry precedes a skip_callbacks entry for the same document' do + before do + cs.add_entry(make_entry(type: :update, doc: doc)) + cs.add_entry(make_entry(type: :update, doc: doc, skip_callbacks: true)) + end + + it 'fires after_flush exactly once' do + count = 0 + klass.after_flush { count += 1 } + cs.flush + expect(count).to eq(1) + end + + it 'fires after_commit exactly once' do + count = 0 + klass.after_commit { count += 1 } + cs.flush + expect(count).to eq(1) + end + end + end + + context 'dirty_fields cleanup' do + let(:doc) { klass.new(name: 'Alice') } + let(:view) { instance_double(Mongo::Collection::View) } + + before do + allow(coll).to receive(:find).and_return(view) + allow(view).to receive(:update_one) + end + + it 'removes named fields from changed_attributes after flush' do + doc.changed_attributes['name'] = 'Bob' + cs.add_entry(make_entry(type: :update, doc: doc, skip_callbacks: true, dirty_fields: [ 'name' ])) + cs.flush + expect(doc.changed_attributes).not_to have_key('name') + end + + it 'accumulates dirty_fields across multiple entries for the same document' do + allow(coll).to receive(:bulk_write) + doc.changed_attributes['name'] = 'Bob' + doc.changed_attributes['age'] = 0 + cs.add_entry(make_entry(type: :update, doc: doc, skip_callbacks: true, dirty_fields: [ 'name' ])) + cs.add_entry(make_entry(type: :update, doc: doc, skip_callbacks: true, dirty_fields: [ 'age' ])) + cs.flush + expect(doc.changed_attributes).not_to have_key('name') + expect(doc.changed_attributes).not_to have_key('age') + end + + it 'ignores nil dirty_fields' do + cs.add_entry(make_entry(type: :update, doc: doc, skip_callbacks: true)) + expect { cs.flush }.not_to raise_error + end + end + + context 'document state updates' do + it 'marks a document with :insert entry as not new_record after flush' do + doc = klass.new(name: 'Alice') + expect(doc.new_record?).to be(true) + allow(coll).to receive(:insert_one) + cs.add_entry(make_entry(type: :insert, doc: doc)) + cs.flush + expect(doc.new_record?).to be(false) + end + + it 'marks a document with :delete entry as destroyed after flush' do + doc = klass.new(name: 'Alice') + doc.new_record = false + view = instance_double(Mongo::Collection::View) + allow(coll).to receive(:find).with(selector).and_return(view) + allow(view).to receive(:delete_one) + cs.add_entry(make_entry(type: :delete, pay: nil, doc: doc)) + cs.flush + expect(doc.destroyed?).to be(true) + end + + it 'applies all state transitions in order when a document has multiple entries in one batch' do + doc = klass.new(name: 'Alice') + expect(doc.new_record?).to be(true) + view = instance_double(Mongo::Collection::View) + allow(coll).to receive(:insert_one) + allow(coll).to receive(:bulk_write) + allow(coll).to receive(:find).with(selector).and_return(view) + allow(view).to receive(:update_one) + allow(view).to receive(:delete_one) + cs.add_entry(make_entry(type: :insert, doc: doc)) + cs.add_entry(make_entry(type: :update, doc: doc)) + cs.add_entry(make_entry(type: :delete, pay: nil, doc: doc)) + cs.flush + expect(doc.new_record?).to be(false) + expect(doc.destroyed?).to be(true) + end + end + end +end diff --git a/spec/mongoid/clients/transactions_spec.rb b/spec/mongoid/clients/transactions_spec.rb index 9d06dca48d..373e35a64b 100644 --- a/spec/mongoid/clients/transactions_spec.rb +++ b/spec/mongoid/clients/transactions_spec.rb @@ -1220,7 +1220,14 @@ def capture_exception rescue RuntimeError end - it_behaves_like 'rollback callbacks are called' + it 'does not call any transaction callbacks' do + # after_save runs inside the changeset block, before the + # driver write. A raise from after_save aborts the changeset + # so no write is dispatched and the transaction has nothing + # to roll back. Post-write logic belongs in after_flush. + expect(subject.after_commit_counter.value).to eq(0) + expect(subject.after_rollback_counter.value).to eq(0) + end end end end @@ -1329,4 +1336,24 @@ def capture_exception end end end + + context 'when called inside a changeset' do + after { Mongoid::Threaded.current_changeset = nil } + + it 'raises TransactionInChangeset on a model class' do + expect do + Mongoid.changeset do + TransactionsSpecPerson.transaction { nil } + end + end.to raise_error(Mongoid::Errors::TransactionInChangeset) + end + + it 'raises TransactionInChangeset on the Mongoid module' do + expect do + Mongoid.changeset do + Mongoid.transaction { nil } + end + end.to raise_error(Mongoid::Errors::TransactionInChangeset) + end + end end diff --git a/spec/mongoid/contextual/memory_spec.rb b/spec/mongoid/contextual/memory_spec.rb index 2e1e093349..d41c26fd71 100644 --- a/spec/mongoid/contextual/memory_spec.rb +++ b/spec/mongoid/contextual/memory_spec.rb @@ -2781,4 +2781,46 @@ end end end + + describe 'Mongoid.changeset deferral' do + let(:person) { Person.create! } + let!(:hobrecht) { person.addresses.create!(street: 'hobrecht') } + let!(:friedel) { person.addresses.create!(street: 'friedel') } + + describe '#delete inside a changeset block' do + let(:criteria) do + Address.where(street: 'hobrecht').tap do |crit| + crit.documents = [ hobrecht, friedel ] + end + end + + let(:context) { described_class.new(criteria) } + + it 'defers the delete until the block exits' do + Mongoid.changeset do + context.delete + expect(person.reload.addresses.count).to eq(2) + end + expect(person.reload.addresses.count).to eq(1) + end + end + + describe '#update_all inside a changeset block' do + let(:criteria) do + Address.where(street: 'hobrecht').tap do |crit| + crit.documents = [ hobrecht, friedel ] + end + end + + let(:context) { described_class.new(criteria) } + + it 'defers the update until the block exits' do + Mongoid.changeset do + context.update_all(number: 42) + expect(hobrecht.reload.number).to be_nil + end + expect(hobrecht.reload.number).to eq(42) + end + end + end end diff --git a/spec/mongoid/contextual/mongo_spec.rb b/spec/mongoid/contextual/mongo_spec.rb index 99d62cc161..e9eddbe1f7 100644 --- a/spec/mongoid/contextual/mongo_spec.rb +++ b/spec/mongoid/contextual/mongo_spec.rb @@ -279,7 +279,7 @@ expect(Band.count).to eq(1) end - it 'returns the number of documents deleted' do + it 'returns the number of deleted documents' do expect(deleted).to eq(1) end @@ -304,7 +304,7 @@ expect(Band.count).to eq(1) end - it 'returns the number of documents deleted' do + it 'returns the number of deleted documents' do expect(deleted).to eq(1) end end @@ -343,6 +343,18 @@ expect(deleted).to eq(0) end end + + context 'when inside a containing changeset' do + let!(:deleted) do + Mongoid.changeset do + Band.where(name: 'Depeche Mode').send(method) + end + end + + it 'returns nil' do + expect(deleted).to be_nil + end + end end end diff --git a/spec/mongoid/interceptable_spec.rb b/spec/mongoid/interceptable_spec.rb index ac2db75760..08bf4e459b 100644 --- a/spec/mongoid/interceptable_spec.rb +++ b/spec/mongoid/interceptable_spec.rb @@ -2577,4 +2577,41 @@ def log_callback end end end + + describe 'before_flush / after_flush callbacks' do + let(:klass) do + Class.new do + include Mongoid::Document + + store_in collection: 'interceptable_flush_test' + + attr_accessor :log + + def initialize(*) + super + @log = [] + end + + before_flush { @log << :before_flush } + after_flush { @log << :after_flush } + end + end + + let(:doc) { klass.new } + + it 'fires before_flush callbacks via run_before_callbacks(:flush)' do + doc.run_before_callbacks(:flush) + expect(doc.log).to eq([ :before_flush ]) + end + + it 'fires after_flush callbacks via run_after_callbacks(:flush)' do + doc.run_after_callbacks(:flush) + expect(doc.log).to eq([ :after_flush ]) + end + + it 'does not fire after_flush when running before_flush' do + doc.run_before_callbacks(:flush) + expect(doc.log).not_to include(:after_flush) + end + end end diff --git a/spec/mongoid/interceptable_spec_models.rb b/spec/mongoid/interceptable_spec_models.rb index e7a8202dd2..d27bf04603 100644 --- a/spec/mongoid/interceptable_spec_models.rb +++ b/spec/mongoid/interceptable_spec_models.rb @@ -49,7 +49,7 @@ def initialize(callback_registry, *args, **kwargs) end module RootInsertable - def insert_as_root + def _stage_insert_as_root @callback_registry&.record_call(self.class, :insert_into_database) super end diff --git a/spec/mongoid/persistable/atomic_callbacks_spec.rb b/spec/mongoid/persistable/atomic_callbacks_spec.rb new file mode 100644 index 0000000000..884f895dde --- /dev/null +++ b/spec/mongoid/persistable/atomic_callbacks_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Atomic operations (inc, bit, set, etc.) must not fire flush or commit +# callbacks, because they did not do so before the changeset layer was +# introduced. When an atomic op and a regular save share the same changeset, +# callbacks should fire exactly once (driven by the save entry). +describe 'atomic operation callbacks' do + let!(:person) { Person.create!(age: 10, score: 50, aliases: %w[foo bar]) } + let(:events) { [] } + + before do + ev = events + Person.before_flush { ev << :before_flush } + Person.after_flush { ev << :after_flush } + Person.after_commit { ev << :commit } + end + + after do + Person.reset_callbacks(:flush) + Person.reset_callbacks(:commit) + end + + shared_examples 'an atomic op that suppresses callbacks' do + it 'does not fire flush or commit callbacks' do + subject + expect(events).to be_empty + end + end + + describe '#inc' do + subject { person.inc(age: 1) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#bit' do + subject { person.bit(score: { or: 1 }) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#set_max' do + subject { person.set_max(score: 999) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#set_min' do + subject { person.set_min(score: 1) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#mul' do + subject { person.mul(age: 2) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#pop' do + subject { person.pop(aliases: 1) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#pull' do + subject { person.pull(aliases: 'foo') } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#pull_all' do + subject { person.pull_all(aliases: %w[foo]) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#add_to_set' do + subject { person.add_to_set(aliases: 'baz') } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#push' do + subject { person.push(aliases: 'baz') } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#rename' do + subject { person.rename(title: :ssn) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#set' do + subject { person.set(title: 'Sir') } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe '#unset' do + subject { person.unset(:title) } + + it_behaves_like 'an atomic op that suppresses callbacks' + end + + describe 'combined with a save in the same changeset' do + it 'fires each callback exactly once' do + person.title = 'Sir' + Mongoid.changeset do + person.inc(age: 1) + person.save + end + expect(events).to eq(%i[before_flush after_flush commit]) + end + end +end diff --git a/spec/mongoid/persistable/deletable_spec.rb b/spec/mongoid/persistable/deletable_spec.rb index f6727e796b..15a73fa108 100644 --- a/spec/mongoid/persistable/deletable_spec.rb +++ b/spec/mongoid/persistable/deletable_spec.rb @@ -186,6 +186,20 @@ end end + context 'inside a Mongoid.changeset block' do + it 'defers the embedded delete until block exit' do + person = Person.create!(title: 'Mr') + address = person.addresses.create!(street: 'Test St') + Mongoid.changeset do + address.delete + # Still present in DB during the block + expect(person.reload.addresses.count).to eq(1) + end + # Gone after flush + expect(person.reload.addresses.count).to eq(0) + end + end + context 'when deleting subclasses' do let!(:firefox) do Firefox.create!(name: 'firefox') diff --git a/spec/mongoid/persistable/destroyable_spec.rb b/spec/mongoid/persistable/destroyable_spec.rb index 8deaad869c..4d80b92f2b 100644 --- a/spec/mongoid/persistable/destroyable_spec.rb +++ b/spec/mongoid/persistable/destroyable_spec.rb @@ -74,6 +74,17 @@ end end + context 'inside a Mongoid.changeset block' do + it 'defers the delete until block exit' do + person = Person.create!(title: 'Mr') + Mongoid.changeset do + person.destroy + expect(Person.where(id: person.id).first).not_to be_nil + end + expect(Person.where(id: person.id).first).to be_nil + end + end + context 'when destroying an embedded document' do let(:address) do person.addresses.build(street: 'Bond Street') diff --git a/spec/mongoid/persistable/incrementable_spec.rb b/spec/mongoid/persistable/incrementable_spec.rb index 098d4ed49b..79f434e84e 100644 --- a/spec/mongoid/persistable/incrementable_spec.rb +++ b/spec/mongoid/persistable/incrementable_spec.rb @@ -213,12 +213,19 @@ Person.create!(age: 10, score: 100) end - it 'marks a dirty change for the incremented fields' do + it 'marks dirty changes for the incremented fields during the block' do person.atomically do person.inc age: 15, score: 2 expect(person.changes).to eq({ 'age' => [ 10, 25 ], 'score' => [ 100, 102 ] }) end end + + it 'clears dirty changes after the block' do + person.atomically do + person.inc age: 15, score: 2 + end + expect(person.changes).to be_empty + end end context 'when executing on a readonly document' do diff --git a/spec/mongoid/persistable/logical_spec.rb b/spec/mongoid/persistable/logical_spec.rb index 59ba5eba50..0bccbddcd5 100644 --- a/spec/mongoid/persistable/logical_spec.rb +++ b/spec/mongoid/persistable/logical_spec.rb @@ -137,12 +137,17 @@ Person.create!(age: 10, score: 100) end - it 'marks a dirty change for the modified fields' do + it 'marks dirty changes for the bit-operated fields during the block' do person.atomically do person.bit age: { and: 6 }, score: { or: 122 } expect(person.changes).to eq({ 'age' => [ 10, 2 ], 'score' => [ 100, 126 ] }) end end + + it 'clears dirty changes after the block' do + person.atomically { person.bit age: { and: 6 }, score: { or: 122 } } + expect(person.changes).to be_empty + end end context 'when executing on a readonly document' do diff --git a/spec/mongoid/persistable/maxable_spec.rb b/spec/mongoid/persistable/maxable_spec.rb index b31946b839..107e2da415 100644 --- a/spec/mongoid/persistable/maxable_spec.rb +++ b/spec/mongoid/persistable/maxable_spec.rb @@ -123,13 +123,17 @@ context 'when executing atomically' do let(:band) { Band.create!(member_count: 10, name: 'Manhattan Transfer') } - it 'marks a dirty change for the modified fields' do + it 'marks dirty changes for fields that actually changed during the block' do band.atomically do band.send(max_method, member_count: 15, name: 'Manhattan Transfer') - expect(band.changes) - .to eq({ 'member_count' => [ 10, 15 ] }) + expect(band.changes).to eq({ 'member_count' => [ 10, 15 ] }) end end + + it 'clears dirty changes after the block' do + band.atomically { band.send(max_method, member_count: 15, name: 'Manhattan Transfer') } + expect(band.changes).to be_empty + end end end diff --git a/spec/mongoid/persistable/minable_spec.rb b/spec/mongoid/persistable/minable_spec.rb index 67e07d870b..342998175e 100644 --- a/spec/mongoid/persistable/minable_spec.rb +++ b/spec/mongoid/persistable/minable_spec.rb @@ -123,13 +123,17 @@ context 'when executing atomically' do let(:band) { Band.create!(member_count: 10, name: 'Manhattan Transfer') } - it 'marks a dirty change for the modified fields' do + it 'marks dirty changes for fields that actually changed during the block' do band.atomically do band.send(min_method, member_count: 5, name: 'Manhattan Transfer') - expect(band.changes) - .to eq({ 'member_count' => [ 10, 5 ] }) + expect(band.changes).to eq({ 'member_count' => [ 10, 5 ] }) end end + + it 'clears dirty changes after the block' do + band.atomically { band.send(min_method, member_count: 5, name: 'Manhattan Transfer') } + expect(band.changes).to be_empty + end end end diff --git a/spec/mongoid/persistable/multipliable_spec.rb b/spec/mongoid/persistable/multipliable_spec.rb index ccbae2b66e..bf65990482 100644 --- a/spec/mongoid/persistable/multipliable_spec.rb +++ b/spec/mongoid/persistable/multipliable_spec.rb @@ -165,12 +165,17 @@ Person.create!(age: 10, score: 100) end - it 'marks a dirty change for the multiplied fields' do + it 'marks dirty changes for the multiplied fields during the block' do person.atomically do person.mul age: 15, score: 2 expect(person.changes).to eq({ 'age' => [ 10, 150 ], 'score' => [ 100, 200 ] }) end end + + it 'clears dirty changes after the block' do + person.atomically { person.mul age: 15, score: 2 } + expect(person.changes).to be_empty + end end context 'when executing on a readonly document' do diff --git a/spec/mongoid/persistable/poppable_spec.rb b/spec/mongoid/persistable/poppable_spec.rb index 8368f6be76..ae94c527f7 100644 --- a/spec/mongoid/persistable/poppable_spec.rb +++ b/spec/mongoid/persistable/poppable_spec.rb @@ -109,10 +109,10 @@ Person.create!(test_array: [ 1, 2, 3 ]) end - it 'marks a dirty change for the popped fields' do + it 'stages the operation and clears dirty tracking immediately' do person.atomically do person.pop test_array: 1 - expect(person.changes).to eq({ 'test_array' => [ [ 1, 2, 3 ], [ 1, 2 ] ] }) + expect(person.changes).to be_empty end end end diff --git a/spec/mongoid/persistable/pullable_spec.rb b/spec/mongoid/persistable/pullable_spec.rb index 8696d4a846..68e4616c4a 100644 --- a/spec/mongoid/persistable/pullable_spec.rb +++ b/spec/mongoid/persistable/pullable_spec.rb @@ -109,10 +109,10 @@ Person.create!(test_array: [ 1, 1, 2, 3 ]) end - it 'marks a dirty change for the pulled fields' do + it 'stages the operation and clears dirty tracking immediately' do person.atomically do person.pull test_array: 1 - expect(person.changes).to eq({ 'test_array' => [ [ 1, 1, 2, 3 ], [ 2, 3 ] ] }) + expect(person.changes).to be_empty end end end @@ -261,10 +261,10 @@ Person.create!(test_array: [ 1, 1, 2, 3, 4 ]) end - it 'marks a dirty change for the pulled fields' do + it 'stages the operation and clears dirty tracking immediately' do person.atomically do person.pull_all test_array: [ 1, 2 ] - expect(person.changes).to eq({ 'test_array' => [ [ 1, 1, 2, 3, 4 ], [ 3, 4 ] ] }) + expect(person.changes).to be_empty end end end diff --git a/spec/mongoid/persistable/pushable_spec.rb b/spec/mongoid/persistable/pushable_spec.rb index 7b244c323b..2f09742148 100644 --- a/spec/mongoid/persistable/pushable_spec.rb +++ b/spec/mongoid/persistable/pushable_spec.rb @@ -185,10 +185,10 @@ Person.create!(test_array: [ 1, 2, 3 ]) end - it 'marks a dirty change for the modified fields' do + it 'stages the operation and clears dirty tracking immediately' do person.atomically do person.add_to_set test_array: [ 1, 4 ] - expect(person.changes).to eq({ 'test_array' => [ [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ] }) + expect(person.changes).to be_empty end end end @@ -355,10 +355,10 @@ Person.create!(test_array: [ 1, 2, 3 ]) end - it 'marks a dirty change for the pushed fields' do + it 'stages the operation and clears dirty tracking immediately' do person.atomically do person.push test_array: [ 1, 4 ] - expect(person.changes).to eq({ 'test_array' => [ [ 1, 2, 3 ], [ 1, 2, 3, 1, 4 ] ] }) + expect(person.changes).to be_empty end end end diff --git a/spec/mongoid/persistable/renamable_spec.rb b/spec/mongoid/persistable/renamable_spec.rb index 763331abb5..b5a65f2aa6 100644 --- a/spec/mongoid/persistable/renamable_spec.rb +++ b/spec/mongoid/persistable/renamable_spec.rb @@ -129,12 +129,17 @@ Person.create!(title: 'sir') end - it 'marks a dirty change for the renamed fields' do + it 'marks dirty changes for renamed fields during the block' do person.atomically do person.rename title: :salutation expect(person.changes).to eq({ 'title' => [ 'sir', nil ], 'salutation' => [ nil, 'sir' ] }) end end + + it 'clears dirty changes after the block' do + person.atomically { person.rename title: :salutation } + expect(person.changes).to be_empty + end end context 'when executing on a readonly document' do diff --git a/spec/mongoid/persistable/settable_spec.rb b/spec/mongoid/persistable/settable_spec.rb index 271cdd9f32..dd5e5b428f 100644 --- a/spec/mongoid/persistable/settable_spec.rb +++ b/spec/mongoid/persistable/settable_spec.rb @@ -161,12 +161,17 @@ Person.create!(title: 'sir', age: 30) end - it 'marks a dirty change for the set fields' do + it 'marks dirty changes for the set fields during the block' do person.atomically do person.set title: 'miss', age: 21 expect(person.changes).to eq({ 'title' => %w[sir miss], 'age' => [ 30, 21 ] }) end end + + it 'clears dirty changes after the block' do + person.atomically { person.set title: 'miss', age: 21 } + expect(person.changes).to be_empty + end end context 'when executing on a readonly document' do diff --git a/spec/mongoid/persistable/unsettable_spec.rb b/spec/mongoid/persistable/unsettable_spec.rb index 4f7b380827..bf396efb88 100644 --- a/spec/mongoid/persistable/unsettable_spec.rb +++ b/spec/mongoid/persistable/unsettable_spec.rb @@ -145,12 +145,17 @@ Person.create!(title: 'sir', age: 30) end - it 'marks a dirty change for the unset fields' do + it 'marks dirty changes for the unset fields during the block' do person.atomically do person.unset :title expect(person.changes).to eq({ 'title' => [ 'sir', nil ] }) end end + + it 'clears dirty changes after the block' do + person.atomically { person.unset :title } + expect(person.changes).to be_empty + end end context 'when executing on a readonly document' do diff --git a/spec/mongoid/persistable/upsertable_spec.rb b/spec/mongoid/persistable/upsertable_spec.rb index befd21fb14..aba0328051 100644 --- a/spec/mongoid/persistable/upsertable_spec.rb +++ b/spec/mongoid/persistable/upsertable_spec.rb @@ -174,6 +174,33 @@ end end + context 'inside a Mongoid.changeset block' do + it 'defers the upsert until the block exits' do + band = Band.new(name: 'Tool') + Mongoid.changeset do + band.upsert + expect(Band.count).to eq(0) + end + expect(Band.count).to eq(1) + end + + it 'stages a :upsert entry for a non-replace upsert' do + band = Band.new(name: 'Deftones') + Mongoid.changeset do |cs| + band.upsert + expect(cs.entries.last.type).to eq(:upsert) + end + end + + it 'stages a :upsert_replace entry for a replace upsert' do + band = Band.new(name: 'Deftones') + Mongoid.changeset do |cs| + band.upsert(replace: true) + expect(cs.entries.last.type).to eq(:upsert_replace) + end + end + end + context 'when the document is readonly' do context 'when legacy_readonly is true' do config_override :legacy_readonly, true diff --git a/spec/mongoid/persistable_spec.rb b/spec/mongoid/persistable_spec.rb index 5b4afb9d63..b51c76322e 100644 --- a/spec/mongoid/persistable_spec.rb +++ b/spec/mongoid/persistable_spec.rb @@ -6,286 +6,87 @@ class PersistableSpecTestException < StandardError; end describe '#atomically' do - let(:document) do - Band.create!(member_count: 0, likes: 60, origin: 'London') - end - - context 'when providing a block' do - shared_examples_for 'an atomically updatable root document' do - it 'performs inc updates' do - expect(document.member_count).to eq(10) - end - - it 'performs bit updates' do - expect(document.likes).to eq(12) - end - - it 'performs set updates' do - expect(document.name).to eq('Placebo') - end - - it 'performs unset updates' do - expect(document.origin).to be_nil - end - - it 'returns true' do - expect(update).to be true - end - - it 'persists inc updates' do - expect(document.reload.member_count).to eq(10) - end - - it 'persists bit updates' do - expect(document.reload.likes).to eq(12) - end - - it 'persists set updates' do - expect(document.reload.name).to eq('Placebo') - end - - it 'persists unset updates' do - expect(document.reload.origin).to be_nil - end - end - - context 'when not chaining the operations' do - let(:operations) do - [ { - '$inc' => { 'member_count' => 10 }, - '$bit' => { 'likes' => { and: 13 } }, - '$set' => { 'name' => 'Placebo' }, - '$unset' => { 'origin' => true } - }, - { session: nil } ] - end - - let(:update) do - document.atomically do - document.inc(member_count: 10) - document.bit(likes: { and: 13 }) - document.set(name: 'Placebo') - document.unset(:origin) - end - end - - before do - expect_any_instance_of(Mongo::Collection::View).to receive(:update_one).with(*operations).and_call_original - update - end - - it_behaves_like 'an atomically updatable root document' - end - - context 'when chaining the operations' do - let(:operations) do - [ { - '$inc' => { 'member_count' => 10 }, - '$bit' => { 'likes' => { and: 13 } }, - '$set' => { 'name' => 'Placebo' }, - '$unset' => { 'origin' => true } - }, - { session: nil } ] - end - - let(:update) do - document.atomically do - document - .inc(member_count: 10) - .bit(likes: { and: 13 }) - .set(name: 'Placebo') - .unset(:origin) - end - end - - before do - expect_any_instance_of(Mongo::Collection::View).to receive(:update_one).with(*operations).and_call_original - update - end - - it_behaves_like 'an atomically updatable root document' - end - - context 'when given multiple operations of the same type' do - let(:operations) do - [ { - '$inc' => { 'member_count' => 10, 'other_count' => 10 }, - '$bit' => { 'likes' => { and: 13 } }, - '$set' => { 'name' => 'Placebo' }, - '$unset' => { 'origin' => true } - }, - { session: nil } ] - end - - let(:update) do - document.atomically do - document - .inc(member_count: 10) - .inc(other_count: 10) - .bit(likes: { and: 13 }) - .set(name: 'Placebo') - .unset(:origin) - end - end - - before do - expect_any_instance_of(Mongo::Collection::View).to receive(:update_one).with(*operations).and_call_original - update - end - - it_behaves_like 'an atomically updatable root document' - - it 'performs multiple inc updates' do - expect(document.other_count).to eq(10) - end + let!(:doc) { Band.create!(member_count: 0, likes: 60, origin: 'London') } - it 'persists multiple inc updates' do - expect(document.reload.other_count).to eq(10) - end + it 'applies all operations when the block succeeds' do + doc.atomically do |d| + d.inc(member_count: 10) + d.set(name: 'Placebo') end + doc.reload + expect(doc.member_count).to eq(10) + expect(doc.name).to eq('Placebo') + end - context 'when expecting the document to be yielded' do - let(:operations) do - [ { - '$inc' => { 'member_count' => 10 }, - '$bit' => { 'likes' => { and: 13 } }, - '$set' => { 'name' => 'Placebo' }, - '$unset' => { 'origin' => true } - }, - { session: nil } ] - end - - let(:update) do - document.atomically do |doc| - doc - .inc(member_count: 10) - .bit(likes: { and: 13 }) - .set(name: 'Placebo') - .unset(:origin) - end + it 'does not persist any operations when the block raises' do + expect do + doc.atomically do |d| + d.inc(member_count: 10) + raise 'abort' end + end.to raise_error(RuntimeError, 'abort') + expect(doc.reload.member_count).to eq(0) + end - before do - expect_any_instance_of(Mongo::Collection::View).to receive(:update_one).with(*operations).and_call_original - update + it 'nests correctly: inner atomically merges into outer' do + doc.atomically do |d| + d.inc(member_count: 5) + d.atomically do |d2| + d2.inc(member_count: 5) end - - it_behaves_like 'an atomically updatable root document' end + expect(doc.reload.member_count).to eq(10) + end - context 'when nesting atomically calls' do - before do - class Band - def my_updates(**args) - atomically(**args) do |d| - d.set(name: 'Placebo') - d.unset(:origin) - end - end - end - end - - context 'when given join_context: false' do - let(:run_update) do - document.atomically do |doc| - doc.set origin: 'Paris' - doc.atomically(join_context: false) do |doc2| - doc2.inc(member_count: 10) - end - doc.inc likes: 1 - raise PersistableSpecTestException, 'oops' - end - end - - it_behaves_like 'an atomically updatable root document' do - let!(:update) do - document.atomically do |doc| - doc.inc(member_count: 10) - doc.my_updates join_context: false - doc.bit(likes: { and: 13 }) - end - end - end - - it "independently persists the non-joining block's operations" do - begin run_update; rescue PersistableSpecTestException; end - - document.reload - - expect(document.origin).to eq 'London' - expect(document.likes).to eq 60 - expect(document.member_count).to eq 10 - end - - it 'resets in-memory changes that did not successfully persist' do - begin run_update; rescue PersistableSpecTestException; end - - expect(document.origin).to eq 'London' - expect(document.likes).to eq 60 - expect(document.member_count).to eq 10 - end - end - - context 'when given join_context: true' do - let(:run_update) do - document.atomically do |doc| - doc.inc(member_count: 10) - doc.my_updates join_context: true - doc.bit(likes: { and: 13 }) - end - end - - it_behaves_like 'an atomically updatable root document' do - let!(:update) { run_update } - end - - it 'performs an update_one exactly once' do - expect_any_instance_of(Mongo::Collection::View).to receive(:update_one).exactly(:once).and_call_original - run_update - end - - it 'resets in-memory changes that did not successfully persist' do - begin - document.atomically do |doc| - doc.set origin: 'Paris' - doc.atomically(join_context: true) do |doc2| - doc2.inc(member_count: 10) - end - doc.atomically(join_context: true) do |_doc3| - doc.inc likes: 1 - end - raise PersistableSpecTestException, 'oops' - end - rescue PersistableSpecTestException - end - - expect(document.origin).to eq 'London' - expect(document.likes).to eq 60 - expect(document.member_count).to eq 0 + it 'with join_context: false persists independently even when outer raises' do + begin + doc.atomically do |d| + d.set(origin: 'Paris') + d.atomically(join_context: false) do |d2| + d2.inc(member_count: 10) end + d.inc(likes: 1) + raise 'abort' end + rescue RuntimeError + # expected end + doc.reload + expect(doc.member_count).to eq(10) + expect(doc.origin).to eq('London') + expect(doc.likes).to eq(60) end - context 'when providing no block' do - it 'returns true' do - expect(document.atomically).to be true - end + it 'returns true when given no block' do + expect(doc.atomically).to be true end context 'when the block has no operations' do - before do - expect_any_instance_of(Mongo::Collection::View).not_to receive(:update_one) + it 'does not update the document' do + doc.atomically {} + expect(doc.reload.origin).to eq('London') end + end - let!(:update) do - document.atomically do - end + it 'persists multiple same-type operations (two inc calls)' do + doc.atomically do |d| + d.inc(member_count: 3) + d.inc(member_count: 7) end + expect(doc.reload.member_count).to eq(10) + end - it "doesn't update the document" do - expect(document.reload.origin).to eq('London') + it 'with join_context: false and no enclosing context, still persists' do + doc.atomically(join_context: false) do |d| + d.inc(member_count: 5) end + expect(doc.reload.member_count).to eq(5) + end + + it 'with join_context: false emits a deprecation warning' do + Mongoid::Warnings.instance_variable_set(:@join_context_false_deprecated, nil) + expect(Mongoid.logger).to receive(:warn).with(/deprecated/) + doc.atomically(join_context: false) { |d| d.inc(member_count: 1) } end end diff --git a/spec/mongoid/reloadable_spec.rb b/spec/mongoid/reloadable_spec.rb index 35eff0de07..1e381f48b1 100644 --- a/spec/mongoid/reloadable_spec.rb +++ b/spec/mongoid/reloadable_spec.rb @@ -4,59 +4,53 @@ describe Mongoid::Reloadable do describe '#reload' do - context 'when called during after_save' do + context 'when called during after_flush' do context 'when using non-sharded documents' do class NonShardedProfile include Mongoid::Document - attr_reader :after_save_count, :after_save_name_val + attr_reader :after_flush_count, :after_flush_name_val field :name, type: String - after_save :on_after_save + after_flush :on_after_flush - def on_after_save - @after_save_count = @after_save_count ? @after_save_count + 1 : 1 + def on_after_flush + @after_flush_count = @after_flush_count ? @after_flush_count + 1 : 1 reload - @after_save_name_val = name + @after_flush_name_val = name end end - context 'when using reload during a post-persist callback' do - context 'when document is not yet persisted' do - context 'when after_save' do - let(:profile) do - NonShardedProfile.new(name: 'Alice') - end + context 'when document is not yet persisted' do + let(:profile) do + NonShardedProfile.new(name: 'Alice') + end - it 'reloads successfully' do - expect(profile.after_save_count).to be_nil - expect(profile.after_save_name_val).to be_nil - profile.name = 'Bob' - profile.save - expect(profile.after_save_count).to eq(1) - expect(profile.after_save_name_val).to eq('Bob') - end - end + it 'reloads successfully' do + expect(profile.after_flush_count).to be_nil + expect(profile.after_flush_name_val).to be_nil + profile.name = 'Bob' + profile.save + expect(profile.after_flush_count).to eq(1) + expect(profile.after_flush_name_val).to eq('Bob') + end + end + + context 'when document is already persisted' do + let(:profile) do + NonShardedProfile.create(name: 'Alice') end - context 'when document is already persisted' do - context 'when after_save' do - let(:profile) do - NonShardedProfile.create(name: 'Alice') - end - - it 'reloads successfully' do - expect(profile.after_save_count).to eq(1) - expect(profile.after_save_name_val).to eq('Alice') - profile.name = 'Bob' - profile.save - expect(profile.after_save_count).to eq(2) - expect(profile.after_save_name_val).to eq('Bob') - end - end + it 'reloads successfully' do + expect(profile.after_flush_count).to eq(1) + expect(profile.after_flush_name_val).to eq('Alice') + profile.name = 'Bob' + profile.save + expect(profile.after_flush_count).to eq(2) + expect(profile.after_flush_name_val).to eq('Bob') end end end @@ -67,56 +61,50 @@ def on_after_save class ShardedProfile include Mongoid::Document - attr_reader :after_save_count, :after_save_name_val + attr_reader :after_flush_count, :after_flush_name_val field :name, type: String shard_key :name - after_save :on_after_save + after_flush :on_after_flush - def on_after_save - @after_save_count = @after_save_count ? @after_save_count + 1 : 1 + def on_after_flush + @after_flush_count = @after_flush_count ? @after_flush_count + 1 : 1 reload - @after_save_name_val = name + @after_flush_name_val = name end end - context 'when using reload during a post-persist callback' do - context 'when document is not yet persisted' do - context 'when after_save' do - let(:profile) do - ShardedProfile.new(name: 'Alice') - end + context 'when document is not yet persisted' do + let(:profile) do + ShardedProfile.new(name: 'Alice') + end - it 'reloads successfully' do - expect(profile.after_save_count).to be_nil - expect(profile.after_save_name_val).to be_nil - profile.name = 'Bob' - profile.save - expect(profile.after_save_count).to eq(1) - expect(profile.after_save_name_val).to eq('Bob') - end - end + it 'reloads successfully' do + expect(profile.after_flush_count).to be_nil + expect(profile.after_flush_name_val).to be_nil + profile.name = 'Bob' + profile.save + expect(profile.after_flush_count).to eq(1) + expect(profile.after_flush_name_val).to eq('Bob') + end + end + + context 'when document is already persisted' do + let(:profile) do + ShardedProfile.create(name: 'Alice') end - context 'when document is already persisted' do - context 'when after_save' do - let(:profile) do - ShardedProfile.create(name: 'Alice') - end - - it 'reloads successfully' do - expect(profile.after_save_count).to eq(1) - expect(profile.after_save_name_val).to eq('Alice') - profile.name = 'Bob' - profile.save - expect(profile.after_save_count).to eq(2) - expect(profile.after_save_name_val).to eq('Bob') - end - end + it 'reloads successfully' do + expect(profile.after_flush_count).to eq(1) + expect(profile.after_flush_name_val).to eq('Alice') + profile.name = 'Bob' + profile.save + expect(profile.after_flush_count).to eq(2) + expect(profile.after_flush_name_val).to eq('Bob') end end end diff --git a/spec/mongoid/stateful_spec.rb b/spec/mongoid/stateful_spec.rb index 21b78e0f12..e3e01e037e 100644 --- a/spec/mongoid/stateful_spec.rb +++ b/spec/mongoid/stateful_spec.rb @@ -120,6 +120,77 @@ end end + describe '#staged?' do + let(:person) { Person.new } + + context 'when no changeset is active' do + it 'returns false' do + expect(person.staged?).to be false + end + end + + context 'when inside a changeset block' do + it 'returns false before the document is saved' do + Mongoid.changeset do + expect(person.staged?).to be false + end + end + + it 'returns true after the document is saved' do + Mongoid.changeset do + person.save! + expect(person.staged?).to be true + end + end + + it 'returns false after the changeset block exits' do + Mongoid.changeset do + person.save! + end + expect(person.staged?).to be false + end + end + end + + describe '#staged' do + let(:person) { Person.new } + + context 'when no changeset is active' do + it 'returns an empty array' do + expect(person.staged).to eq([]) + end + end + + context 'when inside a changeset block' do + it 'returns one entry after one save' do + Mongoid.changeset do + person.save! + expect(person.staged.length).to eq(1) + expect(person.staged.first.document).to equal(person) + end + end + + it 'returns two entries after two saves inside a changeset block' do + person.save! # persist first so subsequent saves are staged updates, not inserts + Mongoid.changeset do + person.title = 'First' + person.save! + person.title = 'Second' + person.save! + expect(person.staged.length).to eq(2) + expect(person.staged.all? { |e| e.document.equal?(person) }).to be true + end + end + + it 'returns an empty array after the changeset block exits' do + Mongoid.changeset do + person.save! + end + expect(person.staged).to eq([]) + end + end + end + describe '#readonly?' do let(:document) do Band.new diff --git a/spec/mongoid/threaded_spec.rb b/spec/mongoid/threaded_spec.rb index 4ba258b22d..b505ad4d2f 100644 --- a/spec/mongoid/threaded_spec.rb +++ b/spec/mongoid/threaded_spec.rb @@ -334,4 +334,24 @@ end end end + + describe '.current_changeset' do + after { Mongoid::Threaded.current_changeset = nil } + + it 'returns nil when not set' do + expect(Mongoid::Threaded.current_changeset).to be_nil + end + + it 'returns the assigned changeset' do + cs = Mongoid::Changeset.new + Mongoid::Threaded.current_changeset = cs + expect(Mongoid::Threaded.current_changeset).to eq(cs) + end + + it 'can be cleared by assigning nil' do + Mongoid::Threaded.current_changeset = Mongoid::Changeset.new + Mongoid::Threaded.current_changeset = nil + expect(Mongoid::Threaded.current_changeset).to be_nil + end + end end