From 42464b67487bde7f1f9e340d06c852e3bdfd66fe Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 16 Sep 2020 21:10:30 +0100 Subject: [PATCH] Minor refactoring and polishing - Remove (unused) GraphQLResponseBody - Collapse servlet and reactive packages and rename handlers to WebFluxGraphQLHandler and WebMvcGraphQLHandler. - Minor refactoring and polishing in each handler also removing some protected methods that overlap in purpose. - Rename GraphQLRequestBody to RequestInput and make it package private. --- .../GraphQLWebFluxAutoConfiguration.java | 8 +-- .../servlet/GraphQLWebAutoConfiguration.java | 8 +-- .../graphql/GraphQLInterceptor.java | 1 + .../graphql/GraphQLResponseBody.java | 36 ------------- ...phQLRequestBody.java => RequestInput.java} | 6 +-- .../graphql/WebFluxGraphQLHandler.java | 50 ++++++++++++++++++ ...Handler.java => WebMvcGraphQLHandler.java} | 36 +++++++------ .../graphql/reactive/GraphQLHandler.java | 52 ------------------- .../graphql/reactive/package-info.java | 6 --- .../graphql/servlet/package-info.java | 6 --- 10 files changed, 81 insertions(+), 128 deletions(-) delete mode 100644 spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLResponseBody.java rename spring-graphql-web/src/main/java/org/springframework/graphql/{GraphQLRequestBody.java => RequestInput.java} (84%) create mode 100644 spring-graphql-web/src/main/java/org/springframework/graphql/WebFluxGraphQLHandler.java rename spring-graphql-web/src/main/java/org/springframework/graphql/{servlet/GraphQLHandler.java => WebMvcGraphQLHandler.java} (68%) delete mode 100644 spring-graphql-web/src/main/java/org/springframework/graphql/reactive/GraphQLHandler.java delete mode 100644 spring-graphql-web/src/main/java/org/springframework/graphql/reactive/package-info.java delete mode 100644 spring-graphql-web/src/main/java/org/springframework/graphql/servlet/package-info.java diff --git a/spring-graphql-web/src/main/java/org/springframework/boot/graphql/reactive/GraphQLWebFluxAutoConfiguration.java b/spring-graphql-web/src/main/java/org/springframework/boot/graphql/reactive/GraphQLWebFluxAutoConfiguration.java index 209a5f89..352120f6 100644 --- a/spring-graphql-web/src/main/java/org/springframework/boot/graphql/reactive/GraphQLWebFluxAutoConfiguration.java +++ b/spring-graphql-web/src/main/java/org/springframework/boot/graphql/reactive/GraphQLWebFluxAutoConfiguration.java @@ -10,7 +10,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplicat import org.springframework.boot.graphql.GraphQLAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.graphql.reactive.GraphQLHandler; +import org.springframework.graphql.WebFluxGraphQLHandler; import org.springframework.web.reactive.function.server.RouterFunction; import org.springframework.web.reactive.function.server.RouterFunctions; import org.springframework.web.reactive.function.server.ServerResponse; @@ -24,12 +24,12 @@ public class GraphQLWebFluxAutoConfiguration { @Bean @ConditionalOnMissingBean - public GraphQLHandler graphQLHandler(GraphQL.Builder graphQLBuilder) { - return new GraphQLHandler(graphQLBuilder); + public WebFluxGraphQLHandler graphQLHandler(GraphQL.Builder graphQLBuilder) { + return new WebFluxGraphQLHandler(graphQLBuilder); } @Bean - public RouterFunction graphQLQueryEndpoint(GraphQLHandler handler) { + public RouterFunction graphQLQueryEndpoint(WebFluxGraphQLHandler handler) { return RouterFunctions.route().POST("/graphql", handler::handle).build(); } diff --git a/spring-graphql-web/src/main/java/org/springframework/boot/graphql/servlet/GraphQLWebAutoConfiguration.java b/spring-graphql-web/src/main/java/org/springframework/boot/graphql/servlet/GraphQLWebAutoConfiguration.java index eba15e89..e1b52947 100644 --- a/spring-graphql-web/src/main/java/org/springframework/boot/graphql/servlet/GraphQLWebAutoConfiguration.java +++ b/spring-graphql-web/src/main/java/org/springframework/boot/graphql/servlet/GraphQLWebAutoConfiguration.java @@ -10,7 +10,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplicat import org.springframework.boot.graphql.GraphQLAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.graphql.servlet.GraphQLHandler; +import org.springframework.graphql.WebMvcGraphQLHandler; import org.springframework.http.MediaType; import org.springframework.web.servlet.function.RouterFunction; import org.springframework.web.servlet.function.RouterFunctions; @@ -27,12 +27,12 @@ public class GraphQLWebAutoConfiguration { @Bean @ConditionalOnMissingBean - public GraphQLHandler graphQLHandler(GraphQL.Builder graphQLBuilder) { - return new GraphQLHandler(graphQLBuilder); + public WebMvcGraphQLHandler graphQLHandler(GraphQL.Builder graphQLBuilder) { + return new WebMvcGraphQLHandler(graphQLBuilder); } @Bean - public RouterFunction graphQLQueryEndpoint(GraphQLHandler handler) { + public RouterFunction graphQLQueryEndpoint(WebMvcGraphQLHandler handler) { return RouterFunctions.route() .POST("/graphql", accept(MediaType.APPLICATION_JSON), handler::handle) .build(); diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLInterceptor.java b/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLInterceptor.java index e75d744c..b7cb867a 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLInterceptor.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLInterceptor.java @@ -6,6 +6,7 @@ import graphql.ExecutionResult; import org.springframework.http.HttpHeaders; public interface GraphQLInterceptor { + ExecutionInput preHandle(ExecutionInput input, HttpHeaders headers); ExecutionResult postHandle(ExecutionResult result); diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLResponseBody.java b/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLResponseBody.java deleted file mode 100644 index 679e78f5..00000000 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLResponseBody.java +++ /dev/null @@ -1,36 +0,0 @@ -package org.springframework.graphql; - -import java.util.Collections; -import java.util.List; -import java.util.Map; - -public class GraphQLResponseBody { - - private T data; - - private List> errors = Collections.emptyList(); - - public GraphQLResponseBody() { - } - - public GraphQLResponseBody(T data) { - this.data = data; - this.errors = errors; - } - - public T getData() { - return this.data; - } - - public void setData(T data) { - this.data = data; - } - - public List> getErrors() { - return this.errors; - } - - public void setErrors(List> errors) { - this.errors = errors; - } -} diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLRequestBody.java b/spring-graphql-web/src/main/java/org/springframework/graphql/RequestInput.java similarity index 84% rename from spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLRequestBody.java rename to spring-graphql-web/src/main/java/org/springframework/graphql/RequestInput.java index 37d58185..d46dd0a8 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLRequestBody.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/RequestInput.java @@ -9,7 +9,7 @@ import org.springframework.lang.Nullable; * @author Andreas Marek * @author Brian Clozel */ -public class GraphQLRequestBody { +class RequestInput { private String query; @@ -17,13 +17,13 @@ public class GraphQLRequestBody { private Map variables = Collections.emptyMap(); - public GraphQLRequestBody(String query, String operationName, Map variables) { + public RequestInput(String query, String operationName, Map variables) { this.query = query; this.operationName = operationName; this.variables = variables; } - public GraphQLRequestBody() { + public RequestInput() { } @Nullable 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 new file mode 100644 index 00000000..20f8d724 --- /dev/null +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/WebFluxGraphQLHandler.java @@ -0,0 +1,50 @@ +package org.springframework.graphql; + +import graphql.ExecutionInput; +import graphql.ExecutionResult; +import graphql.GraphQL; +import reactor.core.publisher.Mono; + +import org.springframework.http.HttpHeaders; +import org.springframework.web.reactive.function.server.ServerRequest; +import org.springframework.web.reactive.function.server.ServerResponse; + +public class WebFluxGraphQLHandler { + + private final GraphQL graphQL; + + public WebFluxGraphQLHandler(GraphQL.Builder graphQLBuilder) { + this.graphQL = graphQLBuilder.build(); + } + + public Mono handle(ServerRequest request) { + return request.bodyToMono(RequestInput.class) + .flatMap(body -> { + String query = body.getQuery(); + if (query == null) { + query = ""; + } + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query(query) + .operationName(body.getOperationName()) + .variables(body.getVariables()) + .build(); + // Invoke GraphQLInterceptor's preHandle here + return customizeExecutionInput(executionInput, request.headers().asHttpHeaders()); + }) + .flatMap(input -> { + // Invoke GraphQLInterceptor's postHandle here + return execute(input); + }) + .flatMap(result -> ServerResponse.ok().bodyValue(result.toSpecification())); + } + + protected Mono customizeExecutionInput(ExecutionInput input, HttpHeaders headers) { + return Mono.just(input); + } + + protected Mono execute(ExecutionInput input) { + return Mono.fromFuture(graphQL.executeAsync(input)); + } + +} diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/servlet/GraphQLHandler.java b/spring-graphql-web/src/main/java/org/springframework/graphql/WebMvcGraphQLHandler.java similarity index 68% rename from spring-graphql-web/src/main/java/org/springframework/graphql/servlet/GraphQLHandler.java rename to spring-graphql-web/src/main/java/org/springframework/graphql/WebMvcGraphQLHandler.java index d9669c55..71af3b0f 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/servlet/GraphQLHandler.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/WebMvcGraphQLHandler.java @@ -1,6 +1,7 @@ -package org.springframework.graphql.servlet; +package org.springframework.graphql; import java.io.IOException; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -10,25 +11,24 @@ import graphql.ExecutionInput; import graphql.ExecutionResult; import graphql.GraphQL; -import org.springframework.graphql.GraphQLRequestBody; import org.springframework.http.HttpHeaders; import org.springframework.web.server.ServerErrorException; import org.springframework.web.server.ServerWebInputException; import org.springframework.web.servlet.function.ServerRequest; import org.springframework.web.servlet.function.ServerResponse; -public class GraphQLHandler { +public class WebMvcGraphQLHandler { private final GraphQL graphQL; - public GraphQLHandler(GraphQL.Builder graphQL) { + public WebMvcGraphQLHandler(GraphQL.Builder graphQL) { this.graphQL = graphQL.build(); } public ServerResponse handle(ServerRequest serverRequest) { - GraphQLRequestBody body; + RequestInput body; try { - body = serverRequest.body(GraphQLRequestBody.class); + body = serverRequest.body(RequestInput.class); } catch (ServletException | IOException ex) { throw new ServerWebInputException("Failed to read request body", null, ex); @@ -42,11 +42,19 @@ public class GraphQLHandler { .operationName(body.getOperationName()) .variables(body.getVariables()) .build(); + // Invoke GraphQLInterceptor's preHandle here - CompletableFuture resultFuture = - customizeExecutionInput(input, serverRequest.headers().asHttpHeaders()).thenCompose(this::execute); + + CompletableFuture> future = + customizeExecutionInput(input, serverRequest.headers().asHttpHeaders()) + .thenCompose(this::execute) + .thenApply(ExecutionResult::toSpecification); + // Invoke GraphQLInterceptor's postHandle here - return customizeExecutionResult(resultFuture); + + return future.isDone() ? + ServerResponse.ok().body(getResult(future)) : + ServerResponse.ok().body(future); } protected CompletableFuture customizeExecutionInput(ExecutionInput input, HttpHeaders headers) { @@ -57,15 +65,9 @@ public class GraphQLHandler { return graphQL.executeAsync(input); } - protected ServerResponse customizeExecutionResult(CompletableFuture resultFuture) { - return resultFuture.isDone() ? - ServerResponse.ok().body(getResult(resultFuture)) : - ServerResponse.ok().body(resultFuture); - } - - private ExecutionResult getResult(CompletableFuture resultFuture) { + private Map getResult(CompletableFuture> future) { try { - return resultFuture.get(); + return future.get(); } catch (InterruptedException | ExecutionException ex) { throw new ServerErrorException("Failed to get result", ex); diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/reactive/GraphQLHandler.java b/spring-graphql-web/src/main/java/org/springframework/graphql/reactive/GraphQLHandler.java deleted file mode 100644 index 725a488b..00000000 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/reactive/GraphQLHandler.java +++ /dev/null @@ -1,52 +0,0 @@ -package org.springframework.graphql.reactive; - -import graphql.ExecutionInput; -import graphql.ExecutionResult; -import graphql.GraphQL; -import reactor.core.publisher.Mono; - -import org.springframework.graphql.GraphQLRequestBody; -import org.springframework.http.HttpHeaders; -import org.springframework.web.reactive.function.server.ServerRequest; -import org.springframework.web.reactive.function.server.ServerResponse; - -public class GraphQLHandler { - - private final GraphQL graphQL; - - public GraphQLHandler(GraphQL.Builder graphQLBuilder) { - this.graphQL = graphQLBuilder.build(); - } - - public Mono handle(ServerRequest request) { - Mono bodyMono = request.bodyToMono(GraphQLRequestBody.class); - return bodyMono.map(body -> { - String query = body.getQuery(); - if (query == null) { - query = ""; - } - ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(query) - .operationName(body.getOperationName()) - .variables(body.getVariables()) - .build(); - return customizeExecutionInput(executionInput, request.headers().asHttpHeaders()) - .then(execute(executionInput)); - }) - .flatMap(this::toServerResponse); - } - - protected Mono customizeExecutionInput(ExecutionInput input, HttpHeaders headers) { - return Mono.just(input); - } - - protected Mono execute(ExecutionInput input) { - return Mono.fromFuture((graphQL.executeAsync(input))); - } - - protected Mono toServerResponse(Mono result) { - return result.map(ExecutionResult::toSpecification) - .flatMap(spec -> ServerResponse.ok().bodyValue(spec)); - } - -} diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/reactive/package-info.java b/spring-graphql-web/src/main/java/org/springframework/graphql/reactive/package-info.java deleted file mode 100644 index 93ba6435..00000000 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/reactive/package-info.java +++ /dev/null @@ -1,6 +0,0 @@ -@NonNullApi -@NonNullFields -package org.springframework.graphql.reactive; - -import org.springframework.lang.NonNullApi; -import org.springframework.lang.NonNullFields; diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/servlet/package-info.java b/spring-graphql-web/src/main/java/org/springframework/graphql/servlet/package-info.java deleted file mode 100644 index b58e8344..00000000 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/servlet/package-info.java +++ /dev/null @@ -1,6 +0,0 @@ -@NonNullApi -@NonNullFields -package org.springframework.graphql.servlet; - -import org.springframework.lang.NonNullApi; -import org.springframework.lang.NonNullFields;