Move AnnotatedControllerExceptionResolver to base class

See gh-864

Author: rstoyanchev

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

@@ -58,7 +58,6 @@
 import org.springframework.graphql.data.pagination.CursorStrategy;
 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;
 import org.springframework.graphql.execution.SelfDescribingDataFetcher;
 import org.springframework.graphql.execution.SubscriptionPublisherException;
@@ -115,9 +114,6 @@ public class AnnotatedControllerConfigurer
 	@Nullable
 	private ValidationHelper validationHelper;
 
-	@Nullable
-	private AnnotatedControllerExceptionResolver exceptionResolver;
-
 
 	/**
 	 * Add a {@link HandlerMethodArgumentResolver} for custom controller method
@@ -131,26 +127,6 @@ public void addCustomArgumentResolver(HandlerMethodArgumentResolver resolver) {
 		this.customArgumentResolvers.add(resolver);
 	}
 
-	/**
-	 * Return a {@link DataFetcherExceptionResolver} that resolves exceptions with
-	 * {@code @GraphQlExceptionHandler} methods in {@code @ControllerAdvice}
-	 * classes declared in Spring configuration. This is useful primarily for
-	 * exceptions from non-controller {@link DataFetcher}s since exceptions from
-	 * {@code @SchemaMapping} controller methods are handled automatically at
-	 * the point of invocation.
-	 *
-	 * @return a resolver instance that can be plugged into
-	 * {@link org.springframework.graphql.execution.GraphQlSource.Builder#exceptionResolvers(List)
-	 * GraphQlSource.Builder}
-	 *
-	 * @since 1.2.0
-	 */
-	public DataFetcherExceptionResolver getExceptionResolver() {
-		Assert.notNull(this.exceptionResolver,
-				"DataFetcherExceptionResolver is not yet initialized, was afterPropertiesSet called?");
-		return (ex, env) -> this.exceptionResolver.resolveException(ex, env, null);
-	}
-
 	/**
 	 * Configure an initializer that configures the {@link DataBinder} before the binding process.
 	 * @param consumer the data binder initializer
@@ -167,11 +143,6 @@ public void setDataBinderInitializer(@Nullable Consumer<DataBinder> consumer) {
 	public void afterPropertiesSet() {
 		super.afterPropertiesSet();
 
-		this.exceptionResolver = new AnnotatedControllerExceptionResolver(getArgumentResolvers());
-		if (getApplicationContext() != null) {
-			this.exceptionResolver.registerControllerAdvice(getApplicationContext());
-		}
-
 		if (beanValidationPresent) {
 			this.validationHelper = ValidationHelper.createIfValidatorPresent(obtainApplicationContext());
 		}
@@ -251,13 +222,11 @@ private void addSortMethodArgumentResolver(HandlerMethodArgumentResolverComposit
 
 	@Override
 	public void configure(RuntimeWiring.Builder runtimeWiringBuilder) {
-		Assert.state(this.exceptionResolver != null, "`exceptionResolver` is not initialized");
-
 		detectHandlerMethods().forEach(info -> {
 			DataFetcher<?> dataFetcher;
 			if (!info.isBatchMapping()) {
 				dataFetcher = new SchemaMappingDataFetcher(
-						info, getArgumentResolvers(), this.validationHelper, this.exceptionResolver, getExecutor());
+						info, getArgumentResolvers(), this.validationHelper, getExceptionResolver(), getExecutor());
 			}
 			else {
 				dataFetcher = registerBatchLoader(info);
@@ -415,7 +384,7 @@ static class SchemaMappingDataFetcher implements SelfDescribingDataFetcher<Objec
 		@Nullable
 		private final BiConsumer<Object, Object[]> methodValidationHelper;
 
-		private final AnnotatedControllerExceptionResolver exceptionResolver;
+		private final HandlerDataFetcherExceptionResolver exceptionResolver;
 
 		@Nullable
 		private final Executor executor;
@@ -424,7 +393,7 @@ static class SchemaMappingDataFetcher implements SelfDescribingDataFetcher<Objec
 
 		SchemaMappingDataFetcher(
 				DataFetcherMappingInfo info, HandlerMethodArgumentResolverComposite argumentResolvers,
-				@Nullable ValidationHelper helper, AnnotatedControllerExceptionResolver exceptionResolver,
+				@Nullable ValidationHelper helper, HandlerDataFetcherExceptionResolver exceptionResolver,
 				@Nullable Executor executor) {
 
 			this.mappingInfo = info;
@@ -433,10 +402,6 @@ static class SchemaMappingDataFetcher implements SelfDescribingDataFetcher<Objec
 			this.methodValidationHelper =
 					(helper != null ? helper.getValidationHelperFor(info.getHandlerMethod()) : null);
 
-			// Register controllers early to validate exception handler return types
-			Class<?> controllerType = info.getHandlerMethod().getBeanType();
-			exceptionResolver.registerController(controllerType);
-
 			this.exceptionResolver = exceptionResolver;
 
 			this.executor = executor;

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

@@ -21,12 +21,14 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Executor;
 import java.util.stream.Collectors;
 
+import graphql.schema.DataFetcher;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -42,6 +44,7 @@
 import org.springframework.format.support.FormattingConversionService;
 import org.springframework.graphql.data.method.HandlerMethod;
 import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite;
+import org.springframework.graphql.execution.DataFetcherExceptionResolver;
 import org.springframework.lang.Nullable;
 import org.springframework.stereotype.Controller;
 import org.springframework.util.Assert;
@@ -81,6 +84,9 @@ public abstract class AnnotatedControllerDetectionSupport<M> implements Applicat
 
 	private boolean fallBackOnDirectFieldAccess;
 
+	@Nullable
+	private AnnotatedControllerExceptionResolver exceptionResolver;
+
 	@Nullable
 	private Executor executor;
 
@@ -120,6 +126,25 @@ protected boolean isFallBackOnDirectFieldAccess() {
 		return this.fallBackOnDirectFieldAccess;
 	}
 
+	/**
+	 * Return a {@link DataFetcherExceptionResolver} that resolves exceptions with
+	 * {@code @GraphQlExceptionHandler} methods in {@code @ControllerAdvice}
+	 * classes declared in Spring configuration. This is useful primarily for
+	 * exceptions from non-controller {@link DataFetcher}s since exceptions from
+	 * {@code @SchemaMapping} controller methods are handled automatically at
+	 * the point of invocation.
+	 *
+	 * @return a resolver instance that can be plugged into
+	 * {@link org.springframework.graphql.execution.GraphQlSource.Builder#exceptionResolvers(List)
+	 * GraphQlSource.Builder}
+	 *
+	 * @since 1.2.0
+	 */
+	public HandlerDataFetcherExceptionResolver getExceptionResolver() {
+		Assert.notNull(this.exceptionResolver, "afterPropertiesSet not called yet");
+		return this.exceptionResolver;
+	}
+
 	/**
 	 * Configure an {@link Executor} to use for asynchronous handling of
 	 * {@link Callable} return values from controller methods.
@@ -140,7 +165,7 @@ public Executor getExecutor() {
 	 * Return the configured argument resolvers.
 	 */
 	protected HandlerMethodArgumentResolverComposite getArgumentResolvers() {
-		Assert.notNull(this.argumentResolvers, "Not yet initialized, was afterPropertiesSet called?");
+		Assert.notNull(this.argumentResolvers, "afterPropertiesSet not called yet");
 		return this.argumentResolvers;
 	}
 
@@ -163,6 +188,11 @@ protected final ApplicationContext obtainApplicationContext() {
 	@Override
 	public void afterPropertiesSet() {
 		this.argumentResolvers = initArgumentResolvers();
+
+		this.exceptionResolver = new AnnotatedControllerExceptionResolver(this.argumentResolvers);
+		if (getApplicationContext() != null) {
+			this.exceptionResolver.registerControllerAdvice(getApplicationContext());
+		}
 	}
 
 	protected abstract HandlerMethodArgumentResolverComposite initArgumentResolvers();
@@ -192,17 +222,7 @@ protected Set<M> detectHandlerMethods() {
 				continue;
 			}
 			Class<?> beanClass = context.getType(beanName);
-			findHandlerMethods(beanName, beanClass).forEach(info -> {
-				HandlerMethod handlerMethod = getHandlerMethod(info);
-				M existing = results.stream().filter(o -> o.equals(info)).findFirst().orElse(null);
-				if (existing != null && !getHandlerMethod(existing).equals(handlerMethod)) {
-					throw new IllegalStateException(
-							"Ambiguous mapping. Cannot map '" + handlerMethod.getBean() + "' method \n" +
-									handlerMethod + "\n" + ": There is already '" +
-									getHandlerMethod(existing).getBean() + "' bean method\n" + existing + " mapped.");
-				}
-				results.add(info);
-			});
+			findHandlerMethods(beanName, beanClass).forEach(info -> registerHandlerMethod(info, results));
 		}
 		return results;
 	}
@@ -230,13 +250,6 @@ private Collection<M> findHandlerMethods(Object handler, @Nullable Class<?> hand
 	@Nullable
 	protected abstract M getMappingInfo(Method method, Object handler, Class<?> handlerType);
 
-	protected HandlerMethod createHandlerMethod(Method originalMethod, Object handler, Class<?> handlerType) {
-		Method method = AopUtils.selectInvocableMethod(originalMethod, handlerType);
-		return (handler instanceof String beanName ?
-				new HandlerMethod(beanName, obtainApplicationContext().getAutowireCapableBeanFactory(), method) :
-				new HandlerMethod(handler, method));
-	}
-
 	private String formatMappings(Class<?> handlerType, Collection<M> infos) {
 		String formattedType = Arrays.stream(ClassUtils.getPackageName(handlerType).split("\\."))
 				.map(p -> p.substring(0, 1))
@@ -252,4 +265,25 @@ private String formatMappings(Class<?> handlerType, Collection<M> infos) {
 				.collect(Collectors.joining("\n\t", "\n\t" + formattedType + ":" + "\n\t", ""));
 	}
 
+	private void registerHandlerMethod(M info, Set<M> results) {
+		Assert.state(this.exceptionResolver != null, "afterPropertiesSet not called");
+		HandlerMethod handlerMethod = getHandlerMethod(info);
+		M existing = results.stream().filter(o -> o.equals(info)).findFirst().orElse(null);
+		if (existing != null && !getHandlerMethod(existing).equals(handlerMethod)) {
+			throw new IllegalStateException(
+					"Ambiguous mapping. Cannot map '" + handlerMethod.getBean() + "' method \n" +
+							handlerMethod + "\n" + ": There is already '" +
+							getHandlerMethod(existing).getBean() + "' bean method\n" + existing + " mapped.");
+		}
+		results.add(info);
+		this.exceptionResolver.registerController(handlerMethod.getBeanType());
+	}
+
+	protected HandlerMethod createHandlerMethod(Method originalMethod, Object handler, Class<?> handlerType) {
+		Method method = AopUtils.selectInvocableMethod(originalMethod, handlerType);
+		return (handler instanceof String beanName ?
+				new HandlerMethod(beanName, obtainApplicationContext().getAutowireCapableBeanFactory(), method) :
+				new HandlerMethod(handler, method));
+	}
+
 }

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

@@ -73,7 +73,7 @@
  * @author Rossen Stoyanchev
  * @since 1.2.0
  */
-final class AnnotatedControllerExceptionResolver {
+final class AnnotatedControllerExceptionResolver implements HandlerDataFetcherExceptionResolver {
 
 	private static final Log logger = LogFactory.getLog(AnnotatedControllerExceptionResolver.class);
 
@@ -121,7 +121,7 @@ public void registerControllerAdvice(ApplicationContext context) {
 		}
 		if (logger.isDebugEnabled()) {
 			logger.debug("@GraphQlException methods in ControllerAdvice beans: " +
-					(this.controllerAdviceCache.size() == 0 ? "none" : this.controllerAdviceCache.size()));
+					(this.controllerAdviceCache.isEmpty() ? "none" : this.controllerAdviceCache.size()));
 		}
 	}
 
@@ -167,6 +167,7 @@ private static Map<Class<? extends Throwable>, Method> findExceptionHandlers(Cla
 	 * @return a {@code Mono} with resolved {@code GraphQLError}s as specified in
 	 * {@link DataFetcherExceptionResolver#resolveException(Throwable, DataFetchingEnvironment)}
 	 */
+	@Override
 	public Mono<List<GraphQLError>> resolveException(
 			Throwable ex, DataFetchingEnvironment environment, @Nullable Object controller) {
 

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

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2002-2024 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 graphql.GraphQLError;
+import graphql.schema.DataFetchingEnvironment;
+import reactor.core.publisher.Mono;
+
+import org.springframework.graphql.execution.DataFetcherExceptionResolver;
+import org.springframework.lang.Nullable;
+
+/**
+ * Extension of {@link DataFetcherExceptionResolver} with overloaded method to
+ * apply at the point of DataFetcher invocation to allow local exception handling.
+ *
+ * @author Rossen Stoyanchev
+ * @since 1.3
+ */
+public interface HandlerDataFetcherExceptionResolver extends DataFetcherExceptionResolver {
+
+
+	@Override
+	default Mono<List<GraphQLError>> resolveException(Throwable exception, DataFetchingEnvironment environment) {
+		return resolveException(exception, environment, null);
+	}
+
+	/**
+	 * Resolve an exception raised by the given handler.
+	 * @param ex the exception to resolve
+	 * @param environment the environment for the invoked {@code DataFetcher}
+	 * @param handler the handler that raised the exception, if applicable
+	 * @return a {@code Mono} with resolved {@code GraphQLError}s as specified in
+	 * {@link DataFetcherExceptionResolver#resolveException(Throwable, DataFetchingEnvironment)}
+	 */
+	Mono<List<GraphQLError>> resolveException(
+			Throwable ex, DataFetchingEnvironment environment, @Nullable Object handler);
+
+}