jamesfredley opened a new pull request, #15418: URL: https://github.com/apache/grails-core/pull/15418
## Summary Fixes #15416 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. Reproducer: https://github.com/jamesfredley/grails-15416-where-connection-routing ## Root Cause Two issues were found: ### 1. Wrong method node passed to `findConnectionId()` Both `AbstractWhereImplementer` and `AbstractDetachedCriteriaServiceImplementor` called `findConnectionId(newMethodNode)` instead of `findConnectionId(abstractMethodNode)`. - `newMethodNode` belongs to the generated `$ServiceImplementation` class, which does not carry the `@Transactional` annotation - `abstractMethodNode` belongs to the original interface/abstract class where `@Transactional(connection)` is declared - `findConnectionId()` resolves the connection by walking up to `methodNode.getDeclaringClass()` to find `@Transactional` - so passing the wrong node meant it could never find the annotation This is the same class of bug fixed in #15395 for `SaveImplementer`, `DeleteImplementer`, and `FindAndDeleteImplementer`. ### 2. `build()` after `withConnection()` in `AbstractWhereImplementer` In the `@Where` code path, the original order was: 1. `query = new DetachedCriteria(Foo)` 2. `query = query.withConnection('secondary')` - sets `connectionName` 3. `query = query.build(closure)` - internally calls `clone()`, which does **not** copy `connectionName` The `clone()` call inside `build()` creates a new `DetachedCriteria` instance without the connection setting, so the connection was silently lost. Fixed by reordering to: `new DetachedCriteria` -> `build(closure)` -> `withConnection(name)`. > **Note**: `AbstractDetachedCriteria.clone()` not copying `connectionName` is arguably a separate bug. The non-`@Where` path in `AbstractDetachedCriteriaServiceImplementor` is not affected because it does not call `build()` after `withConnection()`. ## Changes | File | Change | |------|--------| | `AbstractWhereImplementer.groovy` | `findConnectionId(newMethodNode)` -> `findConnectionId(abstractMethodNode)` + reorder `build()`/`withConnection()` | | `AbstractDetachedCriteriaServiceImplementor.groovy` | `findConnectionId(newMethodNode)` -> `findConnectionId(abstractMethodNode)` | ## Tests ### Unit Tests (`WhereConnectionRoutingSpec`) - 5 tests - Verifies `@Where` method generates `withConnection()` call when `@Transactional(connection)` is present - Verifies `@Where` method does NOT generate `withConnection()` when no connection specified - Verifies `DetachedCriteria` `count()` generates `withConnection()` call - Verifies `DetachedCriteria` `list()` generates `withConnection()` call - Verifies `DetachedCriteria` `findByName()` generates `withConnection()` call ### Integration Tests (`WhereQueryMultiDataSourceSpec`) - 5 tests - Uses two real H2 in-memory databases (default + secondary) - Domain class mapped with `datasource 'ALL'` - Inserts data only into secondary, verifies queries route correctly: - `@Where`-annotated method returns data from secondary - `@Where`-annotated method returns empty from default (proving isolation) - `count()` routes to secondary - `list()` routes to secondary - `findByName()` routes to secondary All existing `ServiceTransformSpec` tests (30) continue to pass with no regressions. -- 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]
