From 1f1ce72272db0e91523df8566a4b00d2573f59f1 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 18 Sep 2020 22:24:43 +0100 Subject: [PATCH] Add WebInterceptorExecution Arguably a better way to encapsulate support for WebInterceptor vs a base class, more testable, and reduced public API surface. --- .../graphql/WebFluxGraphQLHandler.java | 8 +- .../org/springframework/graphql/WebInput.java | 4 +- .../graphql/WebInterceptor.java | 6 +- ...port.java => WebInterceptorExecution.java} | 31 ++--- .../graphql/WebMvcGraphQLHandler.java | 8 +- .../springframework/graphql/WebOutput.java | 4 +- .../WebFluxApplicationContextTests.java | 1 + .../WebMvcApplicationContextTests.java | 3 +- .../{boot => }/graphql/Book.java | 2 +- .../graphql/GraphQLDataFetchers.java | 2 +- .../graphql/WebInterceptorExecutionTests.java | 113 ++++++++++++++++++ 11 files changed, 149 insertions(+), 33 deletions(-) rename spring-graphql-web/src/main/java/org/springframework/graphql/{WebHandlerSupport.java => WebInterceptorExecution.java} (68%) rename spring-graphql-web/src/test/java/org/springframework/{boot => }/graphql/Book.java (94%) rename spring-graphql-web/src/test/java/org/springframework/{boot => }/graphql/GraphQLDataFetchers.java (94%) create mode 100644 spring-graphql-web/src/test/java/org/springframework/graphql/WebInterceptorExecutionTests.java diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/WebFluxGraphQLHandler.java b/spring-graphql-web/src/main/java/org/springframework/graphql/WebFluxGraphQLHandler.java index 16925ff6..1ac4031a 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/WebFluxGraphQLHandler.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/WebFluxGraphQLHandler.java @@ -28,11 +28,13 @@ import org.springframework.web.reactive.function.server.ServerResponse; * GraphQL handler to expose as a WebFlux.fn endpoint via * {@link org.springframework.web.reactive.function.server.RouterFunctions}. */ -public class WebFluxGraphQLHandler extends WebHandlerSupport implements HandlerFunction { +public class WebFluxGraphQLHandler implements HandlerFunction { + + private final WebInterceptorExecution executionChain; public WebFluxGraphQLHandler(GraphQL graphQL, List interceptors) { - super(graphQL, interceptors); + this.executionChain = new WebInterceptorExecution(graphQL, interceptors); } @@ -40,7 +42,7 @@ public class WebFluxGraphQLHandler extends WebHandlerSupport implements HandlerF return request.bodyToMono(WebInput.MAP_PARAMETERIZED_TYPE_REF) .flatMap(body -> { WebInput webInput = new WebInput(request.uri(), request.headers().asHttpHeaders(), body); - return executeQuery(webInput); + return this.executionChain.execute(webInput); }) .flatMap(output -> ServerResponse.ok().bodyValue(output.toSpecification())); } diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/WebInput.java b/spring-graphql-web/src/main/java/org/springframework/graphql/WebInput.java index be22fcd4..06f38b95 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/WebInput.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/WebInput.java @@ -30,8 +30,8 @@ import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; /** - * Represents the input to a GraphQL HTTP endpoint including URI, headers, and - * the query, operationName, and variables from the request body. + * Container for input from an HTTP request to a GraphQL endpoint, including + * URI, headers, and other inputs extracted from the body of the request. */ public class WebInput { diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/WebInterceptor.java b/spring-graphql-web/src/main/java/org/springframework/graphql/WebInterceptor.java index 5580381d..d388f229 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/WebInterceptor.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/WebInterceptor.java @@ -18,11 +18,13 @@ package org.springframework.graphql; import java.util.function.Consumer; import graphql.ExecutionInput; +import graphql.GraphQL; import reactor.core.publisher.Mono; /** - * Allows interception of GraphQL over HTTP requests with possible customization - * of the input and the result of query execution. + * Interceptor for GraphQL over HTTP requests that allows customization of the + * {@link ExecutionInput} and the {@link graphql.ExecutionResult} of + * {@link GraphQL} query execution. */ public interface WebInterceptor { diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/WebHandlerSupport.java b/spring-graphql-web/src/main/java/org/springframework/graphql/WebInterceptorExecution.java similarity index 68% rename from spring-graphql-web/src/main/java/org/springframework/graphql/WebHandlerSupport.java rename to spring-graphql-web/src/main/java/org/springframework/graphql/WebInterceptorExecution.java index 22a0d3fa..12239e18 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/WebHandlerSupport.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/WebInterceptorExecution.java @@ -18,6 +18,7 @@ package org.springframework.graphql; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.CompletableFuture; import graphql.ExecutionInput; import graphql.ExecutionResult; @@ -27,39 +28,32 @@ import reactor.core.publisher.Mono; import org.springframework.util.CollectionUtils; /** - * Base class for GraphQL over HTTP handlers. + * Supports the use of {@link WebInterceptor}s to customize the + * {@link ExecutionInput} and the {@link ExecutionResult} of {@link GraphQL} + * query execution. */ -public abstract class WebHandlerSupport { +class WebInterceptorExecution { private final GraphQL graphQL; private final List interceptors; - public WebHandlerSupport(GraphQL graphQL, List interceptors) { + WebInterceptorExecution(GraphQL graphQL, List interceptors) { this.graphQL = graphQL; this.interceptors = (!CollectionUtils.isEmpty(interceptors) ? Collections.unmodifiableList(new ArrayList<>(interceptors)) : Collections.emptyList()); } - public GraphQL getGraphQL() { - return this.graphQL; - } - - public List getInterceptors() { - return this.interceptors; - } - - - protected Mono executeQuery(WebInput webInput) { + public Mono execute(WebInput webInput) { return createInputChain(webInput).flatMap(executionInput -> { - Mono resultMono = Mono.fromFuture(getGraphQL().executeAsync(executionInput)); - return createOutputChain(resultMono); + CompletableFuture future = this.graphQL.executeAsync(executionInput); + return createOutputChain(Mono.fromFuture(future)); }); } - protected Mono createInputChain(WebInput webInput) { + private Mono createInputChain(WebInput webInput) { Mono preHandleMono = Mono.just(webInput.toExecutionInput()); for (WebInterceptor interceptor : this.interceptors) { preHandleMono = preHandleMono.flatMap(input -> interceptor.preHandle(input, webInput)); @@ -67,9 +61,10 @@ public abstract class WebHandlerSupport { return preHandleMono; } - protected Mono createOutputChain(Mono resultMono) { + private Mono createOutputChain(Mono resultMono) { Mono outputMono = resultMono.map(WebOutput::new); - for (WebInterceptor interceptor : this.interceptors) { + for (int i = this.interceptors.size() - 1 ; i >= 0; i--) { + WebInterceptor interceptor = this.interceptors.get(i); outputMono = outputMono.flatMap(interceptor::postHandle); } return outputMono; diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/WebMvcGraphQLHandler.java b/spring-graphql-web/src/main/java/org/springframework/graphql/WebMvcGraphQLHandler.java index 85ddbe7b..e0817840 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/WebMvcGraphQLHandler.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/WebMvcGraphQLHandler.java @@ -35,11 +35,13 @@ import org.springframework.web.servlet.function.ServerResponse; * GraphQL handler to expose as a WebMvc.fn endpoint via * {@link org.springframework.web.servlet.function.RouterFunctions}. */ -public class WebMvcGraphQLHandler extends WebHandlerSupport implements HandlerFunction { +public class WebMvcGraphQLHandler implements HandlerFunction { + + private final WebInterceptorExecution executionChain; public WebMvcGraphQLHandler(GraphQL graphQL, List interceptors) { - super(graphQL, interceptors); + this.executionChain = new WebInterceptorExecution(graphQL, interceptors); } @@ -51,7 +53,7 @@ public class WebMvcGraphQLHandler extends WebHandlerSupport implements HandlerFu */ public ServerResponse handle(ServerRequest request) throws ServletException { WebInput webInput = new WebInput(request.uri(), request.headers().asHttpHeaders(), readBody(request)); - Mono outputMono = executeQuery(webInput); + Mono outputMono = this.executionChain.execute(webInput); return ServerResponse.ok().body(outputMono.map(ExecutionResult::toSpecification)); } diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/WebOutput.java b/spring-graphql-web/src/main/java/org/springframework/graphql/WebOutput.java index 849c1eea..9cdfa53f 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/WebOutput.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/WebOutput.java @@ -27,8 +27,8 @@ import org.springframework.lang.Nullable; /** - * Simple wrapper around a GraphQL {@link ExecutionResult} that allows - * {@link #transform(Consumer) transformation} via a {@link Builder Builder}. + * {@link ExecutionResult} that wraps another in order to provide a convenient + * way to {@link #transform(Consumer) transform} it. */ public class WebOutput implements ExecutionResult { diff --git a/spring-graphql-web/src/test/java/org/springframework/boot/graphql/WebFluxApplicationContextTests.java b/spring-graphql-web/src/test/java/org/springframework/boot/graphql/WebFluxApplicationContextTests.java index 9c0ece0c..6709f8ce 100644 --- a/spring-graphql-web/src/test/java/org/springframework/boot/graphql/WebFluxApplicationContextTests.java +++ b/spring-graphql-web/src/test/java/org/springframework/boot/graphql/WebFluxApplicationContextTests.java @@ -14,6 +14,7 @@ import org.springframework.boot.autoconfigure.web.reactive.WebFluxAutoConfigurat import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.graphql.GraphQLDataFetchers; import org.springframework.http.MediaType; import org.springframework.test.web.reactive.server.WebTestClient; diff --git a/spring-graphql-web/src/test/java/org/springframework/boot/graphql/WebMvcApplicationContextTests.java b/spring-graphql-web/src/test/java/org/springframework/boot/graphql/WebMvcApplicationContextTests.java index 54b13a30..77f708a4 100644 --- a/spring-graphql-web/src/test/java/org/springframework/boot/graphql/WebMvcApplicationContextTests.java +++ b/spring-graphql-web/src/test/java/org/springframework/boot/graphql/WebMvcApplicationContextTests.java @@ -11,6 +11,7 @@ import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguratio import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.graphql.GraphQLDataFetchers; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; @@ -84,7 +85,7 @@ class WebMvcApplicationContextTests { @Bean public RuntimeWiringCustomizer bookDataFetcher() { - return (runtimeWiring) -> runtimeWiring.type(newTypeWiring("Query") + return (builder) -> builder.type(newTypeWiring("Query") .dataFetcher("bookById", GraphQLDataFetchers.getBookByIdDataFetcher())); } } diff --git a/spring-graphql-web/src/test/java/org/springframework/boot/graphql/Book.java b/spring-graphql-web/src/test/java/org/springframework/graphql/Book.java similarity index 94% rename from spring-graphql-web/src/test/java/org/springframework/boot/graphql/Book.java rename to spring-graphql-web/src/test/java/org/springframework/graphql/Book.java index 6b877a4e..4b4f20b5 100644 --- a/spring-graphql-web/src/test/java/org/springframework/boot/graphql/Book.java +++ b/spring-graphql-web/src/test/java/org/springframework/graphql/Book.java @@ -1,4 +1,4 @@ -package org.springframework.boot.graphql; +package org.springframework.graphql; public class Book { diff --git a/spring-graphql-web/src/test/java/org/springframework/boot/graphql/GraphQLDataFetchers.java b/spring-graphql-web/src/test/java/org/springframework/graphql/GraphQLDataFetchers.java similarity index 94% rename from spring-graphql-web/src/test/java/org/springframework/boot/graphql/GraphQLDataFetchers.java rename to spring-graphql-web/src/test/java/org/springframework/graphql/GraphQLDataFetchers.java index 44d26e32..475c763f 100644 --- a/spring-graphql-web/src/test/java/org/springframework/boot/graphql/GraphQLDataFetchers.java +++ b/spring-graphql-web/src/test/java/org/springframework/graphql/GraphQLDataFetchers.java @@ -1,4 +1,4 @@ -package org.springframework.boot.graphql; +package org.springframework.graphql; import java.util.Arrays; import java.util.List; diff --git a/spring-graphql-web/src/test/java/org/springframework/graphql/WebInterceptorExecutionTests.java b/spring-graphql-web/src/test/java/org/springframework/graphql/WebInterceptorExecutionTests.java new file mode 100644 index 00000000..5fb5d13f --- /dev/null +++ b/spring-graphql-web/src/test/java/org/springframework/graphql/WebInterceptorExecutionTests.java @@ -0,0 +1,113 @@ +/* + * Copyright 2020-2020 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; + +import java.io.File; +import java.net.URI; +import java.time.Duration; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import com.fasterxml.jackson.databind.ObjectMapper; +import graphql.ExecutionInput; +import graphql.GraphQL; +import graphql.schema.GraphQLSchema; +import graphql.schema.idl.RuntimeWiring; +import graphql.schema.idl.SchemaGenerator; +import graphql.schema.idl.SchemaParser; +import graphql.schema.idl.TypeDefinitionRegistry; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; + +import org.springframework.http.HttpHeaders; +import org.springframework.util.ResourceUtils; + +import static graphql.schema.idl.TypeRuntimeWiring.newTypeWiring; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Unit tests for {@link WebInterceptorExecution}. + */ +public class WebInterceptorExecutionTests { + + @Test + void testInterceptorInvocation() throws Exception { + + StringBuilder sb = new StringBuilder(); + List interceptors = Arrays.asList( + new TestWebInterceptor(sb, 1), new TestWebInterceptor(sb, 2), new TestWebInterceptor(sb, 3)); + + String query = "{" + + " bookById(id: \\\"book-1\\\"){ " + + " id" + + " name" + + " pageCount" + + " author" + + " }" + + "}"; + + ObjectMapper mapper = new ObjectMapper(); + Map body = mapper.reader().readValue("{\"query\": \"" + query + "\"}", Map.class); + WebInput webInput = new WebInput(URI.create("/graphql"), new HttpHeaders(), body); + + WebOutput webOutput = new WebInterceptorExecution(createGraphQL(), interceptors) + .execute(webInput).block(); + + assertEquals(":pre1:pre2:pre3:post3:post2:post1", sb.toString()); + assertTrue(webOutput.isDataPresent()); + } + + private static GraphQL createGraphQL() throws Exception { + RuntimeWiring runtimeWiring = RuntimeWiring.newRuntimeWiring() + .type(newTypeWiring("Query").dataFetcher("bookById", GraphQLDataFetchers.getBookByIdDataFetcher())) + .build(); + + File file = ResourceUtils.getFile("classpath:books/schema.graphqls"); + TypeDefinitionRegistry registry = new SchemaParser().parse(file); + SchemaGenerator generator = new SchemaGenerator(); + GraphQLSchema schema = generator.makeExecutableSchema(registry, runtimeWiring); + + return GraphQL.newGraphQL(schema).build(); + } + + + private static class TestWebInterceptor implements WebInterceptor { + + private final StringBuilder output; + + private final int index; + + public TestWebInterceptor(StringBuilder output, int index) { + this.output = output; + this.index = index; + } + + @Override + public Mono preHandle(ExecutionInput executionInput, WebInput webInput) { + this.output.append(":pre").append(this.index); + return Mono.delay(Duration.ofMillis(50)).map(aLong -> executionInput); + } + + @Override + public Mono postHandle(WebOutput webOutput) { + this.output.append(":post").append(this.index); + return Mono.delay(Duration.ofMillis(50)).map(aLong -> webOutput); + } + } + +}