Skip to content

Commit d463f5f

Browse files
committed
Avoid DTO Constructor Expression rewriting for selection of nested properties.
We back off from rewriting String-based queries to use DTO Constructor expressions if the query selects a property that is assignable to the return type. Closes #3862
1 parent 43062c4 commit d463f5f

File tree

6 files changed

+224
-7
lines changed

6 files changed

+224
-7
lines changed

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

Lines changed: 125 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,17 @@
1818
import jakarta.persistence.EntityManager;
1919
import jakarta.persistence.Query;
2020

21+
import java.util.List;
22+
import java.util.Map;
2123
import java.util.Objects;
24+
import java.util.concurrent.ConcurrentHashMap;
2225

2326
import org.springframework.data.domain.Pageable;
2427
import org.springframework.data.domain.Sort;
2528
import org.springframework.data.expression.ValueEvaluationContextProvider;
2629
import org.springframework.data.jpa.repository.QueryRewriter;
30+
import org.springframework.data.mapping.PropertyPath;
31+
import org.springframework.data.mapping.PropertyReferenceException;
2732
import org.springframework.data.repository.query.ResultProcessor;
2833
import org.springframework.data.repository.query.ReturnedType;
2934
import org.springframework.data.repository.query.ValueExpressionDelegate;
@@ -48,7 +53,8 @@
4853
*/
4954
abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
5055

51-
private final DeclaredQuery query;
56+
private final StringQuery query;
57+
private final Map<Class<?>, Boolean> knownProjections = new ConcurrentHashMap<>();
5258
private final Lazy<DeclaredQuery> countQuery;
5359
private final ValueExpressionDelegate valueExpressionDelegate;
5460
private final QueryParameterSetter.QueryMetadataCache metadataCache = new QueryParameterSetter.QueryMetadataCache();
@@ -120,7 +126,7 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
120126

121127
Sort sort = accessor.getSort();
122128
ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor);
123-
ReturnedType returnedType = processor.getReturnedType();
129+
ReturnedType returnedType = getReturnedType(processor);
124130
String sortedQueryString = getSortedQueryString(sort, returnedType);
125131
Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), returnedType);
126132

@@ -131,6 +137,81 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
131137
return parameterBinder.get().bindAndPrepare(query, metadata, accessor);
132138
}
133139

140+
/**
141+
* Post-process {@link ReturnedType} to determine if the query is projecting by checking the projection and property
142+
* assignability.
143+
*
144+
* @param processor
145+
* @return
146+
*/
147+
private ReturnedType getReturnedType(ResultProcessor processor) {
148+
149+
ReturnedType returnedType = processor.getReturnedType();
150+
Class<?> returnedJavaType = processor.getReturnedType().getReturnedType();
151+
152+
if (query.isDefaultProjection() || !returnedType.isProjecting() || returnedJavaType.isInterface()
153+
|| query.isNativeQuery()) {
154+
return returnedType;
155+
}
156+
157+
Boolean known = knownProjections.get(returnedJavaType);
158+
159+
if (known != null && known) {
160+
return returnedType;
161+
}
162+
163+
if ((known != null && !known) || returnedJavaType.isArray()) {
164+
if (known == null) {
165+
knownProjections.put(returnedJavaType, false);
166+
}
167+
return new NonProjectingReturnedType(returnedType);
168+
}
169+
170+
String alias = query.getAlias();
171+
String projection = query.getProjection();
172+
173+
// we can handle single-column and no function projections here only
174+
if (StringUtils.hasText(projection) && (projection.indexOf(',') != -1 || projection.indexOf('(') != -1)) {
175+
return returnedType;
176+
}
177+
178+
if (StringUtils.hasText(alias) && StringUtils.hasText(projection)) {
179+
alias = alias.trim();
180+
projection = projection.trim();
181+
if (projection.startsWith(alias + ".")) {
182+
projection = projection.substring(alias.length() + 1);
183+
}
184+
}
185+
186+
if (StringUtils.hasText(projection)) {
187+
188+
int space = projection.indexOf(' ');
189+
190+
if (space != -1) {
191+
projection = projection.substring(0, space);
192+
}
193+
194+
Class<?> propertyType;
195+
196+
try {
197+
PropertyPath from = PropertyPath.from(projection, getQueryMethod().getEntityInformation().getJavaType());
198+
propertyType = from.getLeafType();
199+
} catch (PropertyReferenceException ignored) {
200+
propertyType = null;
201+
}
202+
203+
if (propertyType == null
204+
|| (returnedJavaType.isAssignableFrom(propertyType) || propertyType.isAssignableFrom(returnedJavaType))) {
205+
knownProjections.put(returnedJavaType, false);
206+
return new NonProjectingReturnedType(returnedType);
207+
} else {
208+
knownProjections.put(returnedJavaType, true);
209+
}
210+
}
211+
212+
return returnedType;
213+
}
214+
134215
String getSortedQueryString(Sort sort, ReturnedType returnedType) {
135216
return querySortRewriter.getSorted(query, sort, returnedType);
136217
}
@@ -348,4 +429,46 @@ public int hashCode() {
348429
return result;
349430
}
350431
}
432+
433+
/**
434+
* Non-projecting {@link ReturnedType} wrapper that delegates to the original {@link ReturnedType} but always returns
435+
* {@code false} for {@link #isProjecting()}. This type is to indicate that this query is not projecting, even if the
436+
* original {@link ReturnedType} was because we e.g. select a nested property and do not want DTO constructor
437+
* expression rewriting to kick in.
438+
*/
439+
private static class NonProjectingReturnedType extends ReturnedType {
440+
441+
private final ReturnedType delegate;
442+
443+
NonProjectingReturnedType(ReturnedType delegate) {
444+
super(delegate.getDomainType());
445+
this.delegate = delegate;
446+
}
447+
448+
@Override
449+
public boolean isProjecting() {
450+
return false;
451+
}
452+
453+
@Override
454+
public Class<?> getReturnedType() {
455+
return delegate.getReturnedType();
456+
}
457+
458+
@Override
459+
public boolean needsCustomConstruction() {
460+
return false;
461+
}
462+
463+
@Override
464+
@Nullable
465+
public Class<?> getTypeToRead() {
466+
return delegate.getTypeToRead();
467+
}
468+
469+
@Override
470+
public List<String> getInputProperties() {
471+
return delegate.getInputProperties();
472+
}
473+
}
351474
}

spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/Address.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
import jakarta.persistence.Embeddable;
1919

20+
import org.springframework.util.ObjectUtils;
21+
2022
/**
2123
* @author Thomas Darimont
2224
*/
@@ -52,4 +54,26 @@ public String getStreetName() {
5254
public String getStreetNo() {
5355
return streetNo;
5456
}
57+
58+
@Override
59+
public boolean equals(Object o) {
60+
if (!(o instanceof Address address)) {
61+
return false;
62+
}
63+
if (!ObjectUtils.nullSafeEquals(country, address.country)) {
64+
return false;
65+
}
66+
if (!ObjectUtils.nullSafeEquals(city, address.city)) {
67+
return false;
68+
}
69+
if (!ObjectUtils.nullSafeEquals(streetName, address.streetName)) {
70+
return false;
71+
}
72+
return ObjectUtils.nullSafeEquals(streetNo, address.streetNo);
73+
}
74+
75+
@Override
76+
public int hashCode() {
77+
return ObjectUtils.nullSafeHash(country, city, streetName, streetNo);
78+
}
5579
}

spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/Role.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import jakarta.persistence.GeneratedValue;
2020
import jakarta.persistence.Id;
2121

22+
import org.springframework.util.ObjectUtils;
23+
2224
/**
2325
* Sample domain class representing roles. Mapped with XML.
2426
*
@@ -55,4 +57,17 @@ public String toString() {
5557
public boolean isNew() {
5658
return id == null;
5759
}
60+
61+
@Override
62+
public boolean equals(Object o) {
63+
if (!(o instanceof Role role)) {
64+
return false;
65+
}
66+
return ObjectUtils.nullSafeEquals(id, role.id);
67+
}
68+
69+
@Override
70+
public int hashCode() {
71+
return ObjectUtils.nullSafeHash(id);
72+
}
5873
}

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

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,8 @@
1515
*/
1616
package org.springframework.data.jpa.repository;
1717

18-
import static org.assertj.core.api.Assertions.assertThat;
19-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
20-
import static org.springframework.data.domain.Sort.Direction.ASC;
21-
import static org.springframework.data.domain.Sort.Direction.DESC;
18+
import static org.assertj.core.api.Assertions.*;
19+
import static org.springframework.data.domain.Sort.Direction.*;
2220

2321
import jakarta.persistence.EntityManager;
2422

@@ -33,6 +31,7 @@
3331
import org.junit.jupiter.api.extension.ExtendWith;
3432
import org.junit.jupiter.params.ParameterizedTest;
3533
import org.junit.jupiter.params.provider.ValueSource;
34+
3635
import org.springframework.beans.factory.annotation.Autowired;
3736
import org.springframework.dao.InvalidDataAccessApiUsageException;
3837
import org.springframework.data.domain.Limit;
@@ -43,6 +42,7 @@
4342
import org.springframework.data.domain.Slice;
4443
import org.springframework.data.domain.Sort;
4544
import org.springframework.data.domain.Window;
45+
import org.springframework.data.jpa.domain.sample.Address;
4646
import org.springframework.data.jpa.domain.sample.Role;
4747
import org.springframework.data.jpa.domain.sample.User;
4848
import org.springframework.data.jpa.provider.PersistenceProvider;
@@ -422,6 +422,11 @@ void dtoProjectionShouldApplyConstructorExpressionRewriting() {
422422

423423
assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::firstname) //
424424
.contains("Dave", "Carter", "Oliver August");
425+
426+
dtos = userRepository.findRecordProjectionWithFunctions();
427+
428+
assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::lastname) //
429+
.contains("matthews", "beauford");
425430
}
426431

427432
@Test // GH-3076
@@ -442,6 +447,29 @@ void dynamicDtoProjection() {
442447
.contains("Dave", "Carter", "Oliver August");
443448
}
444449

450+
@Test // GH-3862
451+
void shouldNotRewritePrimitiveSelectionToDtoProjection() {
452+
453+
oliver.setAge(28);
454+
em.persist(oliver);
455+
456+
assertThat(userRepository.findAgeByAnnotatedQuery(oliver.getEmailAddress())).contains(28);
457+
}
458+
459+
@Test // GH-3862
460+
void shouldNotRewritePropertySelectionToDtoProjection() {
461+
462+
Address address = new Address("DE", "Dresden", "some street", "12345");
463+
dave.setAddress(address);
464+
userRepository.save(dave);
465+
em.flush();
466+
em.clear();
467+
468+
assertThat(userRepository.findAddressByAnnotatedQuery(dave.getEmailAddress())).contains(address);
469+
assertThat(userRepository.findCityByAnnotatedQuery(dave.getEmailAddress())).contains("Dresden");
470+
assertThat(userRepository.findRolesByAnnotatedQuery(dave.getEmailAddress())).contains(singer);
471+
}
472+
445473
@Test // GH-3076
446474
void dtoProjectionWithEntityAndAggregatedValue() {
447475

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,6 @@ void testOverwritingFinder() {
455455

456456
@Test
457457
void testUsesQueryAnnotation() {
458-
459458
assertThat(repository.findByAnnotatedQuery("gierke@synyx.de")).isNull();
460459
}
461460

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.springframework.data.domain.Slice;
3838
import org.springframework.data.domain.Sort;
3939
import org.springframework.data.domain.Window;
40+
import org.springframework.data.jpa.domain.sample.Address;
4041
import org.springframework.data.jpa.domain.sample.Role;
4142
import org.springframework.data.jpa.domain.sample.SpecialUser;
4243
import org.springframework.data.jpa.domain.sample.User;
@@ -723,12 +724,39 @@ List<String> findAllAndSortByFunctionResultNamedParameter(@Param("namedParameter
723724
@Query("select u from User u")
724725
List<UserExcerpt> findRecordProjection();
725726

727+
@Query("select u.firstname, LOWER(u.lastname) from User u")
728+
List<UserExcerpt> findRecordProjectionWithFunctions();
729+
726730
@Query("select u from User u")
727731
<T> List<T> findRecordProjection(Class<T> projectionType);
728732

729733
@Query("select u.firstname, u.lastname from User u")
730734
List<UserExcerpt> findMultiselectRecordProjection();
731735

736+
/**
737+
* Retrieves a user age by email.
738+
*/
739+
@Query("select u.age from User u where u.emailAddress = ?1")
740+
Optional<Integer> findAgeByAnnotatedQuery(String emailAddress);
741+
742+
/**
743+
* Retrieves a user address by email.
744+
*/
745+
@Query("select u.address from User u where u.emailAddress = ?1")
746+
Optional<Address> findAddressByAnnotatedQuery(String emailAddress);
747+
748+
/**
749+
* Retrieves a user roles by email.
750+
*/
751+
@Query("select u.roles from User u where u.emailAddress = ?1")
752+
Set<Role> findRolesByAnnotatedQuery(String emailAddress);
753+
754+
/**
755+
* Retrieves a user address city by email.
756+
*/
757+
@Query("select u.address.city from User u where u.emailAddress = ?1")
758+
String findCityByAnnotatedQuery(String emailAddress);
759+
732760
@UserRoleCountProjectingQuery
733761
List<UserRoleCountDtoProjection> dtoProjectionEntityAndAggregatedValue();
734762

0 commit comments

Comments
 (0)