Spring/spring-data-commons
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Consider property path root in QuerydslBindings.

Browse Source 
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
Original Pull Request: #2422
This commit is contained in:
Mark Paluch
2021-07-29 11:46:31 +02:00
committed by Christoph Strobl
parent 0505fc4975
commit bb3898add3
10 changed files with 211 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/main/java/org/springframework/data/querydsl/binding/PathInformation.java
View File

@@ -31,6 +31,14 @@ import com.querydsl.core.types.Path;
*/
interface PathInformation {
/**
* The root property owner type.
*
* @return
* @since 2.5.5
*/
Class<?> getRootParentType();
/**
* The type of the leaf property.
*

21
src/main/java/org/springframework/data/querydsl/binding/PropertyPathInformation.java
View File

@@ -35,6 +35,7 @@ import com.querydsl.core.types.dsl.CollectionPathBase;
*
* @author Oliver Gierke
* @author Christoph Strobl
* @author Mark Paluch
* @since 1.13
*/
class PropertyPathInformation implements PathInformation {
@@ -71,6 +72,15 @@ class PropertyPathInformation implements PathInformation {
return new PropertyPathInformation(path);
}
/*
* (non-Javadoc)
* @see org.springframework.data.querydsl.binding.PathInformation#getRootParentType()
*/
@Override
public Class<?> getRootParentType() {
return path.getOwningType().getType();
}
/*
* (non-Javadoc)
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafType()
@@ -162,12 +172,13 @@ class PropertyPathInformation implements PathInformation {
return true;
}
if (!(o instanceof PropertyPathInformation)) {
if (!(o instanceof PathInformation)) {
return false;
}
PropertyPathInformation that = (PropertyPathInformation) o;
return ObjectUtils.nullSafeEquals(path, that.path);
PathInformation that = (PathInformation) o;
return ObjectUtils.nullSafeEquals(getRootParentType(), that.getRootParentType())
&& ObjectUtils.nullSafeEquals(toDotPath(), that.toDotPath());
}
/*
@@ -176,7 +187,9 @@ class PropertyPathInformation implements PathInformation {
*/
@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(path);
int result = ObjectUtils.nullSafeHashCode(getRootParentType());
result = 31 * result + ObjectUtils.nullSafeHashCode(toDotPath());
return result;
}
/*

42
src/main/java/org/springframework/data/querydsl/binding/QuerydslBindings.java
View File

@@ -65,6 +65,7 @@ import com.querydsl.core.types.Path;
*/
public class QuerydslBindings {
// pathSpecs key format: <class.simpleName>.<pathInformation.toDotPath>
private final Map<String, PathAndBinding<?, ?>> pathSpecs;
private final Map<Class<?>, PathAndBinding<?, ?>> typeSpecs;
private final Set<String> allowList;
@@ -203,7 +204,7 @@ public class QuerydslBindings {
Assert.notNull(path, "PropertyPath must not be null!");
PathAndBinding<S, T> pathAndBinding = (PathAndBinding<S, T>) pathSpecs.get(path.toDotPath());
PathAndBinding<S, T> pathAndBinding = (PathAndBinding<S, T>) pathSpecs.get(createKey(path));
if (pathAndBinding != null) {
@@ -229,7 +230,7 @@ public class QuerydslBindings {
Assert.notNull(path, "PropertyPath must not be null!");
return Optional.ofNullable(pathSpecs.get(path.toDotPath())).flatMap(PathAndBinding::getPath);
return Optional.ofNullable(pathSpecs.get(createKey(path))).flatMap(PathAndBinding::getPath);
}
/**
@@ -249,6 +250,15 @@ public class QuerydslBindings {
return null;
}
// fully-qualified path lookup
String key = createKey(type, path);
if (pathSpecs.containsKey(key)) {
return pathSpecs.get(key).getPath()//
.map(QuerydslPathInformation::of)//
.orElse(null);
}
// alias lookup
if (pathSpecs.containsKey(path)) {
return pathSpecs.get(path).getPath()//
.map(QuerydslPathInformation::of)//
@@ -263,6 +273,28 @@ public class QuerydslBindings {
}
}
/**
* Returns the property path key for the given {@link Path}.
*
* @param path can be {@literal null}.
* @return
*/
private static String createKey(Optional<Path<?>> path) {
return path.map(QuerydslPathInformation::of).map(QuerydslBindings::createKey).orElse("");
}
private static String createKey(PathInformation path) {
return createKey(path.getRootParentType(), path.toDotPath());
}
private static String createKey(TypeInformation<?> type, String path) {
return createKey(type.getType(), path);
}
private static String createKey(Class<?> type, String path) {
return type.getSimpleName() + "." + path;
}
/**
* Checks if a given {@link PropertyPath} should be visible for binding values.
*
@@ -385,7 +417,7 @@ public class QuerydslBindings {
}
protected void registerBinding(PathAndBinding<P, T> binding) {
QuerydslBindings.this.pathSpecs.put(toDotPath(binding.getPath()), binding);
QuerydslBindings.this.pathSpecs.put(createKey(binding.getPath()), binding);
}
}
@@ -455,10 +487,12 @@ public class QuerydslBindings {
super.registerBinding(binding);
String dotPath = toDotPath(binding.getPath());
if (alias != null) {
QuerydslBindings.this.pathSpecs.put(alias, binding);
QuerydslBindings.this.aliases.add(alias);
QuerydslBindings.this.denyList.add(toDotPath(binding.getPath()));
QuerydslBindings.this.denyList.add(dotPath);
}
}
}

31
src/main/java/org/springframework/data/querydsl/binding/QuerydslPathInformation.java
View File

@@ -29,6 +29,7 @@ import com.querydsl.core.types.Path;
* {@link PathInformation} based on a Querydsl {@link Path}.
*
* @author Oliver Gierke
* @author Mark Paluch
* @since 1.13
*/
class QuerydslPathInformation implements PathInformation {
@@ -45,7 +46,16 @@ class QuerydslPathInformation implements PathInformation {
/*
* (non-Javadoc)
* @see org.springframework.data.querydsl.binding.MappedPath#getLeafType()
* @see org.springframework.data.querydsl.binding.PathInformation#getRootParentType()
*/
@Override
public Class<?> getRootParentType() {
return path.getRoot().getType();
}
/*
* (non-Javadoc)
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafType()
*/
@Override
public Class<?> getLeafType() {
@@ -54,7 +64,7 @@ class QuerydslPathInformation implements PathInformation {
/*
* (non-Javadoc)
* @see org.springframework.data.querydsl.binding.MappedPath#getLeafParentType()
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafParentType()
*/
@Override
public Class<?> getLeafParentType() {
@@ -70,7 +80,7 @@ class QuerydslPathInformation implements PathInformation {
/*
* (non-Javadoc)
* @see org.springframework.data.querydsl.binding.MappedPath#getLeafProperty()
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafProperty()
*/
@Override
public String getLeafProperty() {
@@ -79,7 +89,7 @@ class QuerydslPathInformation implements PathInformation {
/*
* (non-Javadoc)
* @see org.springframework.data.querydsl.binding.MappedPath#getLeafPropertyDescriptor()
* @see org.springframework.data.querydsl.binding.PathInformation#getLeafPropertyDescriptor()
*/
@Nullable
@Override
@@ -89,7 +99,7 @@ class QuerydslPathInformation implements PathInformation {
/*
* (non-Javadoc)
* @see org.springframework.data.querydsl.binding.MappedPath#toDotPath()
* @see org.springframework.data.querydsl.binding.PathInformation#toDotPath()
*/
@Override
public String toDotPath() {
@@ -115,12 +125,13 @@ class QuerydslPathInformation implements PathInformation {
return true;
}
if (!(o instanceof QuerydslPathInformation)) {
if (!(o instanceof PathInformation)) {
return false;
}
QuerydslPathInformation that = (QuerydslPathInformation) o;
return ObjectUtils.nullSafeEquals(path, that.path);
PathInformation that = (PathInformation) o;
return ObjectUtils.nullSafeEquals(getRootParentType(), that.getRootParentType())
&& ObjectUtils.nullSafeEquals(toDotPath(), that.toDotPath());
}
/*
@@ -129,7 +140,9 @@ class QuerydslPathInformation implements PathInformation {
*/
@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(path);
int result = ObjectUtils.nullSafeHashCode(getRootParentType());
result = 31 * result + ObjectUtils.nullSafeHashCode(toDotPath());
return result;
}
/*

1
src/test/java/org/springframework/data/querydsl/Address.java
View File

@@ -25,6 +25,7 @@ public class Address {
public String street, city;
public Double[] lonLat;
public String description;
public Address(String street, String city) {
this.street = street;

5
src/test/java/org/springframework/data/querydsl/User.java
View File

@@ -37,6 +37,7 @@ public class User {
public List<Address> addresses;
public List<String> nickNames;
public Long inceptionYear;
public String description;
public User(String firstname, String lastname, Address address) {
@@ -56,7 +57,3 @@ class SpecialUser extends User {
}
}
@QueryEntity
class UserWrapper {
public User user;
}

23
src/test/java/org/springframework/data/querydsl/UserWrapper.java Normal file
View File

@@ -0,0 +1,23 @@
/*
* Copyright 2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.querydsl;
import com.querydsl.core.annotations.QueryEntity;
@QueryEntity
public final class UserWrapper {
public User user;
}

53
src/test/java/org/springframework/data/querydsl/binding/PropertyPathInformationUnitTests.java Normal file
View File

@@ -0,0 +1,53 @@
/*
* Copyright 2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.querydsl.binding;
import static org.assertj.core.api.Assertions.*;
import org.junit.jupiter.api.Test;
import org.springframework.data.querydsl.QUser;
import org.springframework.data.querydsl.User;
/**
* Unit tests for {@link PropertyPathInformation}.
*
* @author Mark Paluch
*/
class PropertyPathInformationUnitTests {
@Test // GH-2418
void shouldEqualsCorrectly() {
PropertyPathInformation information = PropertyPathInformation.of("address.description", User.class);
QuerydslPathInformation querydslPathInformation = QuerydslPathInformation.of(QUser.user.address.description);
assertThat(information).isEqualTo(querydslPathInformation);
assertThat(querydslPathInformation).isEqualTo(information);
}
@Test // GH-2418
void shouldHashCodeCorrectly() {
PropertyPathInformation information = PropertyPathInformation.of("address.description", User.class);
QuerydslPathInformation querydslPathInformation = QuerydslPathInformation.of(QUser.user.address.description);
assertThat(information).hasSameHashCodeAs(querydslPathInformation);
assertThat(querydslPathInformation).hasSameHashCodeAs(information);
}
}

25
src/test/java/org/springframework/data/querydsl/binding/QuerydslBindingsUnitTests.java
View File

@@ -23,6 +23,8 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.data.querydsl.Address;
import org.springframework.data.querydsl.QAddress;
import org.springframework.data.querydsl.QSpecialUser;
import org.springframework.data.querydsl.QUser;
import org.springframework.data.querydsl.SimpleEntityPathResolver;
@@ -67,6 +69,27 @@ class QuerydslBindingsUnitTests {
assertThat(bindings.getBindingForPath(path)).isEmpty();
}
@Test // GH-2418
void shouldConsiderOwningTypeWhenObtainingBindings() {
bindings.bind(QUser.user.description).first(CONTAINS_BINDING);
assertAdapterWithTargetBinding(bindings.getBindingForPath(PropertyPathInformation.of("description", User.class)),
CONTAINS_BINDING);
assertThat(bindings.getBindingForPath(PropertyPathInformation.of("address.description", User.class))).isEmpty();
assertThat(bindings.getBindingForPath(PropertyPathInformation.of("description", Address.class))).isEmpty();
}
@Test // GH-2418
void shouldConsiderOwningTypeWhenObtainingBindingsWithQuerydslPathInformation() {
bindings.bind(QUser.user.description).first(CONTAINS_BINDING);
assertAdapterWithTargetBinding(bindings.getBindingForPath(QuerydslPathInformation.of(QUser.user.description)),
CONTAINS_BINDING);
assertThat(bindings.getBindingForPath(QuerydslPathInformation.of(QAddress.address.description))).isEmpty();
}
@Test // DATACMNS-669
void returnsRegisteredBindingForSimplePath() {
@@ -78,7 +101,7 @@ class QuerydslBindingsUnitTests {
}
@Test // DATACMNS-669
void getBindingForPathShouldReturnSpeficicBindingForNestedElementsWhenAvailable() {
void getBindingForPathShouldReturnSpecificBindingForNestedElementsWhenAvailable() {
bindings.bind(QUser.user.address.street).first(CONTAINS_BINDING);

28
src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java
View File

@@ -27,11 +27,14 @@ import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.data.querydsl.Address;
import org.springframework.data.querydsl.QSpecialUser;
import org.springframework.data.querydsl.QUser;
import org.springframework.data.querydsl.QUserWrapper;
import org.springframework.data.querydsl.SimpleEntityPathResolver;
import org.springframework.data.querydsl.User;
import org.springframework.data.querydsl.UserWrapper;
import org.springframework.data.querydsl.Users;
import org.springframework.data.util.ClassTypeInformation;
import org.springframework.data.util.Version;
@@ -42,6 +45,7 @@ import org.springframework.util.MultiValueMap;
import com.querydsl.collections.CollQueryFactory;
import com.querydsl.core.types.Constant;
import com.querydsl.core.types.Predicate;
import com.querydsl.core.types.dsl.StringPath;
/**
* Unit tests for {@link QuerydslPredicateBuilder}.
@@ -54,6 +58,7 @@ class QuerydslPredicateBuilderUnitTests {
static final ClassTypeInformation<User> USER_TYPE = ClassTypeInformation.from(User.class);
static final QuerydslBindings DEFAULT_BINDINGS = new QuerydslBindings();
static final SingleValueBinding<StringPath, String> CONTAINS_BINDING = (path, value) -> path.contains(value);
QuerydslPredicateBuilder builder;
MultiValueMap<String, String> values;
@@ -83,6 +88,21 @@ class QuerydslPredicateBuilderUnitTests {
.isTrue();
}
@Test // GH-2418
void shouldLookupCorrectPath() {
DEFAULT_BINDINGS.bind(QUser.user.description).first(CONTAINS_BINDING);
values.add("description", "Linz");
Predicate predicate = this.builder.getPredicate(ClassTypeInformation.from(User.class), values, DEFAULT_BINDINGS);
assertThat(predicate).hasToString("contains(user.description,Linz)");
predicate = this.builder.getPredicate(ClassTypeInformation.from(Address.class), values, DEFAULT_BINDINGS);
assertThat(predicate).hasToString("address.description = Linz");
}
@Test // DATACMNS-669
void resolveArgumentShouldCreateSingleStringParameterPredicateCorrectly() throws Exception {
@@ -206,14 +226,14 @@ class QuerydslPredicateBuilderUnitTests {
values.add("user.specialProperty", "VALUE");
QUserWrapper $ = QUserWrapper.userWrapper;
QUserWrapper wrapper = QUserWrapper.userWrapper;
QuerydslBindings bindings = new QuerydslBindings();
bindings.bind($.user.as(QSpecialUser.class).specialProperty)//
bindings.bind(wrapper.user.as(QSpecialUser.class).specialProperty)//
.first(QuerydslBindingsUnitTests.ContainsBinding.INSTANCE);
assertThat(builder.getPredicate(USER_TYPE, values, bindings))//
.isEqualTo($.user.as(QSpecialUser.class).specialProperty.contains("VALUE"));
assertThat(builder.getPredicate(ClassTypeInformation.from(UserWrapper.class), values, bindings))//
.isEqualTo(wrapper.user.as(QSpecialUser.class).specialProperty.contains("VALUE"));
}
@Test // DATACMNS-1443