From efa585b8876bf707c02e66612c31c3c6ac29497b Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 17 Jun 2021 20:02:27 +0100 Subject: [PATCH] Refine and clarify contracts for context management Use ThreadLocalAccessor, the main API for ThreadLocal value management, to state where this is supported, which is in Spring MVC, GraphQL applications. Improve Javadoc in related contracts as well including a minor refactoring in ContextManager. --- .../graphql/execution/ContextManager.java | 19 +++++--- .../execution/ThreadLocalAccessor.java | 43 +++++++++++-------- .../web/DefaultWebGraphQlHandlerBuilder.java | 7 +-- .../graphql/web/WebGraphQlHandler.java | 15 ++++--- .../ContextDataFetcherDecoratorTests.java | 2 +- ...ceptionResolversExceptionHandlerTests.java | 2 +- 6 files changed, 51 insertions(+), 37 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/ContextManager.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/ContextManager.java index 5f55506a..cc36df21 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/ContextManager.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/ContextManager.java @@ -63,15 +63,24 @@ public abstract class ContextManager { } /** - * Use the given accessor to extract ThreadLocal value, and return a Reactor context - * that contains both the extracted values and the accessor. + * Use the given accessor to extract ThreadLocal values and save them in a + * sub-map in the given {@link Context}, so those can be restored later + * around the execution of data fetchers and exception resolvers. The accessor + * instance is also saved in the Reactor Context so it can be used to + * actually restore and reset ThreadLocal values. * @param accessor the accessor to use - * @return the reactor {@link ContextView} + * @param context the context to write to if there are ThreadLocal values + * @return a new Reactor {@link ContextView} or the {@code Context} instance + * that was passed in, if there were no ThreadLocal values to extract. */ - public static ContextView extractThreadLocalValues(ThreadLocalAccessor accessor) { + public static Context extractThreadLocalValues(ThreadLocalAccessor accessor, Context context) { Map valuesMap = new LinkedHashMap<>(); accessor.extractValues(valuesMap); - return Context.of(THREAD_LOCAL_VALUES_KEY, valuesMap, THREAD_LOCAL_ACCESSOR_KEY, accessor); + if (valuesMap.isEmpty()) { + return context; + } + return context.putAll((ContextView) Context.of( + THREAD_LOCAL_VALUES_KEY, valuesMap, THREAD_LOCAL_ACCESSOR_KEY, accessor)); } /** diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/ThreadLocalAccessor.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/ThreadLocalAccessor.java index 75c9f7d0..3966f5c9 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/ThreadLocalAccessor.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/ThreadLocalAccessor.java @@ -22,44 +22,51 @@ import java.util.Map; import org.springframework.beans.factory.ObjectProvider; /** - * Interface to be implemented by a framework or an application in order to assist with - * extracting ThreadLocal values at the web layer, which can then be re-established for - * DataFetcher's that are potentially executing on a different thread. + * Interface to be implemented to assist with the extraction of ThreadLocal + * values at the start of GraphQL request execution, e.g. in the web layer. + * Those values are saved in {@link graphql.ExecutionInput} and restored later + * around the invocation of data fetchers and exception resolvers which may be + * in a different thread. * - *

- * Implementations may be declared as beans in Spring configuration and ordered as defined - * in {@link ObjectProvider#orderedStream()}. + *

Implementations of this interface are typically declared as beans in + * Spring configuration and are invoked in order as defined in + * {@link ObjectProvider#orderedStream()}. + * + *

Currently supported for GraphQL requests over HTTP and WebSocket in + * Spring MVC applications. * * @author Rossen Stoyanchev * @since 1.0.0 + * @see org.springframework.graphql.web.WebGraphQlHandler.Builder#threadLocalAccessor(ThreadLocalAccessor...) */ public interface ThreadLocalAccessor { /** - * Extract ThreadLocal values and add them to the given Map which is then passed to - * {@link #restoreValues(Map)} and {@link #resetValues(Map)} before and after the - * execution of a {@link graphql.schema.DataFetcher}. - * @param container container for ThreadLocal values + * Extract ThreadLocal values and add them to the given Map, so they can be + * saved and subsequently {@link #restoreValues(Map) restored} around the + * invocation of data fetchers and exception resolvers. + * @param container to add extracted ThreadLocal values to */ void extractValues(Map container); /** - * Re-establish ThreadLocal context by looking up values, previously extracted via - * {@link #extractValues(Map)}. - * @param values the saved ThreadLocal values + * Restore ThreadLocal context by looking up previously + * {@link #extractValues(Map) extracted} values. + * @param values previously extracted saved ThreadLocal values */ void restoreValues(Map values); /** - * Reset ThreadLocal context for the given values, previously extracted via - * {@link #extractValues(Map)}. - * @param values the saved ThreadLocal values + * Reset ThreadLocal context for the given, previously + * {@link #extractValues(Map) extracted} and then + * {@link #restoreValues(Map) restored} values. + * @param values previously extracted saved ThreadLocal values */ void resetValues(Map values); /** - * Create a composite accessor that delegates to all of the given accessors. - * @param accessors the accessors to aggregate + * Create a composite accessor that applies all of the given ThreadLocal accessors. + * @param accessors the accessors to apply * @return the composite accessor */ static ThreadLocalAccessor composite(List accessors) { diff --git a/spring-graphql/src/main/java/org/springframework/graphql/web/DefaultWebGraphQlHandlerBuilder.java b/spring-graphql/src/main/java/org/springframework/graphql/web/DefaultWebGraphQlHandlerBuilder.java index 2de97b3e..7dd38bff 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/web/DefaultWebGraphQlHandlerBuilder.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/web/DefaultWebGraphQlHandlerBuilder.java @@ -23,7 +23,6 @@ import java.util.List; import graphql.ExecutionInput; import reactor.core.publisher.Mono; -import reactor.util.context.ContextView; import org.springframework.graphql.GraphQlService; import org.springframework.graphql.execution.ContextManager; @@ -118,10 +117,8 @@ class DefaultWebGraphQlHandlerBuilder implements WebGraphQlHandler.Builder { @Override public Mono handle(WebInput input) { - return this.delegate.handle(input).contextWrite((context) -> { - ContextView view = ContextManager.extractThreadLocalValues(this.accessor); - return (!view.isEmpty() ? context.putAll(view) : context); - }); + return this.delegate.handle(input).contextWrite((context) -> + ContextManager.extractThreadLocalValues(this.accessor, context)); } } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/web/WebGraphQlHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/web/WebGraphQlHandler.java index 557ff56f..9a43e13f 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/web/WebGraphQlHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/web/WebGraphQlHandler.java @@ -24,8 +24,8 @@ import org.springframework.graphql.GraphQlService; import org.springframework.graphql.execution.ThreadLocalAccessor; /** - * Contract to handle a GraphQL over HTTP or WebSocket request that forms the basis of a - * {@link WebInterceptor} delegation chain. + * Common contract to handle a GraphQL request over HTTP or WebSocket for use + * with both Spring MVC and Spring WebFlux. * * @author Rossen Stoyanchev * @since 1.0.0 @@ -50,8 +50,8 @@ public interface WebGraphQlHandler { } /** - * Builder for {@link WebGraphQlHandler} that represents a {@link WebInterceptor} - * chain followed by a {@link GraphQlService}. + * Builder for a {@link WebGraphQlHandler} that executes a + * {@link WebInterceptor} chain followed by a {@link GraphQlService}. */ interface Builder { @@ -70,9 +70,10 @@ public interface WebGraphQlHandler { Builder interceptors(List interceptors); /** - * Configure accessors for ThreadLocal variables to use to extract ThreadLocal - * values at the Web framework level, have those propagated and re-established at - * the DataFetcher level. + * Configure accessors for ThreadLocal variables to use to extract + * ThreadLocal values at the start of GraphQL execution in the web layer, + * and have those saved, and restored around the invocation of data + * fetchers and exception resolvers. * @param accessors the accessors to add * @return this builder */ diff --git a/spring-graphql/src/test/java/org/springframework/graphql/execution/ContextDataFetcherDecoratorTests.java b/spring-graphql/src/test/java/org/springframework/graphql/execution/ContextDataFetcherDecoratorTests.java index 742ffff4..005e54c0 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/execution/ContextDataFetcherDecoratorTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/execution/ContextDataFetcherDecoratorTests.java @@ -109,7 +109,7 @@ public class ContextDataFetcherDecoratorTests { (env) -> "Hello " + nameThreadLocal.get()); ExecutionInput input = ExecutionInput.newExecutionInput().query("{ greeting }").build(); - ContextView view = ContextManager.extractThreadLocalValues(accessor); + ContextView view = ContextManager.extractThreadLocalValues(accessor, Context.empty()); ContextManager.setReactorContext(view, input); ExecutionResult result = Mono.delay(Duration.ofMillis(10)) diff --git a/spring-graphql/src/test/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandlerTests.java b/spring-graphql/src/test/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandlerTests.java index b607b5fa..3623f313 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandlerTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandlerTests.java @@ -103,7 +103,7 @@ public class ExceptionResolversExceptionHandlerTests { .build())); ExecutionInput input = ExecutionInput.newExecutionInput().query("{ greeting }").build(); - ContextView view = ContextManager.extractThreadLocalValues(accessor); + ContextView view = ContextManager.extractThreadLocalValues(accessor, Context.empty()); ContextManager.setReactorContext(view, input); ExecutionResult result = Mono.delay(Duration.ofMillis(10))