Inserting an `embeds_many` child while another child of the same array has a pending touch update raises `Mongo::Error::OperationFailure [40]`. Regression in 9.1.0, still present on master

XMLWordPrintableJSON

    • Type: Bug
    • Resolution: Unresolved
    • Priority: Unknown
    • None
    • Affects Version/s: 9.1.0
    • Component/s: Persistence
    • None
    • None
    • Ruby Drivers
    • None
    • None
    • None
    • None
    • None
    • None

      Mongoid::Persistable::Creatable#insert_as_embedded merges the parent's pending touch updates into the same `update_one` that performs the embedded insert:

      operations = atomic_inserts
if _touchable_parent?
  touches = _parent._gather_touch_updates(Time.current)
  if touches.present?
    operations['$set'] = (operations['$set'] || {}).merge(touches)
  Threaded.begin_touch_merged(self)
  end
end

       

      The merge does not check whether a touch path collides with a path the insert already targets. For embeds_many, atomic_inserts produces $push on the array, while _gather_touch_updates can return a path inside that array (items.0.updated_at) when a sibling has a pending touch. The resulting update document is:

       

      {
"$push": { "items":
{ ... }
},
"$set":
{ "updated_at": ..., "items.0.updated_at": ... }
}

       

       

      MongoDB rejects any update whose operators target both a path and one of its ancestors, so the write fails with:

      [40]: Updating the path 'items.0.updated_at' would create a conflict at 'items'

      Before 9.1.0 the touch was persisted in its own `update_one`, so the two never shared an update document.

      Reproduction

       

      require "mongoid"

Mongoid.configure { |c| c.clients.default = {uri: "mongodb://localhost:27017/mongoid_repro"} }

class Parent
  include Mongoid::Document
  include Mongoid::Timestamps
  embeds_many :items, class_name: "Item", cascade_callbacks: true, validate: false
end

class Item
  include Mongoid::Document
  include Mongoid::Timestamps
  embedded_in :parent, touch: true

  field :qty, type: Integer
  field :archived, type: Mongoid::Boolean, default: false

  # Saves a NEW sibling into the same array from this document's before_update.
  before_update :spawn_sibling, if: -> { qty_changed? && !archived }

  def spawn_sibling
    throw :abort unless parent.items.build(qty: qty_was, archived: true).save
  end
end

parent = Parent.create!
item = parent.items.create!(qty: 1)

item.update!(qty: 2)
# 9.0.11 => OK; two items persisted, sibling's updated_at touched
# 9.1.0  => Mongo::Error::OperationFailure:
#           [40]: Updating the path 'items.0.updated_at' would create a conflict at 'items'

       

       

      Observed:

      Version Result
      — —
      9.0.11 passes — 2 items persisted, touch applied
      9.1.0 `Mongo::Error::OperationFailure [40]`
      master `8c976ba` `Mongo::Error::OperationFailure [40]`

      Expected

      The insert and the touch both succeed, as in 9.0.x.

      Suggestion:

      Skip the merge when any touch path conflicts with a path the insert already targets, leaving those touches to the normal separate round-trip. This keeps the optimization for the common case (the conflict only arises for `embeds_many` inserts with a dirty sibling) and preserves 9.0.x semantics otherwise.

      if _touchable_parent?
  touches = _parent._gather_touch_updates(Time.current)
  if touches.present? && !touch_paths_conflict?(operations, touches)
    operations['$set'] = (operations['$set'] || {}).merge(touches)
    Threaded.begin_touch_merged(self)
  end
end

# ...

# MongoDB refuses an update whose operators target a path and one of its
# ancestors or descendants (error 40).
def touch_paths_conflict?(operations, touches)
  targeted = operations.each_value.flat_map { |doc| doc.respond_to?(:keys) ? doc.keys.map(&:to_s) : [] }

  touches.each_key.any? do |touch_path|
    path = touch_path.to_s
    targeted.any? { |t| path == t || path.start_with?("#{t}.") || t.start_with?("#{path}.") }
  end
end

      I can provide a PR if needed, or more information for further investigation.

            Assignee:
            Unassigned
            Reporter:
            Brian Lima (EXT)
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated: