Revise QuerydslPredicateBuilder nullability behavior.

We now no longer return a null Predicate from QuerydslPredicateBuilder.getPredicate(…) if the input values are empty or if the constraints are empty. Instead, we return an empty BooleanBuilder instance to avoid null handling on the calling side.

HandlerMethodArgumentsResolvers for QuerydslBindings retain their null/Optional.empty semantics.

Closes #2396

Author: mp911de

src/main/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilder.java

@@ -32,7 +32,6 @@
 import org.springframework.data.mapping.PropertyPath;
 import org.springframework.data.querydsl.EntityPathResolver;
 import org.springframework.data.util.TypeInformation;
-import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.StringUtils;
@@ -80,9 +79,8 @@ public QuerydslPredicateBuilder(ConversionService conversionService, EntityPathR
 	 * @param type the type to create a predicate for.
 	 * @param values the values to bind.
 	 * @param bindings the {@link QuerydslBindings} for the predicate.
-	 * @return
+	 * @return the {@link Predicate}.
 	 */
-	@Nullable
 	public Predicate getPredicate(TypeInformation<?> type, MultiValueMap<String, String> values,
 			QuerydslBindings bindings) {
 
@@ -91,7 +89,7 @@ public Predicate getPredicate(TypeInformation<?> type, MultiValueMap<String, Str
 		BooleanBuilder builder = new BooleanBuilder();
 
 		if (values.isEmpty()) {
-			return builder.getValue();
+			return getPredicate(builder);
 		}
 
 		for (Entry<String, List<String>> entry : values.entrySet()) {
@@ -118,7 +116,19 @@ public Predicate getPredicate(TypeInformation<?> type, MultiValueMap<String, Str
 			predicate.ifPresent(builder::and);
 		}
 
-		return builder.getValue();
+		return getPredicate(builder);
+	}
+
+	/**
+	 * Returns whether the given {@link Predicate} represents an empty predicate instance.
+	 *
+	 * @param predicate
+	 * @return
+	 * @since 2.5.3
+	 * @see BooleanBuilder
+	 */
+	public static boolean isEmpty(Predicate predicate) {
+		return new BooleanBuilder().equals(predicate);
 	}
 
 	/**
@@ -219,4 +229,16 @@ private static TypeDescriptor getTargetTypeDescriptor(PathInformation path) {
 	private static boolean isSingleElementCollectionWithoutText(List<String> source) {
 		return source.size() == 1 && !StringUtils.hasLength(source.get(0));
 	}
+
+	/**
+	 * Returns the {@link Predicate} from {@link BooleanBuilder}.
+	 *
+	 * @param builder
+	 * @return
+	 */
+	private static Predicate getPredicate(BooleanBuilder builder) {
+
+		Predicate predicate = builder.getValue();
+		return predicate == null ? new BooleanBuilder() : predicate;
+	}
 }

src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolver.java

@@ -21,7 +21,6 @@
 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.QuerydslBindingsFactory;
@@ -33,7 +32,6 @@
 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;
 
 /**
@@ -84,13 +82,7 @@ public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewC
 		MultiValueMap<String, String> queryParameters = getQueryParameters(webRequest);
 		Predicate result = getPredicate(parameter, queryParameters);
 
-		if (!parameter.isOptional() && result == null) {
-			return new BooleanBuilder();
-		}
-
-		return OPTIONAL_OF_PREDICATE.isAssignableFrom(ResolvableType.forMethodParameter(parameter)) //
-				? Optional.ofNullable(result) //
-				: result;
+		return potentiallyConvertMethodParameterValue(parameter, result);
 	}
 
 	private static MultiValueMap<String, String> getQueryParameters(NativeWebRequest webRequest) {

src/main/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverSupport.java

@@ -94,7 +94,6 @@ public boolean supportsParameter(MethodParameter parameter) {
 		return false;
 	}
 
-	@Nullable
 	Predicate getPredicate(MethodParameter parameter, MultiValueMap<String, String> queryParameters) {
 
 		MergedAnnotations annotations = MergedAnnotations.from(parameter.getParameter());
@@ -112,6 +111,20 @@ Predicate getPredicate(MethodParameter parameter, MultiValueMap<String, String>
 		return predicateBuilder.getPredicate(domainType, queryParameters, bindings);
 	}
 
+	@Nullable
+	static Object potentiallyConvertMethodParameterValue(MethodParameter parameter, Predicate predicate) {
+
+		if (!parameter.isOptional()) {
+			return predicate;
+		}
+
+		if (OPTIONAL_OF_PREDICATE.isAssignableFrom(ResolvableType.forMethodParameter(parameter))) {
+			return QuerydslPredicateBuilder.isEmpty(predicate) ? Optional.empty() : Optional.of(predicate);
+		}
+
+		return QuerydslPredicateBuilder.isEmpty(predicate) ? null : predicate;
+	}
+
 	/**
 	 * 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.

src/main/java/org/springframework/data/web/querydsl/ReactiveQuerydslPredicateArgumentResolver.java

@@ -17,20 +17,18 @@
 
 import java.util.List;
 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.data.querydsl.binding.QuerydslBindingsFactory;
+import org.springframework.lang.Nullable;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 import org.springframework.web.reactive.BindingContext;
 import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolver;
 import org.springframework.web.reactive.result.method.SyncHandlerMethodArgumentResolver;
 import org.springframework.web.server.ServerWebExchange;
 
-import com.querydsl.core.BooleanBuilder;
 import com.querydsl.core.types.Predicate;
 
 /**
@@ -54,21 +52,17 @@ public ReactiveQuerydslPredicateArgumentResolver(QuerydslBindingsFactory factory
 	 * @see org.springframework.web.reactive.result.method.SyncHandlerMethodArgumentResolver(org.springframework.core.MethodParameter, org.springframework.web.reactive.BindingContext, org.springframework.web.server.ServerWebExchange)
 	 */
 	@Override
+	@Nullable
 	public Object resolveArgumentValue(MethodParameter parameter, BindingContext bindingContext,
 			ServerWebExchange exchange) {
 
 		MultiValueMap<String, String> queryParameters = getQueryParameters(exchange);
 		Predicate result = getPredicate(parameter, queryParameters);
 
-		if (!parameter.isOptional() && result == null) {
-			return new BooleanBuilder();
-		}
-
-		return OPTIONAL_OF_PREDICATE.isAssignableFrom(ResolvableType.forMethodParameter(parameter)) //
-				? Optional.ofNullable(result) //
-				: result;
+		return potentiallyConvertMethodParameterValue(parameter, result);
 	}
 
+
 	private static MultiValueMap<String, String> getQueryParameters(ServerWebExchange exchange) {
 
 		MultiValueMap<String, String> queryParams = exchange.getRequest().getQueryParams();

src/test/java/org/springframework/data/querydsl/binding/QuerydslPredicateBuilderUnitTests.java

@@ -77,8 +77,10 @@ void getPredicateShouldThrowErrorWhenBindingContextIsNull() {
 	}
 
 	@Test // DATACMNS-669, DATACMNS-1168
-	void getPredicateShouldReturnNullWhenPropertiesAreEmpty() {
-		assertThat(builder.getPredicate(ClassTypeInformation.OBJECT, values, DEFAULT_BINDINGS)).isNull();
+	void getPredicateShouldReturnEmptyWhenPropertiesAreEmpty() {
+		assertThat(
+				QuerydslPredicateBuilder.isEmpty(builder.getPredicate(ClassTypeInformation.OBJECT, values, DEFAULT_BINDINGS)))
+						.isTrue();
 	}
 
 	@Test // DATACMNS-669
@@ -228,6 +230,6 @@ void dropsValuesContainingAnEmptyString() {
 
 		values.add("firstname", "");
 
-		assertThat(builder.getPredicate(USER_TYPE, values, DEFAULT_BINDINGS)).isNull();
+		assertThat(QuerydslPredicateBuilder.isEmpty(builder.getPredicate(USER_TYPE, values, DEFAULT_BINDINGS))).isTrue();
 	}
 }