codeconsole opened a new pull request, #16282:
URL: https://github.com/apache/grails-core/pull/16282
### TL;DR
On interception-based stores (MongoDB), two everyday mutation patterns
silently escape dirty checking — `save()` reports success and persists nothing:
```groovy
// 1. The defensive re-init — true for an EMPTY tracked list, because empty
collections are falsy
if (!schedule.shares) {
schedule.shares = [] // replaces the tracked wrapper with a
plain ArrayList
} // and [] == [] means the assignment
isn't even flagged
schedule.shares.add(newShare) // invisible: plain list, nothing
marks the entity dirty
schedule.save(flush: true) // writes nothing
```
```groovy
// 2. Groovy's closure-based removal — removes via iterator().remove()
schedule.shares.removeAll { it.userId == userId } //
DirtyCheckingCollection doesn't override iterator()
schedule.save(flush: true) // writes nothing
```
Hibernate is unaffected — its flush-time snapshot comparison catches
everything. Mongo relies exclusively on the `DirtyChecking*` wrappers, so
anything that escapes them is lost. Hit in production: a schedule-sharing
feature showed "shared" while the document kept `shares: []`.
### The fix (interception only — no snapshots, no flush-time diffing)
**1. Wrappers track every mutation path.** `iterator()`/`listIterator()` now
return dirty-marking iterators (covers `removeAll(Closure)`,
`retainAll(Closure)`, `removeIf`), plus the missing direct overrides:
`retainAll(Collection)`, `List.sort`, `List.replaceAll`. Same approach as
Hibernate's `PersistentCollection`.
**2. Generated setters keep tracking across reassignment.**
Collection/List/Set/Map-typed properties assign through
`DirtyCheckingSupport.rewrap`:
```groovy
// generated setter, before:
void setShares(List shares) { markDirty("shares", shares); this.shares =
shares }
// after:
void setShares(List shares) { markDirty("shares", shares); this.shares =
(List) DirtyCheckingSupport.rewrap(this, "shares", this.shares, shares) }
```
`rewrap` wraps the new value **only when the value being replaced was itself
a tracked wrapper** — otherwise it returns the raw value after one
`instanceof`. Never-tracked properties (transient instances, Hibernate
entities) behave exactly as before, and non-collection properties compile to
identical bytecode.
**3. Replacement wrappers are flagged `isAssigned()`** (default method on
`DirtyCheckableCollection`, so binary-compatible). `PersistentEntityCodec` then
takes the full-rewrite path instead of per-element diffing — a replacement's
layout need not match the stored array. Without the flag, a same-size
replacement holding clean elements emitted no update at all.
### Tests
Each escape is reproduced by a spec that fails without the fix:
- `DirtyCheckingCollectionSpec` — 8 wrapper mutation paths that bypassed
tracking
- `DirtyCheckCollectionReassignmentSpec` — reassignment loses tracking
(List/Set/Map); never-tracked values stay untouched
- `EmbeddedCollectionDirtyTrackingSpec` — end-to-end against MongoDB,
replicating the production shape (an auto-timestamped entity: the `lastUpdated`
write during flush resets the explicit-save dirty marker, so persistence
depends entirely on the wrappers)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
