This is an automated email from the ASF dual-hosted git repository. jamesfredley pushed a commit to branch fix/where-query-reassignment-composition in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 2e3f6a8e7aa5c4b47f1a2953438747f8feb568b7 Author: James Fredley <[email protected]> AuthorDate: Tue Feb 24 10:52:47 2026 -0500 fix: where query composition fails when variable is re-assigned in if/else blocks The DetachedCriteriaTransformer AST transform only tracked variables declared with an initializer (DeclarationExpression) but not variables that were re-assigned (BinaryExpression). When a where query variable was declared without an initializer and then assigned inside if/else blocks, the transformer failed to recognize it as a DetachedCriteria, leaving the closure body untransformed. At runtime, the untransformed criteria were silently ignored. Add visitBinaryExpression() to DetachedCriteriaTransformer to track re-assigned variables in the detachedCriteriaVariables map and update the variable type to DetachedCriteria. Also add junctions to the AbstractDetachedCriteria clone() method for completeness. Assisted-by: OpenCode <[email protected]> --- .../grails/gorm/tests/WhereMethodSpec.groovy | 67 ++++++++++++++++++++ .../query/criteria/AbstractDetachedCriteria.groovy | 1 + .../transform/DetachedCriteriaTransformer.java | 72 ++++++++++++++++++++++ 3 files changed, 140 insertions(+) diff --git a/grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodSpec.groovy b/grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodSpec.groovy index a1d0ba5533..89b8c1fcd7 100644 --- a/grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodSpec.groovy +++ b/grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodSpec.groovy @@ -1553,6 +1553,73 @@ class Project { } } + def "Test where query composition with re-assigned variable in if/else blocks"() { + given: "A bunch of people" + createPeople() + + when: "A where query variable is assigned inside an if/else block and then chained" + def condition = true + def query + if (condition) { + query = Person.where { lastName == 'Simpson' } + } else { + query = Person.where { lastName == 'Rubble' } + } + query = query.where { age > 10 } + def results = query.list() + + then: "Both criteria from the if block and the chained where are applied" + results.size() == 2 + results.every { it.lastName == 'Simpson' && it.age > 10 } + + when: "The else branch is taken instead" + condition = false + if (condition) { + query = Person.where { lastName == 'Simpson' } + } else { + query = Person.where { lastName == 'Rubble' } + } + query = query.where { age > 10 } + results = query.list() + + then: "Both criteria from the else block and the chained where are applied" + results.size() == 1 + results[0].lastName == 'Rubble' + results[0].firstName == 'Barney' + } + + def "Test where query composition with re-assigned variable without initializer"() { + given: "A bunch of people" + createPeople() + + when: "A variable is declared without initializer and then assigned a where query" + def query + query = Person.where { lastName == 'Simpson' } + query = query.where { firstName == 'Bart' } + def results = query.list() + + then: "Both criteria are applied" + results.size() == 1 + results[0].firstName == 'Bart' + results[0].lastName == 'Simpson' + } + + def "Test where query composition with chained where on re-assigned variable"() { + given: "A bunch of people" + createPeople() + + when: "Multiple where queries are chained on a re-assigned variable" + def query + query = Person.where { age > 5 } + query = query.where { age < 42 } + query = query.where { lastName == 'Simpson' } + def results = query.list() + + then: "All three criteria are applied" + results.size() == 3 + results.every { it.age > 5 && it.age < 42 && it.lastName == 'Simpson' } + } + protected createContinentWithCountries() { final continent = new Continent(name: "Africa") continent.countries << new Country(name: "SA", population: 304830) << new Country(name: "Zim", population: 304830) diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/criteria/AbstractDetachedCriteria.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/criteria/AbstractDetachedCriteria.groovy index 077c0379a8..aa79ae542d 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/criteria/AbstractDetachedCriteria.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/criteria/AbstractDetachedCriteria.groovy @@ -881,6 +881,7 @@ abstract class AbstractDetachedCriteria<T> implements Criteria, Cloneable { criteria.defaultOffset = defaultOffset criteria.@fetchStrategies = new HashMap<>(this.fetchStrategies) criteria.@joinTypes = new HashMap<>(this.joinTypes) + criteria.@junctions = new ArrayList(this.junctions) criteria.@connectionName = this.connectionName criteria.@lazyQuery = this.lazyQuery criteria.@associationCriteriaMap = new LinkedHashMap<>(this.associationCriteriaMap) diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/transform/DetachedCriteriaTransformer.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/transform/DetachedCriteriaTransformer.java index 4a924ed982..85fff64fb1 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/transform/DetachedCriteriaTransformer.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/transform/DetachedCriteriaTransformer.java @@ -71,6 +71,7 @@ import org.codehaus.groovy.ast.stmt.WhileStatement; import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.control.messages.LocatedMessage; import org.codehaus.groovy.syntax.Token; +import org.codehaus.groovy.syntax.Types; import org.codehaus.groovy.transform.trait.Traits; import grails.gorm.DetachedCriteria; @@ -312,6 +313,77 @@ public class DetachedCriteriaTransformer extends ClassCodeVisitorSupport { super.visitDeclarationExpression(expression); } + /** + * Handles re-assignment of variables to domain class where queries. + * For example: {@code zoneQuery = ComputeZone.where { field == value }} + * <p> + * This ensures that the variable is tracked in {@link #detachedCriteriaVariables} + * so that subsequent {@code zoneQuery.where { ... }} calls have their closures + * properly transformed. + * <p> + * Without this, only declarations ({@code def query = Domain.where { ... }}) are tracked, + * and re-assignments inside if/else blocks are silently ignored. + */ + @Override + public void visitBinaryExpression(BinaryExpression expression) { + // Only handle assignment expressions (=), not comparisons (==) or other operators + if (expression.getOperation().getType() == Types.ASSIGN) { + Expression leftExpression = expression.getLeftExpression(); + Expression rightExpression = expression.getRightExpression(); + + if (leftExpression instanceof VariableExpression && rightExpression instanceof MethodCallExpression) { + MethodCallExpression call = (MethodCallExpression) rightExpression; + Expression objectExpression = call.getObjectExpression(); + Expression method = call.getMethod(); + Expression arguments = call.getArguments(); + + if (isCandidateMethod(method.getText(), arguments, CANDIDATE_METHODS_WHERE_ONLY)) { + ClassNode targetType = objectExpression.getType(); + if (AstUtils.isDomainClass(targetType)) { + VariableExpression var = (VariableExpression) leftExpression; + String variableName = var.getName(); + + // Track this variable so subsequent .where{} calls are transformed + detachedCriteriaVariables.put(variableName, targetType); + + // Update the variable's type to DetachedCriteria<DomainClass> + ClassNode classNode = new ClassNode(DetachedCriteria.class); + classNode.setGenericsTypes(new GenericsType[]{new GenericsType(targetType)}); + if (var.isClosureSharedVariable()) { + Variable accessedVariable = var.getAccessedVariable(); + if (accessedVariable instanceof VariableExpression) { + ((VariableExpression) accessedVariable).setType(classNode); + } + } else { + var.setType(classNode); + } + } + } + } else if (leftExpression instanceof VariableExpression && rightExpression instanceof ConstructorCallExpression) { + // Handle: zoneQuery = new DetachedCriteria(ComputeZone) + VariableExpression var = (VariableExpression) leftExpression; + String variableName = var.getName(); + ConstructorCallExpression cce = (ConstructorCallExpression) rightExpression; + + ClassNode type = cce.getType(); + if (DETACHED_CRITERIA_CLASS_NODE.getName().equals(type.getName())) { + Expression arguments = cce.getArguments(); + if (arguments instanceof ArgumentListExpression) { + ArgumentListExpression ale = (ArgumentListExpression) arguments; + if (ale.getExpressions().size() == 1) { + Expression exp = ale.getExpression(0); + if (exp instanceof ClassExpression) { + ClassExpression clse = (ClassExpression) exp; + detachedCriteriaVariables.put(variableName, clse.getType()); + } + } + } + } + } + } + super.visitBinaryExpression(expression); + } + private void logTransformationError(ASTNode astNode, Exception e) { StringBuilder message = new StringBuilder("Fatal error occurred applying query transformations [ " + e.getMessage() + "] to source [" + sourceUnit.getName() + "]. Please report an issue."); StringWriter sw = new StringWriter();
