fix: Auto-implemented Data Service CRUD methods ignore @Transactional(connection)

[jamesfredley]

Summary

Auto-implemented CRUD methods on GORM Data Services (save(), delete(), get()) ignore @Transactional(connection = 'secondary') and always route to the default datasource.

The finder implementers (FindAllByImplementer, FindOneByImplementer) already correctly use findStaticApiForConnectionId() to route through GormEnhancer.findStaticApi() with the correct connection qualifier. However, SaveImplementer, DeleteImplementer, FindAndDeleteImplementer, and the get-by-id optimization in AbstractDetachedCriteriaServiceImplementor bypass this mechanism entirely and call instance/static methods directly on the domain class.

Bug Description

Given a Data Service with @Transactional(connection = 'secondary'):

@Service(Metric)
@Transactional(connection = 'secondary')
abstract class MetricService implements MetricDataService {
    // All CRUD methods auto-implemented by GORM
}

• findAllBy*() - correctly routes to secondary (uses findStaticApiForConnectionId)
• save(Metric m) - routes to DEFAULT (calls entity.save() directly)
• delete(Serializable id) returning void/Number - routes to DEFAULT (calls obj.delete() directly)
• delete(Serializable id) returning domain type - routes to DEFAULT (FindAndDeleteImplementer calls obj.delete() directly)
• get(Serializable id) - routes to DEFAULT (calls DomainClass.get(id) directly)

Both patterns are affected:

• Abstract class with @Service + @Transactional(connection) implementing a separate interface
• Interface-only with @Service + @Transactional(connection) directly on the interface (no abstract class)

Root Cause

Four code paths in the auto-implementers skip connection routing:

1. SaveImplementer.doImplement() - single-entity parameter path generates entity.save(failOnError: true), which goes through GormEntity.save() -> GormEnhancer.findInstanceApi(class) without a connection qualifier.

2. AbstractDetachedCriteriaServiceImplementor.doImplement() - the "optimize by id" path generates DomainClass.get(id), a static call that routes to the default datastore. (The detached criteria query path already calls findConnectionId() + withConnection() correctly.)

3. DeleteImplementer.implementById() - generates obj.delete(), which goes through GormEntity.delete() -> default instance API.

4. FindAndDeleteImplementer.buildReturnStatement() - generates obj?.delete(), which goes through GormEntity.delete() -> default instance API. (The find part routes correctly via AbstractDetachedCriteriaServiceImplementor, but the delete call bypasses connection routing.)

Note: AbstractSaveImplementer.bindParametersAndSave() (the multi-parameter constructor-style save) already had the correct findConnectionId() check - only the single-entity path in SaveImplementer was missing it.

Fix

Apply the same findConnectionId() pattern already used by FindAllByImplementer and AbstractSaveImplementer.bindParametersAndSave():

• SaveImplementer: When findConnectionId() returns a connection, route through GormEnhancer.findInstanceApi(DomainClass, connectionId).save(entity, args) instead of entity.save(args)
• AbstractDetachedCriteriaServiceImplementor: When findConnectionId() returns a connection, route get-by-id through GormEnhancer.findStaticApi(DomainClass, connectionId).get(id) instead of DomainClass.get(id)
• DeleteImplementer: When findConnectionId() returns a connection, route through GormEnhancer.findInstanceApi(DomainClass, connectionId).delete(entity) instead of entity.delete()
• FindAndDeleteImplementer: When findConnectionId() returns a connection, route delete through GormEnhancer.findInstanceApi(DomainClass, connectionId).delete(entity) with a null-check guard instead of entity?.delete()

Additionally, harmonized findConnectionId() to consistently use newMethodNode (the generated implementation method) across SaveImplementer, DeleteImplementer, and AbstractSaveImplementer, matching the convention already used by AbstractDetachedCriteriaServiceImplementor. Also renamed the abstractMethodNode parameter in AbstractSaveImplementer.bindParametersAndSave() to newMethodNode for consistency.

Test Coverage

Unit Tests - ConnectionRoutingServiceTransformSpec (6 tests)

1. Save with connection - abstract class with @Transactional(connection='secondary'), verifies save(Foo) and saveFoo(String) compile with correct implementers and connection annotation propagated
2. Delete by id with connection - verifies delete(Serializable) returning domain type uses FindAndDeleteImplementer, void delete(Serializable) uses DeleteImplementer, Number delete(String) uses DeleteImplementer
3. Find by id with connection - verifies find(Serializable) and get(Serializable) use FindOneImplementer with connection propagated
4. Interface service with connection - interface-only (no abstract class) with all CRUD methods, verifies all implementer assignments and @Transactional(connection) propagation
5. No-connection regression - service without @Transactional(connection) still compiles all CRUD correctly
6. Runtime verification - instantiates connection-routed service and confirms methods throw IllegalStateException (proving they invoke GormEnhancer APIs that require an initialized datastore, rather than calling entity.save()/entity.delete() directly)

TCK Integration Tests - DataServiceMultiDataSourceSpec (16 tests)

Added to grails-data-hibernate5 with a real HibernateDatastore, two H2 in-memory databases (default + books), and a Product domain mapped exclusively to the 'books' datasource:

Abstract class pattern (ProductService):

1. Schema is created on the books datasource
2. Save routes to books datasource
3. Get by ID routes to books datasource
4. Delete (FindAndDeleteImplementer) routes to books datasource
5. Void delete (DeleteImplementer) routes to books datasource
6. Save/get/find round-trip through Data Service
7. Save with constructor-style arguments routes to books datasource

Interface-only pattern (ProductDataService):
8. Interface service: save routes to books datasource
9. Interface service: get by ID routes to books datasource
10. Interface service: delete routes to books datasource
11. Interface service: void delete routes to books datasource
12-16. Cross-service sharing, findByName, count, findAll, list operations

Functional Test App

A standalone Grails functional test app in grails-test-examples/datasources/multi-datasource-data-service/ with full boot-up and HTTP endpoint testing.

All tests pass. All 30 existing ServiceTransformSpec tests pass (no regressions).

Example Application

https://github.com/jamesfredley/grails-auto-crud-datasource-routing

A minimal Grails 7 app with two H2 in-memory databases that proves the bug. The METRIC table exists only on the secondary database. Two Data Service patterns are tested:

• Pattern 1: MetricService - abstract class with @Transactional(connection = 'secondary') implementing MetricDataService interface
• Pattern 2: MetricInterfaceOnlyDataService - interface-only with @Service(Metric) + @Transactional(connection = 'secondary') directly on the interface, no abstract class

Both patterns demonstrate the same bug: auto-implemented save() tries to write to primary (where there's no METRIC table), causing a SQL error.

Run ./gradlew bootRun and visit http://localhost:8080/bugDemo/index to see the verified output.

Environment Information

• Grails 7.0.7
• GORM 7.0.7
• Spring Boot 3.5.10
• Groovy 4.0.30
• JDK 17+

Version

7.0.7

[jdaugherty]

We should add the example app scenario as a functional test

[Copilot]

Pull request overview

Fixes GORM Data Service auto-implemented CRUD methods so @Transactional(connection = ...) is honored, ensuring CRUD routes through connection-aware GormEnhancer APIs instead of default-datasource instance/static calls.

Changes:

• Update Data Service implementers (save/delete/get-by-id) to route via connection-qualified GormEnhancer APIs.
• Add new AST/unit and Hibernate integration tests covering multi-datasource CRUD routing.
• Add a new functional test example app/module demonstrating multi-datasource Data Service behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
settings.gradle	Adds the new multi-datasource data-service example subproject.
grails-test-examples/hibernate5/grails-data-service-multi-datasource/**	New example Grails app + integration spec validating CRUD routing to a secondary datasource.
grails-datamapping-core/src/main/groovy/.../SaveImplementer.groovy	Routes single-entity save() through connection-qualified instance API.
grails-datamapping-core/src/main/groovy/.../DeleteImplementer.groovy	Routes delete-by-id through connection-qualified instance API.
grails-datamapping-core/src/main/groovy/.../FindAndDeleteImplementer.groovy	Routes delete call through connection-qualified instance API for find-and-delete.
grails-datamapping-core/src/main/groovy/.../AbstractDetachedCriteriaServiceImplementor.groovy	Routes get-by-id optimization through connection-qualified static API.
grails-datamapping-core/src/test/groovy/.../ConnectionRoutingServiceTransformSpec.groovy	New Spock spec validating implementer selection + connection annotation propagation.
grails-data-hibernate5/core/src/test/groovy/.../DataServiceMultiDataSourceSpec.groovy	New integration test validating runtime CRUD routing against a secondary datasource.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jamesfredley]

#15395 (comment) led me in the wrong direction and caused a legitimate test failure that was only caught in the joint validation build (had nothing to do with groovy version). I am skeptical of the github copilot PR reviews, but this one looked correct, but was not.

@jdaugherty @matrei Sorry this will need another review.