Skip to content

Commit 804e65c

Browse files
christophstroblmp911de
christophstrobl
authored and
mp911de
committed
Avoid repeated query parsing.
This commit makes sure to avoid unnecessary parsing of queries when sorting must not be appended. Additionally the parsed result is cached on a per query string with sort expression basis to avoid parsing repeating occurrences of the same expressions eg. while paging through results. Closes: #3310 Original pull request: #3321
1 parent 005c507 commit 804e65c

File tree

2 files changed

+315
-4
lines changed

2 files changed

+315
-4
lines changed

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

+86-4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import jakarta.persistence.EntityManager;
1919
import jakarta.persistence.Query;
2020

21+
import java.util.Objects;
22+
2123
import org.springframework.data.domain.Pageable;
2224
import org.springframework.data.domain.Sort;
2325
import org.springframework.data.jpa.repository.QueryRewriter;
@@ -28,6 +30,7 @@
2830
import org.springframework.expression.spel.standard.SpelExpressionParser;
2931
import org.springframework.lang.Nullable;
3032
import org.springframework.util.Assert;
33+
import org.springframework.util.ConcurrentLruCache;
3134
import org.springframework.util.StringUtils;
3235

3336
/**
@@ -41,6 +44,7 @@
4144
* @author Mark Paluch
4245
* @author Diego Krupitza
4346
* @author Greg Turnquist
47+
* @author Christoph Strobl
4448
*/
4549
abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
4650

@@ -50,6 +54,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
5054
private final SpelExpressionParser parser;
5155
private final QueryParameterSetter.QueryMetadataCache metadataCache = new QueryParameterSetter.QueryMetadataCache();
5256
private final QueryRewriter queryRewriter;
57+
private ConcurrentLruCache<CachableQuery, String> queryCache = new ConcurrentLruCache<>(16, this::applySorting);
5358
private final Lazy<ParameterBinder> countParameterBinder;
5459

5560
/**
@@ -104,12 +109,12 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri
104109
@Override
105110
public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
106111

107-
String sortedQueryString = QueryEnhancerFactory.forQuery(query) //
108-
.applySorting(accessor.getSort(), query.getAlias());
112+
Sort sort = accessor.getSort();
113+
String sortedQueryString = applySortingIfNecessary(query, sort);
114+
109115
ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor);
110116

111-
Query query = createJpaQuery(sortedQueryString, accessor.getSort(), accessor.getPageable(),
112-
processor.getReturnedType());
117+
Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), processor.getReturnedType());
113118

114119
QueryParameterSetter.QueryMetadata metadata = metadataCache.getMetadata(sortedQueryString, query);
115120

@@ -118,6 +123,10 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
118123
return parameterBinder.get().bindAndPrepare(query, metadata, accessor);
119124
}
120125

126+
protected String applySorting(DeclaredQuery query, Sort sort) {
127+
return queryCache.get(new CachableQuery(query, sort));
128+
}
129+
121130
@Override
122131
protected ParameterBinder createBinder() {
123132
return createBinder(query);
@@ -194,4 +203,77 @@ protected String potentiallyRewriteQuery(String originalQuery, Sort sort, @Nulla
194203
? queryRewriter.rewrite(originalQuery, pageable) //
195204
: queryRewriter.rewrite(originalQuery, sort);
196205
}
206+
207+
String applySorting(CachableQuery cachableQuery) {
208+
209+
return QueryEnhancerFactory.forQuery(cachableQuery.getDeclaredQuery()).applySorting(cachableQuery.getSort(),
210+
cachableQuery.getAlias());
211+
}
212+
213+
private String applySortingIfNecessary(DeclaredQuery query, Sort sort) {
214+
215+
if (sort.isUnsorted()) {
216+
return query.getQueryString();
217+
}
218+
return applySorting(query, sort);
219+
}
220+
221+
/**
222+
* Value object with optimized {@link Object#equals(Object)} to cache a query based on its query string and
223+
* {@link Sort sorting}.
224+
*
225+
* @since 3.2.3
226+
* @author Christoph Strobl
227+
*/
228+
static class CachableQuery {
229+
230+
private DeclaredQuery declaredQuery;
231+
private final String queryString;
232+
private final Sort sort;
233+
234+
CachableQuery(DeclaredQuery query, Sort sort) {
235+
236+
this.declaredQuery = query;
237+
this.queryString = query.getQueryString();
238+
this.sort = sort;
239+
}
240+
241+
DeclaredQuery getDeclaredQuery() {
242+
return declaredQuery;
243+
}
244+
245+
Sort getSort() {
246+
return sort;
247+
}
248+
249+
String getAlias() {
250+
return declaredQuery.getAlias();
251+
}
252+
253+
@Override
254+
public boolean equals(Object o) {
255+
256+
if (this == o) {
257+
return true;
258+
}
259+
if (o == null || getClass() != o.getClass()) {
260+
return false;
261+
}
262+
263+
CachableQuery that = (CachableQuery) o;
264+
265+
if (!Objects.equals(queryString, that.queryString)) {
266+
return false;
267+
}
268+
return Objects.equals(sort, that.sort);
269+
}
270+
271+
@Override
272+
public int hashCode() {
273+
274+
int result = queryString != null ? queryString.hashCode() : 0;
275+
result = 31 * result + (sort != null ? sort.hashCode() : 0);
276+
return result;
277+
}
278+
}
197279
}

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

+229
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
/*
2+
* Copyright 2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.repository.query;
17+
18+
import static org.mockito.Mockito.*;
19+
20+
import jakarta.persistence.EntityManager;
21+
import jakarta.persistence.metamodel.Metamodel;
22+
23+
import java.lang.reflect.Method;
24+
import java.util.ArrayList;
25+
import java.util.List;
26+
import java.util.function.Supplier;
27+
28+
import org.assertj.core.api.Assertions;
29+
import org.assertj.core.util.Arrays;
30+
import org.junit.jupiter.api.Test;
31+
import org.mockito.Mockito;
32+
import org.springframework.core.annotation.AnnotatedElementUtils;
33+
import org.springframework.data.domain.Pageable;
34+
import org.springframework.data.domain.Sort;
35+
import org.springframework.data.jpa.provider.QueryExtractor;
36+
import org.springframework.data.jpa.repository.Query;
37+
import org.springframework.data.jpa.repository.QueryRewriter;
38+
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
39+
import org.springframework.data.repository.Repository;
40+
import org.springframework.data.repository.core.RepositoryMetadata;
41+
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
42+
import org.springframework.data.repository.query.ParametersSource;
43+
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
44+
import org.springframework.data.repository.query.ReturnedType;
45+
import org.springframework.expression.spel.standard.SpelExpressionParser;
46+
import org.springframework.lang.Nullable;
47+
import org.springframework.util.LinkedMultiValueMap;
48+
import org.springframework.util.MultiValueMap;
49+
import org.springframework.util.ReflectionUtils;
50+
51+
/**
52+
* @author Christoph Strobl
53+
*/
54+
public class AbstractStringBasedJpaQueryUnitTests {
55+
56+
@Test // GH-3310
57+
void shouldNotAttemptToAppendSortIfNoSortArgumentPresent() {
58+
59+
InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find");
60+
stringQuery.createQueryWithArguments();
61+
62+
stringQuery.neverCalled("applySorting");
63+
}
64+
65+
@Test // GH-3310
66+
void shouldNotAttemptToAppendSortIfSortIndicatesUnsorted() {
67+
68+
InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find", Sort.class);
69+
stringQuery.createQueryWithArguments(Sort.unsorted());
70+
71+
stringQuery.neverCalled("applySorting");
72+
}
73+
74+
@Test // GH-3310
75+
void shouldAppendSortIfSortPresent() {
76+
77+
InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find", Sort.class);
78+
stringQuery.createQueryWithArguments(Sort.by("name"));
79+
80+
stringQuery.called("applySorting").times(1);
81+
}
82+
83+
@Test // GH-3311
84+
void cachesInvocationBasedOnSortArgument() {
85+
86+
InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find", Sort.class);
87+
stringQuery.createQueryWithArguments(Sort.by("name"));
88+
stringQuery.called("applySorting").times(1);
89+
90+
stringQuery.createQueryWithArguments(Sort.by("name"));
91+
stringQuery.called("applySorting").times(1);
92+
93+
stringQuery.createQueryWithArguments(Sort.by("age"));
94+
stringQuery.called("applySorting").times(2);
95+
}
96+
97+
interface TestRepo extends Repository<Object, Object> {
98+
99+
@Query("SELECT e FROM Employee e")
100+
Object find();
101+
102+
@Query("SELECT e FROM Employee e")
103+
Object find(Sort sort);
104+
}
105+
106+
static InvocationCapturingStringQueryStub forMethod(Class<?> repository, String method, Class<?>... args) {
107+
108+
Method respositoryMethod = ReflectionUtils.findMethod(repository, method, args);
109+
RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(repository);
110+
SpelAwareProxyProjectionFactory projectionFactory = Mockito.mock(SpelAwareProxyProjectionFactory.class);
111+
QueryExtractor queryExtractor = Mockito.mock(QueryExtractor.class);
112+
JpaQueryMethod queryMethod = new JpaQueryMethod(respositoryMethod, repositoryMetadata, projectionFactory,
113+
queryExtractor);
114+
115+
Query query = AnnotatedElementUtils.getMergedAnnotation(respositoryMethod, Query.class);
116+
117+
return new InvocationCapturingStringQueryStub(respositoryMethod, queryMethod, query.value(), query.countQuery(),
118+
new SpelExpressionParser());
119+
120+
}
121+
122+
static class InvocationCapturingStringQueryStub extends AbstractStringBasedJpaQuery {
123+
124+
private final Method targetMethod;
125+
private final MultiValueMap<String, Arguments> capturedArguments = new LinkedMultiValueMap<>(3);
126+
127+
InvocationCapturingStringQueryStub(Method targetMethod, JpaQueryMethod queryMethod, String queryString,
128+
@Nullable String countQueryString, SpelExpressionParser parser) {
129+
super(queryMethod, new Supplier<EntityManager>() {
130+
131+
@Override
132+
public EntityManager get() {
133+
134+
EntityManager em = Mockito.mock(EntityManager.class);
135+
136+
Metamodel meta = mock(Metamodel.class);
137+
when(em.getMetamodel()).thenReturn(meta);
138+
when(em.getDelegate()).thenReturn(new Object()); // some generic jpa
139+
140+
return em;
141+
}
142+
}.get(), queryString, countQueryString, Mockito.mock(QueryRewriter.class),
143+
Mockito.mock(QueryMethodEvaluationContextProvider.class), parser);
144+
145+
this.targetMethod = targetMethod;
146+
}
147+
148+
@Override
149+
protected String applySorting(CachableQuery query) {
150+
151+
captureInvocation("applySorting", query);
152+
153+
return super.applySorting(query);
154+
}
155+
156+
@Override
157+
protected jakarta.persistence.Query createJpaQuery(String queryString, Sort sort, @Nullable Pageable pageable,
158+
ReturnedType returnedType) {
159+
160+
captureInvocation("createJpaQuery", queryString, sort, pageable, returnedType);
161+
162+
jakarta.persistence.Query jpaQuery = super.createJpaQuery(queryString, sort, pageable, returnedType);
163+
return jpaQuery == null ? Mockito.mock(jakarta.persistence.Query.class) : jpaQuery;
164+
}
165+
166+
// --> convenience for tests
167+
168+
JpaParameters getParameters() {
169+
return new JpaParameters(ParametersSource.of(targetMethod));
170+
}
171+
172+
JpaParametersParameterAccessor getParameterAccessor(Object... args) {
173+
return new JpaParametersParameterAccessor(getParameters(), args);
174+
}
175+
176+
jakarta.persistence.Query createQueryWithArguments(Object... args) {
177+
return doCreateQuery(getParameterAccessor(args));
178+
}
179+
180+
// --> capturing methods
181+
182+
private void captureInvocation(String key, Object... args) {
183+
capturedArguments.add(key, new Arguments(args));
184+
}
185+
186+
// --> verification methdos
187+
188+
int getInvocationCount(String method) {
189+
190+
List<Arguments> invocations = capturedArguments.get(method);
191+
return invocations != null ? invocations.size() : 0;
192+
}
193+
194+
public void neverCalled(String method) {
195+
called(method).never();
196+
}
197+
198+
public Times called(String method) {
199+
200+
return (invocationCount -> {
201+
202+
int actualCount = getInvocationCount(method);
203+
Assertions.assertThat(actualCount)
204+
.withFailMessage(
205+
() -> "Expected %d invocations for %s, but recorded %d".formatted(invocationCount, method, actualCount))
206+
.isEqualTo(invocationCount);
207+
});
208+
}
209+
210+
static class Arguments {
211+
212+
List<Object> values = new ArrayList<>(3);
213+
214+
public Arguments(Object... values) {
215+
this.values = Arrays.asList(values);
216+
}
217+
}
218+
219+
interface Times {
220+
221+
void times(int invocationCount);
222+
223+
default void never() {
224+
times(0);
225+
}
226+
}
227+
228+
}
229+
}

0 commit comments

Comments
 (0)