Skip to content

Commit 664e661

Browse files
odrotbohmodrotbohm
committed
DATACMNS-1593, DATACMNS-1635 - Overhauled null handling in QuerydslPredicateArgumentResolver.
QuerydslPredicateArgumentResolver now properly handles predicate lookups that result in null values. The semantics of a handler method parameter of type of Querydsl's Predicate have been tightened to always see a non-null Predicate by default. Users that want to handle the absence of predicates explicitly can opt into seeing null by annotating the parameter with @nullable or use Optional<Predicate>. QuerydslPredicateBuilder now consistently returns null in case the original parameter map is entirely empty or consists of only keys with empty value arrays as empty form submissions do. This partially reverts the work of DATACMNS-1168, which moved into the direction of returning a default Predicate value for empty maps in the first place. That however prevents us from producing empty Optionals. Related tickets: DATACMNS-1168.
1 parent 9b63646 commit 664e661

File tree

4 files changed

+86
-26
lines changed

4 files changed

+86
-26
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public Predicate getPredicate(TypeInformation<?> type, MultiValueMap<String, Str
9191
BooleanBuilder builder = new BooleanBuilder();
9292

9393
if (values.isEmpty()) {
94-
return builder;
94+
return builder.getValue();
9595
}
9696

9797
for (Entry<String, List<String>> entry : values.entrySet()) {

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

+26-7
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@
2121
import java.util.Optional;
2222

2323
import org.springframework.core.MethodParameter;
24+
import org.springframework.core.ResolvableType;
2425
import org.springframework.core.convert.ConversionService;
2526
import org.springframework.core.convert.support.DefaultConversionService;
2627
import org.springframework.data.querydsl.binding.QuerydslBinderCustomizer;
28+
import org.springframework.data.querydsl.binding.QuerydslBindings;
2729
import org.springframework.data.querydsl.binding.QuerydslBindingsFactory;
2830
import org.springframework.data.querydsl.binding.QuerydslPredicate;
2931
import org.springframework.data.querydsl.binding.QuerydslPredicateBuilder;
@@ -38,6 +40,7 @@
3840
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
3941
import org.springframework.web.method.support.ModelAndViewContainer;
4042

43+
import com.querydsl.core.BooleanBuilder;
4144
import com.querydsl.core.types.Predicate;
4245

4346
/**
@@ -50,6 +53,10 @@
5053
*/
5154
public class QuerydslPredicateArgumentResolver implements HandlerMethodArgumentResolver {
5255

56+
private static final ResolvableType PREDICATE = ResolvableType.forClass(Predicate.class);
57+
private static final ResolvableType OPTIONAL_OF_PREDICATE = ResolvableType.forClassWithGenerics(Optional.class,
58+
PREDICATE);
59+
5360
private final QuerydslBindingsFactory bindingsFactory;
5461
private final QuerydslPredicateBuilder predicateBuilder;
5562

@@ -74,7 +81,9 @@ public QuerydslPredicateArgumentResolver(QuerydslBindingsFactory factory,
7481
@Override
7582
public boolean supportsParameter(MethodParameter parameter) {
7683

77-
if (Predicate.class.equals(parameter.getParameterType())) {
84+
ResolvableType type = ResolvableType.forMethodParameter(parameter);
85+
86+
if (PREDICATE.isAssignableFrom(type) || OPTIONAL_OF_PREDICATE.isAssignableFrom(type)) {
7887
return true;
7988
}
8089

@@ -92,7 +101,7 @@ public boolean supportsParameter(MethodParameter parameter) {
92101
*/
93102
@Nullable
94103
@Override
95-
public Predicate resolveArgument(MethodParameter parameter, @Nullable ModelAndViewContainer mavContainer,
104+
public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewContainer mavContainer,
96105
NativeWebRequest webRequest, @Nullable WebDataBinderFactory binderFactory) throws Exception {
97106

98107
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
@@ -105,13 +114,23 @@ public Predicate resolveArgument(MethodParameter parameter, @Nullable ModelAndVi
105114
.ofNullable(parameter.getParameterAnnotation(QuerydslPredicate.class));
106115
TypeInformation<?> domainType = extractTypeInfo(parameter).getRequiredActualType();
107116

108-
Optional<Class<? extends QuerydslBinderCustomizer<?>>> bindings = annotation//
109-
.map(QuerydslPredicate::bindings)//
117+
Optional<Class<? extends QuerydslBinderCustomizer<?>>> bindingsAnnotation = annotation //
118+
.map(QuerydslPredicate::bindings) //
110119
.map(CastUtils::cast);
111120

112-
return predicateBuilder.getPredicate(domainType, parameters,
113-
bindings.map(it -> bindingsFactory.createBindingsFor(domainType, it))
114-
.orElseGet(() -> bindingsFactory.createBindingsFor(domainType)));
121+
QuerydslBindings bindings = bindingsAnnotation //
122+
.map(it -> bindingsFactory.createBindingsFor(domainType, it)) //
123+
.orElseGet(() -> bindingsFactory.createBindingsFor(domainType));
124+
125+
Predicate result = predicateBuilder.getPredicate(domainType, parameters, bindings);
126+
127+
if (!parameter.isOptional() && result == null) {
128+
return new BooleanBuilder();
129+
}
130+
131+
return OPTIONAL_OF_PREDICATE.isAssignableFrom(ResolvableType.forMethodParameter(parameter)) //
132+
? Optional.ofNullable(result) //
133+
: result;
115134
}
116135

117136
/**

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

+3-5
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import org.springframework.util.MultiValueMap;
3939

4040
import com.querydsl.collections.CollQueryFactory;
41-
import com.querydsl.core.BooleanBuilder;
4241
import com.querydsl.core.types.Constant;
4342
import com.querydsl.core.types.Predicate;
4443

@@ -75,9 +74,8 @@ public void getPredicateShouldThrowErrorWhenBindingContextIsNull() {
7574
}
7675

7776
@Test // DATACMNS-669, DATACMNS-1168
78-
public void getPredicateShouldReturnEmptyPredicateWhenPropertiesAreEmpty() {
79-
assertThat(builder.getPredicate(ClassTypeInformation.OBJECT, values, DEFAULT_BINDINGS))
80-
.isEqualTo(new BooleanBuilder());
77+
public void getPredicateShouldReturnNullWhenPropertiesAreEmpty() {
78+
assertThat(builder.getPredicate(ClassTypeInformation.OBJECT, values, DEFAULT_BINDINGS)).isNull();
8179
}
8280

8381
@Test // DATACMNS-669
@@ -87,7 +85,7 @@ public void resolveArgumentShouldCreateSingleStringParameterPredicateCorrectly()
8785

8886
Predicate predicate = builder.getPredicate(USER_TYPE, values, DEFAULT_BINDINGS);
8987

90-
assertThat(predicate).isEqualTo((Predicate) QUser.user.firstname.eq("Oliver"));
88+
assertThat(predicate).isEqualTo(QUser.user.firstname.eq("Oliver"));
9189

9290
List<User> result = CollQueryFactory.from(QUser.user, Users.USERS).where(predicate).fetchResults().getResults();
9391

‎src/test/java/org/springframework/data/web/querydsl/QuerydslPredicateArgumentResolverUnitTests.java

+56-13
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import org.junit.Before;
2424
import org.junit.Test;
25-
2625
import org.springframework.core.MethodParameter;
2726
import org.springframework.data.domain.Page;
2827
import org.springframework.data.domain.Pageable;
@@ -38,6 +37,7 @@
3837
import org.springframework.hateoas.EntityModel;
3938
import org.springframework.http.HttpEntity;
4039
import org.springframework.http.ResponseEntity;
40+
import org.springframework.lang.Nullable;
4141
import org.springframework.mock.web.MockHttpServletRequest;
4242
import org.springframework.test.util.ReflectionTestUtils;
4343
import org.springframework.web.context.request.ServletWebRequest;
@@ -94,7 +94,7 @@ public void resolveArgumentShouldCreateSingleStringParameterPredicateCorrectly()
9494

9595
request.addParameter("firstname", "rand");
9696

97-
Predicate predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null,
97+
Object predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null,
9898
new ServletWebRequest(request), null);
9999

100100
assertThat(predicate).isEqualTo(QUser.user.firstname.eq("rand"));
@@ -106,7 +106,7 @@ public void resolveArgumentShouldCreateMultipleParametersPredicateCorrectly() th
106106
request.addParameter("firstname", "rand");
107107
request.addParameter("lastname", "al'thor");
108108

109-
Predicate predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null,
109+
Object predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null,
110110
new ServletWebRequest(request), null);
111111

112112
assertThat(predicate).isEqualTo(QUser.user.firstname.eq("rand").and(QUser.user.lastname.eq("al'thor")));
@@ -117,23 +117,23 @@ public void resolveArgumentShouldCreateNestedObjectPredicateCorrectly() throws E
117117

118118
request.addParameter("address.city", "two rivers");
119119

120-
Predicate predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null,
120+
Object predicate = resolver.resolveArgument(getMethodParameterFor("simpleFind", Predicate.class), null,
121121
new ServletWebRequest(request), null);
122122

123123
BooleanExpression eq = QUser.user.address.city.eq("two rivers");
124124

125-
assertThat(predicate).isEqualTo((Predicate) eq);
125+
assertThat(predicate).isEqualTo(eq);
126126
}
127127

128128
@Test // DATACMNS-669
129129
public void resolveArgumentShouldResolveTypePropertyFromPageCorrectly() throws Exception {
130130

131131
request.addParameter("address.city", "tar valon");
132132

133-
Predicate predicate = resolver.resolveArgument(getMethodParameterFor("pagedFind", Predicate.class, Pageable.class),
133+
Object predicate = resolver.resolveArgument(getMethodParameterFor("pagedFind", Predicate.class, Pageable.class),
134134
null, new ServletWebRequest(request), null);
135135

136-
assertThat(predicate).isEqualTo((Predicate) QUser.user.address.city.eq("tar valon"));
136+
assertThat(predicate).isEqualTo(QUser.user.address.city.eq("tar valon"));
137137
}
138138

139139
@Test // DATACMNS-669
@@ -142,7 +142,7 @@ public void resolveArgumentShouldHonorCustomSpecification() throws Exception {
142142
request.addParameter("firstname", "egwene");
143143
request.addParameter("lastname", "al'vere");
144144

145-
Predicate predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null,
145+
Object predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null,
146146
new ServletWebRequest(request), null);
147147

148148
assertThat(predicate).isEqualTo(
@@ -154,21 +154,21 @@ public void shouldCreatePredicateForNonStringPropertyCorrectly() throws Exceptio
154154

155155
request.addParameter("inceptionYear", "978");
156156

157-
Predicate predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null,
157+
Object predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null,
158158
new ServletWebRequest(request), null);
159159

160-
assertThat(predicate).isEqualTo((Predicate) QUser.user.inceptionYear.eq(978L));
160+
assertThat(predicate).isEqualTo(QUser.user.inceptionYear.eq(978L));
161161
}
162162

163163
@Test // DATACMNS-669
164164
public void shouldCreatePredicateForNonStringListPropertyCorrectly() throws Exception {
165165

166166
request.addParameter("inceptionYear", new String[] { "978", "998" });
167167

168-
Predicate predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null,
168+
Object predicate = resolver.resolveArgument(getMethodParameterFor("specificFind", Predicate.class), null,
169169
new ServletWebRequest(request), null);
170170

171-
assertThat(predicate).isEqualTo((Predicate) QUser.user.inceptionYear.in(978L, 998L));
171+
assertThat(predicate).isEqualTo(QUser.user.inceptionYear.in(978L, 998L));
172172
}
173173

174174
@Test // DATACMNS-669
@@ -190,7 +190,7 @@ public void extractTypeInformationShouldUseTypeExtractedFromMethodReturnTypeIfPr
190190
TypeInformation<?> type = ReflectionTestUtils.invokeMethod(resolver, "extractTypeInfo",
191191
getMethodParameterFor("predicateWithoutAnnotation", Predicate.class));
192192

193-
assertThat(type).isEqualTo((TypeInformation) ClassTypeInformation.from(User.class));
193+
assertThat(type).isEqualTo(ClassTypeInformation.from(User.class));
194194
}
195195

196196
@Test // DATACMNS-669
@@ -205,6 +205,43 @@ public void detectsDomainTypesCorrectly() {
205205
assertThat(extractTypeInfo(getMethodParameterFor("forModelAndView"))).isEqualTo(MODELA_AND_VIEW_TYPE);
206206
}
207207

208+
@Test // DATACMNS-1593
209+
public void returnsEmptyPredicateForEmptyInput() throws Exception {
210+
211+
MethodParameter parameter = getMethodParameterFor("predicateWithoutAnnotation", Predicate.class);
212+
213+
request.addParameter("firstname", "");
214+
215+
assertThat(resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null)) //
216+
.isNotNull();
217+
}
218+
219+
@Test // DATACMNS-1635
220+
public void forwardsNullValueForNullablePredicate() throws Exception {
221+
222+
MethodParameter parameter = getMethodParameterFor("nullablePredicateWithoutAnnotation", Predicate.class);
223+
224+
request.addParameter("firstname", "");
225+
226+
assertThat(resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null)).isNull();
227+
}
228+
229+
@Test // DATACMNS-1635
230+
public void returnsOptionalIfDeclared() throws Exception {
231+
232+
MethodParameter parameter = getMethodParameterFor("optionalPredicateWithoutAnnotation", Optional.class);
233+
234+
request.addParameter("firstname", "");
235+
236+
assertThat(resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null)) //
237+
.isInstanceOfSatisfying(Optional.class, it -> assertThat(it).isEmpty());
238+
239+
request.addParameter("lastname", "Matthews");
240+
241+
assertThat(resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null)) //
242+
.isInstanceOfSatisfying(Optional.class, it -> assertThat(it).isPresent());
243+
}
244+
208245
private static MethodParameter getMethodParameterFor(String methodName, Class<?>... args) throws RuntimeException {
209246

210247
try {
@@ -244,6 +281,12 @@ static interface Sample {
244281
ModelAndView forModelAndView();
245282

246283
ResponseEntity<EntityModel<User>> forResourceOfUser();
284+
285+
// Nullability
286+
287+
User nullablePredicateWithoutAnnotation(@Nullable Predicate predicate);
288+
289+
User optionalPredicateWithoutAnnotation(Optional<Predicate> predicate);
247290
}
248291

249292
public static class SampleRepo implements QuerydslBinderCustomizer<QUser> {

0 commit comments

Comments
 (0)