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.

Author: odrotbohm

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());
 	}
 }

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.StringUtils;
 
 import com.querydsl.core.types.Path;
-import com.querydsl.core.types.PathMetadata;
 import com.querydsl.core.types.Predicate;
 
 /**
@@ -158,8 +159,34 @@ public final QuerydslBindings excludeUnlistedProperties(boolean excludeUnlistedP
 		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 <S extends Path<? extends T>, T> MultiValueBinding<S, T> getBindingForPat
 	 */
 	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 @@ Path<?> getExistingPath(PropertyPath path) {
 	 */
 	PropertyPath getPropertyPath(String path, TypeInformation<?> type) {
 
+		Assert.notNull(path, "Path must not be null!");
+
 		if (!isPathVisible(path)) {
 			return null;
 		}
@@ -272,23 +303,6 @@ private boolean isPathVisible(String path) {
 		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.
 	 *

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 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 Predicate getPredicate(TypeInformation<?> type, MultiValueMap<String, Str
 
 			String path = entry.getKey();
 
-			if (!bindings.isPathVisible(path, type.getType())) {
+			if (!bindings.isPathAvailable(path, type)) {
 				continue;
 			}
 
@@ -179,6 +180,10 @@ private Path<?> getPath(PropertyPath path, QuerydslBindings bindings) {
 	 */
 	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());

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"));
+	}
+}

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<Address> addresses;
 	public List<String> nickNames;
 	public Long inceptionYear;
 

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

@@ -129,7 +129,7 @@ public void pathIsVisibleIfTypeBasedBindingWasRegistered() {
 
 		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 void explicitlyIncludedPathIsVisible() {
 
 		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 void notExplicitlyIncludedPathIsInvisible() {
 
 		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 void excludedPathIsInvisible() {
 
 		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 void pathIsVisibleIfNotExplicitlyExcluded() {
 
 		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 void pathIsVisibleIfItsBothBlackAndWhitelisted() {
 		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 void nestedPathIsInvisibleIfAParanetPathWasExcluded() {
 
 		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 void pathIsVisibleIfConcretePathIsVisibleButParentExcluded() {
 		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 void isPathVisibleShouldReturnFalseWhenPartialPathContainedInExcludingAnd
 		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 void whitelistsPropertiesCorrectly() {
 
 		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 void aliasesBinding() {
 		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 void explicitlyIncludesOriginalBindingDespiteAlias() {
 		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()));