Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add null handling to sort query parameters #3153

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/main/java/org/springframework/data/web/SortDefault.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -33,6 +34,7 @@
* @since 1.6
* @author Oliver Gierke
* @author Mark Palich
* @author Petar Heyken
*/
@Documented
@Retention(RetentionPolicy.RUNTIME)
Expand Down Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,6 +42,7 @@
* @author Mark Paluch
* @author Vedran Pavic
* @author Johannes Englmeier
* @author Petar Heyken
* @see SortHandlerMethodArgumentResolver
* @see ReactiveSortHandlerMethodArgumentResolver
* @since 2.2
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -214,6 +223,7 @@ Sort parseParameterIntoSort(List<String> source, String delimiter) {
}

SortOrderParser.parse(part, delimiter) //
.parseNullHandling() //
.parseIgnoreCase() //
.parseDirection() //
.forEachOrder(allOrders::add);
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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();
}

Expand All @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
* @author Nick Williams
* @author Mark Paluch
* @author Vedran Pavic
* @author Petar Heyken
*/
class SortHandlerMethodArgumentResolverUnitTests extends SortDefaultUnitTests {

Expand Down Expand Up @@ -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 {

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down