This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch test/document-datamapping-core-finders in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit e705178a7a9daf5f27da84b10f439f36f316b3c9 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Sat Aug 15 14:46:57 2026 -0500 Fix IntelliJ warnings on DynamicFinder - operatorPatterns/operators fields are only ever assigned in the constructor - make them final. - Diamond operator for the methodExpressions map initializer. - Merge two identical-body SecurityException/NoSuchMethodException catch branches into multi-catch, in both the static initializer and registerNewMethodExpression. - Suppress the intentionally-ignored Matcher.find() result in createFinderInvocation (isMethodMatch already guarantees the match succeeds before a finder's invoke() is ever called). - buildMatchSpec: the local pattern "(prefix)([A-Z]\w*)" has no alternation, so group 2 is guaranteed non-null whenever find() succeeds - drop the always-true querySequence != null conjunct, and collapse the now childless outer if into the inner one. - Drop two self-reassignments (x = getInitializedExpression(x, args)) now that the helper is confirmed to always mutate-and-return the same reference; and delete a soloArgs array that was populated but never read. - Extract the ~25-line fetch/cache argument-map handling duplicated verbatim between both populateArgumentsForCriteria overloads into a shared applyFetchAndCacheArguments helper, parameterized over the join/cache operations since Query and BuildableCriteria don't share a supertype exposing them. Convert the surviving sort-map branches to pattern variables, dropping an unused `value` local along the way. - configureQueryWithArguments: fold the instanceof check and cast into a single pattern-variable condition. - Drop the redundant Iterable cast on methodExpressions.keySet() and replace criteria.remove(0) with removeFirst(). - Leave targetClass's unused-parameter warning suppressed rather than removing the parameter (call-site compatibility with grails-data-hibernate5/7 and grails-data-mongodb) and leave the Class<? extends Object> wildcard alone (documented GROOVY-9460 workaround already noted in the existing TODO). Verified via the full grails-datamapping-core and grails-datamapping-core-test suites - no behavior change. Co-Authored-By: Claude Sonnet 5 <[email protected]> --- .../datastore/gorm/finders/DynamicFinder.java | 215 ++++++++++----------- 1 file changed, 101 insertions(+), 114 deletions(-) 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 f93a7a4d8e..63d59320b4 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 @@ -20,6 +20,8 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.function.BiConsumer; +import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -73,7 +75,7 @@ import org.grails.datastore.mapping.reflect.NameUtils; * @author Graeme Rocher * @since 1.0 */ -@SuppressWarnings({"rawtypes", "unchecked"}) +@SuppressWarnings({"rawtypes", "unchecked", "ResultOfMethodCallIgnored"}) public class DynamicFinder implements FinderGrammar { public static final String ARGUMENT_FETCH_SIZE = "fetchSize"; @@ -95,8 +97,8 @@ public class DynamicFinder implements FinderGrammar { private static final String OPERATOR_OR = "Or"; private static final String OPERATOR_AND = "And"; private static final String[] DEFAULT_OPERATORS = {OPERATOR_AND, OPERATOR_OR}; - private Pattern[] operatorPatterns; - private String[] operators; + private final Pattern[] operatorPatterns; + private final String[] operators; private static Pattern methodExpressinPattern; @@ -104,7 +106,7 @@ public class DynamicFinder implements FinderGrammar { private static final Object[] EMPTY_OBJECT_ARRAY = {}; private static final String NOT = "Not"; - private static final Map<String, Constructor> methodExpressions = new LinkedHashMap<String, Constructor>(); + private static final Map<String, Constructor> methodExpressions = new LinkedHashMap<>(); protected final MappingContext mappingContext; private final boolean firstExpressionIsRequiredBoolean; @@ -126,9 +128,7 @@ public class DynamicFinder implements FinderGrammar { for (Class c : classes) { methodExpressions.put(c.getSimpleName(), c.getConstructor(constructorParamTypes)); } - } catch (SecurityException e) { - // ignore - } catch (NoSuchMethodException e) { + } catch (SecurityException | NoSuchMethodException e) { // ignore } @@ -163,11 +163,7 @@ public class DynamicFinder implements FinderGrammar { methodExpressions.put(methodExpression.getSimpleName(), methodExpression.getConstructor( Class.class, String.class)); resetMethodExpressionPattern(); - } catch (SecurityException e) { - throw new IllegalArgumentException("Class [" + methodExpression + - "] does not provide a constructor that takes parameters of type Class and String: " + - e.getMessage(), e); - } catch (NoSuchMethodException e) { + } catch (SecurityException | NoSuchMethodException e) { throw new IllegalArgumentException("Class [" + methodExpression + "] does not provide a constructor that takes parameters of type Class and String: " + e.getMessage(), e); @@ -185,54 +181,52 @@ public class DynamicFinder implements FinderGrammar { public static MatchSpec buildMatchSpec(String prefix, String methodName, int parameterCount) { String methodPattern = "(" + prefix + ")([A-Z]\\w*)"; Matcher matcher = Pattern.compile(methodPattern).matcher(methodName); - if (matcher.find()) { + if (matcher.find() && matcher.groupCount() == 2) { int totalRequiredArguments = 0; List<MethodExpression> expressions = new ArrayList<>(); - if (matcher.groupCount() == 2) { - String querySequence = matcher.group(2); - String operatorInUse; - boolean containsOperator = false; - String[] queryParameters; - for (int i = 0; i < DEFAULT_OPERATORS.length; i++) { - Matcher currentMatcher = defaultOperationPatterns[i].matcher(querySequence); - if (currentMatcher.find()) { - containsOperator = true; - operatorInUse = DEFAULT_OPERATORS[i]; - - queryParameters = querySequence.split(operatorInUse); - // loop through query parameters and create expressions - // calculating the number of arguments required for the expression - for (String queryParameter : queryParameters) { - MethodExpression currentExpression = findMethodExpression(queryParameter); - // add to list of expressions - totalRequiredArguments += currentExpression.argumentsRequired; - expressions.add(currentExpression); - } - break; - } - } - - // otherwise there is only one expression - if (!containsOperator && querySequence != null) { - MethodExpression solo = findMethodExpression(querySequence); + String querySequence = matcher.group(2); + String operatorInUse; + boolean containsOperator = false; + String[] queryParameters; + for (int i = 0; i < DEFAULT_OPERATORS.length; i++) { + Matcher currentMatcher = defaultOperationPatterns[i].matcher(querySequence); + if (currentMatcher.find()) { + containsOperator = true; + operatorInUse = DEFAULT_OPERATORS[i]; - final int requiredArguments = solo.getArgumentsRequired(); - if (requiredArguments > parameterCount) { - return null; + queryParameters = querySequence.split(operatorInUse); + // loop through query parameters and create expressions + // calculating the number of arguments required for the expression + for (String queryParameter : queryParameters) { + MethodExpression currentExpression = findMethodExpression(queryParameter); + // add to list of expressions + totalRequiredArguments += currentExpression.argumentsRequired; + expressions.add(currentExpression); } - - totalRequiredArguments += requiredArguments; - expressions.add(solo); + break; } + } + + // otherwise there is only one expression + if (!containsOperator) { + MethodExpression solo = findMethodExpression(querySequence); - // if the total of all the arguments necessary does not equal the number of arguments - // return null - if (totalRequiredArguments > parameterCount) { + final int requiredArguments = solo.getArgumentsRequired(); + if (requiredArguments > parameterCount) { return null; } - else { - return new MatchSpec(methodName, prefix, querySequence, totalRequiredArguments, expressions); - } + + totalRequiredArguments += requiredArguments; + expressions.add(solo); + } + + // if the total of all the arguments necessary does not equal the number of arguments + // return null + if (totalRequiredArguments > parameterCount) { + return null; + } + else { + return new MatchSpec(methodName, prefix, querySequence, totalRequiredArguments, expressions); } } return null; @@ -325,7 +319,7 @@ public class DynamicFinder implements FinderGrammar { for (int k = 0; k < requiredArgs; k++, argumentCursor++) { currentArguments[k] = arguments[argumentCursor]; } - currentExpression = getInitializedExpression(currentExpression, currentArguments); + getInitializedExpression(currentExpression, currentArguments); PersistentEntity persistentEntity = mappingContext.getPersistentEntity(clazz.getName()); try { @@ -352,9 +346,7 @@ public class DynamicFinder implements FinderGrammar { } totalRequiredArguments += requiredArguments; - Object[] soloArgs = new Object[requiredArguments]; - System.arraycopy(arguments, 0, soloArgs, 0, requiredArguments); - solo = getInitializedExpression(solo, arguments); + getInitializedExpression(solo, arguments); PersistentEntity persistentEntity = mappingContext.getPersistentEntity(clazz.getName()); try { solo.convertArguments(persistentEntity); @@ -395,29 +387,10 @@ public class DynamicFinder implements FinderGrammar { } String orderParam = (String) argMap.get(ARGUMENT_ORDER); - Object fetchObj = argMap.get(ARGUMENT_FETCH); - if (fetchObj instanceof Map) { - Map fetch = (Map) fetchObj; - for (Object o : fetch.keySet()) { - String associationName = (String) o; - Object fetchValue = fetch.get(associationName); - if (fetchValue instanceof FetchType) { - FetchType fetchType = (FetchType) fetchValue; - handleFetchType(query, associationName, fetchType); - } - else if (fetchValue instanceof JoinType) { - JoinType joinType = (JoinType) fetchValue; - query.join(associationName, joinType); - } else { - FetchType fetchType = getFetchMode(fetchValue); - handleFetchType(query, associationName, fetchType); - } - } - } - - if (argMap.containsKey(ARGUMENT_CACHE)) { - query.cache(ClassUtils.getBooleanFromMap(ARGUMENT_CACHE, argMap)); - } + applyFetchAndCacheArguments(argMap, + (associationName, fetchType) -> handleFetchType(query, associationName, fetchType), + query::join, + query::cache); Object sortObject = argMap.get(ARGUMENT_SORT); boolean ignoreCase = !argMap.containsKey(ARGUMENT_IGNORE_CASE) || ClassUtils.getBooleanFromMap(ARGUMENT_IGNORE_CASE, argMap); @@ -431,10 +404,8 @@ public class DynamicFinder implements FinderGrammar { } query.order(order); } - else if (sortObject instanceof Map) { - Map sortMap = (Map) sortObject; + else if (sortObject instanceof Map sortMap) { for (Object key : sortMap.keySet()) { - Object value = sortMap.get(key); String sort = key.toString(); final Query.Order order = ORDER_DESC.equalsIgnoreCase(orderParam) ? Query.Order.desc(sort) : Query.Order.asc(sort); if (ignoreCase) { @@ -452,11 +423,13 @@ public class DynamicFinder implements FinderGrammar { /** * Populates arguments for the given query form the given map + * @param targetClass Unused - kept for call-site/API compatibility with existing callers in + * grails-data-hibernate5/7 and grails-data-mongodb * @param query The query * @param argMap The query arguments */ //TODO: Change {code}Class<? extends Object>{code} to {class<?>} once GROOVY-9460 is fixed. - public static void populateArgumentsForCriteria(Class<? extends Object> targetClass, Query query, Map argMap) { + public static void populateArgumentsForCriteria(@SuppressWarnings("unused") Class<? extends Object> targetClass, Query query, Map argMap) { if (argMap == null) { return; } @@ -472,29 +445,10 @@ public class DynamicFinder implements FinderGrammar { } String orderParam = (String) argMap.get(ARGUMENT_ORDER); - Object fetchObj = argMap.get(ARGUMENT_FETCH); - if (fetchObj instanceof Map) { - Map fetch = (Map) fetchObj; - for (Object o : fetch.keySet()) { - String associationName = (String) o; - Object fetchValue = fetch.get(associationName); - if (fetchValue instanceof FetchType) { - FetchType fetchType = (FetchType) fetchValue; - handleFetchType(query, associationName, fetchType); - } - else if (fetchValue instanceof JoinType) { - JoinType joinType = (JoinType) fetchValue; - query.join(associationName, joinType); - } else { - FetchType fetchType = getFetchMode(fetchValue); - handleFetchType(query, associationName, fetchType); - } - } - } - - if (argMap.containsKey(ARGUMENT_CACHE)) { - query.cache(ClassUtils.getBooleanFromMap(ARGUMENT_CACHE, argMap)); - } + applyFetchAndCacheArguments(argMap, + (associationName, fetchType) -> handleFetchType(query, associationName, fetchType), + query::join, + query::cache); if (argMap.containsKey(ARGUMENT_LOCK)) { query.lock(ClassUtils.getBooleanFromMap(ARGUMENT_LOCK, argMap)); } @@ -516,8 +470,7 @@ public class DynamicFinder implements FinderGrammar { final String order = ORDER_DESC.equalsIgnoreCase(orderParam) ? ORDER_DESC : ORDER_ASC; addSimpleSort(query, sort, order, ignoreCase); } - else if (sortObject instanceof Map) { - Map sortMap = (Map) sortObject; + else if (sortObject instanceof Map sortMap) { applySortForMap(query, sortMap, ignoreCase); } } @@ -527,6 +480,41 @@ public class DynamicFinder implements FinderGrammar { } } + /** + * Shared by both {@code populateArgumentsForCriteria} overloads: applies the {@code fetch}/ + * {@code cache} argument-map entries via the caller-supplied handlers, since {@link Query} and + * {@link BuildableCriteria} don't share a common supertype exposing {@code join}/{@code select}/ + * {@code cache}. + * + * @param argMap The query arguments + * @param fetchHandler Applies a resolved {@link FetchType} to an association + * @param joinHandler Applies an explicit {@link JoinType} to an association + * @param cacheSetter Applies the {@code cache} argument, when present + */ + private static void applyFetchAndCacheArguments(Map argMap, BiConsumer<String, FetchType> fetchHandler, + BiConsumer<String, JoinType> joinHandler, Consumer<Boolean> cacheSetter) { + Object fetchObj = argMap.get(ARGUMENT_FETCH); + if (fetchObj instanceof Map fetch) { + for (Object o : fetch.keySet()) { + String associationName = (String) o; + Object fetchValue = fetch.get(associationName); + if (fetchValue instanceof FetchType fetchType) { + fetchHandler.accept(associationName, fetchType); + } + else if (fetchValue instanceof JoinType joinType) { + joinHandler.accept(associationName, joinType); + } + else { + fetchHandler.accept(associationName, getFetchMode(fetchValue)); + } + } + } + + if (argMap.containsKey(ARGUMENT_CACHE)) { + cacheSetter.accept(ClassUtils.getBooleanFromMap(ARGUMENT_CACHE, argMap)); + } + } + /** * Applies sorting logic to the given query from the given map * @param query The query @@ -709,7 +697,7 @@ public class DynamicFinder implements FinderGrammar { } private static void resetMethodExpressionPattern() { - String expressionPattern = DefaultGroovyMethods.join((Iterable) methodExpressions.keySet(), "|"); + String expressionPattern = DefaultGroovyMethods.join(methodExpressions.keySet(), "|"); methodExpressinPattern = Pattern.compile("\\p{Upper}[\\p{Lower}\\d]+(" + expressionPattern + ")"); } @@ -734,13 +722,12 @@ public class DynamicFinder implements FinderGrammar { } public static void configureQueryWithArguments(Class clazz, Query query, Object[] arguments) { - if (arguments.length == 0 || !(arguments[0] instanceof Map)) { - populateArgumentsForCriteria(clazz, query, Collections.emptyMap()); + if (arguments.length > 0 && arguments[0] instanceof Map<?, ?> argMap) { + populateArgumentsForCriteria(clazz, query, argMap); return; } - Map<?, ?> argMap = (Map<?, ?>) arguments[0]; - populateArgumentsForCriteria(clazz, query, argMap); + populateArgumentsForCriteria(clazz, query, Collections.emptyMap()); } private static String calcPropertyName(String queryParameter, String clause) { @@ -786,7 +773,7 @@ public class DynamicFinder implements FinderGrammar { if (OPERATOR_OR.equals(invocation.getOperator())) { if (firstExpressionIsRequiredBoolean()) { junction = new Query.Conjunction(); - junction.add(criteria.remove(0)); + junction.add(criteria.removeFirst()); var disjunction = new Query.Disjunction(); criteria.forEach(disjunction::add); junction.add(disjunction);
