fix: where query variable scoping fails inside @Transactional methods#15448
Conversation
…chedCriteriaTransformer When @transactional methods contain where queries with local variable names matching domain property names (e.g. Book.where { title == title }), TransactionalTransform re-runs VariableScopeVisitor on the renamed method body. VariableScopeVisitor sees delegate.property(closure) calls with implicitThis=true, finds a local variable with the same name in scope, and rewrites them to variable.call(closure) - causing MissingMethodException at runtime. The fix sets implicitThis=false on all MethodCallExpression nodes generated by DetachedCriteriaTransformer where the object expression is 'delegate'. This is semantically correct since the call IS explicitly on delegate, not implicit this, and prevents VariableScopeVisitor from rewriting the call. Fixes apache#11464 Assisted-by: OpenCode <opencode@opencode.ai>
There was a problem hiding this comment.
Pull request overview
Fixes a GORM where-query failure inside @Transactional methods when a local variable name collides with a domain property name, by preventing Groovy’s VariableScopeVisitor from rewriting transformed delegate.*(...) calls.
Changes:
- Update
DetachedCriteriaTransformerto setimplicitThis=falseon generatedMethodCallExpressions targetingdelegate. - Add functional integration coverage in the gorm test example app for transactional where-query variable collisions.
- Add unit-level compilation coverage in
WhereMethodSpecfor transactional + detached-criteria transforms with name collisions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/transform/DetachedCriteriaTransformer.java |
Marks generated delegate method calls as explicit (implicitThis=false) to avoid scope-rewrite breakage. |
grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodSpec.groovy |
Adds compilation-oriented tests for transactional contexts with variable/property name collisions. |
grails-test-examples/gorm/grails-app/services/gorm/WhereQueryVariableScopeService.groovy |
Introduces a service containing transactional where queries that intentionally collide with local variable names. |
grails-test-examples/gorm/src/integration-test/groovy/gorm/TransactionalWhereQueryVariableScopeSpec.groovy |
Adds integration tests exercising runtime behavior for the reported transactional scoping regression. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...amples/gorm/src/integration-test/groovy/gorm/TransactionalWhereQueryVariableScopeSpec.groovy
Show resolved
Hide resolved
grails-test-examples/gorm/grails-app/services/gorm/WhereQueryVariableScopeService.groovy
Show resolved
Hide resolved
grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodSpec.groovy
Show resolved
Hide resolved
grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodSpec.groovy
Show resolved
Hide resolved
The comment incorrectly described the method as a 'Non-transactional baseline' when it is actually annotated with @transactional(readOnly = true). Updated to accurately describe it as a read-only transactional variant. Assisted-by: OpenCode <opencode@opencode.ai>
davydotcom
left a comment
There was a problem hiding this comment.
We always suspected this had to do with some type of transfomer clobbering over another but was confusing as to what exactly was tripping it up. ordering didnt seem to do much and this explains that. Great find this is a big step in making where{} criteria more consistent in its behavior
Summary
Fixes #11464 - GORM
wherequeries with local variable names matching domain property names (e.g.Book.where { title == title }) fail withMissingMethodExceptionwhen called from@Transactionalmethods, but work correctly outside of them.See also: GROOVY-9629
Root Cause
Three-step AST transform interaction:
DetachedCriteriaTransformer (WHERE_ORDER priority) transforms
where { title == title }into criteria API calls likedelegate.title(closure)withimplicitThis = true(default inMethodCallExpressionconstructor).TransactionalTransform runs later, moves the method body to
$tt__methodName(), then re-runs Groovy'sVariableScopeVisitoron the renamed method (AbstractMethodDecoratingTransformation.groovy L356-362).VariableScopeVisitor sees
delegate.title(closure)withimplicitThis=true, finds a local variable namedtitlein scope, and rewrites it totitle.call(closure)- completely breaking the transformed code.Fix
Set
implicitThis = falseon allMethodCallExpressionnodes generated byDetachedCriteriaTransformerwhere the object expression isdelegate. This is semantically correct - the call IS explicitly ondelegate, not implicitthis. It preventsVariableScopeVisitorfrom rewriting the call.4 locations modified in
DetachedCriteriaTransformer.java:This pattern is already used in
AbstractMethodDecoratingTransformation.groovy(line 370) for similar reasons.Test Coverage
WhereMethodSpecverifying that@Transactional+@ApplyDetachedCriteriaTransformclasses with variable name collisions compile and instantiate successfullyTransactionalWhereQueryVariableScopeSpecverifying runtime behavior of where queries with variable name collisions in@Transactionalservice methodsWhereMethodSpectests continue to passReproducer
A standalone reproducer application is available at: https://github.com/jamesfredley/grails-where-query-transactional-scope-bug