diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/federation/EntityHandlerMethod.java b/spring-graphql/src/main/java/org/springframework/graphql/data/federation/EntityHandlerMethod.java index 859407bf..13771e96 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/federation/EntityHandlerMethod.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/federation/EntityHandlerMethod.java @@ -40,9 +40,9 @@ final class EntityHandlerMethod extends DataFetcherHandlerMethodSupport { EntityHandlerMethod( FederationSchemaFactory.EntityMappingInfo info, HandlerMethodArgumentResolverComposite resolvers, - @Nullable Executor executor) { + @Nullable Executor executor, boolean invokeAsync) { - super(info.handlerMethod(), resolvers, executor); + super(info.handlerMethod(), resolvers, executor, invokeAsync); this.batchHandlerMethod = info.isBatchHandlerMethod(); } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java b/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java index e7b70cab..d6eff3de 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java @@ -92,8 +92,8 @@ public final class FederationSchemaFactory super.afterPropertiesSet(); detectHandlerMethods().forEach((info) -> - this.handlerMethods.put(info.typeName(), - new EntityHandlerMethod(info, getArgumentResolvers(), getExecutor()))); + this.handlerMethods.put(info.typeName(), new EntityHandlerMethod( + info, getArgumentResolvers(), getExecutor(), shouldInvokeAsync(info.handlerMethod())))); if (this.typeResolver == null) { this.typeResolver = new ClassNameTypeResolver(); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/InvocableHandlerMethodSupport.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/InvocableHandlerMethodSupport.java index 99dee348..4b9e21ad 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/InvocableHandlerMethodSupport.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/InvocableHandlerMethodSupport.java @@ -47,25 +47,43 @@ public abstract class InvocableHandlerMethodSupport extends HandlerMethod { private static final Object NO_VALUE = new Object(); - private final boolean hasCallableReturnValue; - @Nullable private final Executor executor; + private final boolean hasCallableReturnValue; + + private final boolean invokeAsync; + + /** * Create an instance. * @param handlerMethod the controller method * @param executor an {@link Executor} to use for {@link Callable} return values + * @deprecated in favor of alternative constructor */ + @Deprecated(since = "1.3.0", forRemoval = true) protected InvocableHandlerMethodSupport(HandlerMethod handlerMethod, @Nullable Executor executor) { + this(handlerMethod, executor, false); + } + + /** + * Create an instance. + * @param handlerMethod the controller method + * @param executor an {@link Executor} to use for {@link Callable} return values + * @param invokeAsync whether to invoke the method through the Executor + * @since 1.3.0 + */ + protected InvocableHandlerMethodSupport( + HandlerMethod handlerMethod, @Nullable Executor executor, boolean invokeAsync) { super(handlerMethod.createWithResolvedBean()); - this.hasCallableReturnValue = getReturnType().getParameterType().equals(Callable.class); this.executor = executor; + this.hasCallableReturnValue = getReturnType().getParameterType().equals(Callable.class); + this.invokeAsync = (invokeAsync && !this.hasCallableReturnValue); - Assert.isTrue(!this.hasCallableReturnValue || executor != null, - "Controller method has Callable return value, but Executor not provided: " + + Assert.isTrue((!this.hasCallableReturnValue && !invokeAsync) || executor != null, + "Controller method has Callable return value or invokeAsync=true, but Executor not provided: " + handlerMethod.getBridgedMethod().toGenericString()); } @@ -81,7 +99,7 @@ public abstract class InvocableHandlerMethodSupport extends HandlerMethod { @Nullable protected Object doInvoke(GraphQLContext graphQLContext, Object... argValues) { if (logger.isTraceEnabled()) { - logger.trace("Arguments: " + Arrays.toString(argValues)); + logger.trace("Invoking " + getBridgedMethod().getName() + "(" + Arrays.toString(argValues) + ")"); } Method method = getBridgedMethod(); try { @@ -89,10 +107,16 @@ public abstract class InvocableHandlerMethodSupport extends HandlerMethod { return invokeSuspendingFunction(getBean(), method, argValues); } - Object result = method.invoke(getBean(), argValues); - - if (this.hasCallableReturnValue && result != null) { - result = adaptCallable(graphQLContext, (Callable) result); + Object result; + if (this.invokeAsync) { + Callable callable = () -> method.invoke(getBean(), argValues); + result = adaptCallable(graphQLContext, callable); + } + else { + result = method.invoke(getBean(), argValues); + if (this.hasCallableReturnValue && result != null) { + result = adaptCallable(graphQLContext, (Callable) result); + } } return result; diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java index f4dbcea8..4049136e 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java @@ -343,7 +343,8 @@ public class AnnotatedControllerConfigurer DataFetcher dataFetcher; if (!info.isBatchMapping()) { dataFetcher = new SchemaMappingDataFetcher( - info, getArgumentResolvers(), this.validationHelper, getExceptionResolver(), getExecutor()); + info, getArgumentResolvers(), this.validationHelper, getExceptionResolver(), + getExecutor(), shouldInvokeAsync(info.getHandlerMethod())); } else { dataFetcher = registerBatchLoader(info); @@ -366,7 +367,8 @@ public class AnnotatedControllerConfigurer } HandlerMethod handlerMethod = info.getHandlerMethod(); - BatchLoaderHandlerMethod invocable = new BatchLoaderHandlerMethod(handlerMethod, getExecutor()); + BatchLoaderHandlerMethod invocable = + new BatchLoaderHandlerMethod(handlerMethod, getExecutor(), shouldInvokeAsync(handlerMethod)); MethodParameter returnType = handlerMethod.getReturnType(); Class clazz = returnType.getParameterType(); @@ -441,12 +443,14 @@ public class AnnotatedControllerConfigurer @Nullable private final Executor executor; + private final boolean invokeAsync; + private final boolean subscription; SchemaMappingDataFetcher( DataFetcherMappingInfo info, HandlerMethodArgumentResolverComposite argumentResolvers, @Nullable ValidationHelper helper, HandlerDataFetcherExceptionResolver exceptionResolver, - @Nullable Executor executor) { + @Nullable Executor executor, boolean invokeAsync) { this.mappingInfo = info; this.argumentResolvers = argumentResolvers; @@ -457,6 +461,7 @@ public class AnnotatedControllerConfigurer this.exceptionResolver = exceptionResolver; this.executor = executor; + this.invokeAsync = invokeAsync; this.subscription = this.mappingInfo.getCoordinates().getTypeName().equalsIgnoreCase("Subscription"); } @@ -497,7 +502,7 @@ public class AnnotatedControllerConfigurer DataFetcherHandlerMethod handlerMethod = new DataFetcherHandlerMethod( getHandlerMethod(), this.argumentResolvers, this.methodValidationHelper, - this.executor, this.subscription); + this.executor, this.invokeAsync, this.subscription); try { Object result = handlerMethod.invoke(environment); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerDetectionSupport.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerDetectionSupport.java index 9a49cbb4..c61028a9 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerDetectionSupport.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerDetectionSupport.java @@ -27,6 +27,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.Executor; +import java.util.function.Predicate; import java.util.stream.Collectors; import graphql.schema.DataFetcher; @@ -37,7 +38,9 @@ import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.InitializingBean; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; +import org.springframework.core.KotlinDetector; import org.springframework.core.MethodIntrospector; +import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.convert.ConversionService; import org.springframework.format.FormatterRegistrar; @@ -47,9 +50,11 @@ 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.scheduling.SchedulingTaskExecutor; import org.springframework.stereotype.Controller; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; /** * Convenient base for classes that find annotated controller method with argument @@ -65,6 +70,9 @@ public abstract class AnnotatedControllerDetectionSupport implements Applicat "org.springframework.security.core.context.SecurityContext", AnnotatedControllerDetectionSupport.class.getClassLoader()); + private static final boolean virtualThreadsPresent = + (ReflectionUtils.findMethod(Thread.class, "ofVirtual") != null); + /** * Bean name prefix for target beans behind scoped proxies. Used to exclude those * targets from handler method detection, in favor of the corresponding proxies. @@ -91,6 +99,9 @@ public abstract class AnnotatedControllerDetectionSupport implements Applicat @Nullable private Executor executor; + private Predicate blockingMethodPredicate = + (virtualThreadsPresent) ? new BlockingHandlerMethodPredicate() : ((method) -> false); + @Nullable private HandlerMethodArgumentResolverComposite argumentResolvers; @@ -148,20 +159,44 @@ public abstract class AnnotatedControllerDetectionSupport implements Applicat /** * Configure an {@link Executor} to use for asynchronous handling of - * {@link Callable} return values from controller methods. + * {@link Callable} return values from controller methods, as well as for + * {@link #setBlockingMethodPredicate(Predicate) blocking controller methods} + * on Java 21+. *

By default, this is not set in which case controller methods with a - * {@code Callable} return value cannot be registered. + * {@code Callable} return value are not supported, and blocking methods + * will be invoked synchronously. * @param executor the executor to use */ public void setExecutor(Executor executor) { this.executor = executor; } + /** + * Return the {@link #setExecutor(Executor) configured Executor}. + */ @Nullable public Executor getExecutor() { return this.executor; } + /** + * Configure a predicate to decide which controller methods are blocking. + * On Java 21+, such methods are invoked asynchronously through the + * {@link #setExecutor(Executor) configured Executor}, unless the executor + * is a thread pool executor as determined via + * {@link SchedulingTaskExecutor#prefersShortLivedTasks() prefersShortLivedTasks}. + *

By default, on Java 21+ the predicate returns false for controller + * method return types known to {@link ReactiveAdapterRegistry} as well as + * {@link KotlinDetector#isSuspendingFunction Kotlin suspending functions}. + * On Java 20 and lower, the predicate returns false. You can configure the + * predicate for more control, or alternatively, return {@link Callable}. + * @param predicate the predicate to use + * @since 1.3 + */ + public void setBlockingMethodPredicate(@Nullable Predicate predicate) { + this.blockingMethodPredicate = ((predicate != null) ? predicate : (handlerMethod) -> false); + } + /** * Return the configured argument resolvers. */ @@ -287,4 +322,20 @@ public abstract class AnnotatedControllerDetectionSupport implements Applicat new HandlerMethod(handler, method); } + protected boolean shouldInvokeAsync(HandlerMethod handlerMethod) { + return (this.blockingMethodPredicate.test(handlerMethod) && this.executor != null && + !(this.executor instanceof SchedulingTaskExecutor ste && ste.prefersShortLivedTasks())); + } + + + private static final class BlockingHandlerMethodPredicate implements Predicate { + + @Override + public boolean test(HandlerMethod hm) { + Class returnType = hm.getReturnType().getParameterType(); + return (ReactiveAdapterRegistry.getSharedInstance().getAdapter(returnType) == null && + !KotlinDetector.isSuspendingFunction(hm.getMethod())); + } + } + } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java index be9d444e..53200bb5 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java @@ -216,7 +216,7 @@ final class AnnotatedControllerExceptionResolver implements HandlerDataFetcherEx DataFetcherHandlerMethod exceptionHandler = new DataFetcherHandlerMethod( new HandlerMethod(controllerOrAdvice, methodHolder.getMethod()), this.argumentResolvers, - null, null, false); + null, null, false, false); List exceptions = new ArrayList<>(); try { diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/BatchLoaderHandlerMethod.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/BatchLoaderHandlerMethod.java index 34f30922..7de3f377 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/BatchLoaderHandlerMethod.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/BatchLoaderHandlerMethod.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * 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. @@ -20,6 +20,7 @@ import java.security.Principal; import java.util.Arrays; import java.util.Collection; import java.util.Map; +import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Function; @@ -59,8 +60,26 @@ public class BatchLoaderHandlerMethod extends InvocableHandlerMethodSupport { private final ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer(); + /** + * Create an instance. + * @param handlerMethod the controller method + * @param executor an {@link Executor} to use for {@link Callable} return values + * @deprecated in favor of alternative constructor + */ + @Deprecated(since = "1.3.0", forRemoval = true) public BatchLoaderHandlerMethod(HandlerMethod handlerMethod, @Nullable Executor executor) { - super(handlerMethod, executor); + this(handlerMethod, executor, false); + } + + /** + * Create an instance. + * @param handlerMethod the controller method + * @param executor an {@link Executor} to use for {@link Callable} return values + * @param invokeAsync whether to invoke the method through the Executor + * @since 1.3.0 + */ + public BatchLoaderHandlerMethod(HandlerMethod handlerMethod, @Nullable Executor executor, boolean invokeAsync) { + super(handlerMethod, executor, invokeAsync); } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java index c6840d1b..1befabcc 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java @@ -53,13 +53,33 @@ public class DataFetcherHandlerMethod extends DataFetcherHandlerMethodSupport { * @param validationHelper to apply bean validation with * @param executor an {@link Executor} to use for {@link Callable} return values * @param subscription whether the field being fetched is of subscription type + * @deprecated in favor of alternative constructor */ + @Deprecated(since = "1.3.0", forRemoval = true) public DataFetcherHandlerMethod( HandlerMethod handlerMethod, HandlerMethodArgumentResolverComposite resolvers, @Nullable BiConsumer validationHelper, @Nullable Executor executor, boolean subscription) { - super(handlerMethod, resolvers, executor); + this(handlerMethod, resolvers, validationHelper, executor, subscription, false); + } + + /** + * Constructor with a parent handler method. + * @param handlerMethod the handler method + * @param resolvers the argument resolvers + * @param validationHelper to apply bean validation with + * @param executor an {@link Executor} to use for {@link Callable} return values + * @param subscription whether the field being fetched is of subscription type + * @param invokeAsync whether to invoke the method through the Executor + * @since 1.3.0 + */ + public DataFetcherHandlerMethod( + HandlerMethod handlerMethod, HandlerMethodArgumentResolverComposite resolvers, + @Nullable BiConsumer validationHelper, + @Nullable Executor executor, boolean invokeAsync, boolean subscription) { + + super(handlerMethod, resolvers, executor, invokeAsync); Assert.isTrue(!resolvers.getResolvers().isEmpty(), "No argument resolvers"); this.validationHelper = (validationHelper != null) ? validationHelper : (controller, args) -> { }; this.subscription = subscription; diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodSupport.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodSupport.java index 1fd0c33b..fb8a42ff 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodSupport.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodSupport.java @@ -48,9 +48,9 @@ public class DataFetcherHandlerMethodSupport extends InvocableHandlerMethodSuppo protected DataFetcherHandlerMethodSupport( HandlerMethod handlerMethod, HandlerMethodArgumentResolverComposite resolvers, - @Nullable Executor executor) { + @Nullable Executor executor, boolean invokeAsync) { - super(handlerMethod, executor); + super(handlerMethod, executor, invokeAsync); this.resolvers = resolvers; } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ContextValueMethodArgumentResolverTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ContextValueMethodArgumentResolverTests.java index 4426245b..9a2e3639 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ContextValueMethodArgumentResolverTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ContextValueMethodArgumentResolverTests.java @@ -125,7 +125,7 @@ public class ContextValueMethodArgumentResolverTests { DataFetcherHandlerMethod handlerMethod = new DataFetcherHandlerMethod( new HandlerMethod(new TestController(), TestController.class.getMethod("handleMono", Mono.class)), - resolvers, null, null, false); + resolvers, null, null, false, false); GraphQLContext graphQLContext = new GraphQLContext.Builder().build(); @@ -147,7 +147,7 @@ public class ContextValueMethodArgumentResolverTests { BatchLoaderHandlerMethod handlerMethod = new BatchLoaderHandlerMethod( new HandlerMethod(controller, - TestController.class.getMethod("getAuthors", List.class, Long.class)), null); + TestController.class.getMethod("getAuthors", List.class, Long.class)), null, false); GraphQLContext context = new GraphQLContext.Builder().build(); context.put("id", 123L); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodTests.java index 7a78bf85..650b0c69 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodTests.java @@ -60,7 +60,7 @@ public class DataFetcherHandlerMethodTests { resolvers.addResolver(new ArgumentMethodArgumentResolver(new GraphQlArgumentBinder())); DataFetcherHandlerMethod handlerMethod = new DataFetcherHandlerMethod( - handlerMethodFor(new TestController(), "hello"), resolvers, null, null, false); + handlerMethodFor(new TestController(), "hello"), resolvers, null, null, false, false); Object result = handlerMethod.invoke( DataFetchingEnvironmentImpl.newDataFetchingEnvironment() @@ -71,14 +71,22 @@ public class DataFetcherHandlerMethodTests { } @Test - void callableReturnValue() throws Exception { + void asyncInvocation() throws Exception { + testAsyncInvocation("handleSync", true); + } + @Test + void asyncInvocationWithCallableReturnValue() throws Exception { + testAsyncInvocation("handleAndReturnCallable", false); + } + + private static void testAsyncInvocation(String methodName, boolean invokeAsync) throws Exception { HandlerMethodArgumentResolverComposite resolvers = new HandlerMethodArgumentResolverComposite(); resolvers.addResolver(Mockito.mock(HandlerMethodArgumentResolver.class)); DataFetcherHandlerMethod handlerMethod = new DataFetcherHandlerMethod( - handlerMethodFor(new TestController(), "handleAndReturnCallable"), resolvers, null, - new SimpleAsyncTaskExecutor(), false); + handlerMethodFor(new TestController(), methodName), resolvers, null, + new SimpleAsyncTaskExecutor(), invokeAsync, false); DataFetchingEnvironment environment = DataFetchingEnvironmentImpl .newDataFetchingEnvironment() @@ -100,7 +108,7 @@ public class DataFetcherHandlerMethodTests { DataFetcherHandlerMethod handlerMethod = new DataFetcherHandlerMethod( handlerMethodFor(new TestController(), "handleAndReturnFuture"), resolvers, - null, null, false); + null, null, false, false); SecurityContextHolder.setContext(new SecurityContextImpl(new TestingAuthenticationToken("usr", "pwd"))); try { @@ -136,6 +144,11 @@ public class DataFetcherHandlerMethodTests { return "Hello, " + name; } + @Nullable + public String handleSync() { + return "A"; + } + @Nullable public Callable handleAndReturnCallable() { return () -> "A";