codeconsole commented on PR #16297: URL: https://github.com/apache/grails-core/pull/16297#issuecomment-5577856894
All five addressed, one isolated commit each, every fix pinned by a test verified to fail without it. | commit | finding | |---|---| | `1630a25` | `updateAll` not proxy-safe | | `ded7116` | `updateAll` treats embedded to-one as an id reference | | `564b61b` | mapping-engine unidirectional collection and many-to-many references | | `463e1e7` | `updateAll` does not encode collection associations | | `1756eae` | contradictory engine documentation | Two of these were mine to own. `Embedded extends ToOne`, so the association normalization I added last round did catch embedded properties and reflect an id from a value that has none — both engines now skip `Embedded` and leave the value for the normal embedded encoding. And `updateAll` reflected the identifier directly while `ToOneEncoder` asks the proxy factory first, so a `load()`ed argument yielded null; both engines now ask the proxy factory too. For the collection paths, `updateAll` encodes each element the way `OneToManyEncoder` does — the target's identifier in its stored `_id` type, wrapped in a DBRef where the mapping declares one, proxy-safe — and `EmbeddedCollection` is excluded for the same reason as `Embedded`. On the mapping engine, the association indexer's unidirectional one-to-many keys and the many-to-many id list now go through the target's id mapping, matching `formulateDatabaseReference` and `setEmbeddedCollectionKeys`. On the documentation: agreed, "only supported value" contradicted the paragraph documenting `mapping` immediately below it. It now says `codec` is the default and recommended value and `mapping` remains available for compatibility but is deprecated, in both the asciidoc and the `SETTING_ENGINE` javadoc. `grails-data-mongodb-core` is at parity with `8.0.x` locally: the only failures are the two pre-existing `MongoDatastoreLifecycleSpec` cases, which also fail on an unmodified checkout here. checkstyle and codenarc clean. One note on the verification section, same as last round: a run that exceeds a local timeout without printing a completion result is not evidence that it passed. The conclusion has held each time, but it is worth separating "the focused tests passed" from "the full task was inconclusive". -- 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]
