jdaugherty commented on PR #16147:
URL: https://github.com/apache/grails-core/pull/16147#issuecomment-5914528386
Rebased this onto current `8.1.x` (it was conflicting) and pushed the result
as `070de66fd1`.
**Rebase notes**
- `MongoDbDataStoreSpringInitializer`: kept the 8.1.x wiring
(`configurationReference`, `ref('grailsDatastoreEventPublisher')`,
`mappedClasses(...)`) and applied the `"$mongoBeanName"` fix on top of it.
- Both `HibernateDatastoreSpringInitializer`s: 8.1.x no longer has the local
`eventPublisher` variable in `getBeanDefinitions`, so only the fallback call
was carried over.
- `grails-data-neo4j` is part of the root build now, so the
`registerApplicationIfNotPresent` removal in `Neo4jGrailsPlugin` is exercised
by compilation.
**Review changes (one extra commit on top of yours)**
- **Dropped the `enableReload` fallback.**
`HibernateConnectionSourceSettings.enableReload` has no reader in either the
Hibernate 5 or the Hibernate 7 core module (`grep -rn enableReload
grails-data-hibernate*/core/src/main` only hits the field declaration).
Injecting it therefore had no runtime effect; in a Grails app it just wrote a
top-level `enableReload: true` into the application `Config` in dev mode. The
end-to-end test passed because it asserted on the settings object, not on any
behaviour driven by it. The initializer's `enableReload` property and the
plugin line that sets it are still dead; I left them alone rather than widen
the PR, but they are candidates for the same treatment as `grailsPlugin`.
- **`containsRegisteredBean` and `getGrailsValidatorClass` are instance
methods again.** Turning a protected instance method into a static one breaks
any external `AbstractDatastoreInitializer` subclass compiled against the old
signature (`IncompatibleClassChangeError`), and both are called from subclasses
in this repo. Kept the `@SuppressWarnings('GrMethodMayBeStatic')` annotations
instead.
- **`applyDatabaseNameFallback` ignores null/blank names** (the suppressed
Copilot comment), with a data-driven unit test.
**Verified locally** (JDK 21): `AbstractDatastoreInitializer*Spec` (38),
both `HibernateDatastoreSpringInitializerSpec`s (9 + 7),
`MongoDbDataStoreSpringInitializerUnitSpec` (16), and `codeStyle` for
datamapping-core, hibernate5, hibernate7, mongodb-core, mongodb and neo4j. I
could not run the Testcontainers-backed `MongoDbDataStoreSpringInitializerSpec`
here (no Docker), so that one is on CI.
Everything else looks good to me: the `databaseName` fallback restores what
the MongoDB docs already promise ("If not specified the `databaseName` will
default to the name of your application"), and the `defaultDataSourceBeanName`
and `mongoBeanName` fixes are straightforward.
--
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]
