Skip to content

Commit b461694

Browse files
jamesfredleyjamesfredley
committed
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 <Claude@Claude.ai>
1 parent 63beaf3 commit b461694

File tree

4 files changed

+399
-5
lines changed

4 files changed

+399
-5
lines changed

‎grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/WhereQueryMultiDataSourceSpec.groovy‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* https://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.grails.orm.hibernate.connections
20+
21+
import org.hibernate.dialect.H2Dialect
22+
import spock.lang.AutoCleanup
23+
import spock.lang.Issue
24+
import spock.lang.Shared
25+
import spock.lang.Specification
26+
27+
import grails.gorm.annotation.Entity
28+
import grails.gorm.services.Service
29+
import grails.gorm.services.Where
30+
import grails.gorm.transactions.Transactional
31+
import org.grails.datastore.gorm.GormEnhancer
32+
import org.grails.datastore.gorm.GormEntity
33+
import org.grails.datastore.mapping.core.DatastoreUtils
34+
import org.grails.orm.hibernate.HibernateDatastore
35+
36+
@Issue("https://github.com/apache/grails-core/issues/15416")
37+
class WhereQueryMultiDataSourceSpec extends Specification {
38+
39+
@Shared Map config = [
40+
'dataSource.url':"jdbc:h2:mem:defaultDB;LOCK_TIMEOUT=10000",
41+
'dataSource.dbCreate': 'create-drop',
42+
'dataSource.dialect': H2Dialect.name,
43+
'dataSource.formatSql': 'true',
44+
'hibernate.flush.mode': 'COMMIT',
45+
'hibernate.cache.queries': 'true',
46+
'hibernate.hbm2ddl.auto': 'create-drop',
47+
'dataSources.secondary':[url:"jdbc:h2:mem:secondaryDB;LOCK_TIMEOUT=10000"],
48+
]
49+
50+
@Shared @AutoCleanup HibernateDatastore datastore = new HibernateDatastore(
51+
DatastoreUtils.createPropertyResolver(config), Item
52+
)
53+
54+
@Shared ItemQueryService itemQueryService
55+
56+
void setupSpec() {
57+
itemQueryService = datastore
58+
.getDatastoreForConnection('secondary')
59+
.getService(ItemQueryService)
60+
}
61+
62+
void setup() {
63+
def api = GormEnhancer.findStaticApi(Item, 'secondary')
64+
api.withNewTransaction {
65+
api.executeUpdate('delete from Item')
66+
}
67+
GormEnhancer.findStaticApi(Item).withNewTransaction {
68+
GormEnhancer.findStaticApi(Item).executeUpdate('delete from Item')
69+
}
70+
}
71+
72+
void "@Where query routes to secondary datasource"() {
73+
given:
74+
saveToSecondary('Cheap', 10.0)
75+
saveToSecondary('Expensive', 500.0)
76+
77+
when:
78+
def results = itemQueryService.findByMinAmount(100.0)
79+
80+
then:
81+
results.size() == 1
82+
results[0].name == 'Expensive'
83+
}
84+
85+
void "@Where query does not return data from default datasource"() {
86+
given: 'an item saved to secondary'
87+
saveToSecondary('OnSecondary', 50.0)
88+
89+
and: 'a different item saved directly to default'
90+
saveToDefault('OnDefault', 999.0)
91+
92+
when: 'querying via @Where for amount >= 500 on secondary-bound service'
93+
def results = itemQueryService.findByMinAmount(500.0)
94+
95+
then: 'only secondary data is searched - default item is NOT found'
96+
results.size() == 0
97+
}
98+
99+
void "count routes to secondary datasource"() {
100+
given:
101+
saveToSecondary('A', 1.0)
102+
saveToSecondary('B', 2.0)
103+
104+
and: 'an item on default that should not be counted'
105+
saveToDefault('C', 3.0)
106+
107+
expect:
108+
itemQueryService.count() == 2
109+
}
110+
111+
void "list routes to secondary datasource"() {
112+
given:
113+
saveToSecondary('X', 10.0)
114+
saveToSecondary('Y', 20.0)
115+
116+
and: 'an item on default that should not be listed'
117+
saveToDefault('Z', 30.0)
118+
119+
when:
120+
def all = itemQueryService.list()
121+
122+
then:
123+
all.size() == 2
124+
}
125+
126+
void "findByName routes to secondary datasource"() {
127+
given:
128+
saveToSecondary('Unique', 77.0)
129+
130+
when:
131+
def found = itemQueryService.findByName('Unique')
132+
133+
then:
134+
found != null
135+
found.name == 'Unique'
136+
found.amount == 77.0
137+
}
138+
139+
private void saveToSecondary(String name, Double amount) {
140+
def api = GormEnhancer.findStaticApi(Item, 'secondary')
141+
api.withNewTransaction {
142+
def instanceApi = GormEnhancer.findInstanceApi(Item, 'secondary')
143+
def item = new Item(name: name, amount: amount)
144+
instanceApi.save(item, [flush: true])
145+
}
146+
}
147+
148+
private void saveToDefault(String name, Double amount) {
149+
def api = GormEnhancer.findStaticApi(Item)
150+
api.withNewTransaction {
151+
def instanceApi = GormEnhancer.findInstanceApi(Item)
152+
def item = new Item(name: name, amount: amount)
153+
instanceApi.save(item, [flush: true])
154+
}
155+
}
156+
}
157+
158+
@Entity
159+
class Item implements GormEntity<Item> {
160+
Long id
161+
Long version
162+
String name
163+
Double amount
164+
165+
static mapping = {
166+
datasource 'ALL'
167+
}
168+
169+
static constraints = {
170+
name blank: false
171+
amount nullable: false
172+
}
173+
}
174+
175+
@Service(Item)
176+
@Transactional(connection = 'secondary')
177+
interface ItemQueryService {
178+
179+
Item findByName(String name)
180+
181+
Number count()
182+
183+
List<Item> list()
184+
185+
@Where({ amount >= minAmount })
186+
List<Item> findByMinAmount(Double minAmount)
187+
}

‎grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ abstract class AbstractDetachedCriteriaServiceImplementor extends AbstractReadOp
7575
body.addStatement(
7676
declS(queryVar, ctorX(getDetachedCriteriaType(domainClassNode), args(classX(domainClassNode.plainNodeReference))))
7777
)
78-
Expression connectionId = findConnectionId(newMethodNode)
78+
Expression connectionId = findConnectionId(abstractMethodNode)
7979

8080
if (connectionId != null) {
8181
body.addStatement(

‎grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,16 @@ abstract class AbstractWhereImplementer extends AbstractReadOperationImplementer
9696
body.addStatement(
9797
declS(queryVar, ctorX(getDetachedCriteriaType(domainClassNode), args(classX(domainClassNode.plainNodeReference))))
9898
)
99-
Expression connectionId = findConnectionId(newMethodNode)
99+
body.addStatement(
100+
assignS(queryVar, callX(queryVar, 'build', closureExpression))
101+
)
102+
Expression connectionId = findConnectionId(abstractMethodNode)
100103

101104
if (connectionId != null) {
102105
body.addStatement(
103106
assignS(queryVar, callX(queryVar, 'withConnection', connectionId))
104107
)
105108
}
106-
body.addStatement(
107-
assignS(queryVar, callX(queryVar, 'build', closureExpression))
108-
)
109109
Expression queryExpression = callX(queryVar, getQueryMethodToExecute(domainClassNode, newMethodNode), argsExpression != null ? argsExpression : AstUtils.ZERO_ARGUMENTS)
110110
body.addStatement(
111111
buildReturnStatement(domainClassNode, abstractMethodNode, newMethodNode, queryExpression)

0 commit comments

Comments
 (0)