codeconsole commented on PR #16282: URL: https://github.com/apache/grails-core/pull/16282#issuecomment-5615773261
Thanks — taken rather than deferred, in `f11e85549b`. `rewrap` now re-binds a borrowed wrapper: it re-wraps that wrapper's **raw target** against the assigning entity with `assigned = true`. Three constraints shaped it: - **Store-specific wrappers stay untouched.** Your wording taken literally (re-wrap on parent/property mismatch) would convert a `Neo4jList` belonging to another entity into a generic wrapper — the regression @jdaugherty caught earlier, since `Neo4jList extends DirtyCheckingList`. The exact-class check now guards the incoming value as well as the replaced one. - **Unwrap, never nest.** Wrapping the foreign wrapper itself would leave its `parent.markDirty` firing underneath, marking *both* entities on every mutation. It unwraps in a loop, because `BasicCollectionTypeEncoder:78` builds a `DirtyCheckingMap` with no already-wrapped guard, so a wrapper can arrive nested inside another. - **Owner compared by identity**, not `equals` — two rows equal by business key are not the same owner, and on an association `equals` can initialise a proxy. Two things worth flagging back: **Your repro passes as-is today.** The codec condition is a three-way AND, and the `add` grows the list, so `hasChangedSize()` is true and the full re-encode runs anyway. I wrote that spec first and it was green pre-fix, so I replaced it. The shape that does fail is the one where the two collections are **equal in content**: the assignment is then equality-suppressed, so A is never flagged, the update carries only `lastUpdated`, and the addition is silently lost while B absorbs it. That is the new Mongo spec, verified red before the change and green after. **It also fires when the property was untracked.** `new Entity(shares: other.shares)` had the same defect and could not self-heal — the insert's write-back assigns the field directly and never reaches `rewrap` — so the re-binding no longer depends on what the property held before. Assigning a plain collection to an untracked property still introduces no tracking. Not addressed here, deliberately: `wrap()` keeps the same owner-blind early return, so a foreign wrapper reaching it through another path is still handed back as-is. It is a public method that has returned the same instance since 2.0 and is on the encode path, so it felt like its own change rather than a rider on this one — happy to do it here if you would rather it were one PR. -- 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]
