Skip to content

Commit d24a37d

Browse files
committed
Consider property path root in QuerydslBindings.
We now consider a qualified property path when registering and querying `QuerydslBindings` to ensure that paths matching various domain types do not accidentally get applied for a different type than they were registered for. Previously, a binding for `Address.description` would be also applied to `User.description` as only the property path `description` was considered when looking up bindings. Closes #2418
1 parent 4d239e3 commit d24a37d

File tree

10 files changed

+211
-26
lines changed

10 files changed

+211
-26
lines changed

src/main/java/org/springframework/data/querydsl/binding/PathInformation.java

+8
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@
3131
*/
3232
interface PathInformation {
3333

34+
/**
35+
* The root property owner type.
36+
*
37+
* @return
38+
* @since 2.5.4
39+
*/
40+
Class<?> getRootParentType();
41+
3442
/**
3543
* The type of the leaf property.
3644
*

src/main/java/org/springframework/data/querydsl/binding/PropertyPathInformation.java

+17-4
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
*
3636
* @author Oliver Gierke
3737
* @author Christoph Strobl
38+
* @author Mark Paluch
3839
* @since 1.13
3940
*/
4041
class PropertyPathInformation implements PathInformation {
@@ -71,6 +72,15 @@ private static PropertyPathInformation of(PropertyPath path) {
7172
return new PropertyPathInformation(path);
7273
}
7374

75+
/*
76+
* (non-Javadoc)
77+
* @see org.springframework.data.querydsl.binding.PathInformation#getRootParentType()
78+
*/
79+
@Override
80+
public Class<?> getRootParentType() {
81+
return path.getOwningType().getType();
82+
}
83+
7484
/*
7585
* (non-Javadoc)
7686
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafType()
@@ -162,12 +172,13 @@ public boolean equals(Object o) {
162172
return true;
163173
}
164174

165-
if (!(o instanceof PropertyPathInformation)) {
175+
if (!(o instanceof PathInformation)) {
166176
return false;
167177
}
168178

169-
PropertyPathInformation that = (PropertyPathInformation) o;
170-
return ObjectUtils.nullSafeEquals(path, that.path);
179+
PathInformation that = (PathInformation) o;
180+
return ObjectUtils.nullSafeEquals(getRootParentType(), that.getRootParentType())
181+
&& ObjectUtils.nullSafeEquals(toDotPath(), that.toDotPath());
171182
}
172183

173184
/*
@@ -176,7 +187,9 @@ public boolean equals(Object o) {
176187
*/
177188
@Override
178189
public int hashCode() {
179-
return ObjectUtils.nullSafeHashCode(path);
190+
int result = ObjectUtils.nullSafeHashCode(getRootParentType());
191+
result = 31 * result + ObjectUtils.nullSafeHashCode(toDotPath());
192+
return result;
180193
}
181194

182195
/*

src/main/java/org/springframework/data/querydsl/binding/QuerydslBindings.java

+38-4
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
*/
6666
public class QuerydslBindings {
6767

68+
// pathSpecs key format: <class.simpleName>.<pathInformation.toDotPath>
6869
private final Map<String, PathAndBinding<?, ?>> pathSpecs;
6970
private final Map<Class<?>, PathAndBinding<?, ?>> typeSpecs;
7071
private final Set<String> allowList;
@@ -203,7 +204,7 @@ public <S extends Path<? extends T>, T> Optional<MultiValueBinding<S, T>> getBin
203204

204205
Assert.notNull(path, "PropertyPath must not be null!");
205206

206-
PathAndBinding<S, T> pathAndBinding = (PathAndBinding<S, T>) pathSpecs.get(path.toDotPath());
207+
PathAndBinding<S, T> pathAndBinding = (PathAndBinding<S, T>) pathSpecs.get(createKey(path));
207208

208209
if (pathAndBinding != null) {
209210

@@ -229,7 +230,7 @@ Optional<Path<?>> getExistingPath(PathInformation path) {
229230

230231
Assert.notNull(path, "PropertyPath must not be null!");
231232

232-
return Optional.ofNullable(pathSpecs.get(path.toDotPath())).flatMap(PathAndBinding::getPath);
233+
return Optional.ofNullable(pathSpecs.get(createKey(path))).flatMap(PathAndBinding::getPath);
233234
}
234235

235236
/**
@@ -249,6 +250,15 @@ PathInformation getPropertyPath(String path, TypeInformation<?> type) {
249250
return null;
250251
}
251252

253+
// fully-qualified path lookup
254+
String key = createKey(type, path);
255+
if (pathSpecs.containsKey(key)) {
256+
return pathSpecs.get(key).getPath()//
257+
.map(QuerydslPathInformation::of)//
258+
.orElse(null);
259+
}
260+
261+
// alias lookup
252262
if (pathSpecs.containsKey(path)) {
253263
return pathSpecs.get(path).getPath()//
254264
.map(QuerydslPathInformation::of)//
@@ -263,6 +273,28 @@ PathInformation getPropertyPath(String path, TypeInformation<?> type) {
263273
}
264274
}
265275

276+
/**
277+
* Returns the property path key for the given {@link Path}.
278+
*
279+
* @param path can be {@literal null}.
280+
* @return
281+
*/
282+
private static String createKey(Optional<Path<?>> path) {
283+
return path.map(QuerydslPathInformation::of).map(QuerydslBindings::createKey).orElse("");
284+
}
285+
286+
private static String createKey(PathInformation path) {
287+
return createKey(path.getRootParentType(), path.toDotPath());
288+
}
289+
290+
private static String createKey(TypeInformation<?> type, String path) {
291+
return createKey(type.getType(), path);
292+
}
293+
294+
private static String createKey(Class<?> type, String path) {
295+
return type.getSimpleName() + "." + path;
296+
}
297+
266298
/**
267299
* Checks if a given {@link PropertyPath} should be visible for binding values.
268300
*
@@ -385,7 +417,7 @@ public void all(MultiValueBinding<P, T> binding) {
385417
}
386418

387419
protected void registerBinding(PathAndBinding<P, T> binding) {
388-
QuerydslBindings.this.pathSpecs.put(toDotPath(binding.getPath()), binding);
420+
QuerydslBindings.this.pathSpecs.put(createKey(binding.getPath()), binding);
389421
}
390422
}
391423

@@ -455,10 +487,12 @@ protected void registerBinding(PathAndBinding<P, T> binding) {
455487

456488
super.registerBinding(binding);
457489

490+
String dotPath = toDotPath(binding.getPath());
491+
458492
if (alias != null) {
459493
QuerydslBindings.this.pathSpecs.put(alias, binding);
460494
QuerydslBindings.this.aliases.add(alias);
461-
QuerydslBindings.this.denyList.add(toDotPath(binding.getPath()));
495+
QuerydslBindings.this.denyList.add(dotPath);
462496
}
463497
}
464498
}

src/main/java/org/springframework/data/querydsl/binding/QuerydslPathInformation.java

+22-9
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
* {@link PathInformation} based on a Querydsl {@link Path}.
3030
*
3131
* @author Oliver Gierke
32+
* @author Mark Paluch
3233
* @since 1.13
3334
*/
3435
class QuerydslPathInformation implements PathInformation {
@@ -45,7 +46,16 @@ public static QuerydslPathInformation of(Path<?> path) {
4546

4647
/*
4748
* (non-Javadoc)
48-
* @see org.springframework.data.querydsl.binding.MappedPath#getLeafType()
49+
* @see org.springframework.data.querydsl.binding.PathInformation#getRootParentType()
50+
*/
51+
@Override
52+
public Class<?> getRootParentType() {
53+
return path.getRoot().getType();
54+
}
55+
56+
/*
57+
* (non-Javadoc)
58+
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafType()
4959
*/
5060
@Override
5161
public Class<?> getLeafType() {
@@ -54,7 +64,7 @@ public Class<?> getLeafType() {
5464

5565
/*
5666
* (non-Javadoc)
57-
* @see org.springframework.data.querydsl.binding.MappedPath#getLeafParentType()
67+
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafParentType()
5868
*/
5969
@Override
6070
public Class<?> getLeafParentType() {
@@ -70,7 +80,7 @@ public Class<?> getLeafParentType() {
7080

7181
/*
7282
* (non-Javadoc)
73-
* @see org.springframework.data.querydsl.binding.MappedPath#getLeafProperty()
83+
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafProperty()
7484
*/
7585
@Override
7686
public String getLeafProperty() {
@@ -79,7 +89,7 @@ public String getLeafProperty() {
7989

8090
/*
8191
* (non-Javadoc)
82-
* @see org.springframework.data.querydsl.binding.MappedPath#getLeafPropertyDescriptor()
92+
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafPropertyDescriptor()
8393
*/
8494
@Nullable
8595
@Override
@@ -89,7 +99,7 @@ public PropertyDescriptor getLeafPropertyDescriptor() {
8999

90100
/*
91101
* (non-Javadoc)
92-
* @see org.springframework.data.querydsl.binding.MappedPath#toDotPath()
102+
* @see org.springframework.data.querydsl.binding.PathInformation#toDotPath()
93103
*/
94104
@Override
95105
public String toDotPath() {
@@ -115,12 +125,13 @@ public boolean equals(Object o) {
115125
return true;
116126
}
117127

118-
if (!(o instanceof QuerydslPathInformation)) {
128+
if (!(o instanceof PathInformation)) {
119129
return false;
120130
}
121131

122-
QuerydslPathInformation that = (QuerydslPathInformation) o;
123-
return ObjectUtils.nullSafeEquals(path, that.path);
132+
PathInformation that = (PathInformation) o;
133+
return ObjectUtils.nullSafeEquals(getRootParentType(), that.getRootParentType())
134+
&& ObjectUtils.nullSafeEquals(toDotPath(), that.toDotPath());
124135
}
125136

126137
/*
@@ -129,7 +140,9 @@ public boolean equals(Object o) {
129140
*/
130141
@Override
131142
public int hashCode() {
132-
return ObjectUtils.nullSafeHashCode(path);
143+
int result = ObjectUtils.nullSafeHashCode(getRootParentType());
144+
result = 31 * result + ObjectUtils.nullSafeHashCode(toDotPath());
145+
return result;
133146
}
134147

135148
/*

src/test/java/org/springframework/data/querydsl/Address.java

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ public class Address {
2525

2626
public String street, city;
2727
public Double[] lonLat;
28+
public String description;
2829

2930
public Address(String street, String city) {
3031
this.street = street;

src/test/java/org/springframework/data/querydsl/User.java

+1-4
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public class User {
3737
public List<Address> addresses;
3838
public List<String> nickNames;
3939
public Long inceptionYear;
40+
public String description;
4041

4142
public User(String firstname, String lastname, Address address) {
4243

@@ -56,7 +57,3 @@ public SpecialUser(String firstname, String lastname, Address address) {
5657
}
5758
}
5859

59-
@QueryEntity
60-
class UserWrapper {
61-
public User user;
62-
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright 2021 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.querydsl;
17+
18+
import com.querydsl.core.annotations.QueryEntity;
19+
20+
@QueryEntity
21+
public final class UserWrapper {
22+
public User user;
23+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright 2021 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.querydsl.binding;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
20+
import org.junit.jupiter.api.Test;
21+
22+
import org.springframework.data.querydsl.QUser;
23+
import org.springframework.data.querydsl.User;
24+
25+
/**
26+
* Unit tests for {@link PropertyPathInformation}.
27+
*
28+
* @author Mark Paluch
29+
*/
30+
class PropertyPathInformationUnitTests {
31+
32+
@Test // GH-2418
33+
void shouldEqualsCorrectly() {
34+
35+
PropertyPathInformation information = PropertyPathInformation.of("address.description", User.class);
36+
37+
QuerydslPathInformation querydslPathInformation = QuerydslPathInformation.of(QUser.user.address.description);
38+
39+
assertThat(information).isEqualTo(querydslPathInformation);
40+
assertThat(querydslPathInformation).isEqualTo(information);
41+
}
42+
43+
@Test // GH-2418
44+
void shouldHashCodeCorrectly() {
45+
46+
PropertyPathInformation information = PropertyPathInformation.of("address.description", User.class);
47+
48+
QuerydslPathInformation querydslPathInformation = QuerydslPathInformation.of(QUser.user.address.description);
49+
50+
assertThat(information).hasSameHashCodeAs(querydslPathInformation);
51+
assertThat(querydslPathInformation).hasSameHashCodeAs(information);
52+
}
53+
}

src/test/java/org/springframework/data/querydsl/binding/QuerydslBindingsUnitTests.java

+24-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import org.junit.jupiter.api.Test;
2424

2525
import org.springframework.core.convert.support.DefaultConversionService;
26+
import org.springframework.data.querydsl.Address;
27+
import org.springframework.data.querydsl.QAddress;
2628
import org.springframework.data.querydsl.QSpecialUser;
2729
import org.springframework.data.querydsl.QUser;
2830
import org.springframework.data.querydsl.SimpleEntityPathResolver;
@@ -67,6 +69,27 @@ void returnsEmptyOptionalIfNoBindingRegisteredForPath() {
6769
assertThat(bindings.getBindingForPath(path)).isEmpty();
6870
}
6971

72+
@Test // GH-2418
73+
void shouldConsiderOwningTypeWhenObtainingBindings() {
74+
75+
bindings.bind(QUser.user.description).first(CONTAINS_BINDING);
76+
77+
assertAdapterWithTargetBinding(bindings.getBindingForPath(PropertyPathInformation.of("description", User.class)),
78+
CONTAINS_BINDING);
79+
assertThat(bindings.getBindingForPath(PropertyPathInformation.of("address.description", User.class))).isEmpty();
80+
assertThat(bindings.getBindingForPath(PropertyPathInformation.of("description", Address.class))).isEmpty();
81+
}
82+
83+
@Test // GH-2418
84+
void shouldConsiderOwningTypeWhenObtainingBindingsWithQuerydslPathInformation() {
85+
86+
bindings.bind(QUser.user.description).first(CONTAINS_BINDING);
87+
88+
assertAdapterWithTargetBinding(bindings.getBindingForPath(QuerydslPathInformation.of(QUser.user.description)),
89+
CONTAINS_BINDING);
90+
assertThat(bindings.getBindingForPath(QuerydslPathInformation.of(QAddress.address.description))).isEmpty();
91+
}
92+
7093
@Test // DATACMNS-669
7194
void returnsRegisteredBindingForSimplePath() {
7295

@@ -78,7 +101,7 @@ void returnsRegisteredBindingForSimplePath() {
78101
}
79102

80103
@Test // DATACMNS-669
81-
void getBindingForPathShouldReturnSpeficicBindingForNestedElementsWhenAvailable() {
104+
void getBindingForPathShouldReturnSpecificBindingForNestedElementsWhenAvailable() {
82105

83106
bindings.bind(QUser.user.address.street).first(CONTAINS_BINDING);
84107

0 commit comments

Comments
 (0)