Summary

On Mongoid 9.1.1 the cases below fail with code 40
(ConflictingUpdateOperators) and nothing is written. Since MONGOID-6013
(https://github.com/mongodb/mongoid/pull/6189) the save succeeds, but part of
the update is applied to a different embedded document than the one that was
changed. No error is raised, so the data is silently wrong. MONGOID-6013 is not
released yet, so this is a regression in the upcoming 9.1.2.

In each case a write that uses the position of a document after a $pull is
applied before that $pull, while the document is still at its old position.

Environment

• Mongoid master 7780ca294 (Mongoid::VERSION is still 9.1.1), and 9.1.1 for comparison
• mongo driver 2.26.0
• Ruby 4.0.0 and 4.0.7
• MongoDB 8.0 (replica set)

Models

class Grade
  include Mongoid::Document
end

class Leaf
  include Mongoid::Document
  field :name, type: String
  embedded_in :child
  has_and_belongs_to_many :grades, inverse_of: nil
end

class Child
  include Mongoid::Document
  field :name, type: String
  embedded_in :root
  has_and_belongs_to_many :grades, inverse_of: nil
  embeds_many :leaves, class_name: 'Leaf'
  accepts_nested_attributes_for :leaves, allow_destroy: true
end

class Root
  include Mongoid::Document
  embeds_many :children
  accepts_nested_attributes_for :children, allow_destroy: true
end

Every child and leaf is persisted with grade_ids: [g0].

Steps to reproduce

A self-contained script is attached (embedded_array_wrong_target.rb, using
bundler/inline; it needs nothing but a reachable MongoDB). It runs against
master by default, and against a released version with MONGOID_VERSION:

ruby embedded_array_wrong_target.rb
MONGOID_VERSION=9.1.1 ruby embedded_array_wrong_target.rb

Case A: $addToSet on a sibling, destroy a grandchild, $addToSet on the remaining grandchild

# children: [b, c], c.leaves: [c1, c2]
root = Root.find(id)
b, c = root.children.to_a
c1, c2 = c.leaves.to_a
b.grade_ids += [g1.id]
root.children_attributes = [{ id: c.id, leaves_attributes: [{ id: c1.id, _destroy: true }] }]
c2.grade_ids += [g1.id]
root.save!

• Expected: b.grade_ids == [g0, g1], c.leaves == [c2],
  c2.grade_ids == [g0, g1]
• Actual: b.grade_ids == [g0, g1], c.leaves == [c2],
  c2.grade_ids == [g0]. The $addToSet for c2 is lost.

The same happens with $pullAll when the foreign keys of both b and c2 are
removed instead of added (b.grade_ids -= [g0.id] and
c2.grade_ids -= [g0.id]): c2 keeps g0.

Case C: destroy a preceding sibling and a grandchild, rename the remaining grandchild

# children: [a, b, c], c.leaves: [c1, c2]
root = Root.find(id)
a, b, c = root.children.to_a
c1, c2 = c.leaves.to_a
root.children_attributes = [
  { id: a.id, _destroy: true },
  { id: b.id, name: 'B' },
  { id: c.id, leaves_attributes: [{ id: c1.id, _destroy: true }, { id: c2.id, name: 'C2' }] }
]
root.save!

• Expected: children.map(&:name) == ['B', 'c'],
  c.leaves.map(&:name) == ['C2']
• Actual: children.map(&:name) == ['B', 'c'],
  c.leaves.map(&:name) == ['c2']. The rename of c2 is lost.

Analysis

Case A

1. $addToSet children.0.grade_ids (b) is added first. When c's
   $pull children.1.leaves is added, pull_conflict? sees
   add_to_set_fields["children"] and moves the $pull into the
   conflicts.
2. The later $addToSet children.1.leaves.0.grade_ids (c2, at its index after
   c1 is removed) goes through add_to_set_conflict?, which only looks at
   $set and $push, so it stays in the main update.
3. The main update runs before the conflicting $pull, while c1 is still at
   index 0 in the database, so the $addToSet hits c1. c1 is then pulled, and
   c2's change is lost.

In the $pullAll variant, b's $pullAll children.0.grade_ids moves the
$pull into the conflicts through pull_all_root?. c2's
$pullAll children.1.leaves.0.grade_ids then stays in the main update,
because pull_all_conflict? only looks at the $pull of the main update
(pull_root?).

Case C

1. The root's $pull children goes into the main update. b's
   $set children.0.name and c2's $set children.1.leaves.0.name go into
   the conflicts first, and c's $pull children.1.leaves follows them.
2. The conflicts are applied in hash order, so the $set for c2 runs before
   the nested $pull, while c1 is still at index 0, and hits c1. c1 is then
   pulled, and c2's change is lost.

Possible fix

The $addToSet, $pullAll and $set paths are built in
Changeable#setters, after process_flagged_destroys has removed the
flagged documents and reindexed the rest (Atomic#atomic_updates). They
already point at the positions the documents have once the $pull is
applied, so they are correct when written after the $pull.

• Route an $addToSet to the conflicts when there is a $pull on the same
  root, and treat a $pull in the conflicts the same as one in the main
  update when routing $addToSet and $pullAll (Case A).
• Put the conflicting $pull first in the conflicts, so the other
  conflicting writes are applied after it (Case C, and the order Case A needs).

Not covered

A $unset can also be applied at the wrong position when a $pull is in
the conflicts, but it needs the stale paths of MONGOID-5364 to be fixed first:

• When a preceding sibling's $addToSet, $pullAll or $push moves the
  $pull into the conflicts, a $unset stays in the main update and runs
  before the $pull. An embeds_one removed through nested attributes is
  then unset on the element that is about to be pulled.
• Sending the $unset after the $pull would break remove_attribute,
  whose key is recorded in delayed_atomic_unsets with the position from
  before the removal. That key currently lands on the right element only
  because it is applied before the $pull.

Related

• MONGOID-6013 (https://github.com/mongodb/mongoid/pull/6189): the change that
  turned these code 40 errors into wrong writes.
• MONGOID-5364 (https://github.com/mongodb/mongoid/pull/6182): positions
  remembered before a sibling is removed, through delayed_atomic_sets and
  remove_attribute.