From 023973a7a02f3f2febf921cbbd11c344616a9d6e Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 7 Jun 2023 16:12:43 +0200 Subject: [PATCH] Upgrade to Context Propagation 1.0.3 Closes gh-717 Closes gh-688 --- platform/build.gradle | 2 +- .../method/InvocableHandlerMethodSupport.java | 5 +- .../ContextDataFetcherDecorator.java | 14 +++-- .../DataFetcherExceptionResolverAdapter.java | 5 +- .../execution/DefaultBatchLoaderRegistry.java | 8 ++- .../DefaultExecutionGraphQlService.java | 4 +- .../ExceptionResolversExceptionHandler.java | 5 +- .../SecurityContextThreadLocalAccessor.java | 51 ++++++++++++++++++- .../SubscriptionExceptionResolverAdapter.java | 5 +- .../DefaultWebGraphQlHandlerBuilder.java | 5 +- .../webmvc/GraphQlWebSocketHandler.java | 5 +- 11 files changed, 90 insertions(+), 19 deletions(-) diff --git a/platform/build.gradle b/platform/build.gradle index 594598713..4f7b5391f 100644 --- a/platform/build.gradle +++ b/platform/build.gradle @@ -25,7 +25,7 @@ dependencies { constraints { api("com.graphql-java:graphql-java:${graphQlJavaVersion}") - api("io.micrometer:context-propagation:1.0.2") + api("io.micrometer:context-propagation:1.0.3") api("jakarta.annotation:jakarta.annotation-api:2.1.1") api("jakarta.servlet:jakarta.servlet-api:6.0.0") 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 02f64ce59..f36089cc7 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 @@ -26,6 +26,7 @@ import graphql.GraphQLContext; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import reactor.core.publisher.Mono; import org.springframework.core.CoroutinesUtils; @@ -44,6 +45,8 @@ public abstract class InvocableHandlerMethodSupport extends HandlerMethod { private static final Object NO_VALUE = new Object(); + private static final ContextSnapshotFactory SNAPSHOT_FACTORY = ContextSnapshotFactory.builder().build(); + private final boolean hasCallableReturnValue; @@ -113,7 +116,7 @@ private Object handleReturnValue(GraphQLContext graphQLContext, @Nullable Object return CompletableFuture.supplyAsync( () -> { try { - return ContextSnapshot.captureFrom(graphQLContext).wrap((Callable) result).call(); + return SNAPSHOT_FACTORY.captureFrom(graphQLContext).wrap((Callable) result).call(); } catch (Exception ex) { throw new IllegalStateException( diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/ContextDataFetcherDecorator.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/ContextDataFetcherDecorator.java index 81afca794..600bd258f 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/ContextDataFetcherDecorator.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/ContextDataFetcherDecorator.java @@ -33,6 +33,7 @@ import graphql.util.TraversalControl; import graphql.util.TraverserContext; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -59,6 +60,8 @@ final class ContextDataFetcherDecorator implements DataFetcher { private final SubscriptionExceptionResolver subscriptionExceptionResolver; + private final ContextSnapshotFactory snapshotFactory = ContextSnapshotFactory.builder().build(); + private ContextDataFetcherDecorator( DataFetcher delegate, boolean subscription, SubscriptionExceptionResolver subscriptionExceptionResolver) { @@ -73,18 +76,13 @@ private ContextDataFetcherDecorator( @Override public Object get(DataFetchingEnvironment environment) throws Exception { - GraphQLContext context; - // temporarily merge global and local graphql context until https://github.com/micrometer-metrics/context-propagation/pull/98 + ContextSnapshot snapshot; if (environment.getLocalContext() instanceof GraphQLContext localContext) { - context = GraphQLContext.newContext() - .of(environment.getGraphQlContext()) - .of(localContext) - .build(); + snapshot = snapshotFactory.captureFrom(environment.getGraphQlContext(), localContext); } else { - context = environment.getGraphQlContext(); + snapshot = snapshotFactory.captureFrom(environment.getGraphQlContext()); } - ContextSnapshot snapshot = ContextSnapshot.captureFrom(context); Object value = snapshot.wrap(() -> this.delegate.get(environment)).call(); if (this.subscription) { diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/DataFetcherExceptionResolverAdapter.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/DataFetcherExceptionResolverAdapter.java index b8c6c471f..5f73637d6 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/DataFetcherExceptionResolverAdapter.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/DataFetcherExceptionResolverAdapter.java @@ -22,6 +22,7 @@ import graphql.GraphQLError; import graphql.schema.DataFetchingEnvironment; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import io.micrometer.context.ThreadLocalAccessor; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -52,6 +53,8 @@ public abstract class DataFetcherExceptionResolverAdapter implements DataFetcher protected final Log logger = LogFactory.getLog(getClass()); + protected final ContextSnapshotFactory snapshotFactory = ContextSnapshotFactory.builder().build(); + private boolean threadLocalContextAware; @@ -98,7 +101,7 @@ private List resolveInternal(Throwable exception, DataFetchingEnvi return resolveToMultipleErrors(exception, env); } try { - return ContextSnapshot.captureFrom(env.getGraphQlContext()) + return snapshotFactory.captureFrom(env.getGraphQlContext()) .wrap(() -> resolveToMultipleErrors(exception, env)) .call(); } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java index bcb0c59ee..d8b887456 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java @@ -27,6 +27,7 @@ import graphql.GraphQLContext; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import org.dataloader.BatchLoaderContextProvider; import org.dataloader.BatchLoaderEnvironment; import org.dataloader.BatchLoaderWithContext; @@ -52,6 +53,8 @@ */ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { + private static final ContextSnapshotFactory SNAPSHOT_FACTORY = ContextSnapshotFactory.builder().build(); + private final List> loaders = new ArrayList<>(); private final List> mappedLoaders = new ArrayList<>(); @@ -59,6 +62,7 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { private final Supplier defaultOptionsSupplier; + /** * Default constructor */ @@ -230,7 +234,7 @@ public DataLoaderOptions getOptions() { @Override public CompletionStage> load(List keys, BatchLoaderEnvironment environment) { GraphQLContext graphQLContext = environment.getContext(); - ContextSnapshot snapshot = ContextSnapshot.captureFrom(graphQLContext); + ContextSnapshot snapshot = SNAPSHOT_FACTORY.captureFrom(graphQLContext); try { return snapshot.wrap(() -> this.loader.apply(keys, environment) @@ -280,7 +284,7 @@ public DataLoaderOptions getOptions() { @Override public CompletionStage> load(Set keys, BatchLoaderEnvironment environment) { GraphQLContext graphQLContext = environment.getContext(); - ContextSnapshot snapshot = ContextSnapshot.captureFrom(graphQLContext); + ContextSnapshot snapshot = SNAPSHOT_FACTORY.captureFrom(graphQLContext); try { return snapshot.wrap(() -> this.loader.apply(keys, environment) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultExecutionGraphQlService.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultExecutionGraphQlService.java index 0b1352fd3..3434c8f8d 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultExecutionGraphQlService.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultExecutionGraphQlService.java @@ -25,6 +25,7 @@ import graphql.GraphQLContext; import graphql.execution.ExecutionIdProvider; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import org.dataloader.DataLoaderRegistry; import reactor.core.publisher.Mono; @@ -45,6 +46,7 @@ public class DefaultExecutionGraphQlService implements ExecutionGraphQlService { private static final BiFunction RESET_EXECUTION_ID_CONFIGURER = (executionInput, builder) -> builder.executionId(null).build(); + private final ContextSnapshotFactory snapshotFactory = ContextSnapshotFactory.builder().build(); private final GraphQlSource graphQlSource; @@ -77,7 +79,7 @@ public final Mono execute(ExecutionGraphQlRequest requ request.configureExecutionInput(RESET_EXECUTION_ID_CONFIGURER); } ExecutionInput executionInput = request.toExecutionInput(); - ContextSnapshot.captureFrom(contextView).updateContext(executionInput.getGraphQLContext()); + snapshotFactory.captureFrom(contextView).updateContext(executionInput.getGraphQLContext()); ExecutionInput updatedExecutionInput = registerDataLoaders(executionInput); return Mono.fromFuture(this.graphQlSource.graphQl().executeAsync(updatedExecutionInput)) .map(result -> new DefaultExecutionGraphQlResponse(updatedExecutionInput, result)); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandler.java index 372003bad..456cc829c 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandler.java @@ -29,6 +29,7 @@ import graphql.execution.ExecutionId; import graphql.schema.DataFetchingEnvironment; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; @@ -47,6 +48,8 @@ class ExceptionResolversExceptionHandler implements DataFetcherExceptionHandler private static final Log logger = LogFactory.getLog(ExceptionResolversExceptionHandler.class); + private final ContextSnapshotFactory snapshotFactory = ContextSnapshotFactory.builder().build(); + private final List resolvers; /** @@ -70,7 +73,7 @@ public DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandler public CompletableFuture handleException(DataFetcherExceptionHandlerParameters params) { Throwable exception = unwrapException(params); DataFetchingEnvironment env = params.getDataFetchingEnvironment(); - ContextSnapshot snapshot = ContextSnapshot.captureFrom(env.getGraphQlContext()); + ContextSnapshot snapshot = snapshotFactory.captureFrom(env.getGraphQlContext()); try { return Flux.fromIterable(this.resolvers) .flatMap(resolver -> resolver.resolveException(exception, env)) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/SecurityContextThreadLocalAccessor.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SecurityContextThreadLocalAccessor.java index 158238bc3..34022e131 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/SecurityContextThreadLocalAccessor.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/SecurityContextThreadLocalAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * 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. @@ -72,10 +72,30 @@ private void setValueInternal(Object value) { } @Override + public void setValue() { + this.delegate.setValue(); + } + + @Override + @Deprecated public void reset() { this.delegate.reset(); } + @Override + public void restore(Object previousValue) { + restoreInternal(previousValue); + } + + @SuppressWarnings("unchecked") + public void restoreInternal(Object previousValue) { + ((ThreadLocalAccessor) this.delegate).restore((V) previousValue); + } + + @Override + public void restore() { + this.delegate.restore(); + } private static class DelegateAccessor implements ThreadLocalAccessor { @@ -95,6 +115,22 @@ public void setValue(Object value) { } @Override + public void setValue() { + SecurityContextHolder.clearContext(); + } + + @Override + public void restore(Object previousValue) { + SecurityContextHolder.setContext((SecurityContext) previousValue); + } + + @Override + public void restore() { + SecurityContextHolder.clearContext(); + } + + @Override + @Deprecated public void reset() { SecurityContextHolder.clearContext(); } @@ -119,6 +155,19 @@ public void setValue(Object value) { } @Override + public void setValue() { + } + + @Override + public void restore(Object previousValue) { + } + + @Override + public void restore() { + } + + @Override + @Deprecated public void reset() { } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionExceptionResolverAdapter.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionExceptionResolverAdapter.java index 836c00be9..03d4f9ace 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionExceptionResolverAdapter.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionExceptionResolverAdapter.java @@ -22,6 +22,7 @@ import graphql.GraphQLError; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import io.micrometer.context.ThreadLocalAccessor; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -50,6 +51,8 @@ public abstract class SubscriptionExceptionResolverAdapter implements Subscripti protected final Log logger = LogFactory.getLog(getClass()); + protected final ContextSnapshotFactory snapshotFactory = ContextSnapshotFactory.builder().build(); + private boolean threadLocalContextAware; @@ -83,7 +86,7 @@ public boolean isThreadLocalContextAware() { public final Mono> resolveException(Throwable exception) { if (this.threadLocalContextAware) { return Mono.deferContextual(contextView -> { - ContextSnapshot snapshot = ContextSnapshot.captureFrom(contextView); + ContextSnapshot snapshot = snapshotFactory.captureFrom(contextView); try { List errors = snapshot.wrap(() -> resolveToMultipleErrors(exception)).call(); return Mono.justOrEmpty(errors); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/DefaultWebGraphQlHandlerBuilder.java b/spring-graphql/src/main/java/org/springframework/graphql/server/DefaultWebGraphQlHandlerBuilder.java index c965fbf0f..33d93b3c3 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/DefaultWebGraphQlHandlerBuilder.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/DefaultWebGraphQlHandlerBuilder.java @@ -21,6 +21,7 @@ import java.util.List; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import reactor.core.publisher.Mono; import org.springframework.graphql.ExecutionGraphQlService; @@ -70,6 +71,8 @@ public WebGraphQlHandler.Builder interceptors(List interc @Override public WebGraphQlHandler build() { + ContextSnapshotFactory snapshotFactory = ContextSnapshotFactory.builder().build(); + Chain endOfChain = request -> this.service.execute(request).map(WebGraphQlResponse::new); Chain executionChain = this.interceptors.stream() @@ -87,7 +90,7 @@ public WebSocketGraphQlInterceptor getWebSocketInterceptor() { @Override public Mono handleRequest(WebGraphQlRequest request) { - ContextSnapshot snapshot = ContextSnapshot.captureAll(); + ContextSnapshot snapshot = snapshotFactory.captureAll(); return executionChain.next(request).contextWrite(snapshot::updateContext); } }; diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandler.java index c4184cb74..08eeb042b 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandler.java @@ -36,6 +36,7 @@ import graphql.GraphQLError; import graphql.GraphqlErrorBuilder; import io.micrometer.context.ContextSnapshot; +import io.micrometer.context.ContextSnapshotFactory; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; @@ -349,12 +350,14 @@ private static class ContextHandshakeInterceptor implements HandshakeInterceptor private static final String KEY = ContextSnapshot.class.getName(); + private static final ContextSnapshotFactory SNAPSHOT_FACTORY = ContextSnapshotFactory.builder().build(); + @Override public boolean beforeHandshake( ServerHttpRequest request, ServerHttpResponse response, WebSocketHandler wsHandler, Map attributes) { - attributes.put(KEY, ContextSnapshot.captureAll()); + attributes.put(KEY, SNAPSHOT_FACTORY.captureAll()); return true; }