The GitHub Actions job "CI" on grails-core.git/feat/gorm-datastore-infra has failed. Run started by GitHub user borinquenkid (triggered by borinquenkid).
Head commit for run: 23f6b48cc7a21f2f3b0e38f6f7ae7d9cb902b0bf / Walter Duque de Estrada <[email protected]> fix: address review feedback on SessionResolver/AbstractDatastore architecture Responds to jdaugherty's CHANGES_REQUESTED review (#15779). Fixes the session/event architecture concerns, narrows the public API surface, and splits out the unrelated behavioral changes he flagged, per that review's blast-radius check against PR2/PR3 (#15780/#15790) - neither depends on anything reworked here beyond hasCurrentSession(), which is now strictly more correct. Session/event architecture (the core concern): - SessionResolver/ThreadLocalSessionResolver no longer maintain independent ThreadLocal state. They're now a thin, stateless view over the same SessionHolder/TSM store DatastoreUtils already uses, so resolve() can never disagree with the transactional session. Nested scopes fall out for free from SessionHolder's existing stack. - DatastoreUtils.doGetSession() no longer short-circuits through the resolver before transaction-synchronization registration and session validation - that shortcut silently bypassed both, plus the allowCreate contract. - AbstractDatastore.hasCurrentSession() collapses to a single check now that resolver and TSM read the same state instead of being OR'd together. - Dropped the unused, asymmetric resolve(String)/bind(String, S) qualifier surface from SessionResolver (zero callers anywhere in the codebase; the concrete class's own bind() admitted the feature was never finished). - Replaced the hand-rolled event publisher with one composing SimpleApplicationEventMulticaster. addApplicationListener() now routes through getApplicationEventPublisher() (virtual) instead of the raw field, so it reaches whatever publisher a subclass (Mongo/Hibernate/Neo4j) actually publishes through, without touching those modules. - Fixed the applicationEventPublisher triple-assignment and the bug where setApplicationContext(null) discarded a caller-installed custom publisher. - @PreDestroy now closes every session held by the current thread's SessionHolder instead of just dropping the reference. API surface: - Datastore.getSessionResolver() is now a default method (was abstract - broke every external implementer); the default is now safe to construct per-call since the resolver holds no private state of its own. - MappingContext.initialize(ConnectionSourceSettings) is back to protected on AbstractMappingContext, not promoted onto the public interface - nothing needed the promotion. Restored/fixed semantics: - DatastoreUtils.bindSession()/bindSession(creator) fail fast again (IllegalStateException) on a double-bind, instead of silently stacking - bindNewSession() already provides stacking for callers that need it (used internally by executeWithNewSession). - CustomizableRollbackTransactionAttribute's copy constructors now deep-copy the rollback-rule list instead of aliasing the source's mutable list, and also copy transaction labels. - AbstractConnectionSourceFactory.createSettings() now composes the same fallback-settings path create(name, configuration) uses, so it also applies the injected TenantResolver/customTypes. - Deduplicated DatastoreUtils.executeWithNewSession's void-overload to delegate instead of copy-pasting the whole method body. Split out (unrelated to SessionResolver infrastructure, reverted from this PR): - KeyValueMappingContext's JpaMappingConfigurationStrategy -> GormMappingConfigurationStrategy swap - untested, no registry-related justification found. - DirtyCheckingSupport's O(elements)/transitive dirty-checking change - algorithmic and semantic change, zero tests. - AstUtils's annotation-copy dedup change - unrelated AST behavior change, no coverage. - Dropped MappingContext.setMultiTenancyMode and ClassUtils.getIntegerFromMap - zero callers anywhere in the codebase. Every touched class has new or updated Spock coverage, including the specific gaps the review called out as untested: transaction precedence (resolver reads the same store as TSM), nested-session restoration, concrete-datastore publisher wiring, and @PreDestroy cleanup. Full test sweep across grails-datastore-core, grails-datamapping-core, grails-data-mongodb-core, grails-data-simple, grails-data-hibernate5-core, and grails-data-hibernate7-core: BUILD SUCCESSFUL, 0 failures. Co-Authored-By: Claude Sonnet 5 <[email protected]> Report URL: https://github.com/apache/grails-core/actions/runs/29070877197 With regards, GitHub Actions via GitBox
