From dc49101850a761645b4f4ead302508b1f2d0b152 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 15 Jul 2015 09:51:55 +0200 Subject: [PATCH] DATACMNS-669 - Ignore non-reachable paths for Querydsl parameter binding. We now ignore invalid PropertyPaths on property binding to Querydsl Predicates. Those may occur since the request also holds non-property-related arguments, such as pagination parameters. Fixed two potential NullPointerExceptions along the way since we no longer depend on the presence of @QuerydslPredicate. Original pull request: #132. --- .../QuerydslPredicateArgumentResolver.java | 21 +++++++++---- .../querydsl/QuerydslPredicateBuilder.java | 14 ++++++--- ...dslPredicateArgumentResolverUnitTests.java | 31 +++++++++++++++++-- .../QuerydslPredicateBuilderUnitTests.java | 15 +++++++++ 4 files changed, 69 insertions(+), 12 deletions(-) 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 caa2ba55c..bdddf17cb 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java @@ -43,6 +43,8 @@ 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; @@ -99,18 +101,25 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR private TypeInformation extractTypeInfo(MethodParameter parameter) { - Class type = parameter.getParameterAnnotation(QuerydslPredicate.class).root(); + QuerydslPredicate annotation = parameter.getParameterAnnotation(QuerydslPredicate.class); - return type == Object.class ? ClassTypeInformation.fromReturnTypeOf(parameter.getMethod()) - : ClassTypeInformation.from(type); + if (annotation == null || Object.class.equals(annotation.root())) { + return ClassTypeInformation.fromReturnTypeOf(parameter.getMethod()); + } + + return ClassTypeInformation.from(annotation.root()); } private QuerydslBindings createBindings(MethodParameter parameter) throws InstantiationException, IllegalAccessException { - Class bindingsType = parameter.getParameterAnnotation(QuerydslPredicate.class) - .bindings(); + QuerydslPredicate annotation = parameter.getParameterAnnotation(QuerydslPredicate.class); - return beanFactory != null ? beanFactory.createBean(bindingsType) : BeanUtils.instantiateClass(bindingsType); + if (annotation == null) { + return DEFAULT_BINDINGS; + } + + Class type = annotation.bindings(); + return beanFactory != null ? beanFactory.createBean(type) : BeanUtils.instantiateClass(type); } } 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 a8dd4c226..153a7c9f0 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilder.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilder.java @@ -26,6 +26,7 @@ import org.springframework.beans.PropertyValue; 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.util.TypeInformation; import org.springframework.util.Assert; @@ -78,12 +79,17 @@ class QuerydslPredicateBuilder { for (PropertyValue propertyValue : values.getPropertyValues()) { - PropertyPath propertyPath = PropertyPath.from(propertyValue.getName(), type); + try { - if (bindings.isPathVisible(propertyPath)) { + PropertyPath propertyPath = PropertyPath.from(propertyValue.getName(), type); - Collection value = convertToPropertyPathSpecificType(propertyValue.getValue(), propertyPath); - builder.and(invokeBinding(propertyPath, bindings, value)); + if (bindings.isPathVisible(propertyPath)) { + + Collection value = convertToPropertyPathSpecificType(propertyValue.getValue(), propertyPath); + builder.and(invokeBinding(propertyPath, bindings, value)); + } + } catch (PropertyReferenceException o_O) { + // not a property of the domain object, continue } } 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 ae9b18d55..8ae96b4ae 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java @@ -15,7 +15,7 @@ */ package org.springframework.data.web.querydsl; -import static org.hamcrest.core.Is.*; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import org.junit.Before; @@ -25,7 +25,10 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.querydsl.QUser; import org.springframework.data.querydsl.User; +import org.springframework.data.util.ClassTypeInformation; +import org.springframework.data.util.TypeInformation; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.context.request.ServletWebRequest; import com.mysema.query.types.Predicate; @@ -202,6 +205,31 @@ 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 + */ + @Test + @SuppressWarnings("rawtypes") + public void extractTypeInformationShouldUseTypeExtractedFromMethodReturnTypeIfPredicateNotAnnotated() { + + TypeInformation type = ReflectionTestUtils.invokeMethod(resolver, "extractTypeInfo", + getMethodParameterFor("predicateWithoutAnnotation", Predicate.class)); + + assertThat(type, is((TypeInformation) ClassTypeInformation.from(User.class))); + } + private static MethodParameter getMethodParameterFor(String methodName, Class... args) throws RuntimeException { try { @@ -249,5 +277,4 @@ public class QuerydslPredicateArgumentResolverUnitTests { User specificFind(@QuerydslPredicate(bindings = SpecificBinding.class) Predicate predicate); } - } 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 96b9475a8..d0ac0ea20 100644 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateBuilderUnitTests.java @@ -30,6 +30,8 @@ import org.springframework.data.querydsl.QUser; import org.springframework.data.querydsl.User; import org.springframework.data.querydsl.Users; 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; @@ -111,4 +113,17 @@ public class QuerydslPredicateBuilderUnitTests { assertThat(result, hasSize(1)); assertThat(result, hasItem(Users.CHRISTOPH)); } + + @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)); + + assertThat(predicate, is((Predicate) QUser.user.firstname.eq("rand"))); + } }