Copilot commented on code in PR #15435:
URL: https://github.com/apache/grails-core/pull/15435#discussion_r2836926682
##########
grails-test-examples/datasources/src/integration-test/groovy/functionaltests/DatasourceSwitchingSpec.groovy:
##########
@@ -367,4 +367,90 @@ class DatasourceSwitchingSpec extends Specification {
and: "secondary books still exist"
SecondBook.withTransaction { SecondBook.countByTitleLike("Delete Me%")
} == 2
}
+
+ def "executeQuery routes to secondary datasource"() {
+ given: "books in both datasources"
+ def primaryTitle = "Primary Query ${UUID.randomUUID()}"
+ def secondaryTitle = "Secondary Query ${UUID.randomUUID()}"
+ new Book(title: primaryTitle).save(flush: true)
+ SecondBook.withTransaction {
+ new SecondBook(title: secondaryTitle).save(flush: true)
+ }
+
+ when: "executing query on secondary"
+ def results
+ SecondBook.withTransaction {
+ results = SecondBook.executeQuery("from ds2.Book where title =
:t", [t: secondaryTitle])
+ }
+
+ then: "only secondary data is returned"
+ results.size() == 1
+ results.first().title == secondaryTitle
+ }
+
+ def "withCriteria routes to secondary datasource"() {
+ given: "books in both datasources"
+ def primaryTitle = "Primary Criteria ${UUID.randomUUID()}"
+ def secondaryTitle = "Secondary Criteria ${UUID.randomUUID()}"
+ new Book(title: primaryTitle).save(flush: true)
+ SecondBook.withTransaction {
+ new SecondBook(title: secondaryTitle).save(flush: true)
+ }
+
+ when: "executing criteria on secondary"
+ def results
+ SecondBook.withTransaction {
+ results = SecondBook.withCriteria {
+ eq('title', secondaryTitle)
+ }
+ }
+
+ then: "only secondary data is returned"
+ results.size() == 1
+ results.first().title == secondaryTitle
+ }
+
+ def "createCriteria routes to secondary datasource"() {
+ given: "books in both datasources"
+ def primaryTitle = "Primary CreateCriteria ${UUID.randomUUID()}"
+ def secondaryTitle = "Secondary CreateCriteria ${UUID.randomUUID()}"
+ new Book(title: primaryTitle).save(flush: true)
+ SecondBook.withTransaction {
+ new SecondBook(title: secondaryTitle).save(flush: true)
+ }
+
+ when: "executing createCriteria on secondary"
+ def results
+ SecondBook.withTransaction {
+ results = SecondBook.createCriteria().list {
+ eq('title', secondaryTitle)
+ }
+ }
+
+ then: "only secondary data is returned"
+ results.size() == 1
+ results.first().title == secondaryTitle
+ }
+
+ def "executeUpdate routes to secondary datasource"() {
+ given: "books in both datasources"
+ def primaryTitle = "Primary Update ${UUID.randomUUID()}"
+ def secondaryTitle = "Secondary Update ${UUID.randomUUID()}"
+ def updatedTitle = "Secondary Updated ${UUID.randomUUID()}"
+ new Book(title: primaryTitle).save(flush: true)
+ SecondBook.withTransaction {
+ new SecondBook(title: secondaryTitle).save(flush: true)
+ }
+
+ when: "executing update on secondary"
+ int updated
+ SecondBook.withTransaction {
+ updated = SecondBook.executeUpdate("update ds2.Book set title =
:newTitle where title = :oldTitle", [newTitle: updatedTitle, oldTitle:
secondaryTitle])
Review Comment:
Same as above: the update HQL hard-codes `ds2.Book`. Using the domain class’
actual name (e.g., derived from `SecondBook`) avoids stringly-typed coupling
and keeps the test focused on datasource routing rather than a fixed package
name.
```suggestion
updated = SecondBook.executeUpdate("update ${SecondBook.name}
set title = :newTitle where title = :oldTitle", [newTitle: updatedTitle,
oldTitle: secondaryTitle])
```
##########
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormEnhancerAllQualifiersSpec.groovy:
##########
@@ -102,6 +102,38 @@ class GormEnhancerAllQualifiersSpec extends Specification {
qualifiers == ['secondary']
}
+ void "registerEntity adds static api under default and secondary for
non-default datasource"() {
+ given: "a non-MultiTenant entity with datasource 'secondary'"
+ def enhancer = createEnhancer()
+ def entity = mockEntity(NonMultiTenantSecondaryEntity, ['secondary'])
+ def staticApis = GormEnhancer.@STATIC_APIS
+ staticApis.get(ConnectionSource.DEFAULT).remove(entity.name)
+ staticApis.get('secondary').remove(entity.name)
+
+ when: "registering the entity"
+ enhancer.registerEntity(entity)
+
+ then: "static api is available under DEFAULT and secondary qualifiers"
+ staticApis.get(ConnectionSource.DEFAULT).containsKey(entity.name)
+ staticApis.get('secondary').containsKey(entity.name)
Review Comment:
These assertions reach into the private `GormEnhancer.@STATIC_APIS` map and
only verify the raw key exists. That makes the test brittle to internal
refactors and can miss real lookup issues (e.g., if `NameUtils.getClassName()`
diverges from `entity.name`). Prefer asserting via the public API
(`GormEnhancer.findStaticApi(YourClass, qualifier)` does not throw) for DEFAULT
and 'secondary' to validate behavior the production code actually uses.
##########
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormEnhancerAllQualifiersSpec.groovy:
##########
@@ -159,6 +191,20 @@ class GormEnhancerAllQualifiersSpec extends Specification {
qualifiers == [ConnectionSource.DEFAULT]
}
+ void "registerEntity adds static api under default for default
datasource"() {
+ given: "a non-MultiTenant entity on the default datasource"
+ def enhancer = createEnhancer()
+ def entity = mockEntity(NonMultiTenantDefaultEntity,
[ConnectionSource.DEFAULT])
+ def staticApis = GormEnhancer.@STATIC_APIS
+ staticApis.get(ConnectionSource.DEFAULT).remove(entity.name)
+
+ when: "registering the entity"
+ enhancer.registerEntity(entity)
+
+ then: "static api is available under DEFAULT qualifier"
+ staticApis.get(ConnectionSource.DEFAULT).containsKey(entity.name)
Review Comment:
This test also inspects `GormEnhancer.@STATIC_APIS` directly. Using
`GormEnhancer.findStaticApi(NonMultiTenantDefaultEntity,
ConnectionSource.DEFAULT)` (and asserting no exception) would both avoid
relying on private state and ensure the registration is visible through the
same lookup path used at runtime.
```suggestion
when: "registering the entity"
enhancer.registerEntity(entity)
def staticApi =
GormEnhancer.findStaticApi(NonMultiTenantDefaultEntity,
ConnectionSource.DEFAULT)
then: "static api is available under DEFAULT qualifier"
staticApi != null
```
##########
grails-test-examples/datasources/src/integration-test/groovy/functionaltests/DatasourceSwitchingSpec.groovy:
##########
@@ -367,4 +367,90 @@ class DatasourceSwitchingSpec extends Specification {
and: "secondary books still exist"
SecondBook.withTransaction { SecondBook.countByTitleLike("Delete Me%")
} == 2
}
+
+ def "executeQuery routes to secondary datasource"() {
+ given: "books in both datasources"
+ def primaryTitle = "Primary Query ${UUID.randomUUID()}"
+ def secondaryTitle = "Secondary Query ${UUID.randomUUID()}"
+ new Book(title: primaryTitle).save(flush: true)
+ SecondBook.withTransaction {
+ new SecondBook(title: secondaryTitle).save(flush: true)
+ }
+
+ when: "executing query on secondary"
+ def results
+ SecondBook.withTransaction {
+ results = SecondBook.executeQuery("from ds2.Book where title =
:t", [t: secondaryTitle])
Review Comment:
The HQL strings hard-code the entity name (`ds2.Book`). This makes the test
brittle to package/entity-name changes and duplicates the information already
available via `SecondBook`. Consider building the HQL using `SecondBook.name`
(or `SecondBook.simpleName` if appropriate) so the test stays correct if the
domain class is moved/renamed while still exercising routing.
```suggestion
results = SecondBook.executeQuery("from ${SecondBook.name} where
title = :t", [t: secondaryTitle])
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
