jdaugherty commented on PR #16624:
URL: https://github.com/apache/grails-core/pull/16624#issuecomment-6325625840
@matrei
Thanks for the thorough probe. All of 1, 2, 4 and the smaller points are
addressed in the amended commit; the approach changed in one respect that also
answers @jamesfredley's fields plugin point.
**Design change.** The validator `JakartaValidatorRegistry` registers for an
annotated entity is now `JakartaPersistentEntityValidator`, a
`PersistentEntityValidator` subclass, rather than a `GormValidatorAdapter`
carrying one. It validates the `constraints` block and cascades as the base
class does, then runs Bean Validation (without cascading) through a new
protected hook, `PersistentEntityValidator.validateBeyondConstraints(obj,
errors)`, which the base class calls for the root object and, in
`cascadeValidationToOne`, for each associated object under the nested path of
its association. So every consumer that does `validator instanceof
PersistentEntityValidator` (the cascade itself, `DomainPropertyImpl` in
grails-fields, `PersistentGraphQLProperty`, and any plugin outside this repo)
sees the merged constraints without a change. `GormValidatorAdapter` is back to
its 8.0.0 shape.
1. **Cascading.** `cascadeValidationToOne` now validates an annotated
associated entity with the constraints its validator *validates* (a new
protected `getValidatedConstrainedProperties()`, the evaluated block with the
nullability relaxed) rather than the merged ones it *exposes*, so the
translated metadata is not checked a second time, and then calls the hook, so
the annotations of the child are checked under `lines[0].label`. One cycle
guard (`validatedObjects`), Bean Validation never cascades. Covered for an
annotated parent, a plain parent, a single execution and `deepValidate: false`
in the new TCK spec, the `DataTest` spec and `JakartaValidatorRegistrySpec`.
2. **Groups.** `JakartaConstraintsTranslator` now reads the `BeanDescriptor`
of the class and keeps only the constraints whose groups intersect the default
group of the class (`Default`, plus the groups of a `@GroupSequence` redefining
it), both for the nullability delegation and for the description. A
`@NotNull(groups = OnPublish)` leaves `nullable: false` of the block in force:
`validate()` and `save()` reject the null with `nullable`, and the metadata
still says required. The guide says "annotations of the default group" now.
3. **8.0.x.** The upgrade note now separates the rules that already shaped
the schema (`nullable`, `unique`, `maxSize`, a failure moves from flush to
`validate()`) from the ones enforced for the first time (`validator`,
`matches`, `inList`, `min`/`max`, `email`, `blank`, and the global
`grails.gorm.default.constraints`), and asks to review the block and the global
defaults of each annotated class.
4. **Class level constraints.** Bullet dropped, the commit message and the
description corrected, and the `propertyName == null` branch of the resolver
kept as a guard with a comment saying Hibernate Validator does not consult it
for a class level constraint (the spec test is renamed accordingly). The TCK
spec pins the behaviour with a class level constraint.
5. Smaller points:
- the link is `link:validation.html#constraints[constraint]` now;
- the Hibernate 5 and 7 additions are replaced by
`org.apache.grails.data.testing.tck.tests.BeanValidationSpec` (six domains
under `tck/domains`), which runs on Hibernate 5, Hibernate 7, MongoDB and Neo4j
and is ignored on the simple datastore of the core suite, which registers no
validator;
- a getter without a backing field (`isRangeValid()`) is validated but no
longer described, unless the block constrains it; reading the descriptors also
means a composed constraint is described with its `@OverridesAttribute` applied;
- `grails-openapi` gets `BeanValidationSchemaSpec` (the fixture now
builds the registry through `ValidatorRegistries`, as the datastores do), and
the upgrade note mentions the OpenAPI descriptions next to scaffolding and the
fields plugin.
Run locally: `PersistentEntityValidatorSpec`, the `jakarta` specs of
grails-datamapping-core, the `DataTest` spec,
`EntityValidatorDomainPropertySpec` (grails-fields, new case through
`JakartaValidatorRegistry`), the grails-openapi suite, and the TCK
`BeanValidationSpec` on Hibernate 5, Hibernate 7 and MongoDB. Neo4j is left to
CI.
@jamesfredley on the fields plugin (`DomainPropertyImpl` line 57):
Addressed by the design change rather than an edit to `DomainPropertyImpl`:
the validator of an annotated entity is now a `PersistentEntityValidator`
subclass, so the `instanceof` check on line 57 passes and the fields plugin
reads the merged constraints as they are. `EntityValidatorDomainPropertySpec`
has a new case that builds the validator through `JakartaValidatorRegistry` and
checks `isRequired()` and `constrained.maxSize` for an annotated class. The
same holds for `PersistentGraphQLProperty` and for any plugin outside this
repository that checks the type.
The three inline threads have their own replies.
--
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]
