Refactor SortStrategy

For a completely generic SortStrategy we can't provide any concrete
support we know neither the input structure nor the Object structure.
At best, a nearly empty argument resolver, which is not any better
than providing your own custom argument resolver.

Hence, SortStrategy is now explicitly based on Spring Data's Sort,
which enables us to provide concrete support with a base class.

See gh-620

Author: rstoyanchev

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

@@ -48,6 +48,7 @@
 
 import org.springframework.aop.support.AopUtils;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.context.expression.BeanFactoryResolver;
@@ -57,7 +58,6 @@
 import org.springframework.core.ResolvableType;
 import org.springframework.core.annotation.AnnotatedElementUtils;
 import org.springframework.core.convert.ConversionService;
-import org.springframework.expression.BeanResolver;
 import org.springframework.format.FormatterRegistrar;
 import org.springframework.format.support.DefaultFormattingConversionService;
 import org.springframework.format.support.FormattingConversionService;
@@ -68,7 +68,7 @@
 import org.springframework.graphql.data.method.annotation.BatchMapping;
 import org.springframework.graphql.data.method.annotation.SchemaMapping;
 import org.springframework.graphql.data.pagination.CursorStrategy;
-import org.springframework.graphql.data.pagination.SortStrategy;
+import org.springframework.graphql.data.query.SortStrategy;
 import org.springframework.graphql.execution.BatchLoaderRegistry;
 import org.springframework.graphql.execution.DataFetcherExceptionResolver;
 import org.springframework.graphql.execution.RuntimeWiringConfigurer;
@@ -128,9 +128,6 @@ public class AnnotatedControllerConfigurer implements ApplicationContextAware, I
 	@Nullable
 	private CursorStrategy<?> cursorStrategy;
 
-	@Nullable
-	private SortStrategy<?> sortStrategy;
-
 	private final List<HandlerMethodArgumentResolver> customArgumentResolvers = new ArrayList<>(8);
 
 	@Nullable
@@ -174,16 +171,6 @@ public void setCursorStrategy(@Nullable CursorStrategy<?> cursorStrategy) {
 		this.cursorStrategy = cursorStrategy;
 	}
 
-	/**
-	 * Configure a {@link SortStrategy} to extract sort details for pagination
-	 * requests. This results in {@link SortMethodArgumentResolver} being added
-	 * as a method argument resolver.
-	 * @since 1.2
-	 */
-	public void setSortStrategy(SortStrategy<?> sortStrategy) {
-		this.sortStrategy = sortStrategy;
-	}
-
 	/**
 	 * Add a {@link HandlerMethodArgumentResolver} for custom controller method
 	 * arguments. Such custom resolvers are ordered after built-in resolvers
@@ -286,13 +273,19 @@ private HandlerMethodArgumentResolverComposite initArgumentResolvers() {
 		if (this.cursorStrategy != null) {
 			resolvers.addResolver(createSubrangeMethodArgumentResolver(this.cursorStrategy));
 		}
-		if (this.sortStrategy != null) {
-			resolvers.addResolver(new SortMethodArgumentResolver(this.sortStrategy));
+		if (springDataPresent) {
+			try {
+				resolvers.addResolver(
+						new SortMethodArgumentResolver(obtainApplicationContext().getBean(SortStrategy.class)));
+			}
+			catch (NoSuchBeanDefinitionException ex) {
+				// ignore
+			}
 		}
 		if (springSecurityPresent) {
+			ApplicationContext context = obtainApplicationContext();
 			resolvers.addResolver(new PrincipalMethodArgumentResolver());
-			BeanResolver beanResolver = new BeanFactoryResolver(obtainApplicationContext());
-			resolvers.addResolver(new AuthenticationPrincipalArgumentResolver(beanResolver));
+			resolvers.addResolver(new AuthenticationPrincipalArgumentResolver(new BeanFactoryResolver(context)));
 		}
 		if (KotlinDetector.isKotlinPresent()) {
 			resolvers.addResolver(new ContinuationHandlerMethodArgumentResolver());

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

@@ -20,36 +20,38 @@
 import graphql.schema.DataFetchingEnvironment;
 
 import org.springframework.core.MethodParameter;
+import org.springframework.data.domain.Sort;
 import org.springframework.graphql.data.method.HandlerMethodArgumentResolver;
-import org.springframework.graphql.data.pagination.SortStrategy;
+import org.springframework.graphql.data.query.SortStrategy;
 import org.springframework.util.Assert;
 
 
 /**
- * Resolver for a Sort object decoded with {@link SortStrategy}.
+ * Resolver for method arguments of type {@link Sort}.
  *
  * @author Rossen Stoyanchev
  * @since 1.2
  */
 public class SortMethodArgumentResolver implements HandlerMethodArgumentResolver {
 
-	private final SortStrategy<?> sortStrategy;
+	private final SortStrategy sortStrategy;
 
 
-	public SortMethodArgumentResolver(SortStrategy<?> sortStrategy) {
+	public SortMethodArgumentResolver(SortStrategy sortStrategy) {
 		Assert.notNull(sortStrategy, "SortStrategy is required");
 		this.sortStrategy = sortStrategy;
 	}
 
 
 	@Override
 	public boolean supportsParameter(MethodParameter parameter) {
-		return this.sortStrategy.supports(parameter.getParameterType());
+		return parameter.getParameterType().equals(Sort.class);
 	}
 
 	@Override
 	public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment environment) {
-		return this.sortStrategy.extract(environment);
+		Sort sort = this.sortStrategy.extract(environment);
+		return (sort != null ? sort : Sort.unsorted());
 	}
 
 }

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

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2020-2023 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.graphql.data.query;
+
+
+import java.util.ArrayList;
+import java.util.List;
+
+import graphql.schema.DataFetchingEnvironment;
+
+import org.springframework.data.domain.Sort;
+import org.springframework.lang.Nullable;
+import org.springframework.util.ObjectUtils;
+
+/**
+ * Convenient base class for a {@link SortStrategy}. Subclasses help to extract
+ * the list of sort {@link #getProperties(DataFetchingEnvironment) properties}
+ * and {@link #getDirection(DataFetchingEnvironment) direction}.
+ *
+ * @author Rossen Stoyanchev
+ * @since 1.2
+ */
+public abstract class AbstractSortStrategy implements SortStrategy {
+
+	@Override
+	public Sort extract(DataFetchingEnvironment environment) {
+		List<String> properties = getProperties(environment);
+		if (!ObjectUtils.isEmpty(properties)) {
+			Sort.Direction direction = getDirection(environment);
+			direction = (direction != null ? direction : Sort.DEFAULT_DIRECTION);
+			List<Sort.Order> sortOrders = new ArrayList<>(properties.size());
+			for (String property : properties) {
+				sortOrders.add(new Sort.Order(direction, property));
+			}
+			return Sort.by(sortOrders);
+		}
+		return null;
+	}
+
+	/**
+	 * Return the sort properties to use, or an empty list if there are none.
+	 */
+	protected abstract List<String> getProperties(DataFetchingEnvironment environment);
+
+	/**
+	 * Return the sort direction to use, or {@code null}.
+	 */
+	@Nullable
+	protected abstract Sort.Direction getDirection(DataFetchingEnvironment environment);
+
+}

…raphql/data/pagination/SortStrategy.java …ork/graphql/data/query/SortStrategy.javaspring-graphql/src/main/java/org/springframework/graphql/data/pagination/SortStrategy.java renamed to spring-graphql/src/main/java/org/springframework/graphql/data/query/SortStrategy.java spring-graphql/src/main/java/org/springframework/graphql/data/pagination/SortStrategy.java renamed to spring-graphql/src/main/java/org/springframework/graphql/data/query/SortStrategy.java

@@ -14,32 +14,26 @@
  * limitations under the License.
  */
 
-package org.springframework.graphql.data.pagination;
+package org.springframework.graphql.data.query;
 
 
 import graphql.schema.DataFetchingEnvironment;
 
+import org.springframework.data.domain.Sort;
 import org.springframework.lang.Nullable;
 
 /**
- * Strategy to extract sort information from GraphQL request arguments.
+ * Strategy to extract {@link Sort} details from GraphQL arguments.
  *
  * @author Rossen Stoyanchev
  * @since 1.2
  */
-public interface SortStrategy<S> {
+public interface SortStrategy {
 
 	/**
-	 * Whether this strategy supports the given Sort object target type.
-	 */
-	boolean supports(Class<?> targetType);
-
-	/**
-	 * Return an Object that contains sort order and direction information.
-	 * @param environment the environment to obtain GraphQL request arguments from
-	 * @return the object with sort details, if present
+	 * Return a {@link Sort} instance initialized from GraphQL arguments, or {@code null}.
 	 */
 	@Nullable
-	S extract(DataFetchingEnvironment environment);
+	Sort extract(DataFetchingEnvironment environment);
 
 }

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

@@ -21,6 +21,7 @@
 
 import org.springframework.context.support.StaticApplicationContext;
 import org.springframework.graphql.data.method.HandlerMethodArgumentResolver;
+import org.springframework.graphql.data.query.SortStrategy;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
@@ -33,7 +34,6 @@
  */
 public class AnnotatedControllerConfigurerTests {
 
-
 	@Test
 	void customArgumentResolvers() {
 		HandlerMethodArgumentResolver customResolver1 = mock(HandlerMethodArgumentResolver.class);
@@ -52,4 +52,29 @@ void customArgumentResolvers() {
 		assertThat(resolvers).element(size -3).isSameAs(customResolver1);
 	}
 
+	@Test
+	void sortArgumentResolver() {
+		SortStrategy sortStrategy = mock(SortStrategy.class);
+
+		StaticApplicationContext context = new StaticApplicationContext();
+		context.registerBean(SortStrategy.class, () -> sortStrategy);
+
+		AnnotatedControllerConfigurer configurer = new AnnotatedControllerConfigurer();
+		configurer.setApplicationContext(context);
+		configurer.afterPropertiesSet();
+
+		List<HandlerMethodArgumentResolver> resolvers = configurer.getArgumentResolvers().getResolvers();
+		assertThat(resolvers.stream().filter(r -> r instanceof SortMethodArgumentResolver).findFirst()).isPresent();
+	}
+
+	@Test
+	void sortArgumentResolverStrategyNotPresent() {
+		AnnotatedControllerConfigurer configurer = new AnnotatedControllerConfigurer();
+		configurer.setApplicationContext(new StaticApplicationContext());
+		configurer.afterPropertiesSet();
+
+		List<HandlerMethodArgumentResolver> resolvers = configurer.getArgumentResolvers().getResolvers();
+		assertThat(resolvers.stream().filter(r -> r instanceof SortMethodArgumentResolver).findFirst()).isNotPresent();
+	}
+
 }

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

@@ -57,4 +57,8 @@ protected DataFetchingEnvironment environment(String argumentsJson) throws JsonP
 		return DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build();
 	}
 
+	protected DataFetchingEnvironment environment(Map<String, Object> arguments) {
+		return DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build();
+	}
+
 }

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

@@ -0,0 +1,106 @@
+/*
+ * Copyright 2002-2023 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.graphql.data.method.annotation.support;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import graphql.schema.DataFetchingEnvironment;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.core.MethodParameter;
+import org.springframework.data.domain.Sort;
+import org.springframework.graphql.Book;
+import org.springframework.graphql.BookCriteria;
+import org.springframework.graphql.data.method.annotation.QueryMapping;
+import org.springframework.graphql.data.query.AbstractSortStrategy;
+import org.springframework.graphql.data.query.SortStrategy;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Unit tests for {@link SortMethodArgumentResolver}.
+ *
+ * @author Rossen Stoyanchev
+ */
+public class SortMethodArgumentResolverTests extends ArgumentResolverTestSupport {
+
+	private final MethodParameter param = methodParam(BookController.class, "getBooks", Sort.class);
+
+
+	@Test
+	void supports() {
+		SortMethodArgumentResolver resolver = resolver(new SimpleSortStrategy());
+		assertThat(resolver.supportsParameter(this.param)).isTrue();
+		
+		MethodParameter param = methodParam(BookController.class, "getBooksByCriteria", BookCriteria.class);
+		assertThat(resolver.supportsParameter(param)).isFalse();
+	}
+
+	@Test
+	void resolve() throws Exception {
+		DataFetchingEnvironment environment = environment("""
+			{ "sortFields": ["firstName", "lastName", "id"], "sortDirection": "DESC"}"
+		""");
+
+		Sort sort = (Sort) resolver(new SimpleSortStrategy()).resolveArgument(param, environment);
+
+		assertThat(sort.stream().collect(Collectors.toList()))
+				.hasSize(3)
+				.containsExactly(
+						new Sort.Order(Sort.Direction.DESC, "firstName"),
+						new Sort.Order(Sort.Direction.DESC, "lastName"),
+						new Sort.Order(Sort.Direction.DESC, "id"));
+	}
+
+	private SortMethodArgumentResolver resolver(SortStrategy sortStrategy) {
+		return new SortMethodArgumentResolver(sortStrategy);
+	}
+
+
+	@SuppressWarnings({"DataFlowIssue", "unused"})
+	private static class BookController {
+
+		@QueryMapping
+		public List<Book> getBooks(Sort sort) {
+			return null;
+		}
+
+		@QueryMapping
+		public List<Book> getBooksByCriteria(BookCriteria criteria) {
+			return null;
+		}
+
+	}
+
+
+	private static class SimpleSortStrategy extends AbstractSortStrategy {
+
+		@Override
+		protected List<String> getProperties(DataFetchingEnvironment environment) {
+			return environment.getArgument("sortFields");
+		}
+
+		@Override
+		protected Sort.Direction getDirection(DataFetchingEnvironment environment) {
+			return (environment.containsArgument("sortDirection") ?
+					Sort.Direction.valueOf(environment.getArgument("sortDirection")) : null);
+		}
+
+	}
+
+}