Polishing.

Simplify Querydsl templates retrieva and String query caching. Update documentation. Skip selection list rewriting if the returned type is an interface. Encapsulate rewrite information for Query. Reformat code. See #2327
spring-projects · Dec 4, 2024 · f3cedc3 · f3cedc3
1 parent 5e800b0
commit f3cedc3
Show file tree

Hide file tree

Showing 13 changed files with 197 additions and 87 deletions.
diff --git a/...ata-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/...ata-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java
@@ -23,8 +23,6 @@
 import jakarta.persistence.TupleElement;
 import jakarta.persistence.TypedQuery;
 
-import java.lang.reflect.InvocationTargetException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
@@ -34,6 +32,7 @@
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
 
+import org.springframework.beans.BeanUtils;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.jpa.provider.PersistenceProvider;
 import org.springframework.data.jpa.repository.EntityGraph;
@@ -46,14 +45,16 @@
 import org.springframework.data.jpa.repository.query.JpaQueryExecution.StreamExecution;
 import org.springframework.data.jpa.repository.support.QueryHints;
 import org.springframework.data.jpa.util.JpaMetamodel;
+import org.springframework.data.mapping.PreferredConstructor;
+import org.springframework.data.mapping.model.PreferredConstructorDiscoverer;
 import org.springframework.data.repository.query.RepositoryQuery;
 import org.springframework.data.repository.query.ResultProcessor;
 import org.springframework.data.repository.query.ReturnedType;
 import org.springframework.data.util.Lazy;
 import org.springframework.jdbc.support.JdbcUtils;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
-import org.springframework.util.ClassUtils;
+import org.springframework.util.ReflectionUtils;
 
 /**
  * Abstract base class to implement {@link RepositoryQuery}s.
@@ -288,8 +289,8 @@ protected Class<?> getTypeToRead(ReturnedType returnedType) {
 
 		return returnedType.isProjecting() && returnedType.getReturnedType().isInterface()
 				&& !getMetamodel().isJpaManaged(returnedType.getReturnedType()) //
-				? Tuple.class //
-				: null;
+						? Tuple.class //
+						: null;
 	}
 
 	/**
@@ -314,6 +315,10 @@ public static class TupleConverter implements Converter<Object, Object> {
 
 		private final UnaryOperator<Tuple> tupleWrapper;
 
+		private final boolean dtoProjection;
+
+		private final @Nullable PreferredConstructor<?, ?> preferredConstructor;
+
 		/**
 		 * Creates a new {@link TupleConverter} for the given {@link ReturnedType}.
 		 *
@@ -336,6 +341,14 @@ public TupleConverter(ReturnedType type, boolean nativeQuery) {
 
 			this.type = type;
 			this.tupleWrapper = nativeQuery ? FallbackTupleWrapper::new : UnaryOperator.identity();
+			this.dtoProjection = type.isProjecting() && !type.getReturnedType().isInterface()
+					&& !type.getInputProperties().isEmpty();
+
+			if (this.dtoProjection) {
+				this.preferredConstructor = PreferredConstructorDiscoverer.discover(String.class);
+			} else {
+				this.preferredConstructor = null;
+			}
 		}
 
 		@Override
@@ -356,23 +369,26 @@ public Object convert(Object source) {
 				}
 			}
 
-			if(type.isProjecting() && !type.getReturnedType().isInterface() && !type.getInputProperties().isEmpty()) {
-				List<Object> ctorArgs = new ArrayList<>(type.getInputProperties().size());
-				type.getInputProperties().forEach(it -> {
-					ctorArgs.add(tuple.get(it));
-				});
-                try {
-                    return type.getReturnedType().getConstructor(ctorArgs.stream().map(Object::getClass).toArray(Class<?>[]::new)).newInstance(ctorArgs.toArray());
-                } catch (InstantiationException e) {
-                    throw new RuntimeException(e);
-                } catch (IllegalAccessException e) {
-                    throw new RuntimeException(e);
-                } catch (InvocationTargetException e) {
-                    throw new RuntimeException(e);
-                } catch (NoSuchMethodException e) {
-                    throw new RuntimeException(e);
-                }
-            }
+			if (dtoProjection) {
+
+				Object[] ctorArgs = new Object[type.getInputProperties().size()];
+
+				for (int i = 0; i < type.getInputProperties().size(); i++) {
+					ctorArgs[i] = tuple.get(i);
+				}
+
+				try {
+
+					if (preferredConstructor.getParameterCount() == ctorArgs.length) {
+						return BeanUtils.instantiateClass(preferredConstructor.getConstructor(), ctorArgs);
+					}
+
+					return BeanUtils.instantiateClass(type.getReturnedType()
+							.getConstructor(Arrays.stream(ctorArgs).map(Object::getClass).toArray(Class<?>[]::new)), ctorArgs);
+				} catch (ReflectiveOperationException e) {
+					ReflectionUtils.handleReflectionException(e);
+				}
+			}
 
 			return new TupleBackedMap(tupleWrapper.apply(tuple));
 		}

diff --git a/.../main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/.../main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java
@@ -69,8 +69,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
 	 * @param valueExpressionDelegate must not be {@literal null}.
 	 */
 	public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, String queryString,
-			@Nullable String countQueryString, QueryRewriter queryRewriter,
-			ValueExpressionDelegate valueExpressionDelegate) {
+			@Nullable String countQueryString, QueryRewriter queryRewriter, ValueExpressionDelegate valueExpressionDelegate) {
 
 		super(method, em);
 
@@ -99,15 +98,17 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri
 		});
 
 		this.queryRewriter = queryRewriter;
-		ReturnedType returnedType = method.getResultProcessor().getReturnedType();
 
 		JpaParameters parameters = method.getParameters();
-		if ((parameters.hasPageableParameter() || parameters.hasSortParameter()) && !parameters.hasDynamicProjection()) {
-			this.querySortRewriter = new CachingQuerySortRewriter();
-		} else if (returnedType.isProjecting() && !returnedType.getReturnedType().isInterface()) {
-			this.querySortRewriter = new ProjectingSortRewriter();
+
+		if (parameters.hasDynamicProjection()) {
+			this.querySortRewriter = SimpleQuerySortRewriter.INSTANCE;
 		} else {
-			this.querySortRewriter = NoOpQuerySortRewriter.INSTANCE;
+			if (parameters.hasPageableParameter() || parameters.hasSortParameter()) {
+				this.querySortRewriter = new CachingQuerySortRewriter();
+			} else {
+				this.querySortRewriter = new UnsortedCachingQuerySortRewriter();
+			}
 		}
 
 		Assert.isTrue(method.isNativeQuery() || !query.usesJdbcStyleParameters(),
@@ -119,19 +120,13 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
 
 		Sort sort = accessor.getSort();
 		ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor);
-
-		String sortedQueryString = null;
-		if(querySortRewriter.equals(NoOpQuerySortRewriter.INSTANCE) && accessor.findDynamicProjection() != null && !accessor.findDynamicProjection().isInterface()) {
-			sortedQueryString = getSortedQueryString(new ProjectingSortRewriter(), query, sort, processor.getReturnedType());
-		} else {
-			 sortedQueryString = getSortedQueryString(sort, processor.getReturnedType());
-		}
-
-		Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), processor.getReturnedType());
+		ReturnedType returnedType = processor.getReturnedType();
+		String sortedQueryString = getSortedQueryString(sort, returnedType);
+		Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), returnedType);
 
 		QueryParameterSetter.QueryMetadata metadata = metadataCache.getMetadata(sortedQueryString, query);
 
-		// it is ok to reuse the binding contained in the ParameterBinder although we create a new query String because the
+		// it is ok to reuse the binding contained in the ParameterBinder, although we create a new query String because the
 		// parameters in the query do not change.
 		return parameterBinder.get().bindAndPrepare(query, metadata, accessor);
 	}
@@ -140,10 +135,6 @@ String getSortedQueryString(Sort sort, ReturnedType returnedType) {
 		return querySortRewriter.getSorted(query, sort, returnedType);
 	}
 
-	private static String getSortedQueryString(QuerySortRewriter rewriter, DeclaredQuery query, Sort sort, ReturnedType returnedType) {
-		return rewriter.getSorted(query, sort, returnedType);
-	}
-
 	@Override
 	protected ParameterBinder createBinder() {
 		return createBinder(query);
@@ -223,8 +214,8 @@ protected String potentiallyRewriteQuery(String originalQuery, Sort sort, @Nulla
 
 	String applySorting(CachableQuery cachableQuery) {
 
-		return QueryEnhancerFactory.forQuery(cachableQuery.getDeclaredQuery()).rewrite(cachableQuery.getSort(),
-				cachableQuery.getReturnedType());
+		return QueryEnhancerFactory.forQuery(cachableQuery.getDeclaredQuery())
+				.rewrite(new DefaultQueryRewriteInformation(cachableQuery.getSort(), cachableQuery.getReturnedType()));
 	}
 
 	/**
@@ -237,21 +228,17 @@ interface QuerySortRewriter {
 	/**
 	 * No-op query rewriter.
 	 */
-	enum NoOpQuerySortRewriter implements QuerySortRewriter {
+	enum SimpleQuerySortRewriter implements QuerySortRewriter {
 
 		INSTANCE;
 
 		public String getSorted(DeclaredQuery query, Sort sort, ReturnedType returnedType) {
 
-			if (sort.isSorted()) {
-				throw new UnsupportedOperationException("NoOpQueryCache does not support sorting");
-			}
-
-			return query.getQueryString();
+			return QueryEnhancerFactory.forQuery(query).rewrite(new DefaultQueryRewriteInformation(sort, returnedType));
 		}
 	}
 
-	static class ProjectingSortRewriter implements QuerySortRewriter {
+	static class UnsortedCachingQuerySortRewriter implements QuerySortRewriter {
 
 		private volatile String cachedQueryString;
 
@@ -263,7 +250,8 @@ public String getSorted(DeclaredQuery query, Sort sort, ReturnedType returnedTyp
 
 			String cachedQueryString = this.cachedQueryString;
 			if (cachedQueryString == null) {
-				this.cachedQueryString = cachedQueryString = QueryEnhancerFactory.forQuery(query).rewrite(sort, returnedType);
+				this.cachedQueryString = cachedQueryString = QueryEnhancerFactory.forQuery(query)
+						.rewrite(new DefaultQueryRewriteInformation(sort, returnedType));
 			}
 
 			return cachedQueryString;

diff --git a/...jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java b/...jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java
@@ -18,7 +18,6 @@
 import java.util.Set;
 
 import org.springframework.data.domain.Sort;
-import org.springframework.data.repository.query.ReturnedType;
 import org.springframework.lang.Nullable;
 
 /**
@@ -54,8 +53,8 @@ public String applySorting(Sort sort, @Nullable String alias) {
 	}
 
 	@Override
-	public String rewrite(Sort sort, ReturnedType returnedType) {
-		return QueryUtils.applySorting(this.query.getQueryString(), sort, alias);
+	public String rewrite(QueryRewriteInformation rewriteInformation) {
+		return QueryUtils.applySorting(this.query.getQueryString(), rewriteInformation.getSort(), alias);
 	}
 
 	@Override

diff --git a/...in/java/org/springframework/data/jpa/repository/query/DefaultQueryRewriteInformation.java b/...in/java/org/springframework/data/jpa/repository/query/DefaultQueryRewriteInformation.java
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2024 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.jpa.repository.query;
+
+import org.springframework.data.domain.Sort;
+import org.springframework.data.repository.query.ReturnedType;
+
+/**
+ * Default {@link org.springframework.data.jpa.repository.query.QueryEnhancer.QueryRewriteInformation} implementation.
+ *
+ * @author Mark Paluch
+ */
+record DefaultQueryRewriteInformation(Sort sort,
+		ReturnedType returnedType) implements QueryEnhancer.QueryRewriteInformation {
+	@Override
+	public Sort getSort() {
+		return sort();
+	}
+
+	@Override
+	public ReturnedType getReturnedType() {
+		return returnedType();
+	}
+}
diff --git a/.../java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java b/.../java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java
@@ -29,6 +29,7 @@
  * </ul>
  *
  * @author Mark Paluch
+ * @since 3.5
  */
 class DtoProjectionTransformerDelegate {
 
@@ -40,7 +41,8 @@ public DtoProjectionTransformerDelegate(ReturnedType returnedType) {
 
 	public QueryTokenStream transformSelectionList(QueryTokenStream selectionList) {
 
-		if (!returnedType.isProjecting() || selectionList.stream().anyMatch(it -> it.equals(TOKEN_NEW))) {
+		if (!returnedType.isProjecting() || returnedType.getReturnedType().isInterface()
+				|| selectionList.stream().anyMatch(it -> it.equals(TOKEN_NEW))) {
 			return selectionList;
 		}
 

diff --git a/.../src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java b/.../src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java
@@ -50,7 +50,6 @@
 import java.util.StringJoiner;
 
 import org.springframework.data.domain.Sort;
-import org.springframework.data.repository.query.ReturnedType;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
@@ -298,17 +297,20 @@ public DeclaredQuery getQuery() {
 
 	@Override
 	public String applySorting(Sort sort) {
-		return applySorting(sort, detectAlias());
+		return doApplySorting(sort, detectAlias());
 	}
 
 	@Override
-	public String rewrite(Sort sort, ReturnedType returnedType) {
-		return applySorting(sort, primaryAlias);
+	public String rewrite(QueryRewriteInformation rewriteInformation) {
+		return doApplySorting(rewriteInformation.getSort(), primaryAlias);
 	}
 
 	@Override
 	public String applySorting(Sort sort, @Nullable String alias) {
+		return doApplySorting(sort, alias);
+	}
 
+	private String doApplySorting(Sort sort, @Nullable String alias) {
 		String queryString = query.getQueryString();
 		Assert.hasText(queryString, "Query must not be null or empty");
 

diff --git a/...ata-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java b/...ata-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java
@@ -200,8 +200,9 @@ public String applySorting(Sort sort) {
 	}
 
 	@Override
-	public String rewrite(Sort sort, ReturnedType returnedType) {
-		return QueryRenderer.TokenRenderer.render(sortFunction.apply(sort, detectAlias(), returnedType).visit(context));
+	public String rewrite(QueryRewriteInformation rewriteInformation) {
+		return QueryRenderer.TokenRenderer.render(sortFunction
+				.apply(rewriteInformation.getSort(), detectAlias(), rewriteInformation.getReturnedType()).visit(context));
 	}
 
 	/**
@@ -319,6 +320,13 @@ public static JpqlQueryParser parseQuery(String query) throws BadJpqlGrammarExce
 		}
 	}
 
+	/**
+	 * Functional interface to rewrite a query considering {@link Sort} and {@link ReturnedType}. The function returns a
+	 * visitor object that can visit the parsed query tree.
+	 *
+	 * @since 3.5
+	 */
+	@FunctionalInterface
 	interface SortedQueryRewriteFunction {
 
 		ParseTreeVisitor<? extends Object> apply(Sort sort, String primaryAlias, @Nullable ReturnedType returnedType);