From cccfa5e5c5eb775b59dd26029bfd7d02aeda3938 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 15 Jul 2015 16:13:10 +0200 Subject: [PATCH] DATACMNS-669 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Javadoc on MultiValueBinding. Hide QuerydslBindings.bind(…), ….include(…) and ….exclude(…) methods that take String arguments to reduce the probability of no query class being present. Refined the value handling for empty request values which are represented as String[] containing an empty String. We now massage those into an empty Collection for MultiValueBindings and a null value for SingleValueInvocations. We currently skip property values that match this pattern entirely but are prepared to allow a configuration flip switch to handle those scenarios in the future. We currently stick to skipping those values to prevent the bindings from having to deal with null values. We now also allow the bindings to return null to indicate they don't want to get applied in the resulting predicate at all. Switched to interface based customization to make it easier to use customizations with Spring Data REST. Added Querydsl-specific adapter of RepositoryInvoker to allow Spring Data REST to hook in the execution of the predicate obtained from request parameters. Original pull request: #132. --- .../QuerydslRepositoryInvokerAdapter.java | 160 ++++++++++++++++++ .../DefaultRepositoryInvokerFactory.java | 7 +- .../data/web/querydsl/MultiValueBinding.java | 12 ++ .../querydsl/QuerydslBinderCustomizer.java | 28 +++ .../data/web/querydsl/QuerydslBindings.java | 28 +-- .../web/querydsl/QuerydslDefaultBinding.java | 30 ++-- .../data/web/querydsl/QuerydslPredicate.java | 7 +- .../QuerydslPredicateArgumentResolver.java | 42 +++-- .../querydsl/QuerydslPredicateBuilder.java | 82 ++++----- .../data/web/querydsl/SingleValueBinding.java | 10 +- ...ydslRepositoryInvokerAdapterUnitTests.java | 121 +++++++++++++ .../QuerydslDefaultBindingUnitTests.java | 42 +++-- ...dslPredicateArgumentResolverUnitTests.java | 22 +-- .../QuerydslPredicateBuilderUnitTests.java | 51 ++++-- 14 files changed, 506 insertions(+), 136 deletions(-) create mode 100644 src/main/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapter.java create mode 100644 src/main/java/org/springframework/data/web/querydsl/QuerydslBinderCustomizer.java create mode 100644 src/test/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapterUnitTests.java diff --git a/src/main/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapter.java b/src/main/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapter.java new file mode 100644 index 000000000..af8aa070a --- /dev/null +++ b/src/main/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapter.java @@ -0,0 +1,160 @@ +/* + * Copyright 2015 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 java.io.Serializable; +import java.lang.reflect.Method; +import java.util.Map; + +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; +import org.springframework.data.repository.support.RepositoryInvoker; +import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; + +import com.mysema.query.types.Predicate; + +/** + * {@link RepositoryInvoker} that is aware of a {@link QueryDslPredicateExecutor} and {@link Predicate} to be executed + * for all flavors of {@code findAll(…)}. All other calls are forwarded to the configured delegate. + * + * @author Oliver Gierke + */ +public class QuerydslRepositoryInvokerAdapter implements RepositoryInvoker { + + private final RepositoryInvoker delegate; + private final QueryDslPredicateExecutor executor; + private final Predicate predicate; + + /** + * Creates a new {@link QuerydslRepositoryInvokerAdapter} for the given delegate {@link RepositoryInvoker}, + * {@link QueryDslPredicateExecutor} and Querydsl {@link Predicate}. + * + * @param delegate must not be {@literal null}. + * @param executor must not be {@literal null}. + * @param predicate can be {@literal null}. + */ + public QuerydslRepositoryInvokerAdapter(RepositoryInvoker delegate, QueryDslPredicateExecutor executor, + Predicate predicate) { + + Assert.notNull(delegate, "Delegate RepositoryInvoker must not be null!"); + Assert.notNull(executor, "QuerydslPredicateExecutor must not be null!"); + + this.delegate = delegate; + this.executor = executor; + this.predicate = predicate; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvoker#invokeFindAll(org.springframework.data.domain.Pageable) + */ + @Override + public Iterable invokeFindAll(Pageable pageable) { + return executor.findAll(predicate, pageable); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvoker#invokeFindAll(org.springframework.data.domain.Sort) + */ + @Override + public Iterable invokeFindAll(Sort sort) { + return executor.findAll(predicate, sort); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvocationInformation#hasDeleteMethod() + */ + @Override + public boolean hasDeleteMethod() { + return delegate.hasDeleteMethod(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvocationInformation#hasFindAllMethod() + */ + @Override + public boolean hasFindAllMethod() { + return delegate.hasFindAllMethod(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvocationInformation#hasFindOneMethod() + */ + @Override + public boolean hasFindOneMethod() { + return delegate.hasFindOneMethod(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvocationInformation#hasSaveMethod() + */ + @Override + public boolean hasSaveMethod() { + return delegate.hasSaveMethod(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvoker#invokeDelete(java.io.Serializable) + */ + @Override + public void invokeDelete(Serializable id) { + delegate.invokeDelete(id); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvoker#invokeFindOne(java.io.Serializable) + */ + @Override + public T invokeFindOne(Serializable id) { + return delegate.invokeFindOne(id); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvoker#invokeQueryMethod(java.lang.reflect.Method, java.util.Map, org.springframework.data.domain.Pageable, org.springframework.data.domain.Sort) + */ + @Override + public Object invokeQueryMethod(Method method, Map parameters, Pageable pageable, Sort sort) { + return delegate.invokeQueryMethod(method, parameters, pageable, sort); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvoker#invokeQueryMethod(java.lang.reflect.Method, org.springframework.util.MultiValueMap, org.springframework.data.domain.Pageable, org.springframework.data.domain.Sort) + */ + @Override + public Object invokeQueryMethod(Method method, MultiValueMap parameters, Pageable pageable, + Sort sort) { + return delegate.invokeQueryMethod(method, parameters, pageable, sort); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.support.RepositoryInvoker#invokeSave(java.lang.Object) + */ + @Override + public T invokeSave(T object) { + return delegate.invokeSave(object); + } +} diff --git a/src/main/java/org/springframework/data/repository/support/DefaultRepositoryInvokerFactory.java b/src/main/java/org/springframework/data/repository/support/DefaultRepositoryInvokerFactory.java index baa369c24..bd2f3de8f 100644 --- a/src/main/java/org/springframework/data/repository/support/DefaultRepositoryInvokerFactory.java +++ b/src/main/java/org/springframework/data/repository/support/DefaultRepositoryInvokerFactory.java @@ -92,13 +92,18 @@ public class DefaultRepositoryInvokerFactory implements RepositoryInvokerFactory * @param domainType * @return */ - @SuppressWarnings("unchecked") private RepositoryInvoker prepareInvokers(Class domainType) { Object repository = repositories.getRepositoryFor(domainType); Assert.notNull(repository, String.format("No repository found for domain type: %s", domainType)); RepositoryInformation information = repositories.getRepositoryInformationFor(domainType); + return createInvoker(information, repository); + } + + @SuppressWarnings("unchecked") + protected RepositoryInvoker createInvoker(RepositoryInformation information, Object repository) { + if (repository instanceof PagingAndSortingRepository) { return new PagingAndSortingRepositoryInvoker((PagingAndSortingRepository) repository, information, conversionService); 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 681e0a0c3..79f968623 100644 --- a/src/main/java/org/springframework/data/web/querydsl/MultiValueBinding.java +++ b/src/main/java/org/springframework/data/web/querydsl/MultiValueBinding.java @@ -21,9 +21,21 @@ import com.mysema.query.types.Path; import com.mysema.query.types.Predicate; /** + * {@link MultiValueBinding} creates a {@link Predicate} out of given {@link Path} and collection value. Used for + * specific parameter treatment in {@link QuerydslBindings}. + * * @author Oliver Gierke + * @since 1.11 */ +@FunctionalInterface public interface MultiValueBinding, S> { + /** + * Returns the predicate to be applied to the given {@link Path} for the given value. + * + * @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. + * @return can be {@literal null}, in which case the binding will not be incorporated in the overall {@link Predicate}. + */ Predicate bind(T path, Collection value); } diff --git a/src/main/java/org/springframework/data/web/querydsl/QuerydslBinderCustomizer.java b/src/main/java/org/springframework/data/web/querydsl/QuerydslBinderCustomizer.java new file mode 100644 index 000000000..a4a3d9a6e --- /dev/null +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslBinderCustomizer.java @@ -0,0 +1,28 @@ +/* + * Copyright 2015 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.web.querydsl; + +import com.mysema.query.types.EntityPath; + +/** + * A component that will customize {@link QuerydslBindings} for the given entity path. + * + * @author Oliver Gierke + */ +public interface QuerydslBinderCustomizer> { + + void customize(QuerydslBindings bindings, T root); +} 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 c4ab1b510..cf96bdc6b 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslBindings.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslBindings.java @@ -18,6 +18,7 @@ package org.springframework.data.web.querydsl; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -39,7 +40,7 @@ import com.mysema.query.types.Predicate; * new QuerydslBindings() { * { * bind(QUser.user.address.city).using((path, value) -> path.like(value.toString())); - * bind("lastname").using((path, value) -> path.like(value.toString())); + * bind(String.class).using((path, value) -> path.like(value.toString())); * } * } * @@ -72,27 +73,27 @@ public class QuerydslBindings { /** * Returns a new {@link PathBinder} for the given {@link Path}s to define bindings for them. * - * @param paths must not be {@literal null} or emtpy. + * @param paths must not be {@literal null} or empty. * @return */ public final , S> PathBinder bind(T... paths) { return new PathBinder(paths); } + public final TypeBinder bind(Class type) { + return new TypeBinder(type); + } + /** * Defines a binding for the given * * @param paths * @return */ - public final PropertyBinder bind(String... paths) { + final PropertyBinder bind(String... paths) { return new PropertyBinder(Arrays.asList(paths)); } - public final TypeBinder bind(Class type) { - return new TypeBinder(type); - } - /** * 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}. @@ -114,7 +115,7 @@ public class QuerydslBindings { * * @param properties */ - public final void excluding(String... properties) { + final void excluding(String... properties) { this.blackList.addAll(Arrays.asList(properties)); } @@ -137,7 +138,7 @@ public class QuerydslBindings { * * @param properties */ - public final void including(String... properties) { + final void including(String... properties) { Assert.notEmpty(properties, "At least one property has to be provided!"); @@ -264,7 +265,7 @@ public class QuerydslBindings { * * @author Oliver Gierke */ - protected final class PathBinder

, T> { + public final class PathBinder

, T> { private final List

paths; @@ -302,7 +303,7 @@ public class QuerydslBindings { } } - protected final class TypeBinder { + public final class TypeBinder { private final Class type; @@ -323,7 +324,7 @@ public class QuerydslBindings { } } - protected final class PropertyBinder { + public final class PropertyBinder { private final List paths; @@ -393,7 +394,8 @@ public class QuerydslBindings { */ @Override public Predicate bind(T path, Collection value) { - return delegate.bind(path, value.iterator().next()); + Iterator iterator = value.iterator(); + return delegate.bind(path, iterator.hasNext() ? iterator.next() : null); } } } diff --git a/src/main/java/org/springframework/data/web/querydsl/QuerydslDefaultBinding.java b/src/main/java/org/springframework/data/web/querydsl/QuerydslDefaultBinding.java index 31e05f2d0..29b3c8432 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslDefaultBinding.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslDefaultBinding.java @@ -17,6 +17,9 @@ package org.springframework.data.web.querydsl; import java.util.Collection; +import org.springframework.util.Assert; + +import com.mysema.query.BooleanBuilder; import com.mysema.query.types.Path; import com.mysema.query.types.Predicate; import com.mysema.query.types.expr.SimpleExpression; @@ -26,7 +29,6 @@ import com.mysema.query.types.path.CollectionPathBase; * Default implementation of {@link MultiValueBinding} creating {@link Predicate} based on the {@link Path}s type. * Binds: *

    - *
  • {@literal null} as {@link SimpleExpression#isNull()}.
    • *
  • {@link java.lang.Object} as {@link SimpleExpression#eq()} on simple properties.
    • *
  • {@link java.lang.Object} as {@link SimpleExpression#contains()} on collection properties.
    • *
  • {@link java.util.Collection} as {@link SimpleExpression#in()} on simple properties.
    • @@ -44,25 +46,33 @@ class QuerydslDefaultBinding implements MultiValueBinding */ @Override @SuppressWarnings({ "unchecked", "rawtypes" }) - public Predicate bind(Path path, Collection source) { + public Predicate bind(Path path, Collection value) { - if ((source == null || source.isEmpty())) { - return ((SimpleExpression) path).isNull(); + Assert.notNull(path, "Path must not be null!"); + Assert.notNull(value, "Value must not be null!"); + + if (value.isEmpty()) { + return null; } - Object firstValue = source.iterator().next(); - if (path instanceof CollectionPathBase) { - return ((CollectionPathBase) path).contains(firstValue); + + BooleanBuilder builder = new BooleanBuilder(); + + for (Object element : value) { + builder.and(((CollectionPathBase) path).contains(element)); + } + + return builder.getValue(); } if (path instanceof SimpleExpression) { - if (source.size() > 1) { - return ((SimpleExpression) path).in(source); + if (value.size() > 1) { + return ((SimpleExpression) path).in(value); } - return ((SimpleExpression) path).eq(firstValue); + return ((SimpleExpression) path).eq(value.iterator().next()); } throw new IllegalArgumentException( 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 fa0bf2ff9..4087e2d98 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicate.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicate.java @@ -27,7 +27,7 @@ import java.lang.annotation.Target; * @author Christoph Strobl * @since 1.11 */ -@Target(ElementType.PARAMETER) +@Target({ ElementType.PARAMETER, ElementType.TYPE }) @Retention(RetentionPolicy.RUNTIME) public @interface QuerydslPredicate { @@ -36,12 +36,13 @@ public @interface QuerydslPredicate { * * @return */ - Class root() default Object.class; + Classroot() default Object.class; /** * Configuration class providing options on a per field base. * * @return */ - Class bindings() default QuerydslBindings.class; + @SuppressWarnings("rawtypes") + Classbindings() default QuerydslBinderCustomizer.class; } 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 bdddf17cb..a3f8a4e64 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java @@ -15,22 +15,28 @@ */ package org.springframework.data.web.querydsl; +import java.util.Arrays; +import java.util.Map.Entry; + import org.springframework.beans.BeanUtils; import org.springframework.beans.BeansException; -import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.factory.config.AutowireCapableBeanFactory; import org.springframework.context.ApplicationContext; 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.SimpleEntityPathResolver; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.ModelAndViewContainer; +import com.mysema.query.types.EntityPath; import com.mysema.query.types.Predicate; /** @@ -43,8 +49,6 @@ import com.mysema.query.types.Predicate; */ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentResolver, ApplicationContextAware { - private static final QuerydslBindings DEFAULT_BINDINGS = new QuerydslBindings(); - private final QuerydslPredicateBuilder predicateBuilder; private AutowireCapableBeanFactory beanFactory; @@ -95,8 +99,16 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR public Predicate resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { - return predicateBuilder.getPredicate(new MutablePropertyValues(webRequest.getParameterMap()), - createBindings(parameter), extractTypeInfo(parameter).getActualType()); + MultiValueMap parameters = new LinkedMultiValueMap(); + + for (Entry entry : webRequest.getParameterMap().entrySet()) { + parameters.put(entry.getKey(), Arrays.asList(entry.getValue())); + } + + QuerydslPredicate annotation = parameter.getParameterAnnotation(QuerydslPredicate.class); + TypeInformation domainType = extractTypeInfo(parameter).getActualType(); + + return predicateBuilder.getPredicate(parameters, createBindings(annotation, domainType.getType()), domainType); } private TypeInformation extractTypeInfo(MethodParameter parameter) { @@ -110,16 +122,22 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR return ClassTypeInformation.from(annotation.root()); } - private QuerydslBindings createBindings(MethodParameter parameter) - throws InstantiationException, IllegalAccessException { + @SuppressWarnings({ "rawtypes", "unchecked" }) + public QuerydslBindings createBindings(QuerydslPredicate annotation, Class domainType) { - QuerydslPredicate annotation = parameter.getParameterAnnotation(QuerydslPredicate.class); + EntityPath path = SimpleEntityPathResolver.INSTANCE.createPath(domainType); - if (annotation == null) { - return DEFAULT_BINDINGS; + QuerydslBindings bindings = new QuerydslBindings(); + + if (annotation == null || annotation.bindings().equals(QuerydslBinderCustomizer.class)) { + return bindings; } - Class type = annotation.bindings(); - return beanFactory != null ? beanFactory.createBean(type) : BeanUtils.instantiateClass(type); + Class type = annotation.bindings(); + QuerydslBinderCustomizer> customizer = beanFactory != null ? beanFactory.createBean(type) + : BeanUtils.instantiateClass(type); + customizer.customize(bindings, path); + + return bindings; } } 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 153a7c9f0..c6c040547 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilder.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilder.java @@ -20,9 +20,10 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Map.Entry; -import org.springframework.beans.PropertyValue; import org.springframework.beans.PropertyValues; import org.springframework.core.convert.ConversionService; import org.springframework.data.mapping.PropertyPath; @@ -30,12 +31,11 @@ import org.springframework.data.mapping.PropertyReferenceException; import org.springframework.data.querydsl.SimpleEntityPathResolver; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; -import org.springframework.util.ObjectUtils; +import org.springframework.util.MultiValueMap; import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; import com.mysema.query.BooleanBuilder; -import com.mysema.query.types.EntityPath; import com.mysema.query.types.Path; import com.mysema.query.types.Predicate; @@ -46,7 +46,7 @@ import com.mysema.query.types.Predicate; * @author Oliver Gierke * @since 1.11 */ -class QuerydslPredicateBuilder { +public class QuerydslPredicateBuilder { private final ConversionService conversionService; private final MultiValueBinding defaultBinding; @@ -67,26 +67,36 @@ class QuerydslPredicateBuilder { * @return */ - public Predicate getPredicate(PropertyValues values, QuerydslBindings bindings, TypeInformation type) { + public Predicate getPredicate(MultiValueMap values, QuerydslBindings bindings, + TypeInformation type) { Assert.notNull(bindings, "Context must not be null!"); - if (values.isEmpty()) { - return new BooleanBuilder(); - } - BooleanBuilder builder = new BooleanBuilder(); - for (PropertyValue propertyValue : values.getPropertyValues()) { + if (values.isEmpty()) { + return builder.getValue(); + } + + for (Entry> entry : values.entrySet()) { + + if (isSingleElementCollectionWithoutText(entry.getValue())) { + continue; + } try { - PropertyPath propertyPath = PropertyPath.from(propertyValue.getName(), type); + PropertyPath propertyPath = PropertyPath.from(entry.getKey(), type); if (bindings.isPathVisible(propertyPath)) { - Collection value = convertToPropertyPathSpecificType(propertyValue.getValue(), propertyPath); - builder.and(invokeBinding(propertyPath, bindings, value)); + Collection value = convertToPropertyPathSpecificType(entry.getValue(), propertyPath); + + Predicate binding = invokeBinding(propertyPath, bindings, value); + + if (binding != null) { + builder.and(binding); + } } } catch (PropertyReferenceException o_O) { // not a property of the domain object, continue @@ -127,48 +137,33 @@ class QuerydslPredicateBuilder { return resolvedPath; } - private static Path reifyPath(PropertyPath path, EntityPath base) { + private static Path reifyPath(PropertyPath path, Path base) { - EntityPath entityPath = base != null ? base + Path entityPath = base != null ? base : SimpleEntityPathResolver.INSTANCE.createPath(path.getOwningType().getType()); Field field = ReflectionUtils.findField(entityPath.getClass(), path.getSegment()); Object value = ReflectionUtils.getField(field, entityPath); - if (path.hasNext() && value instanceof EntityPath) { - return reifyPath(path.next(), (EntityPath) value); + if (path.hasNext()) { + return reifyPath(path.next(), (Path) value); } return (Path) value; } - private Collection convertToPropertyPathSpecificType(Object source, PropertyPath path) { + private Collection convertToPropertyPathSpecificType(List source, PropertyPath path) { Class targetType = path.getLeafProperty().getType(); - if (targetType.isInstance(source)) { - return Collections.singleton(source); + if (isSingleElementCollectionWithoutText(source)) { + return Collections.emptyList(); } - Object value = source; - - if (ObjectUtils.isArray(value)) { - value = CollectionUtils.arrayToList(value); - } - - if (value instanceof Collection) { - return potentiallyConvertCollectionValues((Collection) value, targetType); - } - - return Collections.singleton(potentiallyConvertValue(value, targetType)); - } - - private Collection potentiallyConvertCollectionValues(Collection source, Class elementType) { - Collection target = new ArrayList(source.size()); - for (Object value : source) { - target.add(potentiallyConvertValue(value, elementType)); + for (String value : source) { + target.add(potentiallyConvertValue(value, targetType)); } return target; @@ -178,4 +173,15 @@ class QuerydslPredicateBuilder { 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. + * + * @param source must not be {@literal null}. + * @return + */ + private static boolean isSingleElementCollectionWithoutText(List source) { + return source.size() == 1 && !StringUtils.hasText(source.get(0)); + } } 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 4fda5551b..1f2e8282c 100644 --- a/src/main/java/org/springframework/data/web/querydsl/SingleValueBinding.java +++ b/src/main/java/org/springframework/data/web/querydsl/SingleValueBinding.java @@ -19,8 +19,8 @@ import com.mysema.query.types.Path; import com.mysema.query.types.Predicate; /** - * {@link SingleValueBinding} creates a {@link Predicate} out of given {@link Path} and value. Used for specific parameter - * treatment in {@link QuerydslBindings}. + * {@link SingleValueBinding} creates a {@link Predicate} out of given {@link Path} and value. Used for specific + * parameter treatment in {@link QuerydslBindings}. * * @author Christoph Strobl * @author Oliver Gierke @@ -32,9 +32,9 @@ public interface SingleValueBinding, S> { /** * Returns the predicate to be applied to the given {@link Path} for the given value. * - * @param path {@link Path} to the property. Must not be {@literal null}. - * @param value the value that should be bound. Can be {@literal null}. - * @return must not be {@literal null}. + * @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}. + * @return can be {@literal null}, in which case the binding will not be incorporated in the overall {@link Predicate}. */ Predicate bind(T path, S value); } diff --git a/src/test/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapterUnitTests.java b/src/test/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapterUnitTests.java new file mode 100644 index 000000000..f79b743a8 --- /dev/null +++ b/src/test/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapterUnitTests.java @@ -0,0 +1,121 @@ +/* + * Copyright 2015 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.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import java.io.Serializable; +import java.lang.reflect.Method; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; +import org.springframework.data.repository.support.RepositoryInvoker; +import org.springframework.util.MultiValueMap; + +import com.mysema.query.types.Predicate; + +/** + * Unit tests for {@link QuerydslRepositoryInvokerAdapter}. + * + * @author Oliver Gierke + * @soundtrack Emilie Nicolas - Grown Up + */ +@RunWith(MockitoJUnitRunner.class) +public class QuerydslRepositoryInvokerAdapterUnitTests { + + @Mock RepositoryInvoker delegate; + @Mock QueryDslPredicateExecutor executor; + @Mock Predicate predicate; + + QuerydslRepositoryInvokerAdapter adapter; + + @Before + public void setUp() { + this.adapter = new QuerydslRepositoryInvokerAdapter(delegate, executor, predicate); + } + + /** + * @see DATACMNS-669 + */ + @Test + public void forwardsFindAllToExecutorWithPredicate() { + + Sort sort = new Sort("firstname"); + adapter.invokeFindAll(sort); + + verify(executor, times(1)).findAll(predicate, sort); + verify(delegate, times(0)).invokeFindAll(sort); + } + + /** + * @see DATACMNS-669 + */ + @Test + public void forwardsFindAllWithPageableToExecutorWithPredicate() { + + PageRequest pageable = new PageRequest(0, 10); + adapter.invokeFindAll(pageable); + + verify(executor, times(1)).findAll(predicate, pageable); + verify(delegate, times(0)).invokeFindAll(pageable); + } + + /** + * @see DATACMNS-669 + */ + @Test + @SuppressWarnings({ "deprecation", "unchecked" }) + public void forwardsMethodsToDelegate() { + + adapter.hasDeleteMethod(); + verify(delegate, times(1)).hasDeleteMethod(); + + adapter.hasFindAllMethod(); + verify(delegate, times(1)).hasFindAllMethod(); + + adapter.hasFindOneMethod(); + verify(delegate, times(1)).hasFindOneMethod(); + + adapter.hasSaveMethod(); + verify(delegate, times(1)).hasSaveMethod(); + + adapter.invokeDelete(any(Serializable.class)); + verify(delegate, times(1)).invokeDelete(any(Serializable.class)); + + adapter.invokeFindOne(any(Serializable.class)); + verify(delegate, times(1)).invokeFindOne(any(Serializable.class)); + + adapter.invokeQueryMethod(any(Method.class), any(Map.class), any(Pageable.class), any(Sort.class)); + verify(delegate, times(1)).invokeQueryMethod(any(Method.class), any(Map.class), any(Pageable.class), + any(Sort.class)); + + adapter.invokeQueryMethod(any(Method.class), (MultiValueMap) any(MultiValueMap.class), + any(Pageable.class), any(Sort.class)); + verify(delegate, times(1)).invokeQueryMethod(any(Method.class), + (MultiValueMap) any(MultiValueMap.class), any(Pageable.class), any(Sort.class)); + + adapter.invokeSave(any()); + verify(delegate, times(1)).invokeSave(any()); + } +} diff --git a/src/test/java/org/springframework/data/web/querydsl/QuerydslDefaultBindingUnitTests.java b/src/test/java/org/springframework/data/web/querydsl/QuerydslDefaultBindingUnitTests.java index 30d6cb7fa..392379008 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslDefaultBindingUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslDefaultBindingUnitTests.java @@ -15,7 +15,8 @@ */ package org.springframework.data.web.querydsl; -import static org.hamcrest.core.Is.*; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; import java.util.Arrays; import java.util.Collections; @@ -32,14 +33,15 @@ import com.mysema.query.types.Predicate; /** * @author Christoph Strobl + * @author Oliver Gierke */ public class QuerydslDefaultBindingUnitTests { - QuerydslDefaultBinding builder; + QuerydslDefaultBinding binding; @Before public void setUp() { - builder = new QuerydslDefaultBinding(); + binding = new QuerydslDefaultBinding(); } /** @@ -48,9 +50,9 @@ public class QuerydslDefaultBindingUnitTests { @Test public void shouldCreatePredicateCorrectlyWhenPropertyIsInRoot() { - Predicate predicate = builder.bind(QUser.user.firstname, Collections.singleton("tam")); + Predicate predicate = binding.bind(QUser.user.firstname, Collections.singleton("tam")); - assertThat(predicate, is(QUser.user.firstname.eq("tam"))); + assertPredicate(predicate, is(QUser.user.firstname.eq("tam"))); } /** @@ -59,31 +61,20 @@ public class QuerydslDefaultBindingUnitTests { @Test public void shouldCreatePredicateCorrectlyWhenPropertyIsInNestedElement() { - Predicate predicate = builder.bind(QUser.user.address.city, Collections.singleton("two rivers")); + Predicate predicate = binding.bind(QUser.user.address.city, Collections.singleton("two rivers")); Assert.assertThat(predicate.toString(), is(QUser.user.address.city.eq("two rivers").toString())); } - /** - * @see DATACMNS-669 - */ - @Test - public void shouldCreatePredicateCorrectlyWhenValueIsNull() { - - Predicate predicate = builder.bind(QUser.user.firstname, Collections.emptySet()); - - assertThat(predicate, is(QUser.user.firstname.isNull())); - } - /** * @see DATACMNS-669 */ @Test public void shouldCreatePredicateWithContainingWhenPropertyIsCollectionLikeAndValueIsObject() { - Predicate predicate = builder.bind(QUser.user.nickNames, Collections.singleton("dragon reborn")); + Predicate predicate = binding.bind(QUser.user.nickNames, Collections.singleton("dragon reborn")); - assertThat(predicate, is(QUser.user.nickNames.contains("dragon reborn"))); + assertPredicate(predicate, is(QUser.user.nickNames.contains("dragon reborn"))); } /** @@ -92,17 +83,22 @@ public class QuerydslDefaultBindingUnitTests { @Test public void shouldCreatePredicateWithInWhenPropertyIsAnObjectAndValueIsACollection() { - Predicate predicate = builder.bind(QUser.user.firstname, Arrays.asList("dragon reborn", "shadowkiller")); + Predicate predicate = binding.bind(QUser.user.firstname, Arrays.asList("dragon reborn", "shadowkiller")); - assertThat(predicate, is(QUser.user.firstname.in(Arrays.asList("dragon reborn", "shadowkiller")))); + assertPredicate(predicate, is(QUser.user.firstname.in(Arrays.asList("dragon reborn", "shadowkiller")))); + } + + @Test + public void testname() { + + assertThat(binding.bind(QUser.user.lastname, Collections.emptySet()), is(nullValue())); } /* * just to satisfy generic type boundaries o_O */ @SuppressWarnings({ "rawtypes", "unchecked" }) - private void assertThat(Predicate predicate, Matcher matcher) { + private void assertPredicate(Predicate predicate, Matcher matcher) { Assert.assertThat((Expression) predicate, Is. is((Matcher) matcher)); } - } 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 8ae96b4ae..71e840311 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java @@ -205,18 +205,6 @@ public class QuerydslPredicateArgumentResolverUnitTests { assertThat(predicate.toString(), is(QUser.user.inceptionYear.eq(973L).toString())); } - /** - * @see DATACMNS-669 - */ - @Test - public void createBindingContextShouldUseQuerydslPredicationAnntotationDefaultBindingIfNotAnnotated() { - - Object bindings = ReflectionTestUtils.invokeMethod(resolver, "createBindings", - getMethodParameterFor("predicateWithoutAnnotation", Predicate.class)); - - assertThat(bindings, is(instanceOf(QuerydslBindings.class))); - } - /** * @see DATACMNS-669 */ @@ -239,11 +227,11 @@ public class QuerydslPredicateArgumentResolverUnitTests { } } - static class SpecificBinding extends QuerydslBindings { + static class SpecificBinding implements QuerydslBinderCustomizer { - public SpecificBinding() { + public void customize(QuerydslBindings bindings, QUser user) { - bind("firstname").using(new SingleValueBinding() { + bindings.bind("firstname").using(new SingleValueBinding() { @Override public Predicate bind(StringPath path, String value) { @@ -251,7 +239,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { } }); - bind(QUser.user.lastname).single(new SingleValueBinding() { + bindings.bind(user.lastname).single(new SingleValueBinding() { @Override public Predicate bind(StringPath path, String value) { @@ -259,7 +247,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { } }); - excluding("address"); + bindings.excluding("address"); } } 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 d0ac0ea20..e95a9c03d 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilderUnitTests.java @@ -15,7 +15,6 @@ */ package org.springframework.data.web.querydsl; -import static java.util.Collections.*; import static org.hamcrest.Matchers.*; import static org.hamcrest.core.Is.is; import static org.junit.Assert.*; @@ -24,7 +23,6 @@ import java.util.List; import org.junit.Before; import org.junit.Test; -import org.springframework.beans.MutablePropertyValues; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.querydsl.QUser; import org.springframework.data.querydsl.User; @@ -33,9 +31,9 @@ import org.springframework.data.util.ClassTypeInformation; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import com.mysema.query.BooleanBuilder; import com.mysema.query.collections.CollQueryFactory; import com.mysema.query.types.Predicate; +import com.mysema.query.types.path.StringPath; /** * Unit tests for {@link QuerydslPredicateBuilder}. @@ -45,13 +43,16 @@ import com.mysema.query.types.Predicate; */ public class QuerydslPredicateBuilderUnitTests { + static final ClassTypeInformation USER_TYPE = ClassTypeInformation.from(User.class); static final QuerydslBindings DEFAULT_BINDINGS = new QuerydslBindings(); QuerydslPredicateBuilder builder; + MultiValueMap values; @Before public void setUp() { - builder = new QuerydslPredicateBuilder(new DefaultConversionService()); + this.builder = new QuerydslPredicateBuilder(new DefaultConversionService()); + this.values = new LinkedMultiValueMap(); } /** @@ -67,7 +68,7 @@ public class QuerydslPredicateBuilderUnitTests { */ @Test(expected = IllegalArgumentException.class) public void getPredicateShouldThrowErrorWhenBindingContextIsNull() { - builder.getPredicate(new MutablePropertyValues(), null, null); + builder.getPredicate(values, null, null); } /** @@ -76,8 +77,7 @@ public class QuerydslPredicateBuilderUnitTests { @Test public void getPredicateShouldReturnEmptyPredicateWhenPropertiesAreEmpty() { - assertThat(builder.getPredicate(new MutablePropertyValues(), DEFAULT_BINDINGS, ClassTypeInformation.OBJECT), - is((Predicate) new BooleanBuilder())); + assertThat(builder.getPredicate(values, DEFAULT_BINDINGS, ClassTypeInformation.OBJECT), is(nullValue())); } /** @@ -86,8 +86,9 @@ public class QuerydslPredicateBuilderUnitTests { @Test public void resolveArgumentShouldCreateSingleStringParameterPredicateCorrectly() throws Exception { - Predicate predicate = builder.getPredicate(new MutablePropertyValues(singletonMap("firstname", "Oliver")), - DEFAULT_BINDINGS, ClassTypeInformation.from(User.class)); + values.add("firstname", "Oliver"); + + Predicate predicate = builder.getPredicate(values, DEFAULT_BINDINGS, USER_TYPE); assertThat(predicate, is((Predicate) QUser.user.firstname.eq("Oliver"))); @@ -103,8 +104,9 @@ public class QuerydslPredicateBuilderUnitTests { @Test public void resolveArgumentShouldCreateNestedStringParameterPredicateCorrectly() throws Exception { - Predicate predicate = builder.getPredicate(new MutablePropertyValues(singletonMap("address.city", "Linz")), - DEFAULT_BINDINGS, ClassTypeInformation.from(User.class)); + values.add("address.city", "Linz"); + + Predicate predicate = builder.getPredicate(values, DEFAULT_BINDINGS, USER_TYPE); assertThat(predicate, is((Predicate) QUser.user.address.city.eq("Linz"))); @@ -114,16 +116,37 @@ public class QuerydslPredicateBuilderUnitTests { assertThat(result, hasItem(Users.CHRISTOPH)); } + /** + * @see DATACMNS-669 + */ @Test public void ignoresNonDomainTypeProperties() { - MultiValueMap values = new LinkedMultiValueMap(); values.add("firstname", "rand"); values.add("lastname".toUpperCase(), "al'thor"); - Predicate predicate = builder.getPredicate(new MutablePropertyValues(values), new QuerydslBindings(), - ClassTypeInformation.from(User.class)); + Predicate predicate = builder.getPredicate(values, new QuerydslBindings(), USER_TYPE); assertThat(predicate, is((Predicate) QUser.user.firstname.eq("rand"))); } + + /** + * @see DATACMNS-669 + */ + @Test + public void forwardsNullForEmptyParameterToSingleValueBinder() { + + values.add("lastname", null); + + QuerydslBindings bindings = new QuerydslBindings(); + bindings.bind(QUser.user.lastname).single(new SingleValueBinding() { + + @Override + public Predicate bind(StringPath path, String value) { + return value == null ? null : path.contains(value); + } + }); + + builder.getPredicate(values, bindings, USER_TYPE); + } }