DATACMNS-1551 - Polishing.

Method order in SortHandlerMethodArgumentResolver.

Author: odrotbohm

src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolver.java

@@ -95,6 +95,19 @@ public void setQualifierDelimiter(String qualifierDelimiter) {
 		this.qualifierDelimiter = qualifierDelimiter == null ? DEFAULT_QUALIFIER_DELIMITER : qualifierDelimiter;
 	}
 
+	/**
+	 * Configures the {@link Sort} to be used as fallback in case no {@link SortDefault} or {@link SortDefaults} (the
+	 * latter only supported in legacy mode) can be found at the method parameter to be resolved.
+	 * <p>
+	 * If you set this to {@literal null}, be aware that you controller methods will get {@literal null} handed into them
+	 * in case no {@link Sort} data can be found in the request.
+	 *
+	 * @param fallbackSort the {@link Sort} to be used as general fallback.
+	 */
+	public void setFallbackSort(Sort fallbackSort) {
+		this.fallbackSort = fallbackSort;
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.web.method.support.HandlerMethodArgumentResolver#supportsParameter(org.springframework.core.MethodParameter)
@@ -128,79 +141,81 @@ public Sort resolveArgument(MethodParameter parameter, @Nullable ModelAndViewCon
 	}
 
 	/**
-	 * Reads the default {@link Sort} to be used from the given {@link MethodParameter}. Rejects the parameter if both an
-	 * {@link SortDefaults} and {@link SortDefault} annotation is found as we cannot build a reliable {@link Sort}
-	 * instance then (property ordering).
+	 * Returns the sort parameter to be looked up from the request. Potentially applies qualifiers to it.
 	 *
-	 * @param parameter will never be {@literal null}.
-	 * @return the default {@link Sort} instance derived from the parameter annotations or the configured fallback-sort
-	 *         {@link #setFallbackSort(Sort)}.
+	 * @param parameter can be {@literal null}.
+	 * @return
 	 */
-	private Sort getDefaultFromAnnotationOrFallback(MethodParameter parameter) {
-
-		SortDefaults annotatedDefaults = parameter.getParameterAnnotation(SortDefaults.class);
-		SortDefault annotatedDefault = parameter.getParameterAnnotation(SortDefault.class);
-
-		if (annotatedDefault != null && annotatedDefaults != null) {
-			throw new IllegalArgumentException(
-					String.format("Cannot use both @%s and @%s on parameter %s! Move %s into %s to define sorting order!",
-							SORT_DEFAULTS_NAME, SORT_DEFAULT_NAME, parameter.toString(), SORT_DEFAULT_NAME, SORT_DEFAULTS_NAME));
-		}
-
-		if (annotatedDefault != null) {
-			return appendOrCreateSortTo(annotatedDefault, Sort.unsorted());
-		}
-
-		if (annotatedDefaults != null) {
+	protected String getSortParameter(@Nullable MethodParameter parameter) {
 
-			Sort sort = Sort.unsorted();
+		StringBuilder builder = new StringBuilder();
 
-			for (SortDefault currentAnnotatedDefault : annotatedDefaults.value()) {
-				sort = appendOrCreateSortTo(currentAnnotatedDefault, sort);
-			}
+		Qualifier qualifier = parameter != null ? parameter.getParameterAnnotation(Qualifier.class) : null;
 
-			return sort;
+		if (qualifier != null) {
+			builder.append(qualifier.value()).append(qualifierDelimiter);
 		}
 
-		return fallbackSort;
+		return builder.append(sortParameter).toString();
 	}
 
 	/**
-	 * Creates a new {@link Sort} instance from the given {@link SortDefault} or appends it to the given {@link Sort}
-	 * instance if it's not {@literal null}.
+	 * Folds the given {@link Sort} instance into a {@link List} of sort expressions, accumulating {@link Order} instances
+	 * of the same direction into a single expression if they are in order.
 	 *
-	 * @param sortDefault
-	 * @param sortOrNull
+	 * @param sort must not be {@literal null}.
 	 * @return
 	 */
-	private Sort appendOrCreateSortTo(SortDefault sortDefault, Sort sortOrNull) {
+	protected List<String> foldIntoExpressions(Sort sort) {
 
-		String[] fields = SpringDataAnnotationUtils.getSpecificPropertyOrDefaultFromValue(sortDefault, "sort");
+		List<String> expressions = new ArrayList<>();
+		ExpressionBuilder builder = null;
+
+		for (Order order : sort) {
 
-		if (fields.length == 0) {
-			return Sort.unsorted();
+			Direction direction = order.getDirection();
+
+			if (builder == null) {
+				builder = new ExpressionBuilder(direction);
+			} else if (!builder.hasSameDirectionAs(order)) {
+				builder.dumpExpressionIfPresentInto(expressions);
+				builder = new ExpressionBuilder(direction);
+			}
+
+			builder.add(order.getProperty());
 		}
 
-		return sortOrNull.and(Sort.by(sortDefault.direction(), fields));
+		return builder == null ? Collections.emptyList() : builder.dumpExpressionIfPresentInto(expressions);
 	}
 
 	/**
-	 * Returns the sort parameter to be looked up from the request. Potentially applies qualifiers to it.
+	 * Folds the given {@link Sort} instance into two expressions. The first being the property list, the second being the
+	 * direction.
 	 *
-	 * @param parameter can be {@literal null}.
+	 * @throws IllegalArgumentException if a {@link Sort} with multiple {@link Direction}s has been handed in.
+	 * @param sort must not be {@literal null}.
 	 * @return
 	 */
-	protected String getSortParameter(@Nullable MethodParameter parameter) {
+	protected List<String> legacyFoldExpressions(Sort sort) {
 
-		StringBuilder builder = new StringBuilder();
+		List<String> expressions = new ArrayList<>();
+		ExpressionBuilder builder = null;
 
-		Qualifier qualifier = parameter != null ? parameter.getParameterAnnotation(Qualifier.class) : null;
+		for (Order order : sort) {
 
-		if (qualifier != null) {
-			builder.append(qualifier.value()).append(qualifierDelimiter);
+			Direction direction = order.getDirection();
+
+			if (builder == null) {
+				builder = new ExpressionBuilder(direction);
+			} else if (!builder.hasSameDirectionAs(order)) {
+				throw new IllegalArgumentException(String.format(
+						"%s in legacy configuration only supports a single direction to sort by!", getClass().getSimpleName()));
+			}
+
+			builder.add(order.getProperty());
 		}
 
-		return builder.append(sortParameter).toString();
+		return builder == null ? Collections.emptyList() : builder.dumpExpressionIfPresentInto(expressions);
 	}
 
 	/**
@@ -226,10 +241,12 @@ Sort parseParameterIntoSort(String[] source, String delimiter) {
 					.filter(SortHandlerMethodArgumentResolver::notOnlyDots) //
 					.toArray(String[]::new);
 
-			Optional<Direction> direction = elements.length == 0 ? Optional.empty()
+			Optional<Direction> direction = elements.length == 0 //
+					? Optional.empty() //
 					: Direction.fromOptionalString(elements[elements.length - 1]);
 
-			int lastIndex = direction.map(it -> elements.length - 1).orElseGet(() -> elements.length);
+			int lastIndex = direction.map(it -> elements.length - 1) //
+					.orElseGet(() -> elements.length);
 
 			for (int i = 0; i < lastIndex; i++) {
 				toOrder(elements[i], direction).ifPresent(allOrders::add);
@@ -240,81 +257,76 @@ Sort parseParameterIntoSort(String[] source, String delimiter) {
 	}
 
 	/**
-	 * Returns whether the given source {@link String} consists of dots only.
+	 * Reads the default {@link Sort} to be used from the given {@link MethodParameter}. Rejects the parameter if both an
+	 * {@link SortDefaults} and {@link SortDefault} annotation is found as we cannot build a reliable {@link Sort}
+	 * instance then (property ordering).
 	 *
-	 * @param source must not be {@literal null}.
-	 * @return
+	 * @param parameter will never be {@literal null}.
+	 * @return the default {@link Sort} instance derived from the parameter annotations or the configured fallback-sort
+	 *         {@link #setFallbackSort(Sort)}.
 	 */
-	private static boolean notOnlyDots(String source) {
-		return StringUtils.hasText(source.replace(".", ""));
-	}
+	private Sort getDefaultFromAnnotationOrFallback(MethodParameter parameter) {
 
-	private static Optional<Order> toOrder(String property, Optional<Direction> direction) {
+		SortDefaults annotatedDefaults = parameter.getParameterAnnotation(SortDefaults.class);
+		SortDefault annotatedDefault = parameter.getParameterAnnotation(SortDefault.class);
 
-		if (!StringUtils.hasText(property)) {
-			return Optional.empty();
+		if (annotatedDefault != null && annotatedDefaults != null) {
+			throw new IllegalArgumentException(
+					String.format("Cannot use both @%s and @%s on parameter %s! Move %s into %s to define sorting order!",
+							SORT_DEFAULTS_NAME, SORT_DEFAULT_NAME, parameter.toString(), SORT_DEFAULT_NAME, SORT_DEFAULTS_NAME));
 		}
 
-		return Optional.of(direction.map(it -> new Order(it, property)).orElseGet(() -> Order.by(property)));
-	}
-
-	/**
-	 * Folds the given {@link Sort} instance into a {@link List} of sort expressions, accumulating {@link Order} instances
-	 * of the same direction into a single expression if they are in order.
-	 *
-	 * @param sort must not be {@literal null}.
-	 * @return
-	 */
-	protected List<String> foldIntoExpressions(Sort sort) {
-
-		List<String> expressions = new ArrayList<>();
-		ExpressionBuilder builder = null;
+		if (annotatedDefault != null) {
+			return appendOrCreateSortTo(annotatedDefault, Sort.unsorted());
+		}
 
-		for (Order order : sort) {
+		if (annotatedDefaults != null) {
 
-			Direction direction = order.getDirection();
+			Sort sort = Sort.unsorted();
 
-			if (builder == null) {
-				builder = new ExpressionBuilder(direction);
-			} else if (!builder.hasSameDirectionAs(order)) {
-				builder.dumpExpressionIfPresentInto(expressions);
-				builder = new ExpressionBuilder(direction);
+			for (SortDefault currentAnnotatedDefault : annotatedDefaults.value()) {
+				sort = appendOrCreateSortTo(currentAnnotatedDefault, sort);
 			}
 
-			builder.add(order.getProperty());
+			return sort;
 		}
 
-		return builder == null ? Collections.emptyList() : builder.dumpExpressionIfPresentInto(expressions);
+		return fallbackSort;
 	}
 
 	/**
-	 * Folds the given {@link Sort} instance into two expressions. The first being the property list, the second being the
-	 * direction.
+	 * Creates a new {@link Sort} instance from the given {@link SortDefault} or appends it to the given {@link Sort}
+	 * instance if it's not {@literal null}.
 	 *
-	 * @throws IllegalArgumentException if a {@link Sort} with multiple {@link Direction}s has been handed in.
-	 * @param sort must not be {@literal null}.
+	 * @param sortDefault
+	 * @param sortOrNull
 	 * @return
 	 */
-	protected List<String> legacyFoldExpressions(Sort sort) {
-
-		List<String> expressions = new ArrayList<>();
-		ExpressionBuilder builder = null;
+	private Sort appendOrCreateSortTo(SortDefault sortDefault, Sort sortOrNull) {
 
-		for (Order order : sort) {
+		String[] fields = SpringDataAnnotationUtils.getSpecificPropertyOrDefaultFromValue(sortDefault, "sort");
 
-			Direction direction = order.getDirection();
+		return fields.length == 0 //
+				? Sort.unsorted()
+				: sortOrNull.and(Sort.by(sortDefault.direction(), fields));
+	}
 
-			if (builder == null) {
-				builder = new ExpressionBuilder(direction);
-			} else if (!builder.hasSameDirectionAs(order)) {
-				throw new IllegalArgumentException(String.format(
-						"%s in legacy configuration only supports a single direction to sort by!", getClass().getSimpleName()));
-			}
+	/**
+	 * Returns whether the given source {@link String} consists of dots only.
+	 *
+	 * @param source must not be {@literal null}.
+	 * @return
+	 */
+	private static boolean notOnlyDots(String source) {
+		return StringUtils.hasText(source.replace(".", ""));
+	}
 
-			builder.add(order.getProperty());
-		}
+	private static Optional<Order> toOrder(String property, Optional<Direction> direction) {
 
-		return builder == null ? Collections.emptyList() : builder.dumpExpressionIfPresentInto(expressions);
+		return !StringUtils.hasText(property) //
+				? Optional.empty() //
+				: Optional.of(direction.map(it -> new Order(it, property)) //
+						.orElseGet(() -> Order.by(property)));
 	}
 
 	/**
@@ -376,17 +388,4 @@ public List<String> dumpExpressionIfPresentInto(List<String> expressions) {
 			return expressions;
 		}
 	}
-
-	/**
-	 * Configures the {@link Sort} to be used as fallback in case no {@link SortDefault} or {@link SortDefaults} (the
-	 * latter only supported in legacy mode) can be found at the method parameter to be resolved.
-	 * <p>
-	 * If you set this to {@literal null}, be aware that you controller methods will get {@literal null} handed into them
-	 * in case no {@link Sort} data can be found in the request.
-	 *
-	 * @param fallbackSort the {@link Sort} to be used as general fallback.
-	 */
-	public void setFallbackSort(Sort fallbackSort) {
-		this.fallbackSort = fallbackSort;
-	}
 }