jdaugherty commented on PR #16624: URL: https://github.com/apache/grails-core/pull/16624#issuecomment-6524519443
@matrei Thanks. The PR now targets `9.0.x`, and the three points are fixed in ab0a14663e ([the commit's diff](https://github.com/apache/grails-core/commit/ab0a14663ea9f87541fade75d4816e0d49ba50fa)). ### Target branch - The base is `9.0.x`. I merged `9.0.x` into the branch (fdca024351, clean) rather than rebasing, so the commits the reviews refer to keep their hashes. - The upgrade note moved to section 21 of the Grails 9 notes, reworded for Grails 9 (`upgrading80x.adoc` is back to the `9.0.x` version), and the `@since` tags say `9.0.0`. ### 1. A cycle leading back to an instance validated with hints A cycle now ends at an instance validation is cascading from, whatever hints it is reached with. `Cascade` holds the instance whose associations are being cascaded to and the cascade it was reached through, so the chain on the thread is the path from the object validation started with to the current object, and `cascadeValidationToOne` returns when that chain leads back to the associated instance. That covers the root, your case, and any instance on the path, so a cycle behaves as on 8.0.x for a plain entity and for an annotated one. An instance reached again through another path is still validated once with each distinct set of hints, as before. Regressions, each failing without the fix: - **`PersistentEntityValidatorSpec`:** a plain account whose region's office leads back to it, validated with a hint through `ValidationUtils`, reports `owner` once; and with hinting validators, an office leading back to the account and to the region it was reached through leaves both validated once, with the hints they were first reached with. - **`JakartaValidatorRegistrySpec`:** your example with `@Valid` on `region` and an office leading back to the account and to the region, validated in `OnPublish`: `summary` and `region.code`, as the provider reports, and nothing under `region.office`. Without hints, `owner` and `region.name` once each. - **TCK `BeanValidationSpec`:** the same graph as `BeanValidatedAccount`, `BeanValidatedRegion` and `BeanValidatedOffice`, through `ValidationUtils` with `PublishChecks` and through `validate()`. ### 2. A `@Valid` association of a bean reached through a transient property `MappingContextTraversableResolver.isCascadable` refuses a persistent association only on the root bean of the provider's validation, the instance GORM itself is validating. A bean the provider reached through a transient `@Valid` property cascades on through every property marked `@Valid`, a persistent association included, since GORM never reaches it. The root bean is told by its path naming no property: the provider gives the root bean's own node, a nameless bean node, as the path to the root bean, and the check accepts an empty path as well. Regressions: - **`MappingContextTraversableResolverSpec`:** with the cascade flag off, a persistent association of the root bean is refused, whether the path is empty or the nameless bean node, and one of a bean reached through a named node is cascaded to. - **`JakartaValidatorRegistrySpec`:** your example reports `featured.title` and `featured.author.name`, as the provider does. The item validated on its own still reports `author.name` once: GORM cascades to the author, and the provider leaves it to GORM. - **TCK `BeanValidationSpec`:** the same with `BeanValidatedHolder`, `BeanValidatedFeatured` and `BeanValidatedFeaturedAuthor`, plus a save once both are valid. The guide says that the provider validates the annotations of each value it reaches that way, not a `constraints` block, which it never did. ### 3. A subclass instance handed to the validator of its superclass Once `validate()` has unwrapped the instance, an instance whose class is another entity is handed to the validator of that entity, with the same hints and the same cascade flag. `supports` still accepts the subclasses, so Spring's checks pass, and a proxy of a report handed to the validator of documents is validated by the validator of reports. Regressions: - **`PersistentEntityValidatorSpec`:** `Report extends Document` with `pages nullable: false` and an appendix of its own: handed to the document validator with a hint, the report validator receives the hint and reports `pages` and `appendix.label`; without cascading, `pages` alone; a proxy of the report the same; a document is validated by the document validator itself. - **`JakartaValidatorRegistrySpec`:** an annotated report with a block of its own, handed to the document validator: `pages` (`NotNull`) and `code` (`nullable`), as its own validator reports them; in `OnPublish`, `summary` and `code`. - **TCK `BeanValidationSpec`:** `BeanValidatedReport extends BeanValidatedDocument`, handed to the document validator through `ValidationUtils`: `pages` and `summary` (`maxSize.exceeded`), the same as `validate()`, then a save once valid. ### Docs - **Guide:** the cycle sentence, the provider's cascade from a value it reaches, and the subclass sentence. - **Upgrade note:** the same three points, in section 21 of the Grails 9 notes. ### Run locally - **Specs:** `PersistentEntityValidatorSpec` (19), `JakartaValidatorRegistrySpec` (30), `MappingContextTraversableResolverSpec` (11), `JakartaConstraintsTranslatorSpec` (21) and `GormValidatorAdapterSpec` (9). - **TCK `BeanValidationSpec`:** all 23 features pass on Hibernate 5, Hibernate 7, MongoDB and Neo4j. - **Style:** `codeStyle` on `grails-datamapping-core`, `grails-datamapping-validation` and `grails-datamapping-tck` is clean. The full module suites, the fields, OpenAPI and testing-support specs, and the docs build are left to CI this round. -- 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]
