This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch merge-hibernate6 in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 5457296af5042eae77eb4fc3ce8fd1e3bff33bdf Author: Walter Duque de Estrada <[email protected]> AuthorDate: Wed Jun 18 22:34:17 2025 -0500 OR and ANDS really fixed and refactored --- .../hibernate/query/AbstractHibernateQuery.java | 48 ++++++++++++---------- .../specs/hibernatequery/HibernateQuerySpec.groovy | 8 ++-- .../gorm/finders/AbstractFindByFinder.java | 38 +++-------------- .../datastore/gorm/finders/CountByFinder.java | 36 +++++----------- .../datastore/gorm/finders/DynamicFinder.java | 33 +++++++++++++++ .../datastore/gorm/finders/FindAllByFinder.java | 46 ++++----------------- 6 files changed, 89 insertions(+), 120 deletions(-) diff --git a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/query/AbstractHibernateQuery.java b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/query/AbstractHibernateQuery.java index 60baa295c4..eadffa0e10 100644 --- a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/query/AbstractHibernateQuery.java +++ b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/query/AbstractHibernateQuery.java @@ -170,14 +170,7 @@ public abstract class AbstractHibernateQuery extends Query { * @param criterion The criterion instance */ public void add(Criterion criterion) { - Conjunction conjunction = (Conjunction) detachedCriteria.getCriteria() - .stream() - .filter(it -> it instanceof Conjunction) - .map(Criterion.class::cast) - .findFirst() - .orElse(new Conjunction()); - conjunction.add(criterion); - detachedCriteria.add(conjunction); + detachedCriteria.add(criterion); } @@ -216,12 +209,14 @@ public abstract class AbstractHibernateQuery extends Query { @Override public Query and(Criterion a, Criterion b) { - and(new Closure(AbstractHibernateQuery.this) { - public void doCall() { - DetachedCriteria owner = (DetachedCriteria) getDelegate(); - owner.add(Restrictions.and(a,b)); - } - }); + and(List.of(a,b)); + return this; + } + + public Query and(List<Criterion> criteria) { + var conjunction = new Conjunction(); + criteria.forEach(conjunction::add); + detachedCriteria.add(conjunction); return this; } @@ -233,13 +228,14 @@ public abstract class AbstractHibernateQuery extends Query { @Override public Query or(Criterion a, Criterion b) { - Closure orClosure = new Closure(this) { - public void doCall() { - DetachedCriteria owner = (DetachedCriteria) getDelegate(); - owner.add(Restrictions.or(a,b)); - } - }; - detachedCriteria.or(orClosure); + or(List.of(a,b)); + return this; + } + + public Query or(List<Criterion> criteria) { + var disjunction = new Disjunction(); + criteria.forEach(disjunction::add); + detachedCriteria.add(disjunction); return this; } @@ -258,6 +254,16 @@ public abstract class AbstractHibernateQuery extends Query { return this; } + public Query not(List<Criterion> criteria) { + var conjunction = new Conjunction(); + criteria.forEach(conjunction::add); + var negation = new Negation(); + negation.add(conjunction); + detachedCriteria.add(negation); + return this; + } + + public Query not(Closure closure) { detachedCriteria.not(closure); return this; diff --git a/grails-data-hibernate6/core/src/test/groovy/grails/gorm/specs/hibernatequery/HibernateQuerySpec.groovy b/grails-data-hibernate6/core/src/test/groovy/grails/gorm/specs/hibernatequery/HibernateQuerySpec.groovy index ac7c85c025..b7b008e635 100644 --- a/grails-data-hibernate6/core/src/test/groovy/grails/gorm/specs/hibernatequery/HibernateQuerySpec.groovy +++ b/grails-data-hibernate6/core/src/test/groovy/grails/gorm/specs/hibernatequery/HibernateQuerySpec.groovy @@ -224,8 +224,9 @@ class HibernateQuerySpec extends HibernateGormDatastoreSpec { def or() { given: new Person(firstName: "Bob", lastName: "Builder", age: 51).save(flush: true) - Query.Criterion lastNameWrong = new Query.Equals("lastName", "Rogers") - Query.Criterion ageCorrect = new Query.Equals("age", 50) + def lastNameWrong = new Query.Equals("lastName", "Rogers") + def ageCorrect = new Query.Equals("age", 50) + hibernateQuery.or(lastNameWrong, ageCorrect) when: def newBob = hibernateQuery.singleResult() @@ -237,7 +238,8 @@ class HibernateQuerySpec extends HibernateGormDatastoreSpec { given: new Person(firstName: "Fred", lastName: "Rogers", age: 51).save(flush: true) Query.Criterion lastNameWrong = new Query.Equals("lastName", "Rogers") - hibernateQuery.not(lastNameWrong) + Query.Criterion firstNameWrong = new Query.Equals("firstName", "Fred") + hibernateQuery.not([lastNameWrong,firstNameWrong]) when: def newBob = hibernateQuery.singleResult() then: diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/AbstractFindByFinder.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/AbstractFindByFinder.java index aab25a7d63..e828a836fc 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/AbstractFindByFinder.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/AbstractFindByFinder.java @@ -20,6 +20,7 @@ package org.grails.datastore.gorm.finders; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.grails.datastore.mapping.core.Datastore; import org.grails.datastore.mapping.core.Session; @@ -44,7 +45,9 @@ public abstract class AbstractFindByFinder extends DynamicFinder { protected Object doInvokeInternal(final DynamicFinderInvocation invocation) { return execute(new SessionCallback<Object>() { public Object doInSession(final Session session) { - return invokeQuery(buildQuery(invocation, session)); + Query query = buildQuery(invocation, session); + adjustQuery(query); + return invokeQuery(query); } }); } @@ -54,40 +57,11 @@ public abstract class AbstractFindByFinder extends DynamicFinder { } public boolean firstExpressionIsRequiredBoolean() { - return false; + return super.firstExpressionIsRequiredBoolean(); } - public Query buildQuery(DynamicFinderInvocation invocation, Session session) { - final Class<?> clazz = invocation.getJavaClass(); - Query q = session.createQuery(clazz); - return buildQuery(invocation, clazz, q); - } - - protected Query buildQuery(DynamicFinderInvocation invocation, Class<?> clazz, Query query) { - applyAdditionalCriteria(query, invocation.getCriteria()); - applyDetachedCriteria(query, invocation.getDetachedCriteria()); - configureQueryWithArguments(clazz, query, invocation.getArguments()); - - final String operatorInUse = invocation.getOperator(); - - if (operatorInUse != null && operatorInUse.equals(OPERATOR_OR)) { - if (firstExpressionIsRequiredBoolean()) { - MethodExpression expression = invocation.getExpressions().remove(0); - query.add(expression.createCriterion()); - } - - Query.Junction disjunction = query.disjunction(); - for (MethodExpression expression : invocation.getExpressions()) { - query.add(disjunction, expression.createCriterion()); - } - } - else { - for (MethodExpression expression : invocation.getExpressions()) { - query.add(expression.createCriterion()); - } - } - return query; + protected void adjustQuery(Query query) { } } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/CountByFinder.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/CountByFinder.java index b07eefaf7a..b1002293cd 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/CountByFinder.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/CountByFinder.java @@ -19,6 +19,7 @@ package org.grails.datastore.gorm.finders; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.grails.datastore.mapping.core.Datastore; import org.grails.datastore.mapping.core.Session; @@ -49,8 +50,9 @@ public class CountByFinder extends DynamicFinder implements QueryBuildingFinder protected Object doInvokeInternal(final DynamicFinderInvocation invocation) { return execute(new SessionCallback<Object>() { public Object doInSession(final Session session) { - Query q = buildQuery(invocation, session); - return invokeQuery(q); + Query query = buildQuery(invocation, session); + adjustQuery(query); + return invokeQuery(query); } }); } @@ -59,31 +61,13 @@ public class CountByFinder extends DynamicFinder implements QueryBuildingFinder return q.singleResult(); } - public Query buildQuery(DynamicFinderInvocation invocation, Session session) { - final Class<?> clazz = invocation.getJavaClass(); - Query q = session.createQuery(clazz); - return buildQuery(invocation, clazz, q); - } - - protected Query buildQuery(DynamicFinderInvocation invocation, Class<?> clazz, Query q) { - applyAdditionalCriteria(q, invocation.getCriteria()); - configureQueryWithArguments(clazz, q, invocation.getArguments()); - - String operatorInUse = invocation.getOperator(); - if (operatorInUse != null && operatorInUse.equals(OPERATOR_OR)) { - Query.Junction disjunction = q.disjunction(); - for (MethodExpression expression : invocation.getExpressions()) { - q.add(disjunction, expression.createCriterion()); - } - } - else { - for (MethodExpression expression : invocation.getExpressions()) { - q.add( expression.createCriterion() ); - } - } + protected Query adjustQuery(Query query) { + query.projections().count(); + return query; + } - q.projections().count(); - return q; + public boolean firstExpressionIsRequiredBoolean() { + return super.firstExpressionIsRequiredBoolean(); } } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinder.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinder.java index 17bd93ca0b..9a69c21be7 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinder.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinder.java @@ -22,6 +22,7 @@ import java.lang.reflect.Constructor; import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import jakarta.persistence.FetchType; import jakarta.persistence.criteria.JoinType; @@ -46,6 +47,7 @@ import org.grails.datastore.gorm.finders.MethodExpression.NotEqual; import org.grails.datastore.gorm.finders.MethodExpression.Rlike; import org.grails.datastore.gorm.query.criteria.AbstractDetachedCriteria; import org.grails.datastore.mapping.core.Datastore; +import org.grails.datastore.mapping.core.Session; import org.grails.datastore.mapping.model.MappingContext; import org.grails.datastore.mapping.model.PersistentEntity; import org.grails.datastore.mapping.model.types.Basic; @@ -773,4 +775,35 @@ public abstract class DynamicFinder extends AbstractFinder implements QueryBuild } + public boolean firstExpressionIsRequiredBoolean() { + return false; + } + + protected Query.Junction getJunction(DynamicFinderInvocation invocation) { + var criteria = invocation.getExpressions().stream().map(MethodExpression::createCriterion).collect(Collectors.toList()); + Query.Junction junction; + if (FindAllByFinder.OPERATOR_OR.equals(invocation.getOperator())) { + junction = new Query.Disjunction(); + if (firstExpressionIsRequiredBoolean()) { + var conjunction = new Query.Conjunction(); + conjunction.add(criteria.remove(0)); + junction.add(conjunction); + } + } + else { + junction = new Query.Conjunction(); + } + criteria.forEach(junction::add); + return junction; + } + + public Query buildQuery(DynamicFinderInvocation invocation, Session session) { + final Class<?> clazz = invocation.getJavaClass(); + var query = session.createQuery(clazz); + applyAdditionalCriteria(query, invocation.getCriteria()); + applyDetachedCriteria(query, invocation.getDetachedCriteria()); + configureQueryWithArguments(clazz, query, invocation.getArguments()); + query.add(getJunction(invocation)); + return query; + } } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindAllByFinder.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindAllByFinder.java index abdeabe4ab..01cbdc437b 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindAllByFinder.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindAllByFinder.java @@ -31,10 +31,10 @@ import org.grails.datastore.mapping.query.Query; */ public class FindAllByFinder extends DynamicFinder { - private static final String OPERATOR_OR = "Or"; - private static final String OPERATOR_AND = "And"; + protected static final String OPERATOR_OR = "Or"; + protected static final String OPERATOR_AND = "And"; private static final String METHOD_PATTERN = "(findAllBy)([A-Z]\\w*)"; - private static final String[] OPERATORS = { OPERATOR_AND, OPERATOR_OR }; + protected static final String[] OPERATORS = { OPERATOR_AND, OPERATOR_OR }; public FindAllByFinder(final Datastore datastore) { super(Pattern.compile(METHOD_PATTERN), OPERATORS, datastore); @@ -48,8 +48,9 @@ public class FindAllByFinder extends DynamicFinder { protected Object doInvokeInternal(final DynamicFinderInvocation invocation) { return execute(new SessionCallback<Object>() { public Object doInSession(final Session session) { - Query q = buildQuery(invocation, session); - return invokeQuery(q); + Query query = buildQuery(invocation, session); + adjustQuery(query); + return invokeQuery(query); } }); } @@ -58,39 +59,8 @@ public class FindAllByFinder extends DynamicFinder { return q.list(); } - public boolean firstExpressionIsRequiredBoolean() { - return false; - } - - public Query buildQuery(DynamicFinderInvocation invocation, Session session) { - final Class<?> clazz = invocation.getJavaClass(); - Query q = session.createQuery(clazz); - return buildQuery(invocation, clazz, q); - } - - protected Query buildQuery(DynamicFinderInvocation invocation, Class<?> clazz, Query query) { - applyAdditionalCriteria(query, invocation.getCriteria()); - applyDetachedCriteria(query, invocation.getDetachedCriteria()); - configureQueryWithArguments(clazz, query, invocation.getArguments()); - - final String operatorInUse = invocation.getOperator(); - if (operatorInUse != null && operatorInUse.equals(OPERATOR_OR)) { - if (firstExpressionIsRequiredBoolean()) { - MethodExpression expression = invocation.getExpressions().remove(0); - query.add(expression.createCriterion()); - } - Query.Junction disjunction = query.disjunction(); - - for (MethodExpression expression : invocation.getExpressions()) { - query.add(disjunction, expression.createCriterion()); - } - } - else { - for (MethodExpression expression : invocation.getExpressions()) { - query.add( expression.createCriterion() ); - } - } + protected void adjustQuery( Query query) { query.projections().distinct(); - return query; } + }
