Skip to content

Commit 2b0934c

Browse files
committed
MONGOID-5972 Clean up review findings in the touch-merge code
Clear the merged-touch flag if the insert update fails, so the thread-local state does not leak. Fix a stale doc reference, correct the mechanism described in a spec comment, and hoist the touchable chain path computation out of the per-key loop.
1 parent b5aa99f commit 2b0934c

2 files changed

Lines changed: 18 additions & 10 deletions

File tree

‎lib/mongoid/persistable/creatable.rb‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,12 @@ def conflicting_touch_paths?(operations, touches)
9090
def rewritten_touch_updates(selector, parent, field, touches)
9191
rewritten = positionally(selector, { '$set' => touches })['$set']
9292

93+
chain_paths = parent._touchable_chain_paths(field)
9394
touches.each_key do |key|
9495
path = key.to_s
9596
next if rewritten.key?(path)
9697

97-
return nil unless parent._touchable_chain_paths(field).include?(path)
98+
return nil unless chain_paths.include?(path)
9899
end
99100
rewritten
100101
end
@@ -129,18 +130,24 @@ def insert_as_embedded
129130

130131
deferred_touches = merge_touch_updates(selector, operations)
131132

132-
_root.collection.find(selector).update_one(
133-
positionally(selector, operations),
134-
session: _session
135-
)
136-
137-
_root.send(:persist_atomic_operations, '$set' => deferred_touches) if deferred_touches
133+
begin
134+
_root.collection.find(selector).update_one(
135+
positionally(selector, operations),
136+
session: _session
137+
)
138+
_root.send(:persist_atomic_operations, '$set' => deferred_touches) if deferred_touches
139+
rescue StandardError
140+
# If the insert failed, the after_save callback will not run to
141+
# consume and clear the merged-touch flag, so clear it here.
142+
Threaded.exit_touch_merged(self)
143+
raise
144+
end
138145
end
139146
end
140147

141148
# Merge the parent chain's pending touch updates into the insert
142149
# operations when doing so would not produce a conflicting update
143-
# (see +conflicting_touch_paths?+ and +misdirected_touch_paths?+).
150+
# (see +conflicting_touch_paths?+ and +rewritten_touch_updates+).
144151
# Either way, marks the touch as merged so the after_save callback
145152
# does not persist the updates a second time. Returns the touch
146153
# updates that could not be merged; they are persisted in their own

‎spec/mongoid/touchable_spec.rb‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,8 +1737,9 @@ class TouchableChild
17371737

17381738
before do
17391739
first_floor
1740-
# Leave a pending touch on the other floor so the touch updates
1741-
# conflict with the insert and cannot be merged into it.
1740+
# Leave a pending touch on the other floor: rewriting it and this
1741+
# chain's own touch would collapse both onto the same positional
1742+
# path, so the touch updates cannot be merged into the insert.
17421743
second_floor.updated_at = pending_touch_time
17431744
end
17441745

0 commit comments

Comments
 (0)