Polishing.
Avoid unconditional ConcurrentLruCache instances for each String-based query. See: #3310 Original pull request: #3321
This commit is contained in:
@@ -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 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
|
||||
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 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
|
||||
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 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
|
||||
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 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
|
||||
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 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
|
||||
*/
|
||||
static class CachableQuery {
|
||||
|
||||
private DeclaredQuery declaredQuery;
|
||||
private final DeclaredQuery declaredQuery;
|
||||
private final String queryString;
|
||||
private final Sort sort;
|
||||
|
||||
@@ -246,6 +283,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
|
||||
return sort;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
String getAlias() {
|
||||
return declaredQuery.getAlias();
|
||||
}
|
||||
|
||||
@@ -39,7 +39,16 @@ import jakarta.persistence.metamodel.SingularAttribute;
|
||||
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 abstract class QueryUtils {
|
||||
*
|
||||
* @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 abstract class QueryUtils {
|
||||
* @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) {
|
||||
|
||||
@@ -49,9 +49,11 @@ import org.springframework.util.MultiValueMap;
|
||||
import org.springframework.util.ReflectionUtils;
|
||||
|
||||
/**
|
||||
* Unit tests for {@link AbstractStringBasedJpaQuery}.
|
||||
*
|
||||
* @author Christoph Strobl
|
||||
*/
|
||||
public class AbstractStringBasedJpaQueryUnitTests {
|
||||
class AbstractStringBasedJpaQueryUnitTests {
|
||||
|
||||
@Test // GH-3310
|
||||
void shouldNotAttemptToAppendSortIfNoSortArgumentPresent() {
|
||||
|
||||
Reference in New Issue
Block a user