From 0330cdc3d39a5221816417ba41a8e415f20def7e Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 17 Jul 2015 19:41:45 +0200 Subject: [PATCH] DATACMNS-669 - Final polishing. Mostly Javadoc. Removed all API that took String paths so far. Changed return type of bean definition for QuerydslPredicateArgumentResolver to HandlerMethodArgumentResolver so that Querydsl stays optional during class loading. Original pull request: #132. --- .../config/SpringDataWebConfiguration.java | 9 +- .../data/web/querydsl/MultiValueBinding.java | 4 +- .../data/web/querydsl/QuerydslBindings.java | 120 +++++++------- .../data/web/querydsl/QuerydslPredicate.java | 13 +- .../QuerydslPredicateArgumentResolver.java | 153 ++++++++++++++---- .../querydsl/QuerydslPredicateBuilder.java | 78 ++++++--- .../data/web/querydsl/SingleValueBinding.java | 3 +- .../querydsl/QuerydslBindingsUnitTests.java | 38 +++-- ...dslPredicateArgumentResolverUnitTests.java | 73 +++++++-- .../QuerydslPredicateBuilderUnitTests.java | 19 +-- 10 files changed, 340 insertions(+), 170 deletions(-) diff --git a/src/main/java/org/springframework/data/web/config/SpringDataWebConfiguration.java b/src/main/java/org/springframework/data/web/config/SpringDataWebConfiguration.java index 2d3c6fb3b..f3bc93c1c 100644 --- a/src/main/java/org/springframework/data/web/config/SpringDataWebConfiguration.java +++ b/src/main/java/org/springframework/data/web/config/SpringDataWebConfiguration.java @@ -38,6 +38,8 @@ import org.springframework.format.support.FormattingConversionService; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; +import com.mysema.query.types.Predicate; + /** * Configuration class to register {@link PageableHandlerMethodArgumentResolver}, * {@link SortHandlerMethodArgumentResolver} and {@link DomainClassConverter}. @@ -70,13 +72,14 @@ public class SpringDataWebConfiguration extends WebMvcConfigurerAdapter { } /** - * Default QuerydslPredicateArgumentResolver. + * Default {@link QuerydslPredicateArgumentResolver} to create Querydsl {@link Predicate} instances for Spring MVC + * controller methods. * * @return */ - @Bean @Lazy - public QuerydslPredicateArgumentResolver querydslPredicateArgumentResolver() { + @Bean + public HandlerMethodArgumentResolver querydslPredicateArgumentResolver() { return new QuerydslPredicateArgumentResolver(conversionService.getObject()); } diff --git a/src/main/java/org/springframework/data/web/querydsl/MultiValueBinding.java b/src/main/java/org/springframework/data/web/querydsl/MultiValueBinding.java index 79f968623..87c0620ad 100644 --- a/src/main/java/org/springframework/data/web/querydsl/MultiValueBinding.java +++ b/src/main/java/org/springframework/data/web/querydsl/MultiValueBinding.java @@ -31,7 +31,9 @@ import com.mysema.query.types.Predicate; public interface MultiValueBinding, S> { /** - * Returns the predicate to be applied to the given {@link Path} for the given value. + * Returns the predicate to be applied to the given {@link Path} for the given collection value, which will contain + * all values submitted for the path bind. If a single value was provided only the collection will consist of exactly + * one element. * * @param path {@link Path} to the property. Will not be {@literal null}. * @param value the value that should be bound. Will not be {@literal null} or empty. diff --git a/src/main/java/org/springframework/data/web/querydsl/QuerydslBindings.java b/src/main/java/org/springframework/data/web/querydsl/QuerydslBindings.java index cf96bdc6b..9b708be37 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslBindings.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslBindings.java @@ -80,18 +80,14 @@ public class QuerydslBindings { return new PathBinder(paths); } - public final TypeBinder bind(Class type) { - return new TypeBinder(type); - } - /** - * Defines a binding for the given + * Returns a new {@link TypeBinder} for the given type. * - * @param paths + * @param type must not be {@literal null}. * @return */ - final PropertyBinder bind(String... paths) { - return new PropertyBinder(Arrays.asList(paths)); + public final TypeBinder bind(Class type) { + return new TypeBinder(type); } /** @@ -109,16 +105,6 @@ public class QuerydslBindings { } } - /** - * Exclude properties from binding. Exclusion of all properties of a nested type can be done by exclusion on a higher - * level. E.g. {@code address} would exclude both {@code address.city} and {@code address.street}. - * - * @param properties - */ - final void excluding(String... properties) { - this.blackList.addAll(Arrays.asList(properties)); - } - /** * Include properties for binding. Include the property considered a binding candidate. * @@ -133,18 +119,6 @@ public class QuerydslBindings { } } - /** - * Include properties for binding. Include the property considered a binding candidate. - * - * @param properties - */ - final void including(String... properties) { - - Assert.notEmpty(properties, "At least one property has to be provided!"); - - this.whiteList.addAll(Arrays.asList(properties)); - } - /** * Returns whether to exclude all properties for which no explicit binding has been defined or it has been explicitly * white-listed. This defaults to {@literal false} which means that for properties without an explicitly defined @@ -281,19 +255,25 @@ public class QuerydslBindings { } /** - * Defines the given {@link SingleValueBinding} to be used for the paths, + * Defines the given {@link SingleValueBinding} to be used for the paths. * * @param binding must not be {@literal null}. * @return */ - public void single(SingleValueBinding binding) { + public void first(SingleValueBinding binding) { Assert.notNull(binding, "Binding must not be null!"); - multi(new MultiValueBindingAdapter(binding)); + all(new MultiValueBindingAdapter(binding)); } - public void multi(MultiValueBinding binding) { + /** + * Defines the given {@link MultiValueBinding} to be used for the paths. + * + * @param binding must not be {@literal null}. + * @return + */ + public void all(MultiValueBinding binding) { Assert.notNull(binding, "Binding must not be null!"); @@ -303,54 +283,53 @@ public class QuerydslBindings { } } + /** + * A binder for types. + * + * @author Oliver Gierke + */ public final class TypeBinder { private final Class type; - public TypeBinder(Class type) { + /** + * Creates a new {@link TypeBinder} for the given type. + * + * @param type must not be {@literal null}. + */ + private TypeBinder(Class type) { + + Assert.notNull(type, "Type must not be null!"); + this.type = type; } - public

> void single(SingleValueBinding binding) { + /** + * Configures the given {@link SingleValueBinding} to be used for the current type. + * + * @param binding must not be {@literal null}. + */ + public

> void first(SingleValueBinding binding) { Assert.notNull(binding, "Binding must not be null!"); - multi(new MultiValueBindingAdapter(binding)); + all(new MultiValueBindingAdapter(binding)); } - public

> void multi(MultiValueBinding binding) { + /** + * Configures the given {@link MultiValueBinding} to be used for the current type. + * + * @param binding must not be {@literal null}. + */ + public

> void all(MultiValueBinding binding) { Assert.notNull(binding, "Binding must not be null!"); QuerydslBindings.this.typeSpecs.put(type, new PathAndBinding(null, binding)); } } - public final class PropertyBinder { - - private final List paths; - - private PropertyBinder(List paths) { - this.paths = paths; - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - public void using(SingleValueBinding binding) { - - Assert.notNull(binding, "Binding must not be null!"); - using(new MultiValueBindingAdapter(binding)); - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - public void using(MultiValueBinding binding) { - - Assert.notNull(binding, "Binding must not be null!"); - - for (String path : paths) { - QuerydslBindings.this.pathSpecs.put(path, new PathAndBinding(null, binding)); - } - } - } - /** + * A pair of a {@link Path} and the registered {@link MultiValueBinding}. + * * @author Christoph Strobl * @since 1.11 */ @@ -359,6 +338,12 @@ public class QuerydslBindings { private final Path path; private final MultiValueBinding binding; + /** + * Creates a new {@link PathAndBinding} for the given {@link Path} and {@link MultiValueBinding}. + * + * @param path must not be {@literal null}. + * @param binding must not be {@literal null}. + */ public PathAndBinding(S path, MultiValueBinding binding) { this.path = path; @@ -375,6 +360,9 @@ public class QuerydslBindings { } /** + * {@link MultiValueBinding} that forwards the first value of the collection values to the delegate + * {@link SingleValueBinding}. + * * @author Oliver Gierke */ static class MultiValueBindingAdapter, S> implements MultiValueBinding { @@ -382,7 +370,9 @@ public class QuerydslBindings { private final SingleValueBinding delegate; /** - * @param delegate + * Creates a new {@link MultiValueBindingAdapter} for the given {@link SingleValueBinding}. + * + * @param delegate must not be {@literal null}. */ public MultiValueBindingAdapter(SingleValueBinding delegate) { this.delegate = delegate; diff --git a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicate.java b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicate.java index 4087e2d98..40c0b69bd 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicate.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicate.java @@ -21,10 +21,11 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** - * {@link java.lang.annotation.Annotation} to specify aspects of {@link com.mysema.query.types.Predicate} used in Spring - * MVC {@link org.springframework.web.servlet.mvc.Controller}. + * Annotation to customize the binding of HTTP request parameters to a Querydsl {@link com.mysema.query.types.Predicate} + * in Spring MVC handler methods. * * @author Christoph Strobl + * @author Oliver Gierke * @since 1.11 */ @Target({ ElementType.PARAMETER, ElementType.TYPE }) @@ -32,14 +33,18 @@ import java.lang.annotation.Target; public @interface QuerydslPredicate { /** - * Root type to be used for {@link com.mysema.query.types.Path} + * The root type to create the {@link com.mysema.query.types.Predicate}. Specify this explictly if the type is not + * contained in the controller method's return type. * * @return */ Classroot() default Object.class; /** - * Configuration class providing options on a per field base. + * To customize the way individual properties' values should be bound to the predicate a + * {@link QuerydslBinderCustomizer} can be specified here. We'll try to obtain a Spring bean of this type but fall + * back to a plain instantiation if no bean is found in the current + * {@link org.springframework.beans.factory.BeanFactory}. * * @return */ diff --git a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java index d41c083ed..62c146693 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java @@ -16,6 +16,7 @@ package org.springframework.data.web.querydsl; import java.util.Arrays; +import java.util.Map; import java.util.Map.Entry; import org.springframework.beans.BeanUtils; @@ -27,10 +28,12 @@ import org.springframework.context.ApplicationContextAware; import org.springframework.core.MethodParameter; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.data.querydsl.EntityPathResolver; import org.springframework.data.querydsl.SimpleEntityPathResolver; import org.springframework.data.repository.support.Repositories; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; +import org.springframework.util.ConcurrentReferenceHashMap; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.bind.support.WebDataBinderFactory; @@ -51,10 +54,12 @@ import com.mysema.query.types.Predicate; */ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentResolver, ApplicationContextAware { + private static final String INVALID_DOMAIN_TYPE = "Unable to find Querydsl root type for detected domain type %s! User @%s's root attribute to define the domain type manually!"; private final QuerydslPredicateBuilder predicateBuilder; + private final EntityPathResolver resolver; + private final Map, EntityPath> entityPaths; private AutowireCapableBeanFactory beanFactory; - private Repositories repositories; /** @@ -63,8 +68,11 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR * @param conversionService defaults to {@link DefaultConversionService} if {@literal null}. */ public QuerydslPredicateArgumentResolver(ConversionService conversionService) { + + this.resolver = SimpleEntityPathResolver.INSTANCE; + this.entityPaths = new ConcurrentReferenceHashMap, EntityPath>(); this.predicateBuilder = new QuerydslPredicateBuilder( - conversionService == null ? new DefaultConversionService() : conversionService); + conversionService == null ? new DefaultConversionService() : conversionService, resolver); } /* @@ -114,53 +122,61 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR QuerydslPredicate annotation = parameter.getParameterAnnotation(QuerydslPredicate.class); TypeInformation domainType = extractTypeInfo(parameter).getActualType(); - return predicateBuilder.getPredicate(parameters, createBindings(annotation, domainType.getType()), domainType); + return predicateBuilder.getPredicate(domainType, parameters, createBindings(annotation, domainType)); } - private TypeInformation extractTypeInfo(MethodParameter parameter) { + QuerydslBindings createBindings(QuerydslPredicate annotation, TypeInformation domainType) { - QuerydslPredicate annotation = parameter.getParameterAnnotation(QuerydslPredicate.class); - - if (annotation == null || Object.class.equals(annotation.root())) { - return ClassTypeInformation.fromReturnTypeOf(parameter.getMethod()); - } - - return ClassTypeInformation.from(annotation.root()); - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - public QuerydslBindings createBindings(QuerydslPredicate annotation, Class domainType) { - - EntityPath path = SimpleEntityPathResolver.INSTANCE.createPath(domainType); - - QuerydslBinderCustomizer customizer = findCustomizerForDomainType(annotation, domainType); + EntityPath path = verifyEntityPathPresent(domainType); QuerydslBindings bindings = new QuerydslBindings(); - if (customizer != null) { - customizer.customize(bindings, path); - } + findCustomizerForDomainType(annotation, domainType.getType()).customize(bindings, path); + return bindings; } - @SuppressWarnings("unchecked") + /** + * Obtains the {@link QuerydslBinderCustomizer} for the given domain type. Will inspect the given annotation for a + * dedicatedly configured one or consider the domain types's repository. + * + * @param annotation + * @param domainType + * @return + */ + @SuppressWarnings({ "unchecked", "rawtypes" }) private QuerydslBinderCustomizer> findCustomizerForDomainType(QuerydslPredicate annotation, Class domainType) { - if (annotation == null || (annotation != null && annotation.bindings().equals(QuerydslBinderCustomizer.class))) { - if (repositories != null && repositories.hasRepositoryFor(domainType)) { + if (annotation != null) { - Object repository = this.repositories.getRepositoryFor(domainType); - if (repository instanceof QuerydslBinderCustomizer) { - return (QuerydslBinderCustomizer>) repository; - } + Class bindings = annotation.bindings(); + + if (bindings != QuerydslBinderCustomizer.class) { + return createQuerydslBinderCustomizer(bindings); } - - return null; } - return createQuerydslBinderCustomizer(annotation.bindings()); + if (repositories != null && repositories.hasRepositoryFor(domainType)) { + + Object repository = repositories.getRepositoryFor(domainType); + + if (repository instanceof QuerydslBinderCustomizer) { + return (QuerydslBinderCustomizer>) repository; + } + } + + return NoOpCustomizer.INSTANCE; } + /** + * Obtains a {@link QuerydslBinderCustomizer} for the given type. Will try to obtain a bean from the + * {@link org.springframework.beans.factory.BeanFactory} first or fall back to create a fresh instance through the + * {@link org.springframework.beans.factory.BeanFactory} or finally falling back to a plain instantiation if no + * {@link org.springframework.beans.factory.BeanFactory} is present. + * + * @param type must not be {@literal null}. + * @return + */ @SuppressWarnings({ "unchecked", "rawtypes" }) private QuerydslBinderCustomizer> createQuerydslBinderCustomizer( Class type) { @@ -172,9 +188,80 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR try { return beanFactory.getBean(type); } catch (NoSuchBeanDefinitionException e) { + return beanFactory.createBean(type); + } + } + /** + * Tries to detect a Querydsl query type for the given domain type candidate via the configured + * {@link EntityPathResolver}. + * + * @param candidate must not be {@literal null}. + * @throws IllegalStateException to indicate the query type can't be found and manual configuration is necessary. + */ + private EntityPath verifyEntityPathPresent(TypeInformation candidate) { + + EntityPath path = entityPaths.get(candidate); + + if (path != null) { + return path; } - return beanFactory.createBean(type); + Class type = candidate.getType(); + + try { + path = resolver.createPath(type); + } catch (IllegalArgumentException o_O) { + throw new IllegalStateException( + String.format(INVALID_DOMAIN_TYPE, candidate.getType(), QuerydslPredicate.class.getSimpleName()), o_O); + } + + entityPaths.put(candidate, path); + return path; + } + + /** + * Obtains the domain type information from the given method parameter. Will favor an explicitly registered on through + * {@link QuerydslPredicate#root()} but use the actual type of the method's return type as fallback. + * + * @param parameter must not be {@literal null}. + * @return + */ + static TypeInformation extractTypeInfo(MethodParameter parameter) { + + QuerydslPredicate annotation = parameter.getParameterAnnotation(QuerydslPredicate.class); + + if (annotation != null && !Object.class.equals(annotation.root())) { + return ClassTypeInformation.from(annotation.root()); + } + + return detectDomainType(ClassTypeInformation.fromReturnTypeOf(parameter.getMethod())); + } + + private static TypeInformation detectDomainType(TypeInformation source) { + + if (source.getTypeArguments().isEmpty()) { + return source; + } + + TypeInformation actualType = source.getActualType(); + + if (source != actualType) { + return detectDomainType(actualType); + } + + if (source instanceof Iterable) { + return source; + } + + return detectDomainType(source.getComponentType()); + } + + private static enum NoOpCustomizer implements QuerydslBinderCustomizer> { + + INSTANCE; + + @Override + public void customize(QuerydslBindings bindings, EntityPath root) {} } } diff --git a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilder.java b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilder.java index c6c040547..8049c4d9b 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilder.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilder.java @@ -28,7 +28,7 @@ import org.springframework.beans.PropertyValues; import org.springframework.core.convert.ConversionService; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.mapping.PropertyReferenceException; -import org.springframework.data.querydsl.SimpleEntityPathResolver; +import org.springframework.data.querydsl.EntityPathResolver; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; import org.springframework.util.MultiValueMap; @@ -51,24 +51,29 @@ public class QuerydslPredicateBuilder { private final ConversionService conversionService; private final MultiValueBinding defaultBinding; private final Map> paths; + private final EntityPathResolver resolver; - public QuerydslPredicateBuilder(ConversionService conversionService) { + public QuerydslPredicateBuilder(ConversionService conversionService, EntityPathResolver resolver) { Assert.notNull(conversionService, "ConversionService must not be null!"); this.defaultBinding = new QuerydslDefaultBinding(); this.conversionService = conversionService; this.paths = new HashMap>(); + this.resolver = resolver; } /** - * @param values - * @param context + * Creates a Querydsl {@link Predicate} for the given values, {@link QuerydslBindings} on the given + * {@link TypeInformation}. + * + * @param type the type to create a predicate for. + * @param values the values to bind. + * @param bindings the {@link QuerydslBindings} for the predicate. * @return */ - - public Predicate getPredicate(MultiValueMap values, QuerydslBindings bindings, - TypeInformation type) { + public Predicate getPredicate(TypeInformation type, MultiValueMap values, + QuerydslBindings bindings) { Assert.notNull(bindings, "Context must not be null!"); @@ -92,10 +97,10 @@ public class QuerydslPredicateBuilder { Collection value = convertToPropertyPathSpecificType(entry.getValue(), propertyPath); - Predicate binding = invokeBinding(propertyPath, bindings, value); + Predicate predicate = invokeBinding(propertyPath, bindings, value); - if (binding != null) { - builder.and(binding); + if (predicate != null) { + builder.and(predicate); } } } catch (PropertyReferenceException o_O) { @@ -106,17 +111,34 @@ public class QuerydslPredicateBuilder { return builder.getValue(); } + /** + * Invokes the binding of the given values, for the given {@link PropertyPath} and {@link QuerydslBindings}. + * + * @param dotPath must not be {@literal null}. + * @param bindings must not be {@literal null}. + * @param values must not be {@literal null}. + * @return + */ @SuppressWarnings({ "unchecked", "rawtypes" }) - private Predicate invokeBinding(PropertyPath dotPath, QuerydslBindings bindings, Collection value) { + private Predicate invokeBinding(PropertyPath dotPath, QuerydslBindings bindings, Collection values) { Path path = getPath(dotPath, bindings); MultiValueBinding binding = bindings.getBindingForPath(dotPath); binding = binding == null ? defaultBinding : binding; - return binding.bind(path, value); + return binding.bind(path, values); } + /** + * Returns the {@link Path} for the given {@link PropertyPath} and {@link QuerydslBindings}. Will try to obtain the + * {@link Path} from the bindings first but fall back to reifying it from the PropertyPath in case no specific binding + * has been configured. + * + * @param path must not be {@literal null}. + * @param bindings must not be {@literal null}. + * @return + */ private Path getPath(PropertyPath path, QuerydslBindings bindings) { Path resolvedPath = bindings.getExistingPath(path); @@ -137,10 +159,16 @@ public class QuerydslPredicateBuilder { return resolvedPath; } - private static Path reifyPath(PropertyPath path, Path base) { + /** + * Tries to reify a Querydsl {@link Path} from the given {@link PropertyPath} and base. + * + * @param path must not be {@literal null}. + * @param base can be {@literal null}. + * @return + */ + private Path reifyPath(PropertyPath path, Path base) { - Path entityPath = base != null ? base - : SimpleEntityPathResolver.INSTANCE.createPath(path.getOwningType().getType()); + Path entityPath = base != null ? base : resolver.createPath(path.getOwningType().getType()); Field field = ReflectionUtils.findField(entityPath.getClass(), path.getSegment()); Object value = ReflectionUtils.getField(field, entityPath); @@ -152,28 +180,34 @@ public class QuerydslPredicateBuilder { return (Path) value; } + /** + * Converts the given source values into a collection of elements that are of the given {@link PropertyPath}'s type. + * Considers a single element list with an empty {@link String} an empty collection because this basically indicates + * the property having been submitted but no value provided. + * + * @param source must not be {@literal null}. + * @param path must not be {@literal null}. + * @return + */ private Collection convertToPropertyPathSpecificType(List source, PropertyPath path) { Class targetType = path.getLeafProperty().getType(); - if (isSingleElementCollectionWithoutText(source)) { + if (source.isEmpty() || isSingleElementCollectionWithoutText(source)) { return Collections.emptyList(); } Collection target = new ArrayList(source.size()); for (String value : source) { - target.add(potentiallyConvertValue(value, targetType)); + + target.add(conversionService.canConvert(value.getClass(), targetType) + ? conversionService.convert(value, targetType) : value); } return target; } - private Object potentiallyConvertValue(Object source, Class targetType) { - return conversionService.canConvert(source.getClass(), targetType) ? conversionService.convert(source, targetType) - : source; - } - /** * Returns whether the given collection has exactly one element that doesn't contain any text. This is basically an * indicator that a request parameter has been submitted but no value for it. diff --git a/src/main/java/org/springframework/data/web/querydsl/SingleValueBinding.java b/src/main/java/org/springframework/data/web/querydsl/SingleValueBinding.java index 1f2e8282c..c2c70ca59 100644 --- a/src/main/java/org/springframework/data/web/querydsl/SingleValueBinding.java +++ b/src/main/java/org/springframework/data/web/querydsl/SingleValueBinding.java @@ -30,7 +30,8 @@ import com.mysema.query.types.Predicate; public interface SingleValueBinding, S> { /** - * Returns the predicate to be applied to the given {@link Path} for the given value. + * Returns the predicate to be applied to the given {@link Path} for the given value. The given value will be the first + * the first one provided for the given path and converted into the expected type. * * @param path {@link Path} to the property. Will not be {@literal null}. * @param value the value that should be bound. Will not be {@literal null}. diff --git a/src/test/java/org/springframework/data/web/querydsl/QuerydslBindingsUnitTests.java b/src/test/java/org/springframework/data/web/querydsl/QuerydslBindingsUnitTests.java index 02f999451..a31ad1bcf 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslBindingsUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslBindingsUnitTests.java @@ -23,6 +23,7 @@ import org.junit.Test; import org.springframework.core.convert.support.DefaultConversionService; 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.test.util.ReflectionTestUtils; @@ -52,7 +53,7 @@ public class QuerydslBindingsUnitTests { @Before public void setUp() { - this.builder = new QuerydslPredicateBuilder(new DefaultConversionService()); + this.builder = new QuerydslPredicateBuilder(new DefaultConversionService(), SimpleEntityPathResolver.INSTANCE); this.bindings = new QuerydslBindings(); } @@ -78,7 +79,7 @@ public class QuerydslBindingsUnitTests { @Test public void returnsRegisteredBindingForSimplePath() { - bindings.bind("firstname").using(CONTAINS_BINDING); + bindings.bind(QUser.user.firstname).first(CONTAINS_BINDING); assertAdapterWithTargetBinding(bindings.getBindingForPath(PropertyPath.from("firstname", User.class)), CONTAINS_BINDING); @@ -90,7 +91,7 @@ public class QuerydslBindingsUnitTests { @Test public void getBindingForPathShouldReturnSpeficicBindingForNestedElementsWhenAvailable() { - bindings.bind("address.street").using(CONTAINS_BINDING); + bindings.bind(QUser.user.address.street).first(CONTAINS_BINDING); assertAdapterWithTargetBinding(bindings.getBindingForPath(PropertyPath.from("address.street", User.class)), CONTAINS_BINDING); @@ -102,7 +103,7 @@ public class QuerydslBindingsUnitTests { @Test public void getBindingForPathShouldReturnSpeficicBindingForTypes() { - bindings.bind(String.class).single(CONTAINS_BINDING); + bindings.bind(String.class).first(CONTAINS_BINDING); assertAdapterWithTargetBinding(bindings.getBindingForPath(PropertyPath.from("address.street", User.class)), CONTAINS_BINDING); @@ -114,7 +115,7 @@ public class QuerydslBindingsUnitTests { @Test public void propertyNotExplicitlyIncludedAndWithoutTypeBindingIsInvisible() { - bindings.bind(String.class).single(CONTAINS_BINDING); + bindings.bind(String.class).first(CONTAINS_BINDING); assertThat(bindings.getBindingForPath(PropertyPath.from("inceptionYear", User.class)), nullValue()); } @@ -125,7 +126,7 @@ public class QuerydslBindingsUnitTests { @Test public void pathIsVisibleIfTypeBasedBindingWasRegistered() { - bindings.bind(String.class).single(CONTAINS_BINDING); + bindings.bind(String.class).first(CONTAINS_BINDING); assertThat(bindings.isPathVisible(PropertyPath.from("inceptionYear", User.class)), is(true)); } @@ -136,7 +137,7 @@ public class QuerydslBindingsUnitTests { @Test public void explicitlyIncludedPathIsVisible() { - bindings.including("inceptionYear"); + bindings.including(QUser.user.inceptionYear); assertThat(bindings.isPathVisible(PropertyPath.from("inceptionYear", User.class)), is(true)); } @@ -147,7 +148,7 @@ public class QuerydslBindingsUnitTests { @Test public void notExplicitlyIncludedPathIsInvisible() { - bindings.including("inceptionYear"); + bindings.including(QUser.user.inceptionYear); assertThat(bindings.isPathVisible(PropertyPath.from("firstname", User.class)), is(false)); } @@ -158,7 +159,7 @@ public class QuerydslBindingsUnitTests { @Test public void excludedPathIsInvisible() { - bindings.excluding("inceptionYear"); + bindings.excluding(QUser.user.inceptionYear); assertThat(bindings.isPathVisible(PropertyPath.from("inceptionYear", User.class)), is(false)); } @@ -169,7 +170,7 @@ public class QuerydslBindingsUnitTests { @Test public void pathIsVisibleIfNotExplicitlyExcluded() { - bindings.excluding("inceptionYear"); + bindings.excluding(QUser.user.inceptionYear); assertThat(bindings.isPathVisible(PropertyPath.from("firstname", User.class)), is(true)); } @@ -180,8 +181,8 @@ public class QuerydslBindingsUnitTests { @Test public void pathIsVisibleIfItsBothBlackAndWhitelisted() { - bindings.excluding("firstname"); - bindings.including("firstname"); + bindings.excluding(QUser.user.firstname); + bindings.including(QUser.user.firstname); assertThat(bindings.isPathVisible(PropertyPath.from("firstname", User.class)), is(true)); } @@ -192,7 +193,7 @@ public class QuerydslBindingsUnitTests { @Test public void nestedPathIsInvisibleIfAParanetPathWasExcluded() { - bindings.excluding("address"); + bindings.excluding(QUser.user.address); assertThat(bindings.isPathVisible(PropertyPath.from("address.city", User.class)), is(false)); } @@ -203,8 +204,8 @@ public class QuerydslBindingsUnitTests { @Test public void pathIsVisibleIfConcretePathIsVisibleButParentExcluded() { - bindings.excluding("address"); - bindings.including("address.city"); + bindings.excluding(QUser.user.address); + bindings.including(QUser.user.address.city); assertThat(bindings.isPathVisible(PropertyPath.from("address.city", User.class)), is(true)); } @@ -215,12 +216,15 @@ public class QuerydslBindingsUnitTests { @Test public void isPathVisibleShouldReturnFalseWhenPartialPathContainedInExcludingAndConcretePathToDifferentPropertyIsIncluded() { - bindings.excluding("address"); - bindings.including("address.city"); + bindings.excluding(QUser.user.address); + bindings.including(QUser.user.address.city); assertThat(bindings.isPathVisible(PropertyPath.from("address.street", User.class)), is(false)); } + /** + * @see DATACMNS-669 + */ @Test public void testname() { 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 0acb1aac0..4ed672fce 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java @@ -18,11 +18,14 @@ package org.springframework.data.web.querydsl; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.mockito.Mockito.*; +import static org.springframework.data.web.querydsl.QuerydslPredicateArgumentResolver.*; import java.util.Collections; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.beans.factory.config.AutowireCapableBeanFactory; import org.springframework.core.MethodParameter; import org.springframework.data.domain.Page; @@ -30,13 +33,16 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.querydsl.QUser; import org.springframework.data.querydsl.User; -import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.support.Repositories; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; +import org.springframework.hateoas.Resource; +import org.springframework.http.HttpEntity; +import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.servlet.ModelAndView; import com.mysema.query.types.Path; import com.mysema.query.types.Predicate; @@ -51,6 +57,10 @@ import com.mysema.query.types.path.StringPath; */ public class QuerydslPredicateArgumentResolverUnitTests { + static final TypeInformation USER_TYPE = ClassTypeInformation.from(User.class); + + public @Rule ExpectedException exception = ExpectedException.none(); + QuerydslPredicateArgumentResolver resolver; MockHttpServletRequest request; @@ -202,7 +212,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { * @see DATACMNS-669 */ @Test - public void shouldExcludePorpertiesCorrectly() throws Exception { + public void shouldExcludePropertiesCorrectly() throws Exception { request.addParameter("address.street", "downhill"); request.addParameter("inceptionYear", "973"); @@ -234,7 +244,6 @@ public class QuerydslPredicateArgumentResolverUnitTests { public void createBindingsShouldHonorQuerydslBinderCustomizerHookWhenPresent() { Repositories repositories = mock(Repositories.class); - RepositoryInformation repoInfo = mock(RepositoryInformation.class); when(repositories.hasRepositoryFor(User.class)).thenReturn(true); when(repositories.getRepositoryFor(User.class)).thenReturn(new SampleRepo()); @@ -242,9 +251,9 @@ public class QuerydslPredicateArgumentResolverUnitTests { resolver = new QuerydslPredicateArgumentResolver(null); ReflectionTestUtils.setField(resolver, "repositories", repositories); - QuerydslBindings bindings = resolver.createBindings(null, User.class); - MultiValueBinding, Object> binding = bindings.getBindingForPath(PropertyPath.from("firstname", - User.class)); + QuerydslBindings bindings = resolver.createBindings(null, USER_TYPE); + MultiValueBinding, Object> binding = bindings + .getBindingForPath(PropertyPath.from("firstname", User.class)); assertThat(binding.bind((Path) QUser.user.firstname, Collections.singleton("rand")), is((Predicate) QUser.user.firstname.contains("rand"))); @@ -259,20 +268,48 @@ public class QuerydslPredicateArgumentResolverUnitTests { AutowireCapableBeanFactory beanFactory = mock(AutowireCapableBeanFactory.class); when(beanFactory.getBean(SpecificBinding.class)).thenReturn(new SpecificBinding()); - QuerydslPredicate annotation = getMethodParameterFor("specificFind", Predicate.class).getParameterAnnotation( - QuerydslPredicate.class); + QuerydslPredicate annotation = getMethodParameterFor("specificFind", Predicate.class) + .getParameterAnnotation(QuerydslPredicate.class); resolver = new QuerydslPredicateArgumentResolver(null); ReflectionTestUtils.setField(resolver, "beanFactory", beanFactory); - QuerydslBindings bindings = resolver.createBindings(annotation, User.class); - MultiValueBinding, Object> binding = bindings.getBindingForPath(PropertyPath.from("firstname", - User.class)); + QuerydslBindings bindings = resolver.createBindings(annotation, USER_TYPE); + MultiValueBinding, Object> binding = bindings + .getBindingForPath(PropertyPath.from("firstname", User.class)); assertThat(binding.bind((Path) QUser.user.firstname, Collections.singleton("rand")), is((Predicate) QUser.user.firstname.eq("RAND"))); } + /** + * @see DATACMNS-669 + */ + @Test + @SuppressWarnings("rawtypes") + public void detectsDomainTypesCorrectly() { + + TypeInformation USER_TYPE = ClassTypeInformation.from(User.class); + TypeInformation MODELA_AND_VIEW_TYPE = ClassTypeInformation.from(ModelAndView.class); + + assertThat(extractTypeInfo(getMethodParameterFor("forEntity")), is(USER_TYPE)); + assertThat(extractTypeInfo(getMethodParameterFor("forResourceOfUser")), is(USER_TYPE)); + assertThat(extractTypeInfo(getMethodParameterFor("forModelAndView")), is(MODELA_AND_VIEW_TYPE)); + } + + /** + * @see DATACMNS-669 + */ + @Test + public void rejectsPredicateResolutionIfDomainTypeCantBeAutoDetected() { + + exception.expect(IllegalStateException.class); + exception.expectMessage(QuerydslPredicate.class.getSimpleName()); + exception.expectMessage("root"); + + resolver.createBindings(null, ClassTypeInformation.from(ModelAndView.class)); + } + private static MethodParameter getMethodParameterFor(String methodName, Class... args) throws RuntimeException { try { @@ -286,7 +323,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { public void customize(QuerydslBindings bindings, QUser user) { - bindings.bind("firstname").using(new SingleValueBinding() { + bindings.bind(user.firstname).first(new SingleValueBinding() { @Override public Predicate bind(StringPath path, String value) { @@ -294,7 +331,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { } }); - bindings.bind(user.lastname).single(new SingleValueBinding() { + bindings.bind(user.lastname).first(new SingleValueBinding() { @Override public Predicate bind(StringPath path, String value) { @@ -302,7 +339,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { } }); - bindings.excluding("address"); + bindings.excluding(user.address); } } @@ -319,6 +356,12 @@ public class QuerydslPredicateArgumentResolverUnitTests { Page pagedFind(@QuerydslPredicate Predicate predicate, Pageable page); User specificFind(@QuerydslPredicate(bindings = SpecificBinding.class) Predicate predicate); + + HttpEntity forEntity(); + + ModelAndView forModelAndView(); + + ResponseEntity> forResourceOfUser(); } public static class SampleRepo implements QuerydslBinderCustomizer { @@ -326,7 +369,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { @Override public void customize(QuerydslBindings bindings, QUser user) { - bindings.bind(QUser.user.firstname).single(new SingleValueBinding() { + bindings.bind(QUser.user.firstname).first(new SingleValueBinding() { @Override public Predicate bind(StringPath path, String value) { diff --git a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilderUnitTests.java b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilderUnitTests.java index e95a9c03d..7ab82bd89 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilderUnitTests.java @@ -25,6 +25,7 @@ import org.junit.Before; import org.junit.Test; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.querydsl.QUser; +import org.springframework.data.querydsl.SimpleEntityPathResolver; import org.springframework.data.querydsl.User; import org.springframework.data.querydsl.Users; import org.springframework.data.util.ClassTypeInformation; @@ -51,7 +52,7 @@ public class QuerydslPredicateBuilderUnitTests { @Before public void setUp() { - this.builder = new QuerydslPredicateBuilder(new DefaultConversionService()); + this.builder = new QuerydslPredicateBuilder(new DefaultConversionService(), SimpleEntityPathResolver.INSTANCE); this.values = new LinkedMultiValueMap(); } @@ -60,7 +61,7 @@ public class QuerydslPredicateBuilderUnitTests { */ @Test(expected = IllegalArgumentException.class) public void rejectsNullConversionService() { - new QuerydslPredicateBuilder(null); + new QuerydslPredicateBuilder(null, SimpleEntityPathResolver.INSTANCE); } /** @@ -68,7 +69,7 @@ public class QuerydslPredicateBuilderUnitTests { */ @Test(expected = IllegalArgumentException.class) public void getPredicateShouldThrowErrorWhenBindingContextIsNull() { - builder.getPredicate(values, null, null); + builder.getPredicate(null, values, null); } /** @@ -77,7 +78,7 @@ public class QuerydslPredicateBuilderUnitTests { @Test public void getPredicateShouldReturnEmptyPredicateWhenPropertiesAreEmpty() { - assertThat(builder.getPredicate(values, DEFAULT_BINDINGS, ClassTypeInformation.OBJECT), is(nullValue())); + assertThat(builder.getPredicate(ClassTypeInformation.OBJECT, values, DEFAULT_BINDINGS), is(nullValue())); } /** @@ -88,7 +89,7 @@ public class QuerydslPredicateBuilderUnitTests { values.add("firstname", "Oliver"); - Predicate predicate = builder.getPredicate(values, DEFAULT_BINDINGS, USER_TYPE); + Predicate predicate = builder.getPredicate(USER_TYPE, values, DEFAULT_BINDINGS); assertThat(predicate, is((Predicate) QUser.user.firstname.eq("Oliver"))); @@ -106,7 +107,7 @@ public class QuerydslPredicateBuilderUnitTests { values.add("address.city", "Linz"); - Predicate predicate = builder.getPredicate(values, DEFAULT_BINDINGS, USER_TYPE); + Predicate predicate = builder.getPredicate(USER_TYPE, values, DEFAULT_BINDINGS); assertThat(predicate, is((Predicate) QUser.user.address.city.eq("Linz"))); @@ -125,7 +126,7 @@ public class QuerydslPredicateBuilderUnitTests { values.add("firstname", "rand"); values.add("lastname".toUpperCase(), "al'thor"); - Predicate predicate = builder.getPredicate(values, new QuerydslBindings(), USER_TYPE); + Predicate predicate = builder.getPredicate(USER_TYPE, values, DEFAULT_BINDINGS); assertThat(predicate, is((Predicate) QUser.user.firstname.eq("rand"))); } @@ -139,7 +140,7 @@ public class QuerydslPredicateBuilderUnitTests { values.add("lastname", null); QuerydslBindings bindings = new QuerydslBindings(); - bindings.bind(QUser.user.lastname).single(new SingleValueBinding() { + bindings.bind(QUser.user.lastname).first(new SingleValueBinding() { @Override public Predicate bind(StringPath path, String value) { @@ -147,6 +148,6 @@ public class QuerydslPredicateBuilderUnitTests { } }); - builder.getPredicate(values, bindings, USER_TYPE); + builder.getPredicate(USER_TYPE, values, bindings); } }