jdaugherty commented on PR #16468:
URL: https://github.com/apache/grails-core/pull/16468#issuecomment-5955671831
Thanks for the review. All four follow-ups are addressed in
9d94eca8323e69e2b090e9c9d850436c7569d17e, pushed to `8.0.x`.
1. **`DataTest` coverage.** `CustomIdGeneratorSpec` in
`grails-testing-support-datamapping` (the module that owns `DataTest`, next to
its other `DataTest` specs) mocks a domain class mapped with `id generator:
'com.example.CustomIdGenerator'` through `getDomainClassesToMock()`, then saves
it and reads it back. With the base-class fallback reverted, it fails in setup
with the original `BeanCreationException` (`No enum constant
ValueGenerator.COM.EXAMPLE.CUSTOMIDGENERATOR`).
2. **Misspelled names and the triplicated lookup.** I took the optional
refactor. `MappingFactory.resolveGenerator(ClassMapping, String)` (`protected
final`) now does the null / upper-case / `valueOf` lookup. It passes any name
that matches no constant to `resolveCustomGenerator(ClassMapping, String)`,
which returns `CUSTOM`. The Hibernate 5 and Hibernate 7 factories override only
that hook, keeping their class-presence check (plus `table` on H7) and the same
`DatastoreConfigurationException` message. New tests:
- core: only names that are not built in reach the hook, its return value
is used, and it can reject a name
- H5: mixed-case built-in name, the invalid-name message, composite id
resolves to `AUTO`
- H7: `table` / `TABLE` resolve to `CUSTOM`, mixed-case built-in name;
the existing invalid-name test now also checks the message
Mutation-checked: making the H5 hook accept every name fails the H5
rejection test, and dropping `table` from the H7 hook fails both `table` cases.
3. **Doc note.** Moved to the end of the section, after the
`ControllerUnitTest` example. It now reads "The mocked datastore generates
identifiers itself and never runs or validates that generator, so a misspelled
generator name still passes a unit test."
4. **Nit.** Dropped `@Unroll` from `IdentityGeneratorMappingSpec`.
Run locally, all green: `IdentityGeneratorMappingSpec`,
`CustomIdGeneratorSpec`, H5 `HibernateMappingContextSpec`, H7
`HibernateMappingFactorySpec`, `SimpleMapIdentifierSpec`,
`MongoMappingContextSpec`, `SnowflakeIdGeneratorSpec`, plus `codeStyle` on the
four touched modules and `validateRepositoryConventions`. The full module
suites were not run locally.
--
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]
