diff --git a/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java b/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java index 2ceda2ac3..7116ffb0a 100644 --- a/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java +++ b/src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java @@ -91,7 +91,7 @@ public class QuerydslPredicateBuilder { BooleanBuilder builder = new BooleanBuilder(); if (values.isEmpty()) { - return builder; + return builder.getValue(); } for (Entry> entry : values.entrySet()) { 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 f6e6c8ff8..3c72892df 100644 --- a/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java @@ -21,9 +21,11 @@ import java.util.Map.Entry; import java.util.Optional; import org.springframework.core.MethodParameter; +import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.querydsl.binding.QuerydslBinderCustomizer; +import org.springframework.data.querydsl.binding.QuerydslBindings; import org.springframework.data.querydsl.binding.QuerydslBindingsFactory; import org.springframework.data.querydsl.binding.QuerydslPredicate; import org.springframework.data.querydsl.binding.QuerydslPredicateBuilder; @@ -38,6 +40,7 @@ import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.ModelAndViewContainer; +import com.querydsl.core.BooleanBuilder; import com.querydsl.core.types.Predicate; /** @@ -50,6 +53,10 @@ import com.querydsl.core.types.Predicate; */ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentResolver { + private static final ResolvableType PREDICATE = ResolvableType.forClass(Predicate.class); + private static final ResolvableType OPTIONAL_OF_PREDICATE = ResolvableType.forClassWithGenerics(Optional.class, + PREDICATE); + private final QuerydslBindingsFactory bindingsFactory; private final QuerydslPredicateBuilder predicateBuilder; @@ -74,7 +81,9 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR @Override public boolean supportsParameter(MethodParameter parameter) { - if (Predicate.class.equals(parameter.getParameterType())) { + ResolvableType type = ResolvableType.forMethodParameter(parameter); + + if (PREDICATE.isAssignableFrom(type) || OPTIONAL_OF_PREDICATE.isAssignableFrom(type)) { return true; } @@ -92,7 +101,7 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR */ @Nullable @Override - public Predicate resolveArgument(MethodParameter parameter, @Nullable ModelAndViewContainer mavContainer, + public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewContainer mavContainer, NativeWebRequest webRequest, @Nullable WebDataBinderFactory binderFactory) throws Exception { MultiValueMap parameters = new LinkedMultiValueMap<>(); @@ -105,13 +114,23 @@ public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentR .ofNullable(parameter.getParameterAnnotation(QuerydslPredicate.class)); TypeInformation domainType = extractTypeInfo(parameter).getRequiredActualType(); - Optional>> bindings = annotation// - .map(QuerydslPredicate::bindings)// + Optional>> bindingsAnnotation = annotation // + .map(QuerydslPredicate::bindings) // .map(CastUtils::cast); - return predicateBuilder.getPredicate(domainType, parameters, - bindings.map(it -> bindingsFactory.createBindingsFor(domainType, it)) - .orElseGet(() -> bindingsFactory.createBindingsFor(domainType))); + QuerydslBindings bindings = bindingsAnnotation // + .map(it -> bindingsFactory.createBindingsFor(domainType, it)) // + .orElseGet(() -> bindingsFactory.createBindingsFor(domainType)); + + Predicate result = predicateBuilder.getPredicate(domainType, parameters, bindings); + + if (!parameter.isOptional() && result == null) { + return new BooleanBuilder(); + } + + return OPTIONAL_OF_PREDICATE.isAssignableFrom(ResolvableType.forMethodParameter(parameter)) // + ? Optional.ofNullable(result) // + : result; } /** diff --git a/src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java b/src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java index 7339aa04a..c94df53ab 100755 --- a/src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java @@ -38,7 +38,6 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import com.querydsl.collections.CollQueryFactory; -import com.querydsl.core.BooleanBuilder; import com.querydsl.core.types.Constant; import com.querydsl.core.types.Predicate; @@ -75,9 +74,8 @@ public class QuerydslPredicateBuilderUnitTests { } @Test // DATACMNS-669, DATACMNS-1168 - public void getPredicateShouldReturnEmptyPredicateWhenPropertiesAreEmpty() { - assertThat(builder.getPredicate(ClassTypeInformation.OBJECT, values, DEFAULT_BINDINGS)) - .isEqualTo(new BooleanBuilder()); + public void getPredicateShouldReturnNullWhenPropertiesAreEmpty() { + assertThat(builder.getPredicate(ClassTypeInformation.OBJECT, values, DEFAULT_BINDINGS)).isNull(); } @Test // DATACMNS-669 @@ -87,7 +85,7 @@ public class QuerydslPredicateBuilderUnitTests { Predicate predicate = builder.getPredicate(USER_TYPE, values, DEFAULT_BINDINGS); - assertThat(predicate).isEqualTo((Predicate) QUser.user.firstname.eq("Oliver")); + assertThat(predicate).isEqualTo(QUser.user.firstname.eq("Oliver")); List result = CollQueryFactory.from(QUser.user, Users.USERS).where(predicate).fetchResults().getResults(); 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 ef53e4188..15154178b 100755 --- a/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java @@ -22,7 +22,6 @@ import java.util.Optional; import org.junit.Before; import org.junit.Test; - import org.springframework.core.MethodParameter; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -38,6 +37,7 @@ import org.springframework.data.util.TypeInformation; import org.springframework.hateoas.EntityModel; import org.springframework.http.HttpEntity; import org.springframework.http.ResponseEntity; +import org.springframework.lang.Nullable; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.context.request.ServletWebRequest; @@ -94,7 +94,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { request.addParameter("firstname", "rand"); - Predicate predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null, + Object predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null, new ServletWebRequest(request), null); assertThat(predicate).isEqualTo(QUser.user.firstname.eq("rand")); @@ -106,7 +106,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { request.addParameter("firstname", "rand"); request.addParameter("lastname", "al'thor"); - Predicate predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null, + Object predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null, new ServletWebRequest(request), null); assertThat(predicate).isEqualTo(QUser.user.firstname.eq("rand").and(QUser.user.lastname.eq("al'thor"))); @@ -117,12 +117,12 @@ public class QuerydslPredicateArgumentResolverUnitTests { request.addParameter("address.city", "two rivers"); - Predicate predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null, + Object predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null, new ServletWebRequest(request), null); BooleanExpression eq = QUser.user.address.city.eq("two rivers"); - assertThat(predicate).isEqualTo((Predicate) eq); + assertThat(predicate).isEqualTo(eq); } @Test // DATACMNS-669 @@ -130,10 +130,10 @@ public class QuerydslPredicateArgumentResolverUnitTests { request.addParameter("address.city", "tar valon"); - Predicate predicate = resolver.resolveArgument(getMethodParameterFor("pagedFind", Predicate.class, Pageable.class), + Object predicate = resolver.resolveArgument(getMethodParameterFor("pagedFind", Predicate.class, Pageable.class), null, new ServletWebRequest(request), null); - assertThat(predicate).isEqualTo((Predicate) QUser.user.address.city.eq("tar valon")); + assertThat(predicate).isEqualTo(QUser.user.address.city.eq("tar valon")); } @Test // DATACMNS-669 @@ -142,7 +142,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { request.addParameter("firstname", "egwene"); request.addParameter("lastname", "al'vere"); - Predicate predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null, + Object predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null, new ServletWebRequest(request), null); assertThat(predicate).isEqualTo( @@ -154,10 +154,10 @@ public class QuerydslPredicateArgumentResolverUnitTests { request.addParameter("inceptionYear", "978"); - Predicate predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null, + Object predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null, new ServletWebRequest(request), null); - assertThat(predicate).isEqualTo((Predicate) QUser.user.inceptionYear.eq(978L)); + assertThat(predicate).isEqualTo(QUser.user.inceptionYear.eq(978L)); } @Test // DATACMNS-669 @@ -165,10 +165,10 @@ public class QuerydslPredicateArgumentResolverUnitTests { request.addParameter("inceptionYear", new String[] { "978", "998" }); - Predicate predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null, + Object predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null, new ServletWebRequest(request), null); - assertThat(predicate).isEqualTo((Predicate) QUser.user.inceptionYear.in(978L, 998L)); + assertThat(predicate).isEqualTo(QUser.user.inceptionYear.in(978L, 998L)); } @Test // DATACMNS-669 @@ -190,7 +190,7 @@ public class QuerydslPredicateArgumentResolverUnitTests { TypeInformation type = ReflectionTestUtils.invokeMethod(resolver, "extractTypeInfo", getMethodParameterFor("predicateWithoutAnnotation", Predicate.class)); - assertThat(type).isEqualTo((TypeInformation) ClassTypeInformation.from(User.class)); + assertThat(type).isEqualTo(ClassTypeInformation.from(User.class)); } @Test // DATACMNS-669 @@ -205,6 +205,43 @@ public class QuerydslPredicateArgumentResolverUnitTests { assertThat(extractTypeInfo(getMethodParameterFor("forModelAndView"))).isEqualTo(MODELA_AND_VIEW_TYPE); } + @Test // DATACMNS-1593 + public void returnsEmptyPredicateForEmptyInput() throws Exception { + + MethodParameter parameter = getMethodParameterFor("predicateWithoutAnnotation", Predicate.class); + + request.addParameter("firstname", ""); + + assertThat(resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null)) // + .isNotNull(); + } + + @Test // DATACMNS-1635 + public void forwardsNullValueForNullablePredicate() throws Exception { + + MethodParameter parameter = getMethodParameterFor("nullablePredicateWithoutAnnotation", Predicate.class); + + request.addParameter("firstname", ""); + + assertThat(resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null)).isNull(); + } + + @Test // DATACMNS-1635 + public void returnsOptionalIfDeclared() throws Exception { + + MethodParameter parameter = getMethodParameterFor("optionalPredicateWithoutAnnotation", Optional.class); + + request.addParameter("firstname", ""); + + assertThat(resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null)) // + .isInstanceOfSatisfying(Optional.class, it -> assertThat(it).isEmpty()); + + request.addParameter("lastname", "Matthews"); + + assertThat(resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null)) // + .isInstanceOfSatisfying(Optional.class, it -> assertThat(it).isPresent()); + } + private static MethodParameter getMethodParameterFor(String methodName, Class... args) throws RuntimeException { try { @@ -244,6 +281,12 @@ public class QuerydslPredicateArgumentResolverUnitTests { ModelAndView forModelAndView(); ResponseEntity> forResourceOfUser(); + + // Nullability + + User nullablePredicateWithoutAnnotation(@Nullable Predicate predicate); + + User optionalPredicateWithoutAnnotation(Optional predicate); } public static class SampleRepo implements QuerydslBinderCustomizer {