From a5809e95df5e8012633f11101447aa6558387c94 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 19 Jul 2022 11:41:47 +0100 Subject: [PATCH] Polishing contribution See gh-398 --- .../AbstractGraphQlSourceBuilder.java | 18 +- ...mpositeSubscriptionExceptionResolver.java} | 59 ++++--- .../ContextDataFetcherDecorator.java | 51 +++--- .../DataFetcherExceptionResolver.java | 29 +++- .../DataFetcherExceptionResolverAdapter.java | 10 +- .../ExceptionResolversExceptionHandler.java | 1 + .../graphql/execution/GraphQlSource.java | 29 ++-- .../SubscriptionExceptionResolver.java | 62 +++++-- .../SubscriptionExceptionResolverAdapter.java | 45 +++-- .../SubscriptionPublisherException.java | 62 +++++++ .../SubscriptionStreamException.java | 20 --- .../support/GraphQlWebSocketMessage.java | 11 +- .../graphql/server/webflux/CodecDelegate.java | 16 +- .../webflux/GraphQlWebSocketHandler.java | 16 +- .../webmvc/GraphQlWebSocketHandler.java | 27 +-- .../client/MockGraphQlWebSocketServer.java | 2 +- .../ContextDataFetcherDecoratorTests.java | 134 +++++---------- ...ceptionResolversExceptionHandlerTests.java | 6 +- .../server/WebGraphQlHandlerTests.java | 9 +- .../webflux/GraphQlWebSocketHandlerTests.java | 155 ++++++++---------- .../webmvc/GraphQlWebSocketHandlerTests.java | 65 +++----- .../springframework/graphql/GraphQlSetup.java | 19 ++- 22 files changed, 447 insertions(+), 399 deletions(-) rename spring-graphql/src/main/java/org/springframework/graphql/execution/{DelegatingSubscriptionExceptionResolver.java => CompositeSubscriptionExceptionResolver.java} (50%) create mode 100644 spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionPublisherException.java delete mode 100644 spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionStreamException.java diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/AbstractGraphQlSourceBuilder.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/AbstractGraphQlSourceBuilder.java index 1fd48b76..1d60e426 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/AbstractGraphQlSourceBuilder.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/AbstractGraphQlSourceBuilder.java @@ -16,6 +16,12 @@ package org.springframework.graphql.execution; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; + import graphql.GraphQL; import graphql.execution.instrumentation.ChainedInstrumentation; import graphql.execution.instrumentation.Instrumentation; @@ -24,9 +30,6 @@ import graphql.schema.GraphQLSchema; import graphql.schema.GraphQLTypeVisitor; import graphql.schema.SchemaTraverser; -import java.util.*; -import java.util.function.Consumer; - /** * Implementation of {@link GraphQlSource.Builder} that leaves it to subclasses @@ -57,8 +60,8 @@ abstract class AbstractGraphQlSourceBuilder> } @Override - public B subscriptionExceptionResolvers(List subscriptionExceptionResolvers) { - this.subscriptionExceptionResolvers.addAll(subscriptionExceptionResolvers); + public B subscriptionExceptionResolvers(List resolvers) { + this.subscriptionExceptionResolvers.addAll(resolvers); return self(); } @@ -110,10 +113,7 @@ abstract class AbstractGraphQlSourceBuilder> protected abstract GraphQLSchema initGraphQlSchema(); private GraphQLSchema applyTypeVisitors(GraphQLSchema schema) { - SubscriptionExceptionResolver subscriptionExceptionResolver = new DelegatingSubscriptionExceptionResolver( - subscriptionExceptionResolvers); - GraphQLTypeVisitor visitor = ContextDataFetcherDecorator.createVisitor(subscriptionExceptionResolver); - + GraphQLTypeVisitor visitor = ContextDataFetcherDecorator.createVisitor(this.subscriptionExceptionResolvers); List visitors = new ArrayList<>(this.typeVisitors); visitors.add(visitor); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/DelegatingSubscriptionExceptionResolver.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/CompositeSubscriptionExceptionResolver.java similarity index 50% rename from spring-graphql/src/main/java/org/springframework/graphql/execution/DelegatingSubscriptionExceptionResolver.java rename to spring-graphql/src/main/java/org/springframework/graphql/execution/CompositeSubscriptionExceptionResolver.java index 3b2024a4..87f7a3a0 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/DelegatingSubscriptionExceptionResolver.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/CompositeSubscriptionExceptionResolver.java @@ -16,59 +16,66 @@ package org.springframework.graphql.execution; -import graphql.ErrorType; +import java.util.Collections; +import java.util.List; + import graphql.GraphQLError; import graphql.GraphqlErrorBuilder; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.util.Assert; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; -import java.util.Collections; -import java.util.List; +import org.springframework.util.Assert; /** - * An implementation of {@link SubscriptionExceptionResolver} that is trying to map exception to GraphQL error - * using provided implementation of {@link SubscriptionExceptionResolver}. - *
- * If none of provided implementations resolve exception to error or if any of implementation throw an exception, - * this {@link SubscriptionExceptionResolver} will return a default error. + * Implementation of {@link SubscriptionExceptionResolver} that is given a list + * of other {@link SubscriptionExceptionResolver}'s to invoke in turn until one + * returns a list of {@link GraphQLError}'s. + * + *

If the exception remains unresolved, it is mapped to a default error of + * type {@link ErrorType#INTERNAL_ERROR} with a generic message. * * @author Mykyta Ivchenko - * @see SubscriptionExceptionResolver + * @author Rossen Stoyanchev + * @since 1.0.1 */ -public class DelegatingSubscriptionExceptionResolver implements SubscriptionExceptionResolver { - private static final Log logger = LogFactory.getLog(DelegatingSubscriptionExceptionResolver.class); +class CompositeSubscriptionExceptionResolver implements SubscriptionExceptionResolver { + + private static final Log logger = LogFactory.getLog(CompositeSubscriptionExceptionResolver.class); + private final List resolvers; - public DelegatingSubscriptionExceptionResolver(List resolvers) { - Assert.notNull(resolvers, "'resolvers' list must be not null."); + + CompositeSubscriptionExceptionResolver(List resolvers) { + Assert.notNull(resolvers, "'resolvers' is required"); this.resolvers = resolvers; } + @Override public Mono> resolveException(Throwable exception) { - return Flux.fromIterable(resolvers) + return Flux.fromIterable(this.resolvers) .flatMap(resolver -> resolver.resolveException(exception)) .next() - .onErrorResume(error -> Mono.just(handleMappingException(error, exception))) - .defaultIfEmpty(createDefaultErrors()); + .onErrorResume(error -> Mono.just(handleResolverException(error, exception))) + .defaultIfEmpty(createDefaultError()); } - private List handleMappingException(Throwable resolverException, Throwable originalException) { + private List handleResolverException( + Throwable resolverException, Throwable originalException) { + if (logger.isWarnEnabled()) { logger.warn("Failure while resolving " + originalException.getClass().getName(), resolverException); } - return createDefaultErrors(); + return createDefaultError(); } - private List createDefaultErrors() { - GraphQLError error = GraphqlErrorBuilder.newError() - .message("Unknown error") - .errorType(ErrorType.DataFetchingException) - .build(); - - return Collections.singletonList(error); + private List createDefaultError() { + return Collections.singletonList(GraphqlErrorBuilder.newError() + .message("Subscription error") + .errorType(ErrorType.INTERNAL_ERROR) + .build()); } + } 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 3c158002..1f2d3d9b 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 @@ -16,17 +16,25 @@ package org.springframework.graphql.execution; +import java.util.List; + import graphql.ExecutionInput; -import graphql.schema.*; +import graphql.schema.DataFetcher; +import graphql.schema.DataFetchingEnvironment; +import graphql.schema.GraphQLCodeRegistry; +import graphql.schema.GraphQLFieldDefinition; +import graphql.schema.GraphQLFieldsContainer; +import graphql.schema.GraphQLSchemaElement; +import graphql.schema.GraphQLTypeVisitor; +import graphql.schema.GraphQLTypeVisitorStub; import graphql.util.TraversalControl; import graphql.util.TraverserContext; import org.reactivestreams.Publisher; -import org.springframework.util.Assert; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.util.context.ContextView; -import java.util.function.Function; +import org.springframework.util.Assert; /** * Wrap a {@link DataFetcher} to enable the following: @@ -35,6 +43,7 @@ import java.util.function.Function; *

  • Support {@link Flux} return value as a shortcut to {@link Flux#collectList()}. *
  • Re-establish Reactor Context passed via {@link ExecutionInput}. *
  • Re-establish ThreadLocal context passed via {@link ExecutionInput}. + *
  • Resolve exceptions from a GraphQL subscription {@link Publisher}. * * * @author Rossen Stoyanchev @@ -50,6 +59,7 @@ final class ContextDataFetcherDecorator implements DataFetcher { private ContextDataFetcherDecorator( DataFetcher delegate, boolean subscription, SubscriptionExceptionResolver subscriptionExceptionResolver) { + Assert.notNull(delegate, "'delegate' DataFetcher is required"); Assert.notNull(subscriptionExceptionResolver, "'subscriptionExceptionResolver' is required"); this.delegate = delegate; @@ -66,8 +76,11 @@ final class ContextDataFetcherDecorator implements DataFetcher { ContextView contextView = ReactorContextManager.getReactorContext(environment.getGraphQlContext()); if (this.subscription) { - Publisher publisher = interceptSubscriptionPublisherWithExceptionHandler((Publisher) value); - return (!contextView.isEmpty() ? Flux.from(publisher).contextWrite(contextView) : publisher); + Assert.state(value instanceof Publisher, "Expected Publisher for a subscription"); + Flux flux = Flux.from((Publisher) value).onErrorResume(exception -> + this.subscriptionExceptionResolver.resolveException(exception) + .flatMap(errors -> Mono.error(new SubscriptionPublisherException(errors, exception)))); + return (!contextView.isEmpty() ? flux.contextWrite(contextView) : flux); } if (value instanceof Flux) { @@ -85,29 +98,15 @@ final class ContextDataFetcherDecorator implements DataFetcher { return value; } - @SuppressWarnings("unchecked") - private Publisher interceptSubscriptionPublisherWithExceptionHandler(Publisher publisher) { - Function> onErrorResumeFunction = e -> - subscriptionExceptionResolver.resolveException(e) - .flatMap(errors -> Mono.error(new SubscriptionStreamException(errors))); - - if (publisher instanceof Flux) { - return ((Flux) publisher).onErrorResume(onErrorResumeFunction); - } - - if (publisher instanceof Mono) { - return ((Mono) publisher).onErrorResume(onErrorResumeFunction); - } - - throw new IllegalArgumentException("Unknown publisher type: '" + publisher.getClass().getName() +"'. " + - "Expected reactor.core.publisher.Mono or reactor.core.publisher.Flux"); - } /** - * {@link GraphQLTypeVisitor} that wraps non-GraphQL data fetchers and adapts them if - * they return {@link Flux} or {@link Mono}. + * Static factory method to create {@link GraphQLTypeVisitor} that wraps + * data fetchers with the {@link ContextDataFetcherDecorator}. */ - static GraphQLTypeVisitor createVisitor(SubscriptionExceptionResolver subscriptionExceptionResolver) { + static GraphQLTypeVisitor createVisitor(List resolvers) { + + SubscriptionExceptionResolver compositeResolver = new CompositeSubscriptionExceptionResolver(resolvers); + return new GraphQLTypeVisitorStub() { @Override public TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition fieldDefinition, @@ -122,7 +121,7 @@ final class ContextDataFetcherDecorator implements DataFetcher { } boolean handlesSubscription = parent.getName().equals("Subscription"); - dataFetcher = new ContextDataFetcherDecorator(dataFetcher, handlesSubscription, subscriptionExceptionResolver); + dataFetcher = new ContextDataFetcherDecorator(dataFetcher, handlesSubscription, compositeResolver); codeRegistry.dataFetcher(parent, fieldDefinition, dataFetcher); return TraversalControl.CONTINUE; } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/DataFetcherExceptionResolver.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/DataFetcherExceptionResolver.java index c859e361..a34ad133 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/DataFetcherExceptionResolver.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/DataFetcherExceptionResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -17,6 +17,7 @@ package org.springframework.graphql.execution; import java.util.List; +import java.util.function.BiFunction; import graphql.GraphQLError; import graphql.schema.DataFetchingEnvironment; @@ -55,9 +56,31 @@ public interface DataFetcherExceptionResolver { * @return a {@code Mono} with errors to add to the GraphQL response; * if the {@code Mono} completes with an empty List, the exception is resolved * without any errors added to the response; if the {@code Mono} completes - * empty, without emitting a List, the exception remains unresolved and gives - * other resolvers a chance. + * empty, without emitting a List, the exception remains unresolved and that + * allows other resolvers to resolve it. */ Mono> resolveException(Throwable exception, DataFetchingEnvironment environment); + + /** + * Factory method to create a {@link DataFetcherExceptionResolver} to resolve + * an exception to a single GraphQL error. Effectively, a shortcut + * for creating {@link DataFetcherExceptionResolverAdapter} and overriding + * its {@code resolveToSingleError} method. + * @param resolver the resolver function to use + * @return the created instance + * @since 1.0.1 + */ + static DataFetcherExceptionResolverAdapter forSingleError( + BiFunction resolver) { + + return new DataFetcherExceptionResolverAdapter() { + + @Override + protected GraphQLError resolveToSingleError(Throwable ex, DataFetchingEnvironment env) { + return resolver.apply(ex, env); + } + }; + } + } 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 1fb06c8f..81c48911 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 @@ -34,14 +34,16 @@ import org.springframework.lang.Nullable; *
  • {@link #resolveToMultipleErrors} * * - *

    Use {@link #from(BiFunction)} to create an instance or extend this class - * and override one of its resolve methods. + *

    Applications may also use + * {@link DataFetcherExceptionResolver#forSingleError(BiFunction)} as a shortcut + * for {@link #resolveToSingleError(Throwable, DataFetchingEnvironment)}. * *

    Implementations can also express interest in ThreadLocal context * propagation, from the underlying transport thread, via * {@link #setThreadLocalContextAware(boolean)}. * * @author Rossen Stoyanchev + * @since 1.0.0 */ public abstract class DataFetcherExceptionResolverAdapter implements DataFetcherExceptionResolver { @@ -57,7 +59,7 @@ public abstract class DataFetcherExceptionResolverAdapter implements DataFetcher /** - * Sub-classes can set this to indicate that ThreadLocal context from the + * Subclasses can set this to indicate that ThreadLocal context from the * transport handler (e.g. HTTP handler) should be restored when resolving * exceptions. *

    Note: This property is applicable only if transports @@ -129,7 +131,9 @@ public abstract class DataFetcherExceptionResolverAdapter implements DataFetcher * resolves exceptions with the given {@code BiFunction}. * @param resolver the resolver function to use * @return the created instance + * @deprecated as of 1.0.1, please use {@link DataFetcherExceptionResolver#forSingleError(BiFunction)} */ + @Deprecated public static DataFetcherExceptionResolverAdapter from( BiFunction resolver) { 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 767d5698..1d35d4df 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 @@ -41,6 +41,7 @@ import org.springframework.util.Assert; * in a sequence until one returns a list of {@link GraphQLError}'s. * * @author Rossen Stoyanchev + * @since 1.0.0 */ class ExceptionResolversExceptionHandler implements DataFetcherExceptionHandler { diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/GraphQlSource.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/GraphQlSource.java index 7f4a076b..b125760b 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/GraphQlSource.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/GraphQlSource.java @@ -16,6 +16,10 @@ package org.springframework.graphql.execution; +import java.util.List; +import java.util.function.BiFunction; +import java.util.function.Consumer; + import graphql.GraphQL; import graphql.execution.instrumentation.Instrumentation; import graphql.schema.GraphQLSchema; @@ -23,12 +27,8 @@ import graphql.schema.GraphQLTypeVisitor; import graphql.schema.TypeResolver; import graphql.schema.idl.RuntimeWiring; import graphql.schema.idl.TypeDefinitionRegistry; -import org.springframework.core.io.Resource; -import java.io.InputStream; -import java.util.List; -import java.util.function.BiFunction; -import java.util.function.Consumer; +import org.springframework.core.io.Resource; /** @@ -83,20 +83,25 @@ public interface GraphQlSource { interface Builder> { /** - * Add {@link DataFetcherExceptionResolver}s for resolving exceptions - * from {@link graphql.schema.DataFetcher}s. + * Add {@link DataFetcherExceptionResolver}'s that are invoked when a + * {@link graphql.schema.DataFetcher} raises an exception. Resolvers + * are invoked in sequence until one emits a list. * @param resolvers the resolvers to add * @return the current builder */ B exceptionResolvers(List resolvers); /** - * Add {@link SubscriptionExceptionResolver}s to map exceptions, thrown by - * GraphQL Subscription publisher. - * @param subscriptionExceptionResolver the subscription exception resolver + * Add {@link SubscriptionExceptionResolver}s that are invoked when a + * GraphQL subscription {@link org.reactivestreams.Publisher} ends with + * error, and given a chance to resolve the exception to one or more + * GraphQL errors to be sent to the client. Resolvers are invoked in + * sequence until one emits a list. + * @param resolvers the subscription exception resolver * @return the current builder + * @since 1.0.1 */ - B subscriptionExceptionResolvers(List subscriptionExceptionResolvers); + B subscriptionExceptionResolvers(List resolvers); /** * Add {@link GraphQLTypeVisitor}s to visit all element of the created @@ -143,7 +148,7 @@ public interface GraphQlSource { /** * Add schema definition resources, typically {@literal ".graphqls"} files, to be - * {@link graphql.schema.idl.SchemaParser#parse(InputStream) parsed} and + * {@link graphql.schema.idl.SchemaParser#parse(java.io.InputStream) parsed} and * {@link TypeDefinitionRegistry#merge(TypeDefinitionRegistry) merged}. * @param resources resources with GraphQL schema definitions * @return the current builder diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionExceptionResolver.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionExceptionResolver.java index 63fa9e00..4fd441a2 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionExceptionResolver.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionExceptionResolver.java @@ -16,35 +16,63 @@ package org.springframework.graphql.execution; +import java.util.List; +import java.util.function.Function; + import graphql.GraphQLError; import reactor.core.publisher.Mono; -import java.util.List; - /** - * Contract to resolve exceptions, that are thrown by subscription publisher. - * Implementations are typically declared as beans in Spring configuration and - * are invoked sequentially until one emits a List of {@link GraphQLError}s. - *
    - * Usually, it is enough to implement this interface by extending {@link SubscriptionExceptionResolverAdapter} - * and overriding one of its {@link SubscriptionExceptionResolverAdapter#resolveToSingleError(Throwable)} - * or {@link SubscriptionExceptionResolverAdapter#resolveToMultipleErrors(Throwable)} + * Contract for a component that is invoked when a GraphQL subscription + * {@link org.reactivestreams.Publisher} ends with an error. + * + *

    Resolver implementations can extend the convenience base class + * {@link SubscriptionExceptionResolverAdapter} and override one of its methods + * {@link SubscriptionExceptionResolverAdapter#resolveToSingleError resolveToSingleError} or + * {@link SubscriptionExceptionResolverAdapter#resolveToMultipleErrors resolveToMultipleErrors} + * that resolve the exception synchronously. + * + *

    Resolved errors are wrapped in a {@link SubscriptionPublisherException} + * and propagated further to the underlying transport which access the errors + * and prepare a final error message to send to the client. * * @author Mykyta Ivchenko + * @author Rossen Stoyanchev + * @since 1.0.1 * @see SubscriptionExceptionResolverAdapter - * @see DelegatingSubscriptionExceptionResolver - * @see org.springframework.graphql.server.webflux.GraphQlWebSocketHandler */ @FunctionalInterface public interface SubscriptionExceptionResolver { + /** - * Resolve given exception as list of {@link GraphQLError}s and send them as WebSocket message. - * @param exception the exception to resolve - * @return a {@code Mono} with errors to send in a WebSocket message; + * Resolve the given exception to a list of {@link GraphQLError}'s to be + * sent in an error message to the client. + * @param exception the exception from the Publisher + * @return a {@code Mono} with the GraphQL errors to send to the client; * if the {@code Mono} completes with an empty List, the exception is resolved - * without any errors added to the response; if the {@code Mono} completes - * empty, without emitting a List, the exception remains unresolved and gives - * other resolvers a chance. + * without any errors to send; if the {@code Mono} completes empty, without + * emitting a List, the exception remains unresolved, and that allows other + * resolvers to resolve it. */ Mono> resolveException(Throwable exception); + + + /** + * Factory method to create a {@link SubscriptionExceptionResolver} to + * resolve an exception to a single GraphQL error. Effectively, a shortcut + * for creating {@link SubscriptionExceptionResolverAdapter} and overriding + * its {@code resolveToSingleError} method. + * @param resolver the resolver function to map the exception + * @return the created instance + */ + static SubscriptionExceptionResolverAdapter forSingleError(Function resolver) { + return new SubscriptionExceptionResolverAdapter() { + + @Override + protected GraphQLError resolveToSingleError(Throwable ex) { + return resolver.apply(ex); + } + }; + } + } 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 3b366f90..1d1ee44a 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 @@ -16,33 +16,58 @@ package org.springframework.graphql.execution; +import java.util.Collections; +import java.util.List; +import java.util.function.Function; + import graphql.GraphQLError; import reactor.core.publisher.Mono; -import java.util.Collections; -import java.util.List; +import org.springframework.lang.Nullable; /** - * Abstract class for {@link SubscriptionExceptionResolver} implementations. - * This class provide an easy way to map an exception as GraphQL error synchronously. - *
    - * To use this class, you need to override either {@link SubscriptionExceptionResolverAdapter#resolveToSingleError(Throwable)} - * or {@link SubscriptionExceptionResolverAdapter#resolveToMultipleErrors(Throwable)}. + * Adapter for {@link SubscriptionExceptionResolver} that pre-implements the + * asynchronous contract and exposes the following synchronous protected methods: + *

      + *
    • {@link #resolveToSingleError} + *
    • {@link #resolveToMultipleErrors} + *
    + * + *

    Applications may also use + * {@link SubscriptionExceptionResolver#forSingleError(Function)} as a shortcut + * for {@link #resolveToSingleError(Throwable)}. * * @author Mykyta Ivchenko + * @author Rossen Stoyanchev + * @since 1.0.1 * @see SubscriptionExceptionResolver */ public abstract class SubscriptionExceptionResolverAdapter implements SubscriptionExceptionResolver { + @Override - public Mono> resolveException(Throwable exception) { - return Mono.just(resolveToMultipleErrors(exception)); + public final Mono> resolveException(Throwable exception) { + return Mono.justOrEmpty(resolveToMultipleErrors(exception)); } + /** + * Override this method to resolve the Exception to multiple GraphQL errors. + * @param exception the exception to resolve + * @return the resolved errors or {@code null} if unresolved + */ + @Nullable protected List resolveToMultipleErrors(Throwable exception) { - return Collections.singletonList(resolveToSingleError(exception)); + GraphQLError error = resolveToSingleError(exception); + return (error != null ? Collections.singletonList(error) : null); } + /** + * Override this method to resolve the Exception to a single GraphQL error. + * @param exception the exception to resolve + * @return the resolved error or {@code null} if unresolved + */ + @Nullable protected GraphQLError resolveToSingleError(Throwable exception) { return null; } + } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionPublisherException.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionPublisherException.java new file mode 100644 index 00000000..82ad10fb --- /dev/null +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionPublisherException.java @@ -0,0 +1,62 @@ +/* + * Copyright 2002-2022 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.execution; + +import java.util.List; + +import graphql.GraphQLError; + +import org.springframework.core.NestedRuntimeException; + +/** + * An exception raised after a GraphQL subscription + * {@link org.reactivestreams.Publisher} ends with an exception, and after that + * exception has been resolved to GraphQL errors. + * + *

    The underlying transport, e.g. WebSocket, can handle a + * {@link SubscriptionPublisherException} and send a final error message to the + * client with the list of GraphQL errors. + * + * @author Mykyta Ivchenko + * @author Rossen Stoyanchev + * @since 1.0.1 + */ +@SuppressWarnings("serial") +public final class SubscriptionPublisherException extends NestedRuntimeException { + + private final List errors; + + + /** + * Constructor with the resolved GraphQL errors and the original exception + * from the GraphQL subscription {@link org.reactivestreams.Publisher}. + */ + public SubscriptionPublisherException(List errors, Throwable cause) { + super("GraphQL subscription ended with error(s): " + errors, cause); + this.errors = errors; + } + + + /** + * Return the GraphQL errors the exception was resolved to by the configured + * {@link SubscriptionExceptionResolver}'s. These errors can be included in + * an error message to be sent to the client by the underlying transport. + */ + public List getErrors() { + return this.errors; + } + +} diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionStreamException.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionStreamException.java deleted file mode 100644 index 71e5ae46..00000000 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/SubscriptionStreamException.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.springframework.graphql.execution; - -import graphql.GraphQLError; -import org.springframework.core.NestedRuntimeException; - -import java.util.List; - -@SuppressWarnings("serial") -public class SubscriptionStreamException extends NestedRuntimeException { - private final List errors; - - public SubscriptionStreamException(List errors) { - super("An exception happened in GraphQL subscription stream."); - this.errors = errors; - } - - public List getErrors() { - return errors; - } -} diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/support/GraphQlWebSocketMessage.java b/spring-graphql/src/main/java/org/springframework/graphql/server/support/GraphQlWebSocketMessage.java index 957aac21..0eabbfb7 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/support/GraphQlWebSocketMessage.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/support/GraphQlWebSocketMessage.java @@ -187,15 +187,12 @@ public class GraphQlWebSocketMessage { /** * Create an {@code "error"} server message. * @param id unique request id - * @param error the error to add as the message payload + * @param errors the error to add as the message payload */ public static GraphQlWebSocketMessage error(String id, List errors) { - Assert.notNull(errors, "GraphQlErrors list is required"); - List> errorsMap = errors.stream() - .map(GraphQLError::toSpecification) - .collect(Collectors.toList()); - - return new GraphQlWebSocketMessage(id, GraphQlWebSocketMessageType.ERROR, errorsMap); + Assert.notNull(errors, "GraphQlError's are required"); + return new GraphQlWebSocketMessage(id, GraphQlWebSocketMessageType.ERROR, + errors.stream().map(GraphQLError::toSpecification).collect(Collectors.toList())); } /** diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/CodecDelegate.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/CodecDelegate.java index a6a685be..bf860e1b 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/CodecDelegate.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/CodecDelegate.java @@ -27,6 +27,8 @@ import org.springframework.core.codec.Decoder; import org.springframework.core.codec.Encoder; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferUtils; +import org.springframework.graphql.execution.ErrorType; +import org.springframework.graphql.execution.SubscriptionPublisherException; import org.springframework.graphql.server.support.GraphQlWebSocketMessage; import org.springframework.http.MediaType; import org.springframework.http.codec.CodecConfigurer; @@ -99,12 +101,13 @@ final class CodecDelegate { return encode(session, GraphQlWebSocketMessage.next(id, responseMap)); } - public WebSocketMessage encodeUnknownError(WebSocketSession session, String id, Throwable ex) { - GraphQLError error = GraphqlErrorBuilder.newError().message(ex.getMessage()).build(); - return encodeError(session, id, Collections.singletonList(error)); - } - - public WebSocketMessage encodeError(WebSocketSession session, String id, List errors) { + public WebSocketMessage encodeError(WebSocketSession session, String id, Throwable ex) { + List errors = ((ex instanceof SubscriptionPublisherException) ? + ((SubscriptionPublisherException) ex).getErrors() : + Collections.singletonList(GraphqlErrorBuilder.newError() + .message("Subscription error") + .errorType(ErrorType.INTERNAL_ERROR) + .build())); return encode(session, GraphQlWebSocketMessage.error(id, errors)); } @@ -112,5 +115,4 @@ final class CodecDelegate { return encode(session, GraphQlWebSocketMessage.complete(id)); } - } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlWebSocketHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlWebSocketHandler.java index 70629558..90db9056 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlWebSocketHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlWebSocketHandler.java @@ -28,12 +28,10 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; import graphql.ExecutionResult; -import graphql.GraphQLError; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; import org.reactivestreams.Subscription; -import org.springframework.graphql.execution.SubscriptionStreamException; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -214,15 +212,11 @@ public class GraphQlWebSocketHandler implements WebSocketHandler { .map(responseMap -> this.codecDelegate.encodeNext(session, id, responseMap)) .concatWith(Mono.fromCallable(() -> this.codecDelegate.encodeComplete(session, id))) .onErrorResume(ex -> { - if (ex instanceof SubscriptionExistsException) { - CloseStatus status = new CloseStatus(4409, "Subscriber for " + id + " already exists"); - return GraphQlStatus.close(session, status); - } - if (ex instanceof SubscriptionStreamException) { - List errors = ((SubscriptionStreamException) ex).getErrors(); - return Mono.fromCallable(() -> this.codecDelegate.encodeError(session, id, errors)); - } - return Mono.fromCallable(() -> this.codecDelegate.encodeUnknownError(session, id, ex)); + if (ex instanceof SubscriptionExistsException) { + CloseStatus status = new CloseStatus(4409, "Subscriber for " + id + " already exists"); + return GraphQlStatus.close(session, status); + } + return Mono.fromCallable(() -> this.codecDelegate.encodeError(session, id, ex)); }); } 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 e3f485ee..052fdd04 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 @@ -41,13 +41,14 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; import org.reactivestreams.Subscription; -import org.springframework.graphql.execution.SubscriptionStreamException; import reactor.core.publisher.BaseSubscriber; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; +import org.springframework.graphql.execution.ErrorType; +import org.springframework.graphql.execution.SubscriptionPublisherException; import org.springframework.graphql.execution.ThreadLocalAccessor; import org.springframework.graphql.server.WebGraphQlHandler; import org.springframework.graphql.server.WebGraphQlResponse; @@ -282,18 +283,18 @@ public class GraphQlWebSocketHandler extends TextWebSocketHandler implements Sub .map(responseMap -> encode(GraphQlWebSocketMessage.next(id, responseMap))) .concatWith(Mono.fromCallable(() -> encode(GraphQlWebSocketMessage.complete(id)))) .onErrorResume((ex) -> { - if (ex instanceof SubscriptionExistsException) { - CloseStatus status = new CloseStatus(4409, "Subscriber for " + id + " already exists"); - GraphQlStatus.closeSession(session, status); - return Flux.empty(); - } - if (ex instanceof SubscriptionStreamException) { - List errors = ((SubscriptionStreamException) ex).getErrors(); - return Mono.just(encode(GraphQlWebSocketMessage.error(id, errors))); - } - String message = ex.getMessage(); - GraphQLError error = GraphqlErrorBuilder.newError().message(message).build(); - return Mono.just(encode(GraphQlWebSocketMessage.error(id, Collections.singletonList(error)))); + if (ex instanceof SubscriptionExistsException) { + CloseStatus status = new CloseStatus(4409, "Subscriber for " + id + " already exists"); + GraphQlStatus.closeSession(session, status); + return Flux.empty(); + } + List errors = ((ex instanceof SubscriptionPublisherException) ? + ((SubscriptionPublisherException) ex).getErrors() : + Collections.singletonList(GraphqlErrorBuilder.newError() + .message("Subscription error") + .errorType(ErrorType.INTERNAL_ERROR) + .build())); + return Mono.just(encode(GraphQlWebSocketMessage.error(id, errors))); }); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/client/MockGraphQlWebSocketServer.java b/spring-graphql/src/test/java/org/springframework/graphql/client/MockGraphQlWebSocketServer.java index d236176b..712a3b3b 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/client/MockGraphQlWebSocketServer.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/client/MockGraphQlWebSocketServer.java @@ -30,8 +30,8 @@ import reactor.core.publisher.Mono; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; -import org.springframework.graphql.support.DefaultGraphQlRequest; import org.springframework.graphql.server.support.GraphQlWebSocketMessage; +import org.springframework.graphql.support.DefaultGraphQlRequest; import org.springframework.http.codec.ClientCodecConfigurer; import org.springframework.lang.Nullable; import org.springframework.web.reactive.socket.WebSocketHandler; 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 8dac171c..56befead 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 @@ -16,25 +16,27 @@ package org.springframework.graphql.execution; -import graphql.*; +import java.time.Duration; +import java.util.Collections; +import java.util.List; + +import graphql.ExecutionInput; +import graphql.ExecutionResult; +import graphql.GraphQL; +import graphql.GraphQLError; +import graphql.GraphqlErrorBuilder; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; -import org.springframework.graphql.GraphQlSetup; -import org.springframework.graphql.ResponseHelper; -import org.springframework.graphql.TestThreadLocalAccessor; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import reactor.util.context.Context; import reactor.util.context.ContextView; -import java.time.Duration; -import java.util.Collections; -import java.util.List; +import org.springframework.graphql.GraphQlSetup; +import org.springframework.graphql.ResponseHelper; +import org.springframework.graphql.TestThreadLocalAccessor; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.verify; /** * Tests for {@link ContextDataFetcherDecorator}. @@ -42,9 +44,13 @@ import static org.mockito.Mockito.verify; */ public class ContextDataFetcherDecoratorTests { + private static final String SCHEMA_CONTENT = + "type Query { greeting: String, greetings: [String] } type Subscription { greetings: String }"; + + @Test void monoDataFetcher() throws Exception { - GraphQL graphQl = GraphQlSetup.schemaContent("type Query { greeting: String }") + GraphQL graphQl = GraphQlSetup.schemaContent(SCHEMA_CONTENT) .queryFetcher("greeting", (env) -> Mono.deferContextual((context) -> { Object name = context.get("name"); @@ -63,7 +69,7 @@ public class ContextDataFetcherDecoratorTests { @Test void fluxDataFetcher() throws Exception { - GraphQL graphQl = GraphQlSetup.schemaContent("type Query { greetings: [String] }") + GraphQL graphQl = GraphQlSetup.schemaContent(SCHEMA_CONTENT) .queryFetcher("greetings", (env) -> Mono.delay(Duration.ofMillis(50)) .flatMapMany((aLong) -> Flux.deferContextual((context) -> { @@ -83,7 +89,7 @@ public class ContextDataFetcherDecoratorTests { @Test void fluxDataFetcherSubscription() throws Exception { - GraphQL graphQl = GraphQlSetup.schemaContent("type Query { greeting: String } type Subscription { greetings: String }") + GraphQL graphQl = GraphQlSetup.schemaContent(SCHEMA_CONTENT) .subscriptionFetcher("greetings", (env) -> Mono.delay(Duration.ofMillis(50)) .flatMapMany((aLong) -> Flux.deferContextual((context) -> { @@ -107,93 +113,43 @@ public class ContextDataFetcherDecoratorTests { @Test void fluxDataFetcherSubscriptionThrowException() throws Exception { - GraphQLError expectedError = GraphqlErrorBuilder.newError() - .message("Error: Example Error") - .errorType(ErrorType.INTERNAL_ERROR) - .extensions(Collections.singletonMap("a", "b")) - .build(); - SubscriptionExceptionResolver subscriptionSingleExceptionResolverAdapter = Mockito.spy( - new SubscriptionExceptionResolverAdapter() { - @Override - protected GraphQLError resolveToSingleError(Throwable exception) { - return GraphqlErrorBuilder.newError() + SubscriptionExceptionResolver resolver = + SubscriptionExceptionResolver.forSingleError(exception -> + GraphqlErrorBuilder.newError() .message("Error: " + exception.getMessage()) - .errorType(ErrorType.INTERNAL_ERROR) + .errorType(ErrorType.BAD_REQUEST) .extensions(Collections.singletonMap("a", "b")) - .build(); - } - } - ); + .build()); - GraphQL graphQl = GraphQlSetup.schemaContent("type Query { greeting: String } type Subscription { greetings: String }") - .subscriptionExceptionResolvers(subscriptionSingleExceptionResolverAdapter) - .subscriptionFetcher("greetings", (env) -> - Mono.delay(Duration.ofMillis(50)) - .flatMapMany((aLong) -> Flux.create(sink -> { + GraphQL graphQl = GraphQlSetup.schemaContent(SCHEMA_CONTENT) + .subscriptionExceptionResolvers(resolver) + .subscriptionFetcher("greetings", + (env) -> Mono.delay(Duration.ofMillis(50)) + .handle((aLong, sink) -> { sink.next("Hi!"); sink.error(new RuntimeException("Example Error")); - }))) + })) .toGraphQl(); - ExecutionInput input = ExecutionInput.newExecutionInput().query("subscription { greetings }").build(); + String query = "subscription { greetings }"; + ExecutionInput input = ExecutionInput.newExecutionInput().query(query).build(); + ExecutionResult result = graphQl.executeAsync(input).get(); - ExecutionResult executionResult = graphQl.executeAsync(input).get(); + Flux flux = ResponseHelper.forSubscription(result) + .map(message -> message.toEntity("greetings", String.class)); - Flux greetingsFlux = ResponseHelper.forSubscription(executionResult) - .map(message -> message.toEntity("greetings", String.class)); - - StepVerifier.create(greetingsFlux) + StepVerifier.create(flux) .expectNext("Hi!") - .expectErrorSatisfies(error -> assertThat(error) - .usingRecursiveComparison() - .isEqualTo(new SubscriptionStreamException(Collections.singletonList(expectedError)))) + .expectErrorSatisfies(ex -> { + List errors = ((SubscriptionPublisherException) ex).getErrors(); + assertThat(errors).hasSize(1); + assertThat(errors.get(0).getMessage()).isEqualTo("Error: Example Error"); + assertThat(errors.get(0).getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); + assertThat(errors.get(0).getExtensions()).isEqualTo(Collections.singletonMap("a", "b")); + + }) .verify(); - - verify(subscriptionSingleExceptionResolverAdapter).resolveException(any(RuntimeException.class)); - } - - @Test - void monoDataFetcherSubscriptionThrowException() throws Exception { - GraphQLError expectedError = GraphqlErrorBuilder.newError() - .message("Error: Example Error") - .errorType(ErrorType.INTERNAL_ERROR) - .extensions(Collections.singletonMap("a", "b")) - .build(); - - SubscriptionExceptionResolver subscriptionSingleExceptionResolverAdapter = Mockito.spy( - new SubscriptionExceptionResolverAdapter() { - @Override - protected GraphQLError resolveToSingleError(Throwable exception) { - return GraphqlErrorBuilder.newError() - .message("Error: " + exception.getMessage()) - .errorType(ErrorType.INTERNAL_ERROR) - .extensions(Collections.singletonMap("a", "b")) - .build(); - } - } - ); - - GraphQL graphQl = GraphQlSetup.schemaContent("type Query { greeting: String } type Subscription { greetings: String }") - .subscriptionExceptionResolvers(subscriptionSingleExceptionResolverAdapter) - .subscriptionFetcher("greetings", (env) -> - Mono.delay(Duration.ofMillis(50)) - .then(Mono.error(new RuntimeException("Example Error")))) - .toGraphQl(); - - ExecutionInput input = ExecutionInput.newExecutionInput().query("subscription { greetings }").build(); - - ExecutionResult executionResult = graphQl.executeAsync(input).get(); - - Flux greetingsFlux = ResponseHelper.forSubscription(executionResult); - - StepVerifier.create(greetingsFlux) - .expectErrorSatisfies(error -> assertThat(error) - .usingRecursiveComparison() - .isEqualTo(new SubscriptionStreamException(Collections.singletonList(expectedError)))) - .verify(); - - verify(subscriptionSingleExceptionResolverAdapter).resolveException(any(RuntimeException.class)); } @Test @@ -202,7 +158,7 @@ public class ContextDataFetcherDecoratorTests { nameThreadLocal.set("007"); TestThreadLocalAccessor accessor = new TestThreadLocalAccessor<>(nameThreadLocal); try { - GraphQL graphQl = GraphQlSetup.schemaContent("type Query { greeting: String }") + GraphQL graphQl = GraphQlSetup.schemaContent(SCHEMA_CONTENT) .queryFetcher("greeting", (env) -> "Hello " + nameThreadLocal.get()) .toGraphQl(); 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 0d2c9d27..87ed8dc3 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 @@ -51,7 +51,7 @@ public class ExceptionResolversExceptionHandlerTests { @Test void resolveException() throws Exception { DataFetcherExceptionResolver resolver = - DataFetcherExceptionResolverAdapter.from((ex, env) -> + DataFetcherExceptionResolver.forSingleError((ex, env) -> GraphqlErrorBuilder.newError(env) .message("Resolved error: " + ex.getMessage()) .errorType(ErrorType.BAD_REQUEST).build()); @@ -93,7 +93,7 @@ public class ExceptionResolversExceptionHandlerTests { TestThreadLocalAccessor accessor = new TestThreadLocalAccessor<>(nameThreadLocal); try { DataFetcherExceptionResolverAdapter resolver = - DataFetcherExceptionResolverAdapter.from((ex, env) -> + DataFetcherExceptionResolver.forSingleError((ex, env) -> GraphqlErrorBuilder.newError(env) .message("Resolved error: " + ex.getMessage() + ", name=" + nameThreadLocal.get()) .errorType(ErrorType.BAD_REQUEST) @@ -119,7 +119,7 @@ public class ExceptionResolversExceptionHandlerTests { @Test void unresolvedException() throws Exception { DataFetcherExceptionResolverAdapter resolver = - DataFetcherExceptionResolverAdapter.from((ex, env) -> null); + DataFetcherExceptionResolver.forSingleError((ex, env) -> null); ExecutionResult result = this.graphQlSetup.exceptionResolver(resolver).toGraphQl() .executeAsync(this.input).get(); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/WebGraphQlHandlerTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/WebGraphQlHandlerTests.java index 569b4608..dfe33c37 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/WebGraphQlHandlerTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/WebGraphQlHandlerTests.java @@ -118,10 +118,11 @@ public class WebGraphQlHandlerTests { nameThreadLocal.set("007"); TestThreadLocalAccessor threadLocalAccessor = new TestThreadLocalAccessor<>(nameThreadLocal); try { - DataFetcherExceptionResolverAdapter exceptionResolver = DataFetcherExceptionResolverAdapter.from((ex, env) -> - GraphqlErrorBuilder.newError(env) - .message("Resolved error: " + ex.getMessage() + ", name=" + nameThreadLocal.get()) - .errorType(ErrorType.BAD_REQUEST).build()); + DataFetcherExceptionResolverAdapter exceptionResolver = + DataFetcherExceptionResolver.forSingleError((ex, env) -> + GraphqlErrorBuilder.newError(env) + .message("Resolved error: " + ex.getMessage() + ", name=" + nameThreadLocal.get()) + .errorType(ErrorType.BAD_REQUEST).build()); exceptionResolver.setThreadLocalContextAware(true); Mono responseMono = this.graphQlSetup.queryFetcher("greeting", this.errorDataFetcher) diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlWebSocketHandlerTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlWebSocketHandlerTests.java index 95b70bc4..6bba8688 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlWebSocketHandlerTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlWebSocketHandlerTests.java @@ -16,31 +16,6 @@ package org.springframework.graphql.server.webflux; -import graphql.GraphQLError; -import graphql.GraphqlErrorBuilder; -import org.assertj.core.api.InstanceOfAssertFactories; -import org.junit.jupiter.api.Test; -import org.mockito.Mockito; -import org.springframework.core.ResolvableType; -import org.springframework.core.io.buffer.DataBuffer; -import org.springframework.core.io.buffer.DataBufferUtils; -import org.springframework.core.io.buffer.DefaultDataBufferFactory; -import org.springframework.graphql.GraphQlSetup; -import org.springframework.graphql.execution.ErrorType; -import org.springframework.graphql.execution.SubscriptionExceptionResolver; -import org.springframework.graphql.execution.SubscriptionExceptionResolverAdapter; -import org.springframework.graphql.server.*; -import org.springframework.graphql.server.support.GraphQlWebSocketMessage; -import org.springframework.graphql.server.support.GraphQlWebSocketMessageType; -import org.springframework.http.codec.ServerCodecConfigurer; -import org.springframework.http.codec.json.Jackson2JsonDecoder; -import org.springframework.web.reactive.socket.CloseStatus; -import org.springframework.web.reactive.socket.WebSocketMessage; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; -import reactor.core.publisher.Sinks; -import reactor.test.StepVerifier; - import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.ArrayList; @@ -50,11 +25,36 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; +import graphql.GraphqlErrorBuilder; +import org.assertj.core.api.InstanceOfAssertFactories; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; +import reactor.core.publisher.Sinks; +import reactor.test.StepVerifier; + +import org.springframework.core.ResolvableType; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferUtils; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; +import org.springframework.graphql.GraphQlSetup; +import org.springframework.graphql.execution.ErrorType; +import org.springframework.graphql.execution.SubscriptionExceptionResolver; +import org.springframework.graphql.server.ConsumeOneAndNeverCompleteInterceptor; +import org.springframework.graphql.server.WebGraphQlHandler; +import org.springframework.graphql.server.WebGraphQlInterceptor; +import org.springframework.graphql.server.WebSocketGraphQlInterceptor; +import org.springframework.graphql.server.WebSocketHandlerTestSupport; +import org.springframework.graphql.server.WebSocketSessionInfo; +import org.springframework.graphql.server.support.GraphQlWebSocketMessage; +import org.springframework.graphql.server.support.GraphQlWebSocketMessageType; +import org.springframework.http.codec.ServerCodecConfigurer; +import org.springframework.http.codec.json.Jackson2JsonDecoder; +import org.springframework.web.reactive.socket.CloseStatus; +import org.springframework.web.reactive.socket.WebSocketMessage; + import static org.assertj.core.api.Assertions.as; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; /** * Unit tests for {@link GraphQlWebSocketHandler}. @@ -312,7 +312,7 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { } @Test - void errorMessagePayloadIsArray() { + void subscriptionErrorPayloadIsArray() { final String GREETING_QUERY = "{" + "\"id\":\"" + SUBSCRIPTION_ID + "\"," + "\"type\":\"subscribe\"," + @@ -324,20 +324,16 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { String schema = "type Subscription { greeting: String! } type Query { greetingUnused: String! }"; - WebGraphQlHandler initHandler = GraphQlSetup.schemaContent(schema) - .subscriptionFetcher("greeting", env -> Flux.just("a", null, "b")) - .interceptor() - .toWebGraphQlHandler(); - - GraphQlWebSocketHandler handler = new GraphQlWebSocketHandler( - initHandler, - ServerCodecConfigurer.create(), - Duration.ofSeconds(60)); - TestWebSocketSession session = new TestWebSocketSession(Flux.just( toWebSocketMessage("{\"type\":\"connection_init\"}"), toWebSocketMessage(GREETING_QUERY))); - handler.handle(session).block(TIMEOUT); + + WebGraphQlHandler webHandler = GraphQlSetup.schemaContent(schema) + .subscriptionFetcher("greeting", env -> Flux.just("a", null, "b")) + .toWebGraphQlHandler(); + + new GraphQlWebSocketHandler(webHandler, ServerCodecConfigurer.create(), TIMEOUT) + .handle(session).block(TIMEOUT); StepVerifier.create(session.getOutput()) .consumeNextWith((message) -> assertMessageType(message, GraphQlWebSocketMessageType.CONNECTION_ACK)) @@ -346,29 +342,24 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { assertThat(actual.getId()).isEqualTo(SUBSCRIPTION_ID); assertThat(actual.resolvedType()).isEqualTo(GraphQlWebSocketMessageType.NEXT); assertThat(actual.>getPayload()) - .extractingByKey("data", as(InstanceOfAssertFactories.map(String.class, Object.class))) - .containsEntry("greeting", "a"); + .containsEntry("data", Collections.singletonMap("greeting", "a")); }) .consumeNextWith((message) -> { GraphQlWebSocketMessage actual = decode(message); assertThat(actual.getId()).isEqualTo(SUBSCRIPTION_ID); assertThat(actual.resolvedType()).isEqualTo(GraphQlWebSocketMessageType.ERROR); - assertThat(actual.>>getPayload()) - .asList().hasSize(1) - .allSatisfy(theError -> assertThat(theError) - .asInstanceOf(InstanceOfAssertFactories.map(String.class, Object.class)) - .hasSize(3) - .hasEntrySatisfying("locations", loc -> assertThat(loc).asList().isEmpty()) - .hasEntrySatisfying("message", msg -> assertThat(msg).asString().contains("Unknown error")) - .extractingByKey("extensions", as(InstanceOfAssertFactories.map(String.class, Object.class))) - .containsEntry("classification", "DataFetchingException")); + List> errors = actual.getPayload(); + assertThat(errors).hasSize(1); + assertThat(errors.get(0)).containsEntry("message", "Subscription error"); + assertThat(errors.get(0)).containsEntry("extensions", + Collections.singletonMap("classification", ErrorType.INTERNAL_ERROR.name())); }) .expectComplete() .verify(TIMEOUT); } @Test - void subscriptionStreamException() { + void subscriptionPublisherExceptionResolved() { final String GREETING_QUERY = "{" + "\"id\":\"" + SUBSCRIPTION_ID + "\"," + "\"type\":\"subscribe\"," + @@ -380,34 +371,26 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { String schema = "type Subscription { greeting: String! } type Query { greetingUnused: String! }"; - WebGraphQlHandler initHandler = GraphQlSetup.schemaContent(schema) - .subscriptionFetcher("greeting", env -> Flux.create(emitter -> { - emitter.next("a"); - emitter.error(new RuntimeException("Test Exception")); - emitter.next("b"); - })) - .subscriptionExceptionResolvers(new SubscriptionExceptionResolverAdapter() { - @Override - protected GraphQLError resolveToSingleError(Throwable exception) { - return GraphqlErrorBuilder.newError() - .errorType(ErrorType.INTERNAL_ERROR) - .message("Error: " + exception.getMessage()) - .extensions(Collections.singletonMap("key", "value")) - .build(); - } - }) - .interceptor() - .toWebGraphQlHandler(); - - GraphQlWebSocketHandler handler = new GraphQlWebSocketHandler( - initHandler, - ServerCodecConfigurer.create(), - Duration.ofSeconds(60)); - TestWebSocketSession session = new TestWebSocketSession(Flux.just( toWebSocketMessage("{\"type\":\"connection_init\"}"), toWebSocketMessage(GREETING_QUERY))); - handler.handle(session).block(TIMEOUT); + + WebGraphQlHandler webHandler = GraphQlSetup.schemaContent(schema) + .subscriptionFetcher("greeting", env -> + Flux.create(emitter -> { + emitter.next("a"); + emitter.error(new RuntimeException("Test Exception")); + emitter.next("b"); + })) + .subscriptionExceptionResolvers(SubscriptionExceptionResolver.forSingleError(exception -> + GraphqlErrorBuilder.newError() + .message("Error: " + exception.getMessage()) + .errorType(ErrorType.BAD_REQUEST) + .build())) + .toWebGraphQlHandler(); + + new GraphQlWebSocketHandler(webHandler, ServerCodecConfigurer.create(), TIMEOUT) + .handle(session).block(TIMEOUT); StepVerifier.create(session.getOutput()) .consumeNextWith((message) -> assertMessageType(message, GraphQlWebSocketMessageType.CONNECTION_ACK)) @@ -416,23 +399,17 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { assertThat(actual.getId()).isEqualTo(SUBSCRIPTION_ID); assertThat(actual.resolvedType()).isEqualTo(GraphQlWebSocketMessageType.NEXT); assertThat(actual.>getPayload()) - .extractingByKey("data", as(InstanceOfAssertFactories.map(String.class, Object.class))) - .containsEntry("greeting", "a"); + .containsEntry("data", Collections.singletonMap("greeting", "a")); }) .consumeNextWith((message) -> { GraphQlWebSocketMessage actual = decode(message); assertThat(actual.getId()).isEqualTo(SUBSCRIPTION_ID); assertThat(actual.resolvedType()).isEqualTo(GraphQlWebSocketMessageType.ERROR); - assertThat(actual.>>getPayload()) - .asList().hasSize(1) - .allSatisfy(theError -> assertThat(theError) - .asInstanceOf(InstanceOfAssertFactories.map(String.class, Object.class)) - .hasSize(3) - .hasEntrySatisfying("locations", loc -> assertThat(loc).asList().isEmpty()) - .hasEntrySatisfying("message", msg -> assertThat(msg).asString().isEqualTo("Error: Test Exception")) - .extractingByKey("extensions", as(InstanceOfAssertFactories.map(String.class, Object.class))) - .containsEntry("classification", "INTERNAL_ERROR") - .containsEntry("key", "value")); + List> errors = actual.getPayload(); + assertThat(errors).hasSize(1); + assertThat(errors.get(0)).containsEntry("message", "Error: Test Exception"); + assertThat(errors.get(0)).containsEntry("extensions", + Collections.singletonMap("classification", ErrorType.BAD_REQUEST.name())); }) .expectComplete() .verify(TIMEOUT); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandlerTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandlerTests.java index f142e582..390f71d1 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandlerTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandlerTests.java @@ -28,18 +28,17 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import java.util.function.Consumer; -import graphql.GraphQLError; import graphql.GraphqlErrorBuilder; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.Test; -import org.springframework.graphql.execution.ErrorType; -import org.springframework.graphql.execution.SubscriptionExceptionResolverAdapter; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import org.springframework.graphql.GraphQlSetup; import org.springframework.graphql.TestThreadLocalAccessor; +import org.springframework.graphql.execution.ErrorType; +import org.springframework.graphql.execution.SubscriptionExceptionResolver; import org.springframework.graphql.execution.ThreadLocalAccessor; import org.springframework.graphql.server.ConsumeOneAndNeverCompleteInterceptor; import org.springframework.graphql.server.WebGraphQlHandler; @@ -327,7 +326,7 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { } @Test - void errorMessagePayloadIsCorrectArray() throws Exception { + void subscriptionErrorPayloadIsArray() throws Exception { final String GREETING_QUERY = "{" + "\"id\":\"" + SUBSCRIPTION_ID + "\"," + "\"type\":\"subscribe\"," + @@ -339,14 +338,11 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { String schema = "type Subscription { greeting: String! }type Query { greetingUnused: String! }"; - WebGraphQlHandler initHandler = GraphQlSetup.schemaContent(schema) + WebGraphQlHandler webHandler = GraphQlSetup.schemaContent(schema) .subscriptionFetcher("greeting", env -> Flux.just("a", null, "b")) - .interceptor() .toWebGraphQlHandler(); - GraphQlWebSocketHandler handler = new GraphQlWebSocketHandler(initHandler, converter, Duration.ofSeconds(60)); - - handle(handler, + handle(new GraphQlWebSocketHandler(webHandler, converter, TIMEOUT), new TextMessage("{\"type\":\"connection_init\"}"), new TextMessage(GREETING_QUERY)); @@ -357,22 +353,17 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { assertThat(actual.getId()).isEqualTo(SUBSCRIPTION_ID); assertThat(actual.resolvedType()).isEqualTo(GraphQlWebSocketMessageType.NEXT); assertThat(actual.>getPayload()) - .extractingByKey("data", as(InstanceOfAssertFactories.map(String.class, Object.class))) - .containsEntry("greeting", "a"); + .containsEntry("data", Collections.singletonMap("greeting", "a")); }) .consumeNextWith((message) -> { GraphQlWebSocketMessage actual = decode(message); assertThat(actual.getId()).isEqualTo(SUBSCRIPTION_ID); assertThat(actual.resolvedType()).isEqualTo(GraphQlWebSocketMessageType.ERROR); - assertThat(actual.>>getPayload()) - .asList().hasSize(1) - .allSatisfy(theError -> assertThat(theError) - .asInstanceOf(InstanceOfAssertFactories.map(String.class, Object.class)) - .hasSize(3) - .hasEntrySatisfying("locations", loc -> assertThat(loc).asList().isEmpty()) - .hasEntrySatisfying("message", msg -> assertThat(msg).asString().contains("Unknown error")) - .extractingByKey("extensions", as(InstanceOfAssertFactories.map(String.class, Object.class))) - .containsEntry("classification", "DataFetchingException")); + List> errors = actual.getPayload(); + assertThat(errors).hasSize(1); + assertThat(errors.get(0)).containsEntry("message", "Subscription error"); + assertThat(errors.get(0)).containsEntry("extensions", + Collections.singletonMap("classification", ErrorType.INTERNAL_ERROR.name())); }) .then(this.session::close) .expectComplete() @@ -380,7 +371,7 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { } @Test - void subscriptionStreamException() throws Exception { + void subscriptionPublisherExceptionResolved() throws Exception { final String GREETING_QUERY = "{" + "\"id\":\"" + SUBSCRIPTION_ID + "\"," + "\"type\":\"subscribe\"," + @@ -398,17 +389,11 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { emitter.error(new RuntimeException("Test Exception")); emitter.next("b"); })) - .subscriptionExceptionResolvers(new SubscriptionExceptionResolverAdapter() { - @Override - protected GraphQLError resolveToSingleError(Throwable exception) { - return GraphqlErrorBuilder.newError() + .subscriptionExceptionResolvers(SubscriptionExceptionResolver.forSingleError(exception -> + GraphqlErrorBuilder.newError() .message("Error: " + exception.getMessage()) - .errorType(ErrorType.INTERNAL_ERROR) - .extensions(Collections.singletonMap("key", "value")) - .build(); - } - }) - .interceptor() + .errorType(ErrorType.BAD_REQUEST) + .build())) .toWebGraphQlHandler(); GraphQlWebSocketHandler handler = new GraphQlWebSocketHandler(initHandler, converter, Duration.ofSeconds(60)); @@ -424,23 +409,17 @@ public class GraphQlWebSocketHandlerTests extends WebSocketHandlerTestSupport { assertThat(actual.getId()).isEqualTo(SUBSCRIPTION_ID); assertThat(actual.resolvedType()).isEqualTo(GraphQlWebSocketMessageType.NEXT); assertThat(actual.>getPayload()) - .extractingByKey("data", as(InstanceOfAssertFactories.map(String.class, Object.class))) - .containsEntry("greeting", "a"); + .containsEntry("data", Collections.singletonMap("greeting", "a")); }) .consumeNextWith((message) -> { GraphQlWebSocketMessage actual = decode(message); assertThat(actual.getId()).isEqualTo(SUBSCRIPTION_ID); assertThat(actual.resolvedType()).isEqualTo(GraphQlWebSocketMessageType.ERROR); - assertThat(actual.>>getPayload()) - .asList().hasSize(1) - .allSatisfy(theError -> assertThat(theError) - .asInstanceOf(InstanceOfAssertFactories.map(String.class, Object.class)) - .hasSize(3) - .hasEntrySatisfying("locations", loc -> assertThat(loc).asList().isEmpty()) - .hasEntrySatisfying("message", msg -> assertThat(msg).asString().contains("Error: Test Exception")) - .extractingByKey("extensions", as(InstanceOfAssertFactories.map(String.class, Object.class))) - .containsEntry("classification", "INTERNAL_ERROR") - .containsEntry("key", "value")); + List> errors = actual.getPayload(); + assertThat(errors).hasSize(1); + assertThat(errors.get(0)).containsEntry("message", "Error: Test Exception"); + assertThat(errors.get(0)).containsEntry("extensions", + Collections.singletonMap("classification", ErrorType.BAD_REQUEST.name())); }) .then(this.session::close) .expectComplete() diff --git a/spring-graphql/src/testFixtures/java/org/springframework/graphql/GraphQlSetup.java b/spring-graphql/src/testFixtures/java/org/springframework/graphql/GraphQlSetup.java index b3a08548..7df4c787 100644 --- a/spring-graphql/src/testFixtures/java/org/springframework/graphql/GraphQlSetup.java +++ b/spring-graphql/src/testFixtures/java/org/springframework/graphql/GraphQlSetup.java @@ -15,26 +15,33 @@ */ package org.springframework.graphql; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import graphql.GraphQL; import graphql.schema.DataFetcher; import graphql.schema.GraphQLTypeVisitor; import graphql.schema.TypeResolver; + import org.springframework.context.ApplicationContext; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.Resource; import org.springframework.graphql.data.method.annotation.support.AnnotatedControllerConfigurer; -import org.springframework.graphql.execution.*; +import org.springframework.graphql.execution.DataFetcherExceptionResolver; +import org.springframework.graphql.execution.DataLoaderRegistrar; +import org.springframework.graphql.execution.DefaultExecutionGraphQlService; +import org.springframework.graphql.execution.GraphQlSource; +import org.springframework.graphql.execution.RuntimeWiringConfigurer; +import org.springframework.graphql.execution.SubscriptionExceptionResolver; +import org.springframework.graphql.execution.ThreadLocalAccessor; import org.springframework.graphql.server.WebGraphQlHandler; import org.springframework.graphql.server.WebGraphQlInterceptor; import org.springframework.graphql.server.WebGraphQlSetup; import org.springframework.graphql.server.webflux.GraphQlHttpHandler; import org.springframework.lang.Nullable; -import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - /** * Workflow for GraphQL tests setup that starts with {@link GraphQlSource.Builder} * related input, and then optionally moving on to the creation of a