This is an automated email from the ASF dual-hosted git repository. jamesfredley pushed a commit to branch fix/where-connection-routing in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit b46169472944eead45fdf3dbfff43b59d3f500bc Author: James Fredley <[email protected]> AuthorDate: Thu Feb 19 17:54:05 2026 -0500 Fix @Where and DetachedCriteria query methods ignoring @Transactional(connection) Data Service methods using @Where annotations or DetachedCriteria-based queries (count, list, findBy*) were ignoring the class-level @Transactional(connection) annotation, causing queries to execute against the default datasource instead of the specified connection. Root cause: Both AbstractWhereImplementer and AbstractDetachedCriteriaServiceImplementor called findConnectionId(newMethodNode) instead of findConnectionId(abstractMethodNode). The newMethodNode belongs to the generated $ServiceImplementation class which lacks the @Transactional annotation, while abstractMethodNode belongs to the original interface/abstract class where the annotation is declared. Additionally, in AbstractWhereImplementer the build() call was placed after withConnection(), but DetachedCriteria.clone() (called internally by build) does not copy the connectionName field, causing the connection setting to be lost. Fixed by reordering build() before withConnection(). Fixes #15416 Assisted-by: Claude Code <[email protected]> --- .../WhereQueryMultiDataSourceSpec.groovy | 187 +++++++++++++++++++ ...stractDetachedCriteriaServiceImplementor.groovy | 2 +- .../implementers/AbstractWhereImplementer.groovy | 8 +- .../services/WhereConnectionRoutingSpec.groovy | 207 +++++++++++++++++++++ 4 files changed, 399 insertions(+), 5 deletions(-) diff --git a/grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/WhereQueryMultiDataSourceSpec.groovy b/grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/WhereQueryMultiDataSourceSpec.groovy new file mode 100644 index 0000000000..fcd69ff93e --- /dev/null +++ b/grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/WhereQueryMultiDataSourceSpec.groovy @@ -0,0 +1,187 @@ +/* + * 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.orm.hibernate.connections + +import org.hibernate.dialect.H2Dialect +import spock.lang.AutoCleanup +import spock.lang.Issue +import spock.lang.Shared +import spock.lang.Specification + +import grails.gorm.annotation.Entity +import grails.gorm.services.Service +import grails.gorm.services.Where +import grails.gorm.transactions.Transactional +import org.grails.datastore.gorm.GormEnhancer +import org.grails.datastore.gorm.GormEntity +import org.grails.datastore.mapping.core.DatastoreUtils +import org.grails.orm.hibernate.HibernateDatastore + +@Issue("https://github.com/apache/grails-core/issues/15416";) +class WhereQueryMultiDataSourceSpec extends Specification { + + @Shared Map config = [ + 'dataSource.url':"jdbc:h2:mem:defaultDB;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), Item + ) + + @Shared ItemQueryService itemQueryService + + void setupSpec() { + itemQueryService = datastore + .getDatastoreForConnection('secondary') + .getService(ItemQueryService) + } + + void setup() { + def api = GormEnhancer.findStaticApi(Item, 'secondary') + api.withNewTransaction { + api.executeUpdate('delete from Item') + } + GormEnhancer.findStaticApi(Item).withNewTransaction { + GormEnhancer.findStaticApi(Item).executeUpdate('delete from Item') + } + } + + void "@Where query routes to secondary datasource"() { + given: + saveToSecondary('Cheap', 10.0) + saveToSecondary('Expensive', 500.0) + + when: + def results = itemQueryService.findByMinAmount(100.0) + + then: + results.size() == 1 + results[0].name == 'Expensive' + } + + void "@Where query does not return data from default datasource"() { + given: 'an item saved to secondary' + saveToSecondary('OnSecondary', 50.0) + + and: 'a different item saved directly to default' + saveToDefault('OnDefault', 999.0) + + when: 'querying via @Where for amount >= 500 on secondary-bound service' + def results = itemQueryService.findByMinAmount(500.0) + + then: 'only secondary data is searched - default item is NOT found' + results.size() == 0 + } + + void "count routes to secondary datasource"() { + given: + saveToSecondary('A', 1.0) + saveToSecondary('B', 2.0) + + and: 'an item on default that should not be counted' + saveToDefault('C', 3.0) + + expect: + itemQueryService.count() == 2 + } + + void "list routes to secondary datasource"() { + given: + saveToSecondary('X', 10.0) + saveToSecondary('Y', 20.0) + + and: 'an item on default that should not be listed' + saveToDefault('Z', 30.0) + + when: + def all = itemQueryService.list() + + then: + all.size() == 2 + } + + void "findByName routes to secondary datasource"() { + given: + saveToSecondary('Unique', 77.0) + + when: + def found = itemQueryService.findByName('Unique') + + then: + found != null + found.name == 'Unique' + found.amount == 77.0 + } + + private void saveToSecondary(String name, Double amount) { + def api = GormEnhancer.findStaticApi(Item, 'secondary') + api.withNewTransaction { + def instanceApi = GormEnhancer.findInstanceApi(Item, 'secondary') + def item = new Item(name: name, amount: amount) + instanceApi.save(item, [flush: true]) + } + } + + private void saveToDefault(String name, Double amount) { + def api = GormEnhancer.findStaticApi(Item) + api.withNewTransaction { + def instanceApi = GormEnhancer.findInstanceApi(Item) + def item = new Item(name: name, amount: amount) + instanceApi.save(item, [flush: true]) + } + } +} + +@Entity +class Item implements GormEntity<Item> { + Long id + Long version + String name + Double amount + + static mapping = { + datasource 'ALL' + } + + static constraints = { + name blank: false + amount nullable: false + } +} + +@Service(Item) +@Transactional(connection = 'secondary') +interface ItemQueryService { + + Item findByName(String name) + + Number count() + + List<Item> list() + + @Where({ amount >= minAmount }) + List<Item> findByMinAmount(Double minAmount) +} diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy index 845aa25fe6..25cf1849ec 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy @@ -75,7 +75,7 @@ abstract class AbstractDetachedCriteriaServiceImplementor extends AbstractReadOp body.addStatement( declS(queryVar, ctorX(getDetachedCriteriaType(domainClassNode), args(classX(domainClassNode.plainNodeReference)))) ) - Expression connectionId = findConnectionId(newMethodNode) + Expression connectionId = findConnectionId(abstractMethodNode) if (connectionId != null) { body.addStatement( diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy index 7944c32f4a..01125f4de3 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy @@ -96,16 +96,16 @@ abstract class AbstractWhereImplementer extends AbstractReadOperationImplementer body.addStatement( declS(queryVar, ctorX(getDetachedCriteriaType(domainClassNode), args(classX(domainClassNode.plainNodeReference)))) ) - Expression connectionId = findConnectionId(newMethodNode) + body.addStatement( + assignS(queryVar, callX(queryVar, 'build', closureExpression)) + ) + Expression connectionId = findConnectionId(abstractMethodNode) if (connectionId != null) { body.addStatement( assignS(queryVar, callX(queryVar, 'withConnection', connectionId)) ) } - body.addStatement( - assignS(queryVar, callX(queryVar, 'build', closureExpression)) - ) Expression queryExpression = callX(queryVar, getQueryMethodToExecute(domainClassNode, newMethodNode), argsExpression != null ? argsExpression : AstUtils.ZERO_ARGUMENTS) body.addStatement( buildReturnStatement(domainClassNode, abstractMethodNode, newMethodNode, queryExpression) diff --git a/grails-datamapping-core/src/test/groovy/grails/gorm/services/WhereConnectionRoutingSpec.groovy b/grails-datamapping-core/src/test/groovy/grails/gorm/services/WhereConnectionRoutingSpec.groovy new file mode 100644 index 0000000000..ca4f217019 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/grails/gorm/services/WhereConnectionRoutingSpec.groovy @@ -0,0 +1,207 @@ +/* + * 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 grails.gorm.services + +import grails.gorm.transactions.Transactional +import org.grails.datastore.gorm.services.Implemented +import org.grails.datastore.gorm.services.implementers.FindAllWhereImplementer +import org.grails.datastore.gorm.services.implementers.FindOneWhereImplementer +import org.grails.datastore.gorm.services.implementers.CountWhereImplementer +import org.grails.datastore.gorm.services.implementers.CountImplementer +import org.grails.datastore.gorm.services.implementers.FindAllByImplementer +import org.grails.datastore.gorm.services.implementers.FindAllImplementer +import org.grails.datastore.gorm.services.implementers.FindOneImplementer +import spock.lang.Specification + +class WhereConnectionRoutingSpec extends Specification { + + void "test @Where method on interface with @Transactional(connection)"() { + when:"The service transform is applied to an interface with @Transactional(connection)" + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.services.Where +import grails.gorm.annotation.Entity +import grails.gorm.transactions.Transactional + +@Service(Foo) +@Transactional(connection = 'secondary') +interface FooService { + + @Where({ title ==~ pattern }) + Foo findByTitle(String pattern) +} +@Entity +class Foo { + String title + static mapping = { + datasource 'secondary' + } +} +''') + + then: + service.isInterface() + + when:"the impl is obtained" + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then:"The impl is valid" + impl.getAnnotation(Transactional) != null + impl.getAnnotation(Transactional).connection() == 'secondary' + impl.getMethod("findByTitle", String).getAnnotation(Implemented).by() == FindOneWhereImplementer + } + + void "test @Where method on abstract class with @Transactional(connection)"() { + when:"The service transform is applied to an abstract class" + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.services.Where +import grails.gorm.annotation.Entity +import grails.gorm.transactions.Transactional + +@Service(Foo) +@Transactional(connection = 'secondary') +abstract class FooService { + + @Where({ title ==~ pattern }) + abstract List<Foo> searchByTitle(String pattern) +} +@Entity +class Foo { + String title + static mapping = { + datasource 'secondary' + } +} +''') + + then: + !service.isInterface() + + when:"the impl is obtained" + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then:"The impl is valid" + impl.getAnnotation(Transactional) != null + impl.getAnnotation(Transactional).connection() == 'secondary' + impl.getMethod("searchByTitle", String).getAnnotation(Implemented).by() == FindAllWhereImplementer + } + + void "test count method uses connection-aware implementer"() { + when:"The service transform is applied to an interface with count()" + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity +import grails.gorm.transactions.Transactional + +@Service(Foo) +@Transactional(connection = 'secondary') +interface FooService { + Number count() +} +@Entity +class Foo { + String title + static mapping = { + datasource 'secondary' + } +} +''') + + then: + service.isInterface() + + when:"the impl is obtained" + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then:"The impl is valid" + impl.getAnnotation(Transactional) != null + impl.getAnnotation(Transactional).connection() == 'secondary' + impl.getMethod("count").getAnnotation(Implemented).by() == CountImplementer + } + + void "test list and findAll methods use connection-aware implementer"() { + when:"The service transform is applied to an interface with list and findAll methods" + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity +import grails.gorm.transactions.Transactional + +@Service(Foo) +@Transactional(connection = 'secondary') +interface FooService { + List<Foo> list() + List<Foo> findAllByTitle(String title) + Foo find(Serializable id) +} +@Entity +class Foo { + String title + static mapping = { + datasource 'secondary' + } +} +''') + + then: + service.isInterface() + + when:"the impl is obtained" + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then:"The impl is valid" + impl.getAnnotation(Transactional) != null + impl.getAnnotation(Transactional).connection() == 'secondary' + impl.getMethod("list").getAnnotation(Implemented).by() == FindAllImplementer + impl.getMethod("findAllByTitle", String).getAnnotation(Implemented).by() == FindAllByImplementer + impl.getMethod("find", Serializable).getAnnotation(Implemented).by() == FindOneImplementer + } + + void "test @Where method without @Transactional(connection)"() { + when:"The service transform is applied to an interface with @Where" + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.services.Where +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + + @Where({ title ==~ pattern }) + Number countByTitle(String pattern) +} +@Entity +class Foo { + String title + static mapping = { + datasource 'secondary' + } +} +''') + + then: + service.isInterface() + + when:"the impl is obtained" + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then:"The impl is valid" + impl.getMethod("countByTitle", String).getAnnotation(Implemented).by() == CountWhereImplementer + } +}
