Polishing.

Avoid unconditional ConcurrentLruCache instances for each String-based query.

See: #3310
Original pull request: #3321

Author: mp911de

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java

@@ -54,7 +54,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
 	private final SpelExpressionParser parser;
 	private final QueryParameterSetter.QueryMetadataCache metadataCache = new QueryParameterSetter.QueryMetadataCache();
 	private final QueryRewriter queryRewriter;
-	private ConcurrentLruCache<CachableQuery, String> queryCache = new ConcurrentLruCache<>(16, this::applySorting);
+	private final QuerySortRewriter querySortRewriter;
 	private final Lazy<ParameterBinder> countParameterBinder;
 
 	/**
@@ -102,6 +102,13 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri
 		this.parser = parser;
 		this.queryRewriter = queryRewriter;
 
+		JpaParameters parameters = method.getParameters();
+		if (parameters.hasPageableParameter() || parameters.hasSortParameter()) {
+			this.querySortRewriter = new CachingQuerySortRewriter();
+		} else {
+			this.querySortRewriter = NoOpQuerySortRewriter.INSTANCE;
+		}
+
 		Assert.isTrue(method.isNativeQuery() || !query.usesJdbcStyleParameters(),
 				"JDBC style parameters (?) are not supported for JPA queries");
 	}
@@ -110,7 +117,7 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri
 	public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
 
 		Sort sort = accessor.getSort();
-		String sortedQueryString = applySortingIfNecessary(query, sort);
+		String sortedQueryString = querySortRewriter.getSorted(query, sort);
 
 		ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor);
 
@@ -123,10 +130,6 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
 		return parameterBinder.get().bindAndPrepare(query, metadata, accessor);
 	}
 
-	protected String applySorting(DeclaredQuery query, Sort sort) {
-		return queryCache.get(new CachableQuery(query, sort));
-	}
-
 	@Override
 	protected ParameterBinder createBinder() {
 		return createBinder(query);
@@ -210,12 +213,46 @@ String applySorting(CachableQuery cachableQuery) {
 				cachableQuery.getAlias());
 	}
 
-	private String applySortingIfNecessary(DeclaredQuery query, Sort sort) {
+	/**
+	 * Query Sort Rewriter interface.
+	 */
+	interface QuerySortRewriter {
+		String getSorted(DeclaredQuery query, Sort sort);
+	}
+
+	/**
+	 * No-op query rewriter.
+	 */
+	enum NoOpQuerySortRewriter implements QuerySortRewriter {
+		INSTANCE;
+
+		public String getSorted(DeclaredQuery query, Sort sort) {
+
+			if (sort.isSorted()) {
+				throw new UnsupportedOperationException("NoOpQueryCache does not support sorting");
+			}
 
-		if (sort.isUnsorted()) {
 			return query.getQueryString();
 		}
-		return applySorting(query, sort);
+	}
+
+	/**
+	 * Caching variant of {@link QuerySortRewriter}.
+	 */
+	class CachingQuerySortRewriter implements QuerySortRewriter {
+
+		private final ConcurrentLruCache<CachableQuery, String> queryCache = new ConcurrentLruCache<>(16,
+				AbstractStringBasedJpaQuery.this::applySorting);
+
+		@Override
+		public String getSorted(DeclaredQuery query, Sort sort) {
+
+			if (sort.isUnsorted()) {
+				return query.getQueryString();
+			}
+
+			return queryCache.get(new CachableQuery(query, sort));
+		}
 	}
 
 	/**
@@ -227,7 +264,7 @@ private String applySortingIfNecessary(DeclaredQuery query, Sort sort) {
 	 */
 	static class CachableQuery {
 
-		private DeclaredQuery declaredQuery;
+		private final DeclaredQuery declaredQuery;
 		private final String queryString;
 		private final Sort sort;
 
@@ -246,6 +283,7 @@ Sort getSort() {
 			return sort;
 		}
 
+		@Nullable
 		String getAlias() {
 			return declaredQuery.getAlias();
 		}

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java

@@ -39,7 +39,16 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Member;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -568,7 +577,7 @@ public static <T> Query applyAndBind(String queryString, Iterable<T> entities, E
 	 *
 	 * @param originalQuery must not be {@literal null} or empty.
 	 * @return Guaranteed to be not {@literal null}.
-	 * @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead.
+	 * @deprecated use {@link DeclaredQuery#deriveCountQuery(String)} instead.
 	 */
 	@Deprecated
 	public static String createCountQueryFor(String originalQuery) {
@@ -582,7 +591,7 @@ public static String createCountQueryFor(String originalQuery) {
 	 * @param countProjection may be {@literal null}.
 	 * @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}.
 	 * @since 1.6
-	 * @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead.
+	 * @deprecated use {@link DeclaredQuery#deriveCountQuery(String)} instead.
 	 */
 	@Deprecated
 	public static String createCountQueryFor(String originalQuery, @Nullable String countProjection) {

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryUnitTests.java

@@ -49,9 +49,11 @@
 import org.springframework.util.ReflectionUtils;
 
 /**
+ * Unit tests for {@link AbstractStringBasedJpaQuery}.
+ *
  * @author Christoph Strobl
  */
-public class AbstractStringBasedJpaQueryUnitTests {
+class AbstractStringBasedJpaQueryUnitTests {
 
 	@Test // GH-3310
 	void shouldNotAttemptToAppendSortIfNoSortArgumentPresent() {