Skip to content

Commit 7e1712a

Browse files
mp911demp911de
committed
Refine @PageableDefault and @SortDefault annotations.
We now use MergedAnnotations to evaluate annotation attributes and to use aliasing across attributes. Also, SortDefault is not repeatable. Closes #2657
1 parent 3a6b04d commit 7e1712a

10 files changed

+71
-68
lines changed

‎src/main/java/org/springframework/data/web/PageableDefault.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.lang.annotation.RetentionPolicy;
2222
import java.lang.annotation.Target;
2323

24+
import org.springframework.core.annotation.AliasFor;
2425
import org.springframework.data.domain.Sort.Direction;
2526
import org.springframework.data.web.SortDefault.SortDefaults;
2627

@@ -31,6 +32,7 @@
3132
*
3233
* @since 1.6
3334
* @author Oliver Gierke
35+
* @author Mark Paluch
3436
*/
3537
@Documented
3638
@Retention(RetentionPolicy.RUNTIME)
@@ -43,22 +45,24 @@
4345
*
4446
* @return
4547
*/
48+
@AliasFor("size")
4649
int value() default 10;
4750

4851
/**
4952
* The default-size the injected {@link org.springframework.data.domain.Pageable} should get if no corresponding
50-
* parameter defined in request (default is 10).
53+
* parameter defined in request (default is {@code 10}).
5154
*/
55+
@AliasFor("value")
5256
int size() default 10;
5357

5458
/**
55-
* The default-pagenumber the injected {@link org.springframework.data.domain.Pageable} should get if no corresponding
56-
* parameter defined in request (default is 0).
59+
* The default page number the injected {@link org.springframework.data.domain.Pageable} should use if no
60+
* corresponding parameter defined in request (default is {@code 0}).
5761
*/
5862
int page() default 0;
5963

6064
/**
61-
* The properties to sort by by default. If unset, no sorting will be applied at all.
65+
* The properties to sort by default. If unset, no sorting will be applied at all.
6266
*
6367
* @return
6468
*/

‎src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverSupport.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
import org.springframework.beans.factory.annotation.Qualifier;
2424
import org.springframework.core.MethodParameter;
25+
import org.springframework.core.annotation.MergedAnnotation;
26+
import org.springframework.core.annotation.MergedAnnotations;
2527
import org.springframework.data.domain.PageRequest;
2628
import org.springframework.data.domain.Pageable;
2729
import org.springframework.data.domain.Sort;
@@ -238,30 +240,34 @@ protected String getParameterNameToUse(String source, @Nullable MethodParameter
238240

239241
private Pageable getDefaultFromAnnotationOrFallback(MethodParameter methodParameter) {
240242

241-
PageableDefault defaults = methodParameter.getParameterAnnotation(PageableDefault.class);
243+
MergedAnnotation<PageableDefault> defaults = MergedAnnotations.from(methodParameter.getParameterAnnotations())
244+
.get(PageableDefault.class);
242245

243-
if (defaults != null) {
246+
if (defaults.isPresent()) {
244247
return getDefaultPageRequestFrom(methodParameter, defaults);
245248
}
246249

247250
return fallbackPageable;
248251
}
249252

250-
private static Pageable getDefaultPageRequestFrom(MethodParameter parameter, PageableDefault defaults) {
253+
private static Pageable getDefaultPageRequestFrom(MethodParameter parameter,
254+
MergedAnnotation<PageableDefault> defaults) {
251255

252-
int defaultPageNumber = defaults.page();
253-
Integer defaultPageSize = getSpecificPropertyOrDefaultFromValue(defaults, "size");
256+
int defaultPageNumber = defaults.getInt("page");
257+
int defaultPageSize = defaults.getInt("size");
254258

255259
if (defaultPageSize < 1) {
256260
Method annotatedMethod = parameter.getMethod();
257261
throw new IllegalStateException(String.format(INVALID_DEFAULT_PAGE_SIZE, annotatedMethod));
258262
}
259263

260-
if (defaults.sort().length == 0) {
264+
String[] sort = defaults.getStringArray("sort");
265+
if (sort.length == 0) {
261266
return PageRequest.of(defaultPageNumber, defaultPageSize);
262267
}
263268

264-
return PageRequest.of(defaultPageNumber, defaultPageSize, defaults.direction(), defaults.sort());
269+
return PageRequest.of(defaultPageNumber, defaultPageSize, defaults.getEnum("direction", Sort.Direction.class),
270+
sort);
265271
}
266272

267273
/**
@@ -287,5 +293,4 @@ private Optional<Integer> parseAndApplyBoundaries(@Nullable String parameter, in
287293
}
288294
}
289295

290-
291296
}

‎src/main/java/org/springframework/data/web/SortDefault.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717

1818
import java.lang.annotation.Documented;
1919
import java.lang.annotation.ElementType;
20+
import java.lang.annotation.Repeatable;
2021
import java.lang.annotation.Retention;
2122
import java.lang.annotation.RetentionPolicy;
2223
import java.lang.annotation.Target;
2324

25+
import org.springframework.core.annotation.AliasFor;
2426
import org.springframework.data.domain.Sort;
2527
import org.springframework.data.domain.Sort.Direction;
2628

@@ -30,24 +32,28 @@
3032
*
3133
* @since 1.6
3234
* @author Oliver Gierke
35+
* @author Mark Palich
3336
*/
3437
@Documented
3538
@Retention(RetentionPolicy.RUNTIME)
3639
@Target(ElementType.PARAMETER)
40+
@Repeatable(SortDefault.SortDefaults.class)
3741
public @interface SortDefault {
3842

3943
/**
4044
* Alias for {@link #sort()} to make a declaration configuring fields only more concise.
4145
*
4246
* @return
4347
*/
48+
@AliasFor("sort")
4449
String[] value() default {};
4550

4651
/**
47-
* The properties to sort by by default. If unset, no sorting will be applied at all.
52+
* The properties to sort by default. If unset, no sorting will be applied at all.
4853
*
4954
* @return
5055
*/
56+
@AliasFor("value")
5157
String[] sort() default {};
5258

5359
/**

‎src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolverSupport.java

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
import java.util.function.Consumer;
2424

2525
import org.springframework.core.MethodParameter;
26+
import org.springframework.core.annotation.MergedAnnotation;
27+
import org.springframework.core.annotation.MergedAnnotations;
28+
import org.springframework.core.annotation.RepeatableContainers;
2629
import org.springframework.data.domain.Sort;
2730
import org.springframework.data.domain.Sort.Direction;
2831
import org.springframework.data.domain.Sort.Order;
@@ -121,31 +124,26 @@ public void setFallbackSort(Sort fallbackSort) {
121124
*/
122125
protected Sort getDefaultFromAnnotationOrFallback(MethodParameter parameter) {
123126

124-
SortDefaults annotatedDefaults = parameter.getParameterAnnotation(SortDefaults.class);
125-
SortDefault annotatedDefault = parameter.getParameterAnnotation(SortDefault.class);
127+
MergedAnnotations mergedAnnotations = MergedAnnotations.from(parameter, parameter.getParameterAnnotations(),
128+
RepeatableContainers.of(SortDefault.class, SortDefaults.class));
126129

127-
if (annotatedDefault != null && annotatedDefaults != null) {
128-
throw new IllegalArgumentException(
129-
String.format("Cannot use both @%s and @%s on parameter %s; Move %s into %s to define sorting order",
130-
SORT_DEFAULTS_NAME, SORT_DEFAULT_NAME, parameter.toString(), SORT_DEFAULT_NAME, SORT_DEFAULTS_NAME));
131-
}
130+
List<MergedAnnotation<SortDefault>> annotations = mergedAnnotations.stream(SortDefault.class).toList();
132131

133-
if (annotatedDefault != null) {
134-
return appendOrCreateSortTo(annotatedDefault, Sort.unsorted());
132+
if (annotations.isEmpty()) {
133+
return fallbackSort;
135134
}
136135

137-
if (annotatedDefaults != null) {
138-
139-
Sort sort = Sort.unsorted();
136+
if (annotations.size() == 1) {
137+
return appendOrCreateSortTo(annotations.get(0), Sort.unsorted());
138+
}
140139

141-
for (SortDefault currentAnnotatedDefault : annotatedDefaults.value()) {
142-
sort = appendOrCreateSortTo(currentAnnotatedDefault, sort);
143-
}
140+
Sort sort = Sort.unsorted();
144141

145-
return sort;
142+
for (MergedAnnotation<SortDefault> currentAnnotatedDefault : annotations) {
143+
sort = appendOrCreateSortTo(currentAnnotatedDefault, sort);
146144
}
147145

148-
return fallbackSort;
146+
return sort;
149147
}
150148

151149
/**
@@ -156,9 +154,9 @@ protected Sort getDefaultFromAnnotationOrFallback(MethodParameter parameter) {
156154
* @param sortOrNull
157155
* @return
158156
*/
159-
private Sort appendOrCreateSortTo(SortDefault sortDefault, Sort sortOrNull) {
157+
private Sort appendOrCreateSortTo(MergedAnnotation<SortDefault> sortDefault, Sort sortOrNull) {
160158

161-
String[] fields = SpringDataAnnotationUtils.getSpecificPropertyOrDefaultFromValue(sortDefault, "sort");
159+
String[] fields = sortDefault.getStringArray("sort");
162160

163161
if (fields.length == 0) {
164162
return Sort.unsorted();
@@ -167,8 +165,8 @@ private Sort appendOrCreateSortTo(SortDefault sortDefault, Sort sortOrNull) {
167165
List<Order> orders = new ArrayList<>(fields.length);
168166
for (String field : fields) {
169167

170-
Order order = new Order(sortDefault.direction(), field);
171-
orders.add(sortDefault.caseSensitive() ? order : order.ignoreCase());
168+
Order order = new Order(sortDefault.getEnum("direction", Sort.Direction.class), field);
169+
orders.add(sortDefault.getBoolean("caseSensitive") ? order : order.ignoreCase());
172170
}
173171

174172
return sortOrNull.and(Sort.by(orders));

‎src/main/java/org/springframework/data/web/SpringDataAnnotationUtils.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,10 @@ private static boolean containsMoreThanOnePageableParameter(Method method) {
9191
* @param annotation must not be {@literal null}.
9292
* @param property must not be {@literal null} or empty.
9393
* @return
94+
* @deprecated since 3.0 as this method is no longer used within the Framework.
9495
*/
9596
@SuppressWarnings("unchecked")
97+
@Deprecated
9698
public static <T> T getSpecificPropertyOrDefaultFromValue(Annotation annotation, String property) {
9799

98100
Object propertyDefaultValue = AnnotationUtils.getDefaultValue(annotation, property);

‎src/test/java/org/springframework/data/web/PageableDefaultUnitTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ void simpleDefaultWithContaineredExternalSort() throws Exception {
9696
}
9797

9898
@Test
99-
void rejectsInvalidQulifiers() throws Exception {
99+
void rejectsInvalidQualifiers() {
100100

101101
var parameter = TestUtils.getParameterOfMethod(getControllerClass(), "invalidQualifiers",
102102
Pageable.class, Pageable.class);
@@ -110,7 +110,7 @@ void rejectsInvalidQulifiers() throws Exception {
110110
}
111111

112112
@Test
113-
void rejectsNoQualifiers() throws Exception {
113+
void rejectsNoQualifiers() {
114114

115115
var parameter = TestUtils.getParameterOfMethod(getControllerClass(), "noQualifiers", Pageable.class,
116116
Pageable.class);

‎src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
* @author Oliver Gierke
3737
* @author Nick Williams
3838
* @author Vedran Pavic
39+
* @author Mark Paluch
3940
*/
4041
class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefaultUnitTests {
4142

@@ -106,13 +107,20 @@ void usesDefaultPageSizeIfRequestPageSizeIsLessThanOne() throws Exception {
106107
@Test // DATACMNS-377
107108
void rejectsInvalidCustomDefaultForPageSize() throws Exception {
108109

109-
var parameter = new MethodParameter(Sample.class.getMethod("invalidDefaultPageSize", Pageable.class),
110-
0);
110+
var parameter = new MethodParameter(Sample.class.getMethod("invalidDefaultPageSize", Pageable.class), 0);
111111

112112
assertThatIllegalStateException().isThrownBy(() -> assertSupportedAndResult(parameter, DEFAULT_PAGE_REQUEST)) //
113113
.withMessageContaining("invalidDefaultPageSize");
114114
}
115115

116+
@Test // GH-2657
117+
void considersValueAlias() throws Exception {
118+
119+
var parameter = new MethodParameter(Sample.class.getMethod("valuePageSize", Pageable.class), 0);
120+
121+
assertSupportedAndResult(parameter, PageRequest.of(0, 2));
122+
}
123+
116124
@Test // DATACMNS-408
117125
void fallsBackToFirstPageIfNegativePageNumberIsGiven() throws Exception {
118126

@@ -289,6 +297,8 @@ interface Sample {
289297

290298
void invalidDefaultPageSize(@PageableDefault(size = 0) Pageable pageable);
291299

300+
void valuePageSize(@PageableDefault(2) Pageable pageable);
301+
292302
void simpleDefault(@PageableDefault(size = PAGE_SIZE, page = PAGE_NUMBER) Pageable pageable);
293303

294304
void simpleDefaultWithSort(

‎src/test/java/org/springframework/data/web/ReactiveSortHandlerMethodArgumentResolverUnitTests.java

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -153,20 +153,9 @@ void sortParamIsInvalidPropertyWhenMultiProperty() {
153153
assertThat(resolve(request, PARAMETER)).isEqualTo(Sort.by(DESC, "property1"));
154154
}
155155

156-
@Test // DATACMNS-1211
157-
void rejectsDoubleAnnotatedMethod() {
158-
159-
var parameter = getParameterOfMethod("invalid");
160-
161-
var resolver = new ReactiveSortHandlerMethodArgumentResolver();
162-
assertThat(resolver.supportsParameter(parameter)).isTrue();
163-
164-
assertThatIllegalArgumentException()
165-
.isThrownBy(() ->
166-
resolver.resolveArgumentValue(parameter, null,
167-
MockServerWebExchange.from(TestUtils.getWebfluxRequest())))
168-
.withMessageContaining(SortDefault.class.getSimpleName())
169-
.withMessageContaining(SortDefaults.class.getSimpleName()).withMessageContaining(parameter.toString());
156+
@Test // GH-2657
157+
void considersRepeatableAnnotation() {
158+
assertSupportedAndResolvedTo(getParameterOfMethod("repeatable"), Sort.by("one", "two", "three").ascending());
170159
}
171160

172161
@Test // DATACMNS-1211
@@ -261,6 +250,6 @@ void simpleDefaultWithDirection(
261250

262251
void containeredDefault(@SortDefaults(@SortDefault({ "foo", "bar" })) Sort sort);
263252

264-
void invalid(@SortDefaults(@SortDefault({ "foo", "bar" })) @SortDefault({ "bar", "foo" }) Sort sort);
253+
void repeatable(@SortDefault({ "one", "two" }) @SortDefault({ "three" }) Sort sort);
265254
}
266255
}

‎src/test/java/org/springframework/data/web/SortDefaultUnitTests.java

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.springframework.data.domain.Sort;
2626
import org.springframework.data.domain.Sort.Direction;
2727
import org.springframework.data.domain.Sort.Order;
28-
import org.springframework.data.web.SortDefault.SortDefaults;
2928
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
3029

3130
/**
@@ -91,19 +90,9 @@ void rejectsNonSortParameter() {
9190
assertThat(getResolver().supportsParameter(parameter)).isFalse();
9291
}
9392

94-
@Test
95-
void rejectsDoubleAnnotatedMethod() {
96-
97-
var parameter = getParameterOfMethod("invalid");
98-
99-
HandlerMethodArgumentResolver resolver = new SortHandlerMethodArgumentResolver();
100-
assertThat(resolver.supportsParameter(parameter)).isTrue();
101-
102-
assertThatIllegalArgumentException()
103-
.isThrownBy(() -> resolver.resolveArgument(parameter, null, TestUtils.getWebRequest(), null)) //
104-
.withMessageContaining(SortDefault.class.getSimpleName()) //
105-
.withMessageContaining(SortDefaults.class.getSimpleName()) //
106-
.withMessageContaining(parameter.toString());
93+
@Test // GH-2657
94+
void considersRepeatableAnnotation() throws Exception {
95+
assertSupportedAndResolvedTo(getParameterOfMethod("repeatable"), Sort.by("one", "two", "three").ascending());
10796
}
10897

10998
@Test

‎src/test/java/org/springframework/data/web/SortHandlerMethodArgumentResolverUnitTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ void simpleDefaultWithDirectionCaseInsensitive(
324324

325325
void containeredDefault(@SortDefaults(@SortDefault({ "foo", "bar" })) Sort sort);
326326

327-
void invalid(@SortDefaults(@SortDefault({ "foo", "bar" })) @SortDefault({ "bar", "foo" }) Sort sort);
327+
void repeatable(@SortDefault({ "one", "two" }) @SortDefault({ "three" }) Sort sort);
328328

329329
void emptyQualifier(@Qualifier Sort sort);
330330

0 commit comments

Comments
 (0)