This is an automated email from the ASF dual-hosted git repository. jamesfredley pushed a commit to branch fix/multi-datasource-osiv in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit afffd5706445949a3e2c53e735b6fc8989638b7b Author: James Fredley <[email protected]> AuthorDate: Fri Feb 20 13:02:34 2026 -0500 fix: extend OSIV to manage sessions for all datasources 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 <[email protected]> --- grails-data-hibernate5/grails-plugin/build.gradle | 1 + .../GrailsOpenSessionInViewInterceptor.java | 111 +++++++++++- .../support/MultiDataSourceSessionSpec.groovy | 193 +++++++++++++++++++++ .../grails-multiple-datasources/build.gradle | 6 + .../datasources/SecondaryBookController.groovy | 78 +++++++++ .../controllers/datasources/UrlMappings.groovy | 34 ++++ .../MultiDataSourceWithSessionSpec.groovy | 86 +++++++++ 7 files changed, 507 insertions(+), 2 deletions(-) diff --git a/grails-data-hibernate5/grails-plugin/build.gradle b/grails-data-hibernate5/grails-plugin/build.gradle index 4a58e2a1af..61d6ffc06f 100644 --- a/grails-data-hibernate5/grails-plugin/build.gradle +++ b/grails-data-hibernate5/grails-plugin/build.gradle @@ -66,6 +66,7 @@ dependencies { testImplementation project(':grails-testing-support-datamapping') testImplementation 'org.spockframework:spock-core' + testImplementation 'jakarta.servlet:jakarta.servlet-api' testRuntimeOnly 'com.h2database:h2' testRuntimeOnly 'org.apache.tomcat:tomcat-jdbc' diff --git a/grails-data-hibernate5/grails-plugin/src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java b/grails-data-hibernate5/grails-plugin/src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java index 62bc0fdf0d..a378db3a0a 100644 --- a/grails-data-hibernate5/grails-plugin/src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java +++ b/grails-data-hibernate5/grails-plugin/src/main/groovy/org/grails/plugin/hibernate/support/GrailsOpenSessionInViewInterceptor.java @@ -18,22 +18,34 @@ */ package org.grails.plugin.hibernate.support; +import java.util.ArrayList; +import java.util.List; + import org.hibernate.FlushMode; import org.hibernate.Session; +import org.hibernate.SessionFactory; import org.springframework.dao.DataAccessException; import org.springframework.dao.DataAccessResourceFailureException; +import org.springframework.orm.hibernate5.SessionFactoryUtils; import org.springframework.orm.hibernate5.SessionHolder; import org.springframework.orm.hibernate5.support.OpenSessionInViewInterceptor; import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.ui.ModelMap; import org.springframework.web.context.request.WebRequest; +import org.grails.datastore.mapping.core.connections.ConnectionSource; import org.grails.orm.hibernate.AbstractHibernateDatastore; +import org.grails.orm.hibernate.HibernateDatastore; +import org.grails.orm.hibernate.connections.HibernateConnectionSourceSettings; /** - * Extends the default spring OSIV and doesn't flush the session if it has been set - * to MANUAL on the session itself. + * Extends the default Spring OSIV to support multiple datasources. + * <p> + * The default datasource's SessionFactory is managed by the parent class. + * Additional (non-default) datasource SessionFactories are managed by this + * subclass, which opens and closes sessions for each one alongside the + * default session. * * @author Graeme Rocher * @since 0.5 @@ -42,6 +54,22 @@ public class GrailsOpenSessionInViewInterceptor extends OpenSessionInViewInterce protected FlushMode hibernateFlushMode = FlushMode.MANUAL; + private final List<AdditionalSessionFactoryConfig> additionalSessionFactories = new ArrayList<>(); + + /** + * Holds configuration for an additional (non-default) SessionFactory + * that needs OSIV session management. + */ + private static class AdditionalSessionFactoryConfig { + final SessionFactory sessionFactory; + final FlushMode flushMode; + + AdditionalSessionFactoryConfig(SessionFactory sessionFactory, FlushMode flushMode) { + this.sessionFactory = sessionFactory; + this.flushMode = flushMode; + } + } + @Override protected Session openSession() throws DataAccessResourceFailureException { Session session = super.openSession(); @@ -53,6 +81,25 @@ public class GrailsOpenSessionInViewInterceptor extends OpenSessionInViewInterce session.setHibernateFlushMode(hibernateFlushMode); } + @Override + public void preHandle(WebRequest request) throws DataAccessException { + super.preHandle(request); + + for (AdditionalSessionFactoryConfig config : additionalSessionFactories) { + SessionFactory sf = config.sessionFactory; + if (TransactionSynchronizationManager.hasResource(sf)) { + continue; + } + if (logger.isDebugEnabled()) { + logger.debug("Opening additional Hibernate Session for datasource in OpenSessionInViewInterceptor"); + } + Session session = sf.openSession(); + session.setHibernateFlushMode(config.flushMode); + SessionHolder sessionHolder = new SessionHolder(session); + TransactionSynchronizationManager.bindResource(sf, sessionHolder); + } + } + @Override public void postHandle(WebRequest request, ModelMap model) throws DataAccessException { SessionHolder sessionHolder = (SessionHolder) TransactionSynchronizationManager.getResource(getSessionFactory()); @@ -73,6 +120,46 @@ public class GrailsOpenSessionInViewInterceptor extends OpenSessionInViewInterce session.setHibernateFlushMode(FlushMode.MANUAL); } } + + for (AdditionalSessionFactoryConfig config : additionalSessionFactories) { + SessionFactory sf = config.sessionFactory; + SessionHolder additionalHolder = (SessionHolder) TransactionSynchronizationManager.getResource(sf); + if (additionalHolder != null) { + Session additionalSession = additionalHolder.getSession(); + try { + FlushMode additionalFlushMode = additionalSession.getHibernateFlushMode(); + boolean additionalIsNotManual = additionalFlushMode != FlushMode.MANUAL && additionalFlushMode != FlushMode.COMMIT; + if (additionalIsNotManual) { + if (logger.isDebugEnabled()) { + logger.debug("Eagerly flushing additional Hibernate session"); + } + additionalSession.flush(); + } + } + finally { + additionalSession.setHibernateFlushMode(FlushMode.MANUAL); + } + } + } + } + + @Override + public void afterCompletion(WebRequest request, Exception ex) throws DataAccessException { + for (int i = additionalSessionFactories.size() - 1; i >= 0; i--) { + AdditionalSessionFactoryConfig config = additionalSessionFactories.get(i); + SessionFactory sf = config.sessionFactory; + SessionHolder sessionHolder = (SessionHolder) TransactionSynchronizationManager.getResource(sf); + if (sessionHolder != null) { + Session session = sessionHolder.getSession(); + TransactionSynchronizationManager.unbindResource(sf); + if (logger.isDebugEnabled()) { + logger.debug("Closing additional Hibernate Session in OpenSessionInViewInterceptor"); + } + SessionFactoryUtils.closeSession(session); + } + } + + super.afterCompletion(request, ex); } public void setHibernateDatastore(AbstractHibernateDatastore hibernateDatastore) { @@ -84,5 +171,25 @@ public class GrailsOpenSessionInViewInterceptor extends OpenSessionInViewInterce this.hibernateFlushMode = FlushMode.valueOf(defaultFlushModeName); } setSessionFactory(hibernateDatastore.getSessionFactory()); + + if (hibernateDatastore instanceof HibernateDatastore) { + HibernateDatastore hibernateDs = (HibernateDatastore) hibernateDatastore; + for (ConnectionSource<SessionFactory, HibernateConnectionSourceSettings> connectionSource : hibernateDs.getConnectionSources().getAllConnectionSources()) { + String connectionName = connectionSource.getName(); + if (!ConnectionSource.DEFAULT.equals(connectionName)) { + AbstractHibernateDatastore childDatastore = hibernateDs.getDatastoreForConnection(connectionName); + FlushMode childFlushMode; + if (childDatastore.isOsivReadOnly()) { + childFlushMode = FlushMode.MANUAL; + } + else { + childFlushMode = FlushMode.valueOf(childDatastore.getDefaultFlushModeName()); + } + additionalSessionFactories.add( + new AdditionalSessionFactoryConfig(childDatastore.getSessionFactory(), childFlushMode) + ); + } + } + } } } diff --git a/grails-data-hibernate5/grails-plugin/src/test/groovy/org/grails/plugin/hibernate/support/MultiDataSourceSessionSpec.groovy b/grails-data-hibernate5/grails-plugin/src/test/groovy/org/grails/plugin/hibernate/support/MultiDataSourceSessionSpec.groovy new file mode 100644 index 0000000000..8b9e07513d --- /dev/null +++ b/grails-data-hibernate5/grails-plugin/src/test/groovy/org/grails/plugin/hibernate/support/MultiDataSourceSessionSpec.groovy @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.plugin.hibernate.support + +import grails.gorm.annotation.Entity +import org.grails.datastore.mapping.core.DatastoreUtils +import org.grails.orm.hibernate.HibernateDatastore +import org.hibernate.Session +import org.hibernate.SessionFactory +import org.hibernate.dialect.H2Dialect +import org.springframework.orm.hibernate5.SessionHolder +import org.springframework.transaction.support.TransactionSynchronizationManager +import org.springframework.web.context.request.WebRequest +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.lang.Specification + +class MultiDataSourceSessionSpec extends Specification { + + @Shared Map config = [ + 'dataSource.url':"jdbc:h2:mem:grailsDB;LOCK_TIMEOUT=10000", + 'dataSource.dbCreate': 'create-drop', + 'dataSource.dialect': H2Dialect.name, + 'dataSource.formatSql': 'true', + 'hibernate.flush.mode': 'COMMIT', + 'hibernate.cache.queries': 'true', + 'hibernate.hbm2ddl.auto': 'create-drop', + 'dataSources.secondary':[url:"jdbc:h2:mem:secondaryDb;LOCK_TIMEOUT=10000"], + ] + + @Shared @AutoCleanup HibernateDatastore datastore = new HibernateDatastore(DatastoreUtils.createPropertyResolver(config), OsivBook, OsivAuthor) + + def "withSession on default datasource works with OSIV"() { + given: "OSIV interceptor configured with the datastore" + def interceptor = new GrailsOpenSessionInViewInterceptor() + interceptor.setHibernateDatastore(datastore) + WebRequest webRequest = Mock(WebRequest) + + when: "OSIV preHandle is called" + interceptor.preHandle(webRequest) + + then: "a session is bound for the default SessionFactory" + TransactionSynchronizationManager.hasResource(datastore.sessionFactory) + + when: "withSession is called on default datasource" + boolean sessionObtained = false + OsivAuthor.withSession { Session s -> + sessionObtained = s != null + } + + then: "session is available" + sessionObtained + + cleanup: + interceptor.afterCompletion(webRequest, null) + } + + def "withSession on secondary datasource works with OSIV"() { + given: "OSIV interceptor configured with the datastore" + def interceptor = new GrailsOpenSessionInViewInterceptor() + interceptor.setHibernateDatastore(datastore) + WebRequest webRequest = Mock(WebRequest) + + when: "OSIV preHandle is called" + interceptor.preHandle(webRequest) + + then: "a session is bound for both the default and secondary SessionFactory" + def secondaryDatastore = datastore.getDatastoreForConnection('secondary') + TransactionSynchronizationManager.hasResource(datastore.sessionFactory) + TransactionSynchronizationManager.hasResource(secondaryDatastore.sessionFactory) + + when: "withSession is called on secondary datasource" + boolean sessionObtained = false + OsivBook.secondary.withSession { Session s -> + sessionObtained = s != null + } + + then: "session is available without 'No Session found for current thread' error" + sessionObtained + + cleanup: + interceptor.afterCompletion(webRequest, null) + } + + def "afterCompletion cleans up sessions for all datasources"() { + given: "OSIV interceptor with preHandle already called" + def interceptor = new GrailsOpenSessionInViewInterceptor() + interceptor.setHibernateDatastore(datastore) + WebRequest webRequest = Mock(WebRequest) + interceptor.preHandle(webRequest) + + def secondaryDatastore = datastore.getDatastoreForConnection('secondary') + + when: "afterCompletion is called" + interceptor.afterCompletion(webRequest, null) + + then: "sessions are unbound for all datasources" + !TransactionSynchronizationManager.hasResource(datastore.sessionFactory) + !TransactionSynchronizationManager.hasResource(secondaryDatastore.sessionFactory) + } + + def "OSIV skips secondary datasource if session already bound"() { + given: "a pre-bound session for the secondary datasource" + def interceptor = new GrailsOpenSessionInViewInterceptor() + interceptor.setHibernateDatastore(datastore) + WebRequest webRequest = Mock(WebRequest) + + def secondaryDatastore = datastore.getDatastoreForConnection('secondary') + SessionFactory secondarySf = secondaryDatastore.sessionFactory + Session preBoundSession = secondarySf.openSession() + SessionHolder preBoundHolder = new SessionHolder(preBoundSession) + TransactionSynchronizationManager.bindResource(secondarySf, preBoundHolder) + + when: "OSIV preHandle is called" + interceptor.preHandle(webRequest) + + then: "the pre-bound session is preserved (not replaced)" + def currentHolder = TransactionSynchronizationManager.getResource(secondarySf) + currentHolder.is(preBoundHolder) + + cleanup: + interceptor.afterCompletion(webRequest, null) + if (TransactionSynchronizationManager.hasResource(secondarySf)) { + TransactionSynchronizationManager.unbindResource(secondarySf) + } + preBoundSession.close() + } + + def "CRUD operations work on secondary datasource with OSIV"() { + given: "OSIV interceptor configured and preHandle called" + def interceptor = new GrailsOpenSessionInViewInterceptor() + interceptor.setHibernateDatastore(datastore) + WebRequest webRequest = Mock(WebRequest) + interceptor.preHandle(webRequest) + + when: "data is saved to secondary datasource within a transaction" + OsivBook book = OsivBook.withTransaction { + new OsivBook(title: "Test Book").save(flush: true) + OsivBook.first() + } + + then: "the book is saved successfully" + book != null + book.title == "Test Book" + + when: "data is read from secondary datasource using withSession" + int count = 0 + OsivBook.secondary.withSession { Session s -> + count = OsivBook.count() + } + + then: "the count is correct" + count == 1 + + cleanup: + OsivBook.withTransaction { + OsivBook.list()*.delete(flush: true) + } + interceptor.afterCompletion(webRequest, null) + } +} + +@Entity +class OsivBook { + String title + Date dateCreated + Date lastUpdated + + static mapping = { + datasource 'secondary' + } +} + +@Entity +class OsivAuthor { + String name +} diff --git a/grails-test-examples/hibernate5/grails-multiple-datasources/build.gradle b/grails-test-examples/hibernate5/grails-multiple-datasources/build.gradle index f9057b3f59..005369f776 100644 --- a/grails-test-examples/hibernate5/grails-multiple-datasources/build.gradle +++ b/grails-test-examples/hibernate5/grails-multiple-datasources/build.gradle @@ -44,12 +44,18 @@ dependencies { runtimeOnly 'com.zaxxer:HikariCP' runtimeOnly 'org.apache.grails:grails-databinding' runtimeOnly 'org.apache.grails:grails-controllers' + runtimeOnly 'org.apache.grails:grails-url-mappings' runtimeOnly 'org.springframework.boot:spring-boot-autoconfigure' runtimeOnly 'org.springframework.boot:spring-boot-starter-logging' runtimeOnly 'org.springframework.boot:spring-boot-starter-tomcat' testImplementation 'org.apache.grails.testing:grails-testing-support-core' + integrationTestImplementation "io.micronaut:micronaut-http-client:$micronautHttpClientVersion" + integrationTestImplementation 'org.spockframework:spock-core' + + integrationTestRuntimeOnly "io.micronaut.serde:micronaut-serde-jackson:$micronautSerdeJacksonVersion" + testRuntimeOnly 'org.apache.grails:grails-testing-support-web' } diff --git a/grails-test-examples/hibernate5/grails-multiple-datasources/grails-app/controllers/datasources/SecondaryBookController.groovy b/grails-test-examples/hibernate5/grails-multiple-datasources/grails-app/controllers/datasources/SecondaryBookController.groovy new file mode 100644 index 0000000000..01a84ac465 --- /dev/null +++ b/grails-test-examples/hibernate5/grails-multiple-datasources/grails-app/controllers/datasources/SecondaryBookController.groovy @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package datasources + +import ds2.Book +import org.hibernate.Session + +class SecondaryBookController { + + def withSessionTest() { + boolean sessionObtained = false + Book.secondary.withSession { Session session -> + sessionObtained = session != null + } + render "sessionObtained:${sessionObtained}" + } + + def crudViaWithSession() { + Book.withTransaction { + new Book(title: 'OSIV Test').save(flush: true) + } + int count = 0 + Book.secondary.withSession { Session session -> + count = Book.count() + } + render "count:${count}" + } + + def validateCommandObject() { + Book book = new Book() + boolean validated = false + Book.secondary.withSession { Session session -> + validated = true + book.validate() + } + render "validated:${validated},hasErrors:${book.hasErrors()}" + } + + def sessionAfterExecuteUpdate() { + Book.withTransaction { + new Book(title: 'Before Update').save(flush: true) + } + Book.secondary.withTransaction { + Book.executeUpdate('UPDATE Book b SET b.title = :newTitle WHERE b.title = :oldTitle', + [newTitle: 'After Update', oldTitle: 'Before Update']) + } + String title = null + Book.secondary.withSession { Session session -> + session.clear() + title = Book.first()?.title + } + render "title:${title}" + } + + def cleanup() { + Book.withTransaction { + Book.list()*.delete(flush: true) + } + render "cleaned" + } +} diff --git a/grails-test-examples/hibernate5/grails-multiple-datasources/grails-app/controllers/datasources/UrlMappings.groovy b/grails-test-examples/hibernate5/grails-multiple-datasources/grails-app/controllers/datasources/UrlMappings.groovy new file mode 100644 index 0000000000..b9b3170933 --- /dev/null +++ b/grails-test-examples/hibernate5/grails-multiple-datasources/grails-app/controllers/datasources/UrlMappings.groovy @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package datasources + +class UrlMappings { + + static mappings = { + "/$controller/$action?/$id?(.$format)?" { + constraints { + } + } + + "/"(view: '/index') + "500"(view: '/error') + "404"(view: '/notFound') + } +} diff --git a/grails-test-examples/hibernate5/grails-multiple-datasources/src/integration-test/groovy/functionaltests/MultiDataSourceWithSessionSpec.groovy b/grails-test-examples/hibernate5/grails-multiple-datasources/src/integration-test/groovy/functionaltests/MultiDataSourceWithSessionSpec.groovy new file mode 100644 index 0000000000..6a8c650809 --- /dev/null +++ b/grails-test-examples/hibernate5/grails-multiple-datasources/src/integration-test/groovy/functionaltests/MultiDataSourceWithSessionSpec.groovy @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package functionaltests + +import datasources.Application +import grails.testing.mixin.integration.Integration +import grails.testing.spock.OnceBefore +import io.micronaut.http.client.HttpClient +import spock.lang.Issue +import spock.lang.Shared +import spock.lang.Specification +import spock.lang.Stepwise + +@Integration(applicationClass = Application) +@Stepwise +class MultiDataSourceWithSessionSpec extends Specification { + + @Shared + HttpClient client + + @OnceBefore + void init() { + String baseUrl = "http://localhost:$serverPort"; + this.client = HttpClient.create(baseUrl.toURL()) + } + + @Issue('https://github.com/apache/grails-core/issues/14333') + void "withSession on secondary datasource does not throw No Session found"() { + when: + String response = client.toBlocking().retrieve('/secondaryBook/withSessionTest') + + then: + response.contains('sessionObtained:true') + } + + @Issue('https://github.com/apache/grails-core/issues/14333') + void "CRUD via withSession on secondary datasource works"() { + when: + String response = client.toBlocking().retrieve('/secondaryBook/crudViaWithSession') + + then: + response.contains('count:1') + + cleanup: + client.toBlocking().retrieve('/secondaryBook/cleanup') + } + + @Issue('https://github.com/apache/grails-core/issues/11798') + void "domain class on secondary datasource can be validated via withSession"() { + when: + String response = client.toBlocking().retrieve('/secondaryBook/validateCommandObject') + + then: + response.contains('validated:true') + response.contains('hasErrors:true') + } + + @Issue('https://github.com/apache/grails-core/issues/14333') + void "withSession works after executeUpdate on secondary datasource"() { + when: + String response = client.toBlocking().retrieve('/secondaryBook/sessionAfterExecuteUpdate') + + then: + response.contains('title:After Update') + + cleanup: + client.toBlocking().retrieve('/secondaryBook/cleanup') + } +}
