From 3ed2f44e60d47adf2ef51f6008c56aac4fbac6bb Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 23 Nov 2015 13:07:19 +0100 Subject: [PATCH] DATACMNS-787 - QuerydsBindings now allows aliasing of paths. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a single path is now handed to QuerydslBindings.bind(…), a specialized AliasingPathBinder is returned which allows defining an alias the binding will be exposed under. By default that alias will cause the original binding being black-listed and thus become unavailable for binding. This can be overridden by explicitly white-listing the properties to be bindable. --- .../querydsl/binding/QuerydslBindings.java | 226 ++++++++++++++---- .../binding/QuerydslPredicateBuilder.java | 24 +- .../binding/QuerydslBindingsUnitTests.java | 106 ++++++-- ...dslPredicateArgumentResolverUnitTests.java | 2 +- 4 files changed, 279 insertions(+), 79 deletions(-) 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 7b0f4456b..6cbafafb4 100644 --- a/src/main/java/org/springframework/data/querydsl/binding/QuerydslBindings.java +++ b/src/main/java/org/springframework/data/querydsl/binding/QuerydslBindings.java @@ -25,6 +25,9 @@ import java.util.Map; import java.util.Set; import org.springframework.data.mapping.PropertyPath; +import org.springframework.data.mapping.PropertyReferenceException; +import org.springframework.data.util.ClassTypeInformation; +import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -33,29 +36,37 @@ import com.querydsl.core.types.PathMetadata; import com.querydsl.core.types.Predicate; /** - * {@link QuerydslBindings} allows definition of path specific {@link SingleValueBinding}. + * {@link QuerydslBindings} allows definition of path specific bindings. * * 
  * 
  * new QuerydslBindings() {
  *   {
- *     bind(QUser.user.address.city).using((path, value) -> path.like(value.toString()));
- *     bind(String.class).using((path, value) -> path.like(value.toString()));
+ *     bind(QUser.user.address.city).first((path, value) -> path.like(value.toString()));
+ *     bind(String.class).first((path, value) -> path.like(value.toString()));
  *   }
  * }
  * 
  *
* + * The bindings can either handle a single - see {@link PathBinder#first(SingleValueBinding)} - (the first in case + * multiple ones are supplied) or multiple - see {@link PathBinder#all(MultiValueBinding)} - value binding. If exactly + * one path is deployed, an {@link AliasingPathBinder} is returned which - as the name suggests - allows aliasing of + * paths, i.e. exposing the path under a different name. + *

+ * {@link QuerydslBindings} are usually manipulated using a {@link QuerydslBinderCustomizer}, either implemented + * directly or using a default method on a Spring Data repository. + * * @author Christoph Strobl * @author Oliver Gierke * @since 1.11 + * @see QuerydslBinderCustomizer */ public class QuerydslBindings { private final Map> pathSpecs; private final Map, PathAndBinding> typeSpecs; - private final Set whiteList; - private final Set blackList; + private final Set whiteList, blackList, aliases; private boolean excludeUnlistedProperties; @@ -68,6 +79,18 @@ public class QuerydslBindings { this.typeSpecs = new LinkedHashMap, PathAndBinding>(); this.whiteList = new HashSet(); this.blackList = new HashSet(); + this.aliases = new HashSet(); + + } + + /** + * Returns an {@link AliasingPathBinder} for the given {@link Path} to define bindings for them. + * + * @param path must not be {@literal null}. + * @return + */ + public final , S> AliasingPathBinder bind(T path) { + return new AliasingPathBinder(path); } /** @@ -135,30 +158,8 @@ public class QuerydslBindings { return this; } - /** - * Checks if a given {@link PropertyPath} should be visible for binding values. - * - * @param path - * @return - */ - boolean isPathVisible(PropertyPath path) { - - List segments = Arrays.asList(path.toDotPath().split("\\.")); - - for (int i = 1; i <= segments.size(); i++) { - - if (!isPathVisible(StringUtils.collectionToDelimitedString(segments.subList(0, i), "."))) { - - // check if full path is on whitelist if though partial one is not - if (!whiteList.isEmpty()) { - return whiteList.contains(path.toDotPath()); - } - - return false; - } - } - - return true; + public boolean isPathVisible(String path, Class type) { + return getPropertyPath(path, ClassTypeInformation.from(type)) != null; } /** @@ -166,17 +167,22 @@ public class QuerydslBindings { * specific path but falls back to the builder registered for a given type. * * @param path must not be {@literal null}. - * @return + * @return can be {@literal null}. */ @SuppressWarnings("unchecked") - public , T> MultiValueBinding getBindingForPath(PropertyPath path) { + public , T> MultiValueBinding getBindingForPath(PropertyPath path) { Assert.notNull(path, "PropertyPath must not be null!"); PathAndBinding pathAndBinding = (PathAndBinding) pathSpecs.get(path.toDotPath()); if (pathAndBinding != null) { - return pathAndBinding.getBinding(); + + MultiValueBinding binding = pathAndBinding.getBinding(); + + if (binding != null) { + return pathAndBinding.getBinding(); + } } pathAndBinding = (PathAndBinding) typeSpecs.get(path.getLeafProperty().getType()); @@ -197,24 +203,73 @@ public class QuerydslBindings { } /** - * Returns whether the given path is visible, which means either on the white list or not on the black list if no - * white list configured. + * @param path + * @param type + * @return + */ + PropertyPath getPropertyPath(String path, TypeInformation type) { + + if (!isPathVisible(path)) { + return null; + } + + if (pathSpecs.containsKey(path)) { + return PropertyPath.from(toDotPath(pathSpecs.get(path).getPath()), type); + } + + try { + PropertyPath propertyPath = PropertyPath.from(path, type); + return isPathVisible(propertyPath) ? propertyPath : null; + } catch (PropertyReferenceException o_O) { + return null; + } + } + + /** + * Checks if a given {@link PropertyPath} should be visible for binding values. + * + * @param path + * @return + */ + private boolean isPathVisible(PropertyPath path) { + + List segments = Arrays.asList(path.toDotPath().split("\\.")); + + for (int i = 1; i <= segments.size(); i++) { + + if (!isPathVisible(StringUtils.collectionToDelimitedString(segments.subList(0, i), "."))) { + + // check if full path is on whitelist although the partial one is not + if (!whiteList.isEmpty()) { + return whiteList.contains(path.toDotPath()); + } + + return false; + } + } + + return true; + } + + /** + * Returns whether the given path is visible, which means either an alias and not explicitly blacklisted, explicitly + * white listed or not on the black list if no white list configured. * * @param path must not be {@literal null}. * @return */ private boolean isPathVisible(String path) { - if (!whiteList.isEmpty()) { - - if (whiteList.contains(path)) { - return true; - } - - return false; + // Aliases are visible if not explicitly blacklisted + if (aliases.contains(path) && !blackList.contains(path)) { + return true; } - return excludeUnlistedProperties ? false : !blackList.contains(path); + if (whiteList.isEmpty()) { + return excludeUnlistedProperties ? false : !blackList.contains(path); + } + + return whiteList.contains(path); } /** @@ -223,7 +278,7 @@ public class QuerydslBindings { * @param path can be {@literal null}. * @return */ - private String toDotPath(Path path) { + private static String toDotPath(Path path) { if (path == null) { return ""; @@ -239,7 +294,7 @@ public class QuerydslBindings { * * @author Oliver Gierke */ - public final class PathBinder

, T> { + public class PathBinder

, T> { private final List

paths; @@ -248,7 +303,7 @@ public class QuerydslBindings { * * @param paths must not be {@literal null} or empty. */ - public PathBinder(P... paths) { + PathBinder(P... paths) { Assert.notEmpty(paths, "At least one path has to be provided!"); this.paths = Arrays.asList(paths); @@ -278,7 +333,86 @@ public class QuerydslBindings { Assert.notNull(binding, "Binding must not be null!"); for (P path : paths) { - QuerydslBindings.this.pathSpecs.put(toDotPath(path), new PathAndBinding(path, binding)); + registerBinding(new PathAndBinding(path, binding)); + } + } + + protected void registerBinding(PathAndBinding binding) { + QuerydslBindings.this.pathSpecs.put(toDotPath(binding.getPath()), binding); + } + } + + /** + * A special {@link PathBinder} that additionally registers the binding under a dedicated alias. The original path is + * still registered but blacklisted so that it becomes unavailable except it's explicitly whitelisted. + * + * @author Oliver Gierke + */ + public class AliasingPathBinder

, T> extends PathBinder { + + private final String alias; + private final P path; + + /** + * Creates a new {@link AliasingPathBinder} for the given {@link Path}. + * + * @param paths must not be {@literal null}. + */ + AliasingPathBinder(P path) { + this(null, path); + } + + /** + * Creates a new {@link AliasingPathBinder} using the given alias and {@link Path}. + * + * @param alias can be {@literal null}. + * @param path must not be {@literal null}. + */ + @SuppressWarnings("unchecked") + private AliasingPathBinder(String alias, P path) { + + super(path); + + Assert.notNull(path, "Path must not be null!"); + + this.alias = alias; + this.path = path; + } + + /** + * Aliases the current binding to be available under the given path. By default, the binding path will be + * blacklisted so that aliasing effectively hides the original path. If you want to keep the original path around, + * include it in an explicit whitelist. + * + * @param alias must not be {@literal null}. + * @return will never be {@literal null}. + */ + public AliasingPathBinder as(String alias) { + + Assert.hasText(alias, "Alias must not be null or empty!"); + return new AliasingPathBinder(alias, path); + } + + /** + * Registers the current aliased binding to use the default binding. + */ + public void withDefaultBinding() { + registerBinding(new PathAndBinding(path, null)); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.querydsl.binding.QuerydslBindings.PathBinder#registerBinding(org.springframework.data.querydsl.binding.QuerydslBindings.PathAndBinding) + */ + @Override + protected void registerBinding(PathAndBinding binding) { + + super.registerBinding(binding); + + if (alias != null) { + QuerydslBindings.this.pathSpecs.put(alias, binding); + QuerydslBindings.this.aliases.add(alias); + QuerydslBindings.this.blackList.add(toDotPath(binding.getPath())); } } } 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 a41af02e7..c2f78540d 100644 --- a/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java +++ b/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java @@ -31,7 +31,6 @@ import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.Property; import org.springframework.core.convert.TypeDescriptor; import org.springframework.data.mapping.PropertyPath; -import org.springframework.data.mapping.PropertyReferenceException; import org.springframework.data.querydsl.EntityPathResolver; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; @@ -100,22 +99,23 @@ public class QuerydslPredicateBuilder { continue; } - try { + String path = entry.getKey(); - PropertyPath propertyPath = PropertyPath.from(entry.getKey(), type); + if (!bindings.isPathVisible(path, type.getType())) { + continue; + } - if (bindings.isPathVisible(propertyPath)) { + PropertyPath propertyPath = bindings.getPropertyPath(path, type); - Collection value = convertToPropertyPathSpecificType(entry.getValue(), propertyPath); + if (propertyPath == null) { + continue; + } - Predicate predicate = invokeBinding(propertyPath, bindings, value); + Collection value = convertToPropertyPathSpecificType(entry.getValue(), propertyPath); + Predicate predicate = invokeBinding(propertyPath, bindings, value); - if (predicate != null) { - builder.and(predicate); - } - } - } catch (PropertyReferenceException o_O) { - // not a property of the domain object, continue + if (predicate != null) { + builder.and(predicate); } } 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 d4f8a2cee..749bc0b43 100644 --- a/src/test/java/org/springframework/data/querydsl/binding/QuerydslBindingsUnitTests.java +++ b/src/test/java/org/springframework/data/querydsl/binding/QuerydslBindingsUnitTests.java @@ -25,6 +25,7 @@ import org.springframework.data.mapping.PropertyPath; import org.springframework.data.querydsl.QUser; import org.springframework.data.querydsl.SimpleEntityPathResolver; import org.springframework.data.querydsl.User; +import org.springframework.data.util.ClassTypeInformation; import org.springframework.test.util.ReflectionTestUtils; import com.querydsl.core.types.Path; @@ -128,7 +129,7 @@ public class QuerydslBindingsUnitTests { bindings.bind(String.class).first(CONTAINS_BINDING); - assertThat(bindings.isPathVisible(PropertyPath.from("inceptionYear", User.class)), is(true)); + assertThat(bindings.isPathVisible("inceptionYear", User.class), is(true)); } /** @@ -139,7 +140,7 @@ public class QuerydslBindingsUnitTests { bindings.including(QUser.user.inceptionYear); - assertThat(bindings.isPathVisible(PropertyPath.from("inceptionYear", User.class)), is(true)); + assertThat(bindings.isPathVisible("inceptionYear", User.class), is(true)); } /** @@ -150,7 +151,7 @@ public class QuerydslBindingsUnitTests { bindings.including(QUser.user.inceptionYear); - assertThat(bindings.isPathVisible(PropertyPath.from("firstname", User.class)), is(false)); + assertThat(bindings.isPathVisible("firstname", User.class), is(false)); } /** @@ -161,7 +162,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.inceptionYear); - assertThat(bindings.isPathVisible(PropertyPath.from("inceptionYear", User.class)), is(false)); + assertThat(bindings.isPathVisible("inceptionYear", User.class), is(false)); } /** @@ -172,7 +173,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.inceptionYear); - assertThat(bindings.isPathVisible(PropertyPath.from("firstname", User.class)), is(true)); + assertThat(bindings.isPathVisible("firstname", User.class), is(true)); } /** @@ -184,7 +185,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.firstname); bindings.including(QUser.user.firstname); - assertThat(bindings.isPathVisible(PropertyPath.from("firstname", User.class)), is(true)); + assertThat(bindings.isPathVisible("firstname", User.class), is(true)); } /** @@ -195,7 +196,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.address); - assertThat(bindings.isPathVisible(PropertyPath.from("address.city", User.class)), is(false)); + assertThat(bindings.isPathVisible("address.city", User.class), is(false)); } /** @@ -207,7 +208,7 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.address); bindings.including(QUser.user.address.city); - assertThat(bindings.isPathVisible(PropertyPath.from("address.city", User.class)), is(true)); + assertThat(bindings.isPathVisible("address.city", User.class), is(true)); } /** @@ -219,29 +220,94 @@ public class QuerydslBindingsUnitTests { bindings.excluding(QUser.user.address); bindings.including(QUser.user.address.city); - assertThat(bindings.isPathVisible(PropertyPath.from("address.street", User.class)), is(false)); + assertThat(bindings.isPathVisible("address.street", User.class), is(false)); } /** * @see DATACMNS-669 */ @Test - public void testname() { - - PropertyPath firstname = PropertyPath.from("firstname", User.class); - PropertyPath lastname = PropertyPath.from("lastname", User.class); - PropertyPath city = PropertyPath.from("address.city", User.class); - PropertyPath street = PropertyPath.from("address.street", User.class); + public void whitelistsPropertiesCorrectly() { bindings.including(QUser.user.firstname, QUser.user.address.street); - assertThat(bindings.isPathVisible(firstname), is(true)); - assertThat(bindings.isPathVisible(street), is(true)); - assertThat(bindings.isPathVisible(lastname), is(false)); - assertThat(bindings.isPathVisible(city), is(false)); + 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)); } - private static

, S> void assertAdapterWithTargetBinding(MultiValueBinding binding, + /** + * @see DATACMNS-787 + */ + @Test(expected = IllegalArgumentException.class) + public void rejectsNullAlias() { + bindings.bind(QUser.user.address).as(null); + } + + /** + * @see DATACMNS-787 + */ + @Test(expected = IllegalArgumentException.class) + public void rejectsEmptyAlias() { + bindings.bind(QUser.user.address).as(""); + } + + /** + * @see DATACMNS-787 + */ + @Test + public void aliasesBinding() { + + bindings.bind(QUser.user.address.city).as("city").first(CONTAINS_BINDING); + + PropertyPath path = bindings.getPropertyPath("city", ClassTypeInformation.from(User.class)); + + assertThat(path, is(notNullValue())); + assertThat(bindings.isPathVisible("city", User.class), is(true)); + + // Aliasing implicitly blacklists original path + assertThat(bindings.isPathVisible("address.city", User.class), is(false)); + } + + /** + * @see DATACMNS-787 + */ + @Test + public void explicitlyIncludesOriginalBindingDespiteAlias() { + + bindings.including(QUser.user.address.city); + bindings.bind(QUser.user.address.city).as("city").first(CONTAINS_BINDING); + + PropertyPath path = bindings.getPropertyPath("city", ClassTypeInformation.from(User.class)); + + assertThat(path, is(notNullValue())); + assertThat(bindings.isPathVisible("city", User.class), is(true)); + + assertThat(bindings.isPathVisible("address.city", User.class), is(true)); + + PropertyPath propertyPath = bindings.getPropertyPath("address.city", ClassTypeInformation.from(User.class)); + assertThat(propertyPath, is(notNullValue())); + + assertAdapterWithTargetBinding(bindings.getBindingForPath(propertyPath), CONTAINS_BINDING); + } + + /** + * @see DATACMNS-787 + */ + @Test + public void registedAliasWithNullBinding() { + + bindings.bind(QUser.user.address.city).as("city").withDefaultBinding(); + + PropertyPath path = bindings.getPropertyPath("city", ClassTypeInformation.from(User.class)); + assertThat(path, is(notNullValue())); + + MultiValueBinding, Object> binding = bindings.getBindingForPath(path); + assertThat(binding, is(nullValue())); + } + + private static

, S> void assertAdapterWithTargetBinding(MultiValueBinding binding, SingleValueBinding, ?> expected) { assertThat(binding, is(instanceOf(QuerydslBindings.MultiValueBindingAdapter.class))); diff --git a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java index 2b22616a4..6deefbcd2 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java @@ -168,7 +168,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { * @see DATACMNS-669 */ @Test - public void resolveArgumentShouldHonorCustomSpeficifcation() throws Exception { + public void resolveArgumentShouldHonorCustomSpecification() throws Exception { request.addParameter("firstname", "egwene"); request.addParameter("lastname", "al'vere");