Minor refactoring

See gh-620

Author: rstoyanchev

spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/SortMethodArgumentResolver.java

@@ -50,12 +50,17 @@ public boolean supportsParameter(MethodParameter parameter) {
 		return parameter.nestedIfOptional().getNestedParameterType().equals(Sort.class);
 	}
 
+	@SuppressWarnings("ConstantValue")
 	@Override
 	public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment environment) {
+
 		Sort sort = this.sortStrategy.extract(environment);
+
 		if (parameter.isOptional()) {
+			sort = (sort == Sort.unsorted() ? null : sort);
 			return Optional.ofNullable(sort);
 		}
+
 		return (sort != null ? sort : Sort.unsorted());
 	}
 

spring-graphql/src/main/java/org/springframework/graphql/data/pagination/Subrange.java

@@ -18,6 +18,7 @@
 
 
 import java.util.Optional;
+import java.util.OptionalInt;
 
 import org.springframework.lang.Nullable;
 
@@ -28,13 +29,12 @@
  * @author Rossen Stoyanchev
  * @since 1.2
  */
+@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
 public class Subrange<P> {
 
-	@Nullable
-	private final P position;
+	private final Optional<P> position;
 
-	@Nullable
-	private final Integer count;
+	private final OptionalInt count;
 
 	private final boolean forward;
 
@@ -43,9 +43,9 @@ public class Subrange<P> {
 	 * Constructor with the relative position, count, and direction.
 	 */
 	public Subrange(@Nullable P position, @Nullable Integer count, boolean forward) {
-		this.position = position;
+		this.position = Optional.ofNullable(position);
+		this.count = (count != null ? OptionalInt.of(count) : OptionalInt.empty());
 		this.forward = forward;
-		this.count = count;
 	}
 
 
@@ -55,15 +55,15 @@ public Subrange(@Nullable P position, @Nullable Integer count, boolean forward)
 	 * Cursor connection spec via {@link CursorStrategy}.
 	 */
 	public Optional<P> position() {
-		return Optional.ofNullable(this.position);
+		return this.position;
 	}
 
 	/**
 	 * The number of elements in the subrange based on the "first" and "last"
 	 * arguments from the GraphQL Cursor connection spec.
 	 */
-	public Optional<Integer> count() {
-		return Optional.ofNullable(this.count);
+	public OptionalInt count() {
+		return this.count;
 	}
 
 	/**

spring-graphql/src/main/java/org/springframework/graphql/data/query/SortStrategy.java

@@ -20,7 +20,6 @@
 import graphql.schema.DataFetchingEnvironment;
 
 import org.springframework.data.domain.Sort;
-import org.springframework.lang.Nullable;
 
 /**
  * Strategy to extract {@link Sort} details from GraphQL arguments.
@@ -31,9 +30,9 @@
 public interface SortStrategy {
 
 	/**
-	 * Return a {@link Sort} instance initialized from GraphQL arguments, or {@code null}.
+	 * Return a {@link Sort} instance by extracting the sort information from
+	 * GraphQL arguments, or {@link Sort#unsorted()} otherwise.
 	 */
-	@Nullable
 	Sort extract(DataFetchingEnvironment environment);
 
 }

spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SubrangeMethodArgumentResolverTests.java

@@ -76,7 +76,7 @@ void backwardPagination() throws Exception {
 	private static void testRequest(int count, int index, Object result, boolean forward) {
 		Subrange<MyPosition> subrange = (Subrange<MyPosition>) result;
 		assertThat(subrange.position().get().index()).isEqualTo(index);
-		assertThat(subrange.count().get()).isEqualTo(count);
+		assertThat(subrange.count().orElse(0)).isEqualTo(count);
 		assertThat(subrange.forward()).isEqualTo(forward);
 	}
 

spring-graphql/src/test/java/org/springframework/graphql/data/query/ScrollSubrangeTests.java

@@ -42,12 +42,12 @@ void offset() {
 
 		ScrollSubrange subrange = new ScrollSubrange(position, count, true);
 		assertThat(((OffsetScrollPosition) subrange.position().get())).isEqualTo(position);
-		assertThat(subrange.count().get()).isEqualTo(count);
+		assertThat(subrange.count().orElse(0)).isEqualTo(count);
 		assertThat(subrange.forward()).isTrue();
 
 		subrange = new ScrollSubrange(position, count, false);
 		assertThat(((OffsetScrollPosition) subrange.position().get()).getOffset()).isEqualTo(20);
-		assertThat(subrange.count().get()).isEqualTo(count);
+		assertThat(subrange.count().orElse(0)).isEqualTo(count);
 		assertThat(subrange.forward()).isTrue();
 	}
 
@@ -65,14 +65,14 @@ void keyset() {
 		KeysetScrollPosition actualPosition = (KeysetScrollPosition) subrange.position().get();
 		assertThat(actualPosition.getKeys()).isEqualTo(keys);
 		assertThat(actualPosition.getDirection()).isEqualTo(Direction.Forward);
-		assertThat(subrange.count().get()).isEqualTo(count);
+		assertThat(subrange.count().orElse(0)).isEqualTo(count);
 		assertThat(subrange.forward()).isTrue();
 
 		subrange = new ScrollSubrange(position, count, false);
 		actualPosition = (KeysetScrollPosition) subrange.position().get();
 		assertThat(actualPosition.getKeys()).isEqualTo(keys);
 		assertThat(actualPosition.getDirection()).isEqualTo(Direction.Backward);
-		assertThat(subrange.count().get()).isEqualTo(count);
+		assertThat(subrange.count().orElse(0)).isEqualTo(count);
 		assertThat(subrange.forward()).isFalse();
 	}