Guard child datastore entity registration against a null GORM enhancer (late registration NPE)#15776
Conversation
Skip late entity registration from Hibernate 5 child datastore listeners that intentionally have no GORM enhancer, leaving the parent enhancer to register public APIs for shared mapping-context entities. Assisted-by: opencode:openai/gpt-5.5 oracle
Skip late entity registration from Hibernate 7 child datastore listeners that intentionally have no GORM enhancer, leaving the parent enhancer to register public APIs for shared mapping-context entities. Assisted-by: opencode:openai/gpt-5.5 oracle
There was a problem hiding this comment.
Pull request overview
This PR hardens multi-datasource Hibernate datastores (Hibernate 5 and Hibernate 7 lines) against late entity registration when child datastores are created without a GORM enhancer, and adds regression coverage to ensure enhancement and datasource routing still behave correctly.
Changes:
- Guard
MappingContext.Listener#persistentEntityAddedin Hibernate 5/7 datastores to avoid NPEs whengormEnhanceris absent (child datastore scenario). - Add regression tests for late entity registration to ensure the parent enhancer still wires up public GORM APIs.
- Verify
withNewSessionrouting for the late-registered entity uses the mappedbooksdatasource.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateDatastore.java | Prevents child-datastore mapping-context listener from failing when gormEnhancer is null. |
| grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/HibernateDatastore.java | Same null-guard for Hibernate 5 line to avoid listener failures on late entity registration. |
| grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/connections/MultipleDataSourceConnectionsSpec.groovy | Adds Hibernate 7 regression test validating late registration is enhanced and routes withNewSession to books. |
| grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/MultipleDataSourceConnectionsSpec.groovy | Adds Hibernate 5 regression test validating late registration is enhanced and routes withNewSession to books. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This was found during #15678 and now carved off into dedicated PR. |
✅ All tests passed ✅🏷️ Commit: c4bd064 Learn more about TestLens at testlens.app. |
|
@borinquenkid FYI This fix for child datastore has been merged. |
Summary
Hibernate child datastores (one per non-default
dataSourcesconnection) share the parent datastore'sMappingContext. Both the parent and every child install aMappingContext.ListenerwhosepersistentEntityAddedcallback callsgormEnhancer.registerEntity(entity). Child datastores are intentionally not GORM-enhanced - theirgormEnhancerfield isnull(the parent is the sole owner of enhancement). When a domain class is registered into the sharedMappingContextafter the child datastores have been constructed, the child's listener fires and dereferences itsnullenhancer:This is the child-datastore-initialization defect identified in #15678 ("Thread 2"). This PR fixes it surgically and nothing else.
Root cause (exact)
HibernateDatastore(H5 and H7) adds, in its constructor, aMappingContext.Listenerthat callsgormEnhancer.registerEntity(entity)on everypersistentEntityAdded.MappingContextis shared - the parent passes its context to each child datastore, so a singlemappingContext.addPersistentEntity(...)notifies the parent listener and every child listener.gormEnhancerisnullby design; only the parent enhances.Fix
Guard both listeners (H5 + H7
HibernateDatastore) so a datastore enhances only when it actually owns an enhancer:The parent (non-null enhancer) still registers the late entity for all qualifiers - including non-default
dataSourcesnames - so multi-datasource routing is unchanged; the child listener simply no-ops.Why this approach (and not #15678's child self-enhancement)
#15678 resolved the same Thread-2 concern by making each child enhance itself (
ChildHibernateDatastore.initialize()builds its ownHibernateGormEnhancer). That required the heavier machinery #15678 then had to add:getDatastoreForConnectionreturningnullduring initialization to avoid aConfigurationExceptionwhen a sibling is looked up before all children are registered, andbindParent()/PARENT_HOLDERthread-local to order parent assignment during the super-constructor chain.Keeping children un-enhanced (the existing 8.0.x design) makes all of that unnecessary: there is no child enhancer to construct or order, no sibling lookup during init, and therefore no init-order
ConfigurationException. The single real defect that remains in this model is the null-enhancer dereference, which this guard removes.Changed files
grails-data-hibernate5/core/.../org/grails/orm/hibernate/HibernateDatastore.java- null-guard the listener.grails-data-hibernate7/core/.../org/grails/orm/hibernate/HibernateDatastore.java- null-guard the listener.grails-data-hibernate5/core/.../connections/MultipleDataSourceConnectionsSpec.groovy- regression test +LateRegisteredBookentity.grails-data-hibernate7/core/.../connections/MultipleDataSourceConnectionsSpec.groovy- regression test +LateRegisteredBookentity.Verification (evidence)
The regression test "late registered entity is enhanced without child datastore listener failure" registers
@Entity LateRegisteredBook { static mapping = { datasource 'books' } }through the publicdatastore.mappingContext.addPersistentEntity(LateRegisteredBook)after child datastores are initialized, then assertsLateRegisteredBook.withNewSession { ... }resolves to the mapped datasource (jdbc:h2:mem:books).MultipleDataSourceConnectionsSpec.groovy:196- confirming the defect is real and the guard is load-bearing.jdbc:h2:mem:books(parent enhancement intact).Local results (
./gradlew --no-daemon --max-workers=1 ... -PmaxTestParallel=1)::grails-data-hibernate5-core:test --tests MultipleDataSourceConnectionsSpec- 5/5 PASS (late-registration + existing multi-datasource routing, first-non-default-datasource, ALL-mapped, and@Transactional(connection='books')tests).:grails-data-hibernate7-core:test --tests MultipleDataSourceConnectionsSpec- 5/5 PASS.:grails-data-hibernate7-core:test --tests ChildHibernateDatastoreUnitSpec- PASS.Scope / non-goals
This PR addresses only the child-datastore null-enhancer NPE (#15678 Thread 2). The
O(entityCount × tenantCount)GORM API-allocation scaling (#15678 Thread 1) and the per-tenant transaction-synchronisation fixes are handled independently in #15771. The two compose cleanly: #15771 keeps the parent as the (now lazy) enhancer; this PR keeps child listeners from dereferencing a null enhancer.