MartinJesusDev opened a new issue, #16425:
URL: https://github.com/apache/grails-core/issues/16425

   ### Environment
   
   - Grails 7.2.3 / GORM 7.2.3 
(`org.apache.grails.data:grails-datamapping-core:7.2.3`), Groovy 4.0.33, JDK 17
   - Also present on current `7.2.x` and `8.0.x` sources (`@DelegatesTo` on 
`where`/`build` still without `strategy`)
   
   ### Description
   
   Under `@CompileStatic`, an inline criteria closure nested inside another 
criteria closure is compiled
   as `((Closure) getOwner()).getDelegate().<dsl>(...)`, so the subquery 
restrictions are added to the
   **enclosing** criteria instead of the new one. Inline `exists`/`notExists` 
subqueries therefore either
   fail with `QueryException: could not resolve property X of: <outer entity>` 
or silently return wrong
   results. The same code works correctly when the subquery is hoisted to a 
method-level variable.
   
   ### Steps to reproduce
   
   ```groovy
   import grails.gorm.DetachedCriteria
   import groovy.transform.CompileStatic
   
   @CompileStatic
   static DetachedCriteria<Author> query() {
       new DetachedCriteria<>(Author).where {
           exists new DetachedCriteria<>(Book).where { title == 'X' }.id()
       }
   }
   ```
   
   There is no need for a DB: compiling the class and inspecting the generated 
closure shows the
   subquery's `eq`/`eqProperty` calls target the outer criteria delegate. 
`javap -c` of the nested
   closure (real app, same pattern):
   
   ```text
   0: aload_0
   1: invokevirtual getOwner:()Ljava/lang/Object;
   4: checkcast groovy/lang/Closure
   7: invokevirtual groovy/lang/Closure.getDelegate:()Ljava/lang/Object;
   10: checkcast 
org/grails/datastore/gorm/query/criteria/AbstractDetachedCriteria
   ...
   invokevirtual grails/gorm/DetachedCriteria.eq(String, Object)   // lands on 
the OUTER criteria
   ```
   
   Hoisting the same subquery to a method-level variable compiles to 
`getDelegate().eq(...)` (its own
   delegate) and works.
   
   ### Root cause
   
   GORM's `@DelegatesTo` on `where`/`whereLazy`/`build`/`buildLazy` (both in 
`AbstractDetachedCriteria`
   and `DetachedCriteria`) declares no resolve strategy. With no `strategy`, 
the Groovy 4 static
   compiler assumes `OWNER_FIRST`; for a closure whose owner is another 
`@DelegatesTo` closure, calls
   resolve through the owner's delegate. This is the 
documented/closed-as-Not-A-Bug behaviour of
   GROOVY-9283, where the recommended remedy is to declare
   `@DelegatesTo(value = ..., strategy = Closure.DELEGATE_FIRST)`.
   
   The runtime already applies `setResolveStrategy(Closure.DELEGATE_FIRST)` in 
`build()` (and
   `projections()`), so only the compile-time annotation is missing.
   
   ### Proposed fix
   
   Add `strategy = Closure.DELEGATE_FIRST` to the `@DelegatesTo` annotations of 
the criteria closure
   API (`where`, `whereLazy`, `build`, `buildLazy`, junctions, subquery 
helpers, projections) and to the
   internal forwarders (`buildQueryableCriteria`, `withPopulatedQuery`, 
`handleJunction`,
   
`GormStaticApi`/`GormEntity`/`GormStaticOperations`/`TenantDelegatingGormOperations`
 where/find
   methods). This has to be done across the whole closure call graph: 
annotating only
   `where`/`build` makes the static compiler fail with "Closure parameter with 
resolve strategy
   OWNER_FIRST passed to method with resolve strategy DELEGATE_FIRST" on every 
forwarding call, so the
   strategy must be consistent. Annotation-only change: no runtime or API 
change (the runtime already
   applies `DELEGATE_FIRST` in `build()`), matching GROOVY-9283's 
recommendation.
   
   Regression test: `NestedCriteriaCompileStaticSpec` (unit, no DB) builds 
inline `exists`/`notExists`
   subqueries nested inside another `where {}` and asserts the restrictions 
stay on the inner criteria
   (fails before, passes after). The full `:grails-datamapping-core:test` suite 
and
   `:grails-datamapping-core:codeStyle` are green with the change. I have the 
patch ready and can send
   the PR immediately after the issue is approved.
   
   ### Workaround
   
   Annotate the affected method `@CompileDynamic`, or hoist the subquery out of 
the outer `where {}`
   into a method-level variable.
   
   I can submit the PR against `7.2.x` (bug fix, no API change) if you confirm 
the target branch and
   approve this issue.
   


-- 
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]

Reply via email to