From f2a6d63dbb5de6536b24c27e64a7bfabdf20d5cf Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sun, 10 Jul 2016 11:39:40 +0200 Subject: [PATCH] DATACMNS-883 - Improved collection path binding for Querydsl-backed web requests. Improved the dot-path translation of existing Querydsl Path instances by not relying on the toString() representation of the metadata but manually traversing the elements up to the root. Extracted the translation method into QueryDslUtils. Furthermore, we now automatically insert an CollectionPathBase.any() step for Path instances of that type when reifying the paths from request attributes. --- .../data/querydsl/QueryDslUtils.java | 50 ++++++++++++++++- .../querydsl/binding/QuerydslBindings.java | 56 ++++++++++++------- .../binding/QuerydslPredicateBuilder.java | 9 ++- .../data/querydsl/QueryDslUtilsUnitTests.java | 37 ++++++++++++ .../springframework/data/querydsl/User.java | 3 +- .../binding/QuerydslBindingsUnitTests.java | 34 +++++------ .../QuerydslPredicateBuilderUnitTests.java | 15 ++++- 7 files changed, 159 insertions(+), 45 deletions(-) create mode 100644 src/test/java/org/springframework/data/querydsl/QueryDslUtilsUnitTests.java diff --git a/src/main/java/org/springframework/data/querydsl/QueryDslUtils.java b/src/main/java/org/springframework/data/querydsl/QueryDslUtils.java index f329a54ec..abc87f0fb 100644 --- a/src/main/java/org/springframework/data/querydsl/QueryDslUtils.java +++ b/src/main/java/org/springframework/data/querydsl/QueryDslUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2015 the original author or authors. + * Copyright 2011-2016 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. @@ -15,17 +15,61 @@ */ package org.springframework.data.querydsl; +import lombok.experimental.UtilityClass; + +import org.springframework.util.StringUtils; + +import com.querydsl.core.types.Path; +import com.querydsl.core.types.PathMetadata; + /** * Utility class for Querydsl. * * @author Oliver Gierke */ -public abstract class QueryDslUtils { +@UtilityClass +public class QueryDslUtils { public static final boolean QUERY_DSL_PRESENT = org.springframework.util.ClassUtils .isPresent("com.querydsl.core.types.Predicate", QueryDslUtils.class.getClassLoader()); - private QueryDslUtils() { + /** + * Returns the property path for the given {@link Path}. + * + * @param path can be {@literal null}. + * @return + */ + public static String toDotPath(Path path) { + return toDotPath(path, ""); + } + /** + * Recursively builds up the dot path for the given {@link Path} instance by walking up the individual segments until + * the root. + * + * @param path can be {@literal null}. + * @param tail must not be {@literal null}. + * @return + */ + private static String toDotPath(Path path, String tail) { + + if (path == null) { + return tail; + } + + PathMetadata metadata = path.getMetadata(); + Path parent = metadata.getParent(); + + if (parent == null) { + return tail; + } + + Object element = metadata.getElement(); + + if (element == null || !StringUtils.hasText(element.toString())) { + return toDotPath(parent, tail); + } + + return toDotPath(parent, StringUtils.hasText(tail) ? String.format("%s.%s", element, tail) : element.toString()); } } diff --git a/src/main/java/org/springframework/data/querydsl/binding/QuerydslBindings.java b/src/main/java/org/springframework/data/querydsl/binding/QuerydslBindings.java index 6cbafafb4..69a0f45f0 100644 --- a/src/main/java/org/springframework/data/querydsl/binding/QuerydslBindings.java +++ b/src/main/java/org/springframework/data/querydsl/binding/QuerydslBindings.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2016 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. @@ -15,6 +15,8 @@ */ package org.springframework.data.querydsl.binding; +import static org.springframework.data.querydsl.QueryDslUtils.*; + import java.util.Arrays; import java.util.Collection; import java.util.HashSet; @@ -32,7 +34,6 @@ import org.springframework.util.Assert; import org.springframework.util.StringUtils; import com.querydsl.core.types.Path; -import com.querydsl.core.types.PathMetadata; import com.querydsl.core.types.Predicate; /** @@ -158,8 +159,34 @@ public class QuerydslBindings { return this; } - public boolean isPathVisible(String path, Class type) { - return getPropertyPath(path, ClassTypeInformation.from(type)) != null; + /** + * Returns whether the given path is available on the given type. + * + * @param path must not be {@literal null}. + * @param type must not be {@literal null}. + * @return + */ + boolean isPathAvailable(String path, Class type) { + + Assert.notNull(path, "Path must not be null!"); + Assert.notNull(type, "Type must not be null!"); + + return isPathAvailable(path, ClassTypeInformation.from(type)); + } + + /** + * Returns whether the given path is available on the given type. + * + * @param path must not be {@literal null}. + * @param type + * @return + */ + boolean isPathAvailable(String path, TypeInformation type) { + + Assert.notNull(path, "Path must not be null!"); + Assert.notNull(type, "Type must not be null!"); + + return getPropertyPath(path, type) != null; } /** @@ -198,6 +225,8 @@ public class QuerydslBindings { */ Path getExistingPath(PropertyPath path) { + Assert.notNull(path, "PropertyPath must not be null!"); + PathAndBinding pathAndBuilder = pathSpecs.get(path.toDotPath()); return pathAndBuilder == null ? null : pathAndBuilder.getPath(); } @@ -209,6 +238,8 @@ public class QuerydslBindings { */ PropertyPath getPropertyPath(String path, TypeInformation type) { + Assert.notNull(path, "Path must not be null!"); + if (!isPathVisible(path)) { return null; } @@ -272,23 +303,6 @@ public class QuerydslBindings { return whiteList.contains(path); } - /** - * Returns the property path for the given {@link Path}. - * - * @param path can be {@literal null}. - * @return - */ - private static String toDotPath(Path path) { - - if (path == null) { - return ""; - } - - PathMetadata metadata = path.getMetadata(); - - return path.toString().substring(metadata.getRootPath().getMetadata().getName().length() + 1); - } - /** * A binder for {@link Path}s. * diff --git a/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java b/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java index c2f78540d..bc53abdcd 100644 --- a/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java +++ b/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2016 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. @@ -41,6 +41,7 @@ import org.springframework.util.StringUtils; import com.querydsl.core.BooleanBuilder; import com.querydsl.core.types.Path; import com.querydsl.core.types.Predicate; +import com.querydsl.core.types.dsl.CollectionPathBase; /** * Builder assembling {@link Predicate} out of {@link PropertyValues}. @@ -101,7 +102,7 @@ public class QuerydslPredicateBuilder { String path = entry.getKey(); - if (!bindings.isPathVisible(path, type.getType())) { + if (!bindings.isPathAvailable(path, type)) { continue; } @@ -179,6 +180,10 @@ public class QuerydslPredicateBuilder { */ private Path reifyPath(PropertyPath path, Path base) { + if (base instanceof CollectionPathBase) { + return reifyPath(path, (Path) ((CollectionPathBase) base).any()); + } + Path entityPath = base != null ? base : resolver.createPath(path.getOwningType().getType()); Field field = ReflectionUtils.findField(entityPath.getClass(), path.getSegment()); diff --git a/src/test/java/org/springframework/data/querydsl/QueryDslUtilsUnitTests.java b/src/test/java/org/springframework/data/querydsl/QueryDslUtilsUnitTests.java new file mode 100644 index 000000000..da33c453e --- /dev/null +++ b/src/test/java/org/springframework/data/querydsl/QueryDslUtilsUnitTests.java @@ -0,0 +1,37 @@ +/* + * Copyright 2016 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 + * + * http://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 static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import org.junit.Test; + +/** + * Unit tests for {@link QueryDslUtils}. + * + * @author Oliver Gierke + */ +public class QueryDslUtilsUnitTests { + + /** + * @see DATACMNS-883 + */ + @Test + public void rendersDotPathForPathTraversalContainingAnyExpression() { + assertThat(QueryDslUtils.toDotPath(QUser.user.addresses.any().street), is("addresses.street")); + } +} diff --git a/src/test/java/org/springframework/data/querydsl/User.java b/src/test/java/org/springframework/data/querydsl/User.java index fc8083caf..0e523f53d 100644 --- a/src/test/java/org/springframework/data/querydsl/User.java +++ b/src/test/java/org/springframework/data/querydsl/User.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2015 the original author or authors. + * Copyright 2011-2016 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. @@ -34,6 +34,7 @@ public class User { public String firstname, lastname; public @DateTimeFormat(iso = ISO.DATE) Date dateOfBirth; public Address address; + public List
addresses; public List nickNames; public Long inceptionYear; diff --git a/src/test/java/org/springframework/data/querydsl/binding/QuerydslBindingsUnitTests.java b/src/test/java/org/springframework/data/querydsl/binding/QuerydslBindingsUnitTests.java index 749bc0b43..6a9bd23c0 100644 --- a/src/test/java/org/springframework/data/querydsl/binding/QuerydslBindingsUnitTests.java +++ b/src/test/java/org/springframework/data/querydsl/binding/QuerydslBindingsUnitTests.java @@ -129,7 +129,7 @@ public class QuerydslBindingsUnitTests { bindings.bind(String.class).first(CONTAINS_BINDING); - assertThat(bindings.isPathVisible("inceptionYear", User.class), is(true)); + assertThat(bindings.isPathAvailable("inceptionYear", User.class), is(true)); } /** @@ -140,7 +140,7 @@ public class QuerydslBindingsUnitTests { bindings.including(QUser.user.inceptionYear); - assertThat(bindings.isPathVisible("inceptionYear", User.class), is(true)); + assertThat(bindings.isPathAvailable("inceptionYear", User.class), is(true)); } /** @@ -151,7 +151,7 @@ public class QuerydslBindingsUnitTests { bindings.including(QUser.user.inceptionYear); - assertThat(bindings.isPathVisible("firstname", User.class), is(false)); + assertThat(bindings.isPathAvailable("firstname", User.class), is(false)); } /** @@ -162,7 +162,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.inceptionYear); - assertThat(bindings.isPathVisible("inceptionYear", User.class), is(false)); + assertThat(bindings.isPathAvailable("inceptionYear", User.class), is(false)); } /** @@ -173,7 +173,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.inceptionYear); - assertThat(bindings.isPathVisible("firstname", User.class), is(true)); + assertThat(bindings.isPathAvailable("firstname", User.class), is(true)); } /** @@ -185,7 +185,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.firstname); bindings.including(QUser.user.firstname); - assertThat(bindings.isPathVisible("firstname", User.class), is(true)); + assertThat(bindings.isPathAvailable("firstname", User.class), is(true)); } /** @@ -196,7 +196,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.address); - assertThat(bindings.isPathVisible("address.city", User.class), is(false)); + assertThat(bindings.isPathAvailable("address.city", User.class), is(false)); } /** @@ -208,7 +208,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.address); bindings.including(QUser.user.address.city); - assertThat(bindings.isPathVisible("address.city", User.class), is(true)); + assertThat(bindings.isPathAvailable("address.city", User.class), is(true)); } /** @@ -220,7 +220,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.address); bindings.including(QUser.user.address.city); - assertThat(bindings.isPathVisible("address.street", User.class), is(false)); + assertThat(bindings.isPathAvailable("address.street", User.class), is(false)); } /** @@ -231,10 +231,10 @@ public class QuerydslBindingsUnitTests { bindings.including(QUser.user.firstname, QUser.user.address.street); - assertThat(bindings.isPathVisible("firstname", User.class), is(true)); - assertThat(bindings.isPathVisible("address.street", User.class), is(true)); - assertThat(bindings.isPathVisible("lastname", User.class), is(false)); - assertThat(bindings.isPathVisible("address.city", User.class), is(false)); + assertThat(bindings.isPathAvailable("firstname", User.class), is(true)); + assertThat(bindings.isPathAvailable("address.street", User.class), is(true)); + assertThat(bindings.isPathAvailable("lastname", User.class), is(false)); + assertThat(bindings.isPathAvailable("address.city", User.class), is(false)); } /** @@ -264,10 +264,10 @@ public class QuerydslBindingsUnitTests { PropertyPath path = bindings.getPropertyPath("city", ClassTypeInformation.from(User.class)); assertThat(path, is(notNullValue())); - assertThat(bindings.isPathVisible("city", User.class), is(true)); + assertThat(bindings.isPathAvailable("city", User.class), is(true)); // Aliasing implicitly blacklists original path - assertThat(bindings.isPathVisible("address.city", User.class), is(false)); + assertThat(bindings.isPathAvailable("address.city", User.class), is(false)); } /** @@ -282,9 +282,9 @@ public class QuerydslBindingsUnitTests { PropertyPath path = bindings.getPropertyPath("city", ClassTypeInformation.from(User.class)); assertThat(path, is(notNullValue())); - assertThat(bindings.isPathVisible("city", User.class), is(true)); + assertThat(bindings.isPathAvailable("city", User.class), is(true)); - assertThat(bindings.isPathVisible("address.city", User.class), is(true)); + assertThat(bindings.isPathAvailable("address.city", User.class), is(true)); PropertyPath propertyPath = bindings.getPropertyPath("address.city", ClassTypeInformation.from(User.class)); assertThat(propertyPath, is(notNullValue())); diff --git a/src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java b/src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java index fcc1421b8..5b75d8b38 100644 --- a/src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2016 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. @@ -206,4 +206,17 @@ public class QuerydslPredicateBuilderUnitTests { assertThat(predicate, is((Predicate) QUser.user.dateOfBirth.eq(format.parseDateTime(date).toDate()))); } + + /** + * @see DATACMNS-883 + */ + @Test + public void automaticallyInsertsAnyStepInCollectionReference() { + + values.add("addresses.street", "VALUE"); + + Predicate predicate = builder.getPredicate(USER_TYPE, values, DEFAULT_BINDINGS); + + assertThat(predicate, is((Predicate) QUser.user.addresses.any().street.eq("VALUE"))); + } }