Add null handling to sort query parameters

Closes spring-projects#3152
pheyken · Sep 13, 2024 · 8134da8 · 8134da8
1 parent f305308
commit 8134da8
Show file tree

Hide file tree

Showing 3 changed files with 137 additions and 7 deletions.
diff --git a/src/main/java/org/springframework/data/web/SortDefault.java b/src/main/java/org/springframework/data/web/SortDefault.java
@@ -25,6 +25,7 @@
 import org.springframework.core.annotation.AliasFor;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
+import org.springframework.data.domain.Sort.NullHandling;
 
 /**
  * Annotation to define the default {@link Sort} options to be used when injecting a {@link Sort} instance into a
@@ -33,6 +34,7 @@
  * @since 1.6
  * @author Oliver Gierke
  * @author Mark Palich
+ * @author Petar Heyken
  */
 @Documented
 @Retention(RetentionPolicy.RUNTIME)
@@ -71,6 +73,14 @@
 	 */
 	boolean caseSensitive() default true;
 
+	/**
+	 * Specifies which null handling to apply. Defaults to {@link NullHandling#NATIVE}.
+	 *
+	 * @return
+	 * @since 3.4
+	 */
+	NullHandling nullHandling() default NullHandling.NATIVE;
+
 	/**
 	 * Wrapper annotation to allow declaring multiple {@link SortDefault} annotations on a method parameter.
 	 *

diff --git a/src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolverSupport.java b/src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolverSupport.java
@@ -28,6 +28,7 @@
 import org.springframework.core.annotation.RepeatableContainers;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
+import org.springframework.data.domain.Sort.NullHandling;
 import org.springframework.data.domain.Sort.Order;
 import org.springframework.data.web.SortDefault.SortDefaults;
 import org.springframework.lang.Nullable;
@@ -41,6 +42,7 @@
  * @author Mark Paluch
  * @author Vedran Pavic
  * @author Johannes Englmeier
+ * @author Petar Heyken
  * @see SortHandlerMethodArgumentResolver
  * @see ReactiveSortHandlerMethodArgumentResolver
  * @since 2.2
@@ -165,7 +167,14 @@ private Sort appendOrCreateSortTo(MergedAnnotation<SortDefault> sortDefault, Sor
 		List<Order> orders = new ArrayList<>(fields.length);
 		for (String field : fields) {
 
-			Order order = new Order(sortDefault.getEnum("direction", Sort.Direction.class), field);
+			Order order = new Order(sortDefault.getEnum("direction", Direction.class), field);
+
+			order = switch (sortDefault.getEnum("nullHandling", NullHandling.class)) {
+				case NATIVE -> order.nullsNative();
+				case NULLS_FIRST -> order.nullsFirst();
+				case NULLS_LAST -> order.nullsLast();
+			};
+
 			orders.add(sortDefault.getBoolean("caseSensitive") ? order : order.ignoreCase());
 		}
 
@@ -214,6 +223,7 @@ Sort parseParameterIntoSort(List<String> source, String delimiter) {
 			}
 
 			SortOrderParser.parse(part, delimiter) //
+					.parseNullHandling() //
 					.parseIgnoreCase() //
 					.parseDirection() //
 					.forEachOrder(allOrders::add);
@@ -360,22 +370,28 @@ List<String> dumpExpressionIfPresentInto(List<String> expressions) {
 	static class SortOrderParser {
 
 		private static final String IGNORECASE = "ignorecase";
+		private static final String NULLSNATIVE = "nullsnative";
+		private static final String NULLSFIRST = "nullsfirst";
+		private static final String NULLSLAST = "nullslast";
 
 		private final String[] elements;
 		private final int lastIndex;
 		private final Optional<Direction> direction;
 		private final Optional<Boolean> ignoreCase;
+		private final Optional<NullHandling> nullHandling;
 
 		private SortOrderParser(String[] elements) {
-			this(elements, elements.length, Optional.empty(), Optional.empty());
+			this(elements, elements.length, Optional.empty(), Optional.empty(), Optional.empty());
 		}
 
 		private SortOrderParser(String[] elements, int lastIndex, Optional<Direction> direction,
-				Optional<Boolean> ignoreCase) {
+				Optional<Boolean> ignoreCase, Optional<NullHandling> nullHandling) {
+
 			this.elements = elements;
 			this.lastIndex = Math.max(0, lastIndex);
 			this.direction = direction;
 			this.ignoreCase = ignoreCase;
+			this.nullHandling = nullHandling;
 		}
 
 		/**
@@ -394,16 +410,34 @@ public static SortOrderParser parse(String part, String delimiter) {
 			return new SortOrderParser(elements);
 		}
 
+		/**
+		 * Parse the {@link NullHandling} portion of the sort specification.
+		 *
+		 * @return a new parsing state object.
+		 */
+		public SortOrderParser parseNullHandling() {
+
+			Optional<NullHandling> nullHandling = lastIndex > 0 ?
+					fromOptionalNullHandlingString(elements[lastIndex - 1]) :
+					Optional.empty();
+
+			return new SortOrderParser(elements, lastIndex - (nullHandling.isPresent() ? 1 : 0), direction, ignoreCase,
+					nullHandling);
+		}
+
 		/**
 		 * Parse the {@code ignoreCase} portion of the sort specification.
 		 *
 		 * @return a new parsing state object.
 		 */
 		public SortOrderParser parseIgnoreCase() {
 
-			Optional<Boolean> ignoreCase = lastIndex > 0 ? fromOptionalString(elements[lastIndex - 1]) : Optional.empty();
+			Optional<Boolean> ignoreCase = lastIndex > 0 ?
+					fromOptionalIgnoreCaseString(elements[lastIndex - 1]) :
+					Optional.empty();
 
-			return new SortOrderParser(elements, lastIndex - (ignoreCase.isPresent() ? 1 : 0), direction, ignoreCase);
+			return new SortOrderParser(elements, lastIndex - (ignoreCase.isPresent() ? 1 : 0), direction, ignoreCase,
+					nullHandling);
 		}
 
 		/**
@@ -416,7 +450,8 @@ public SortOrderParser parseDirection() {
 			Optional<Direction> direction = lastIndex > 0 ? Direction.fromOptionalString(elements[lastIndex - 1])
 					: Optional.empty();
 
-			return new SortOrderParser(elements, lastIndex - (direction.isPresent() ? 1 : 0), direction, ignoreCase);
+			return new SortOrderParser(elements, lastIndex - (direction.isPresent() ? 1 : 0), direction, ignoreCase,
+					nullHandling);
 		}
 
 		/**
@@ -431,7 +466,24 @@ public void forEachOrder(Consumer<? super Order> callback) {
 			}
 		}
 
-		private Optional<Boolean> fromOptionalString(String value) {
+		private Optional<NullHandling> fromOptionalNullHandlingString(String value) {
+
+			if (NULLSNATIVE.equalsIgnoreCase(value)) {
+				return Optional.of(NullHandling.NATIVE);
+			}
+
+			if (NULLSFIRST.equalsIgnoreCase(value)) {
+				return Optional.of(NullHandling.NULLS_FIRST);
+			}
+
+			if (NULLSLAST.equalsIgnoreCase(value)) {
+				return Optional.of(NullHandling.NULLS_LAST);
+			}
+
+			return Optional.empty();
+		}
+
+		private Optional<Boolean> fromOptionalIgnoreCaseString(String value) {
 			return IGNORECASE.equalsIgnoreCase(value) ? Optional.of(true) : Optional.empty();
 		}
 
@@ -443,6 +495,14 @@ private Optional<Order> toOrder(String property) {
 
 			Order order = direction.map(it -> new Order(it, property)).orElseGet(() -> Order.by(property));
 
+			if (nullHandling.isPresent()) {
+				order = switch (nullHandling.get()) {
+					case NATIVE -> order.nullsNative();
+					case NULLS_FIRST -> order.nullsFirst();
+					case NULLS_LAST -> order.nullsLast();
+				};
+			}
+
 			if (ignoreCase.isPresent()) {
 				return Optional.of(order.ignoreCase());
 			}

diff --git a/src/test/java/org/springframework/data/web/SortHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/SortHandlerMethodArgumentResolverUnitTests.java
@@ -47,6 +47,7 @@
  * @author Nick Williams
  * @author Mark Paluch
  * @author Vedran Pavic
+ * @author Petar Heyken
  */
 class SortHandlerMethodArgumentResolverUnitTests extends SortDefaultUnitTests {
 
@@ -210,6 +211,16 @@ void returnsDefaultCaseInsensitive() throws Exception {
 				.isEqualTo(Sort.by(new Order(DESC, "firstname").ignoreCase(), new Order(DESC, "lastname").ignoreCase()));
 	}
 
+	@Test // GH-3152
+	void returnsDefaultNullHandling() throws Exception {
+
+		final var request = new MockHttpServletRequest();
+		request.addParameter("sort", "");
+
+		assertThat(resolveSort(request, getParameterOfMethod("simpleDefaultWithDirectionAndNullHandling"))).isEqualTo(
+				Sort.by(new Order(DESC, "firstname").nullsLast(), new Order(DESC, "lastname").nullsLast()));
+	}
+
 	@Test // DATACMNS-379
 	void parsesCommaParameterForSort() throws Exception {
 
@@ -272,6 +283,52 @@ void readsEncodedSort() {
 		assertSupportedAndResolvedTo(new ServletWebRequest(request), parameter, Sort.by("foo").descending());
 	}
 
+	@Test // GH-3152
+	void sortParamHandlesMultiplePropertiesWithSortOrderAndIgnoreCaseAndNullsLast() throws Exception {
+
+		final var request = new MockHttpServletRequest();
+		request.addParameter("sort", "property1,property2,DESC,IgnoreCase,NullsLast");
+
+		assertThat(resolveSort(request, PARAMETER)).isEqualTo(Sort.by(new Order(DESC, "property1").ignoreCase().nullsLast(),
+				new Order(DESC, "property2").ignoreCase().nullsLast()));
+	}
+
+	@Test // GH-3152
+	void sortParamHandlesSinglePropertyWithIgnoreCaseAndNullsLast() throws Exception {
+
+		final var request = new MockHttpServletRequest();
+		request.addParameter("sort", "property,IgnoreCase,NullsLast");
+
+		assertThat(resolveSort(request, PARAMETER)).isEqualTo(Sort.by(new Order(ASC, "property").ignoreCase().nullsLast()));
+	}
+
+	@Test // GH-3152
+	void sortParamHandlesSinglePropertyWithNullsFirst() throws Exception {
+
+		final var request = new MockHttpServletRequest();
+		request.addParameter("sort", "property,nullsfirst");
+
+		assertThat(resolveSort(request, PARAMETER)).isEqualTo(Sort.by(new Order(ASC, "property").nullsFirst()));
+	}
+
+	@Test // GH-3152
+	void sortParamHandlesSinglePropertyWithSortOrderAndWithNullsFirst() throws Exception {
+
+		final var request = new MockHttpServletRequest();
+		request.addParameter("sort", "property,DESC,nullsfirst");
+
+		assertThat(resolveSort(request, PARAMETER)).isEqualTo(Sort.by(new Order(DESC, "property").nullsFirst()));
+	}
+
+	@Test // GH-3152
+	void sortParamHandlesSinglePropertyWithSortOrderAndWithNullsNative() throws Exception {
+
+		final var request = new MockHttpServletRequest();
+		request.addParameter("sort", "property,DESC,nullsnative");
+
+		assertThat(resolveSort(request, PARAMETER)).isEqualTo(Sort.by(new Order(DESC, "property").nullsNative()));
+	}
+
 	private static Sort resolveSort(HttpServletRequest request, MethodParameter parameter) throws Exception {
 
 		var resolver = new SortHandlerMethodArgumentResolver();
@@ -334,6 +391,9 @@ void simpleDefaultWithDirection(
 		void simpleDefaultWithDirectionCaseInsensitive(
 				@SortDefault(sort = { "firstname", "lastname" }, direction = Direction.DESC, caseSensitive = false) Sort sort);
 
+		void simpleDefaultWithDirectionAndNullHandling(
+			@SortDefault(sort = { "firstname", "lastname" }, direction = Direction.DESC, nullHandling = Sort.NullHandling.NULLS_LAST) Sort sort);
+
 		void containeredDefault(@SortDefaults(@SortDefault({ "foo", "bar" })) Sort sort);
 
 		void repeatable(@SortDefault({ "one", "two" }) @SortDefault({ "three" }) Sort sort);