fix: extend OSIV to manage sessions for all datasources#15425
fix: extend OSIV to manage sessions for all datasources#15425jdaugherty merged 7 commits into7.0.xfrom
Conversation
GrailsOpenSessionInViewInterceptor only registered a session for the default datasource SessionFactory. Calling withSession on a secondary datasource during a web request threw 'No Session found for current thread' because no session was bound for that SessionFactory. The interceptor now iterates all connection sources from the HibernateDatastore and opens/binds sessions for each non-default datasource in preHandle, flushes them in postHandle, and unbinds/closes them in afterCompletion. Existing sessions that are already bound (e.g. by a transaction) are left untouched. Fixes #14333 Fixes #11798 Assisted-by: Claude Code <Claude@Claude.ai>
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical issue where GrailsOpenSessionInViewInterceptor only managed sessions for the default datasource, causing "No Session found for current thread" errors when using withSession on secondary datasources during web requests (issues #14333 and #11798).
Changes:
- Extended
GrailsOpenSessionInViewInterceptorto discover and manage sessions for all configured datasources, not just the default - Added comprehensive unit tests directly testing the interceptor's multi-datasource session management
- Added functional integration tests that verify the fix through HTTP requests to a controller using secondary datasource operations
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
GrailsOpenSessionInViewInterceptor.java |
Core fix: extends OSIV to iterate all connection sources and open/bind/close sessions for each non-default datasource alongside the default |
grails-plugin/build.gradle |
Added servlet-api test dependency for unit tests |
MultiDataSourceSessionSpec.groovy |
New unit tests verifying session binding, cleanup, skip-if-bound behavior, and CRUD operations |
SecondaryBookController.groovy |
Test controller providing endpoints for functional validation of withSession, validation, and executeUpdate scenarios |
UrlMappings.groovy |
URL mappings to route requests to test controller |
MultiDataSourceWithSessionSpec.groovy |
Integration tests exercising the fix via HTTP to verify both reported issues are resolved |
grails-multiple-datasources/build.gradle |
Added HTTP client and URL mappings dependencies for functional tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.../src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java
Outdated
Show resolved
Hide resolved
.../src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java
Show resolved
Hide resolved
.../src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java
Show resolved
Hide resolved
.../src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java
Show resolved
Hide resolved
Wrap the additional session cleanup loop in try-finally so that super.afterCompletion() always runs even if closing an additional session throws. Add per-session try-catch around closeSession with error logging, matching HibernatePersistenceContextInterceptor.destroy() pattern. Assisted-by: Claude Code <Claude@Claude.ai>
.../src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java
Outdated
Show resolved
Hide resolved
...rails-multiple-datasources/grails-app/controllers/datasources/SecondaryBookController.groovy
Show resolved
Hide resolved
Include the datasource connection name in debug and error log messages for GrailsOpenSessionInViewInterceptor to aid multi-datasource debugging. Add a Geb integration test in the datasources test module that verifies OSIV keeps the secondary datasource session open during GSP view rendering, allowing lazy-loaded associations to be accessed without LazyInitializationException. Assisted-by: Claude Code <Claude@Claude.ai>
|
Looks like this has some legitimate test failures |
Use Book.secondary.findByTitle instead of Book.secondary.first to avoid picking up stale records from other integration tests sharing the same H2 database. Assisted-by: Claude Code <Claude@Claude.ai>
Per-session try-catch in postHandle ensures all additional datasource sessions are flushed independently. If one datasource's flush fails (e.g. constraint violation), the remaining datasources still get their flush attempt. Exceptions are accumulated via addSuppressed and re-thrown after all sessions are processed. Addresses jdaugherty's review feedback that a data integrity problem in one datasource should not prevent other independent datasources from saving. Assisted-by: OpenCode <opencode@opencode.ai>
.../src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java
Show resolved
Hide resolved
…eptor When multiple datasource sessions fail to flush, subsequent exceptions are added as suppressed to the first. Log each suppressed exception at DEBUG level with the datasource connection name to aid troubleshooting. Assisted-by: OpenCode <opencode@opencode.ai>
jdaugherty
left a comment
There was a problem hiding this comment.
I think this makes sense, but we need another reviewer for this one given the potential for impact.
davydotcom
left a comment
There was a problem hiding this comment.
Test line up with what would happen in a real world scenario. This has been broken a while I dont think we often use multi datasource in a project. But it makes sense that these should be activated in that scenario.
Summary
Fixes #14333 and #11798 - both have the same root cause.
GrailsOpenSessionInViewInterceptoronly registered a session for the default datasource'sSessionFactory. CallingwithSessionon a secondary datasource during a web request threwNo Session found for current threadbecause no session was bound for thatSessionFactoryinTransactionSynchronizationManager.Root Cause
In
HibernateDatastoreSpringInitializer(lines 189-194), only one OSIV interceptor is created referencinghibernateDatastore(the default). The interceptor'ssetHibernateDatastore()calledsetSessionFactory(hibernateDatastore.getSessionFactory())- binding only the defaultSessionFactory.Call chain for #14333:
GormEntity.withSession->AbstractHibernateGormStaticApi.withSessionAbstractHibernateDatastore.withSession->getHibernateTemplate().execute(callable)GrailsHibernateTemplate.doExecute->getSession()->sessionFactory.getCurrentSession()GrailsSessionContext.currentSession()checksTransactionSynchronizationManager.getResource(sessionFactory)for the secondarySessionFactoryNo Session found#11798 has the same root cause: domain class mapped to non-default datasource used as command object fails validation because
validate()needs a session but OSIV never opened one.Fix
Modified
GrailsOpenSessionInViewInterceptorto handle multiple datasourceSessionFactoryinstances in a single interceptor:setHibernateDatastore()now iterates all connection sources fromHibernateDatastoreand stores non-default datasourceSessionFactoryreferences in anadditionalSessionFactorieslistpreHandle()opens sessions and bindsSessionHoldertoTransactionSynchronizationManagerfor each additional datasource (skipping any that already have a session bound)postHandle()flushes additional sessions if their flush mode warrants itafterCompletion()unbinds and closes additional sessions in reverse orderThis approach keeps a single interceptor bean (backward compatible) and avoids the complexity of registering per-datasource OSIV beans.
Tests
Unit Tests (5 tests)
MultiDataSourceSessionSpec- Tests OSIV interceptor directly:Functional/Integration Tests (4 tests via HTTP)
MultiDataSourceWithSessionSpec- Tests through actual HTTP requests to a controller:withSessionon secondary datasource does not throw (covers Grails7 - Multi DB withSession does not work #14333)withSessionon secondary datasourcewithSessionon secondary datasource (covers Problem With Multiple Datasources #11798)withSessionworks afterexecuteUpdateon secondary datasourceExisting Tests
grails-data-hibernate5tests passgrails-data-hibernate5-coreconnection tests passgrails-multiple-datasourcesintegration tests passChanged Files
GrailsOpenSessionInViewInterceptor.javagrails-data-hibernate5/grails-plugin/build.gradleMultiDataSourceSessionSpec.groovySecondaryBookController.groovyUrlMappings.groovyMultiDataSourceWithSessionSpec.groovygrails-multiple-datasources/build.gradle