From b35e5f8c2ff104a62d053c363eed118affe83a17 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 9 Apr 2021 12:50:42 +0100 Subject: [PATCH] WebOutput minor updates Rename "headers" to "responseHeaders" and clarify those work for HTTP request queries but not for queries over a WebSocket session. Update getter for response headers to return a read-only wrapper. Provide only one public constructor without headers. See gh-42 --- .../boot/WebFluxApplicationContextTests.java | 4 +- .../boot/WebMvcApplicationContextTests.java | 5 +- .../graphql/AbstractWebGraphQLService.java | 2 +- .../graphql/GraphQLService.java | 10 ++-- .../springframework/graphql/WebOutput.java | 59 ++++++++++++++----- .../graphql/webflux/GraphQLHttpHandler.java | 4 +- .../graphql/webmvc/GraphQLHttpHandler.java | 4 +- .../DefaultWebGraphQLServiceTests.java | 9 ++- 8 files changed, 63 insertions(+), 34 deletions(-) diff --git a/graphql-spring-boot-starter/src/test/java/org/springframework/graphql/boot/WebFluxApplicationContextTests.java b/graphql-spring-boot-starter/src/test/java/org/springframework/graphql/boot/WebFluxApplicationContextTests.java index 65171a58..5eb31d95 100644 --- a/graphql-spring-boot-starter/src/test/java/org/springframework/graphql/boot/WebFluxApplicationContextTests.java +++ b/graphql-spring-boot-starter/src/test/java/org/springframework/graphql/boot/WebFluxApplicationContextTests.java @@ -141,8 +141,8 @@ class WebFluxApplicationContextTests { public WebInterceptor customWebInterceptor() { return new WebInterceptor() { @Override - public Mono postHandle(WebOutput webOutput) { - return Mono.just(webOutput.transform(output -> output.header("X-Custom-Header", "42"))); + public Mono postHandle(WebOutput output) { + return Mono.just(output.transform(builder -> builder.responseHeader("X-Custom-Header", "42"))); } }; } diff --git a/graphql-spring-boot-starter/src/test/java/org/springframework/graphql/boot/WebMvcApplicationContextTests.java b/graphql-spring-boot-starter/src/test/java/org/springframework/graphql/boot/WebMvcApplicationContextTests.java index 5845eefd..c7636e2e 100644 --- a/graphql-spring-boot-starter/src/test/java/org/springframework/graphql/boot/WebMvcApplicationContextTests.java +++ b/graphql-spring-boot-starter/src/test/java/org/springframework/graphql/boot/WebMvcApplicationContextTests.java @@ -137,8 +137,9 @@ class WebMvcApplicationContextTests { public WebInterceptor customWebInterceptor() { return new WebInterceptor() { @Override - public Mono postHandle(WebOutput webOutput) { - return Mono.just(webOutput.transform(output -> output.header("X-Custom-Header", "42"))); + public Mono postHandle(WebOutput output) { + return Mono.just(output.transform(builder -> + builder.responseHeader("X-Custom-Header", "42"))); } }; } diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/AbstractWebGraphQLService.java b/spring-graphql-web/src/main/java/org/springframework/graphql/AbstractWebGraphQLService.java index 08e3c392..f2a53b14 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/AbstractWebGraphQLService.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/AbstractWebGraphQLService.java @@ -55,7 +55,7 @@ public abstract class AbstractWebGraphQLService implements WebGraphQLService { public final Mono execute(WebInput input) { return preHandle(input) .flatMap(executionInput -> Mono.fromFuture(executeInternal(executionInput))) - .flatMap(executionResult -> postHandle(new WebOutput(input, executionResult, null))); + .flatMap(executionResult -> postHandle(new WebOutput(input, executionResult))); } private Mono preHandle(WebInput input) { diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLService.java b/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLService.java index 07a5373d..5c3ef291 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLService.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/GraphQLService.java @@ -21,17 +21,17 @@ import reactor.core.publisher.Mono; /** * Contract to execute a GraphQL request. * - * @param the GraphQL query container along with any additional context - * depending on the environment in which the request is handled - * @param the result of query execution and additional environment output + * @param container for the GraphQL query along additional transport + * related context such as the HTTP url or headers for web. + * @param the query execution result */ -public interface GraphQLService { +public interface GraphQLService { /** * Perform the request and return the result. * @param input the GraphQL query container * @return the execution result */ - Mono execute(I input); + Mono execute(IN input); } 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 14c109c2..c6167f2e 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2020 the original author or authors. + * Copyright 2020-2021 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. @@ -30,8 +30,9 @@ import org.springframework.util.Assert; /** - * {@link ExecutionResult} that wraps another in order to provide a convenient - * way to {@link #transform(Consumer) transform} it. + * Decorate an {@link ExecutionResult}, provide a way to + * {@link #transform(Consumer) transform} it, and collect input for custom + * HTTP response headers for GraphQL over HTTP requests. */ public class WebOutput implements ExecutionResult { @@ -40,18 +41,22 @@ public class WebOutput implements ExecutionResult { private final ExecutionResult executionResult; @Nullable - private final HttpHeaders headers; + private final HttpHeaders responseHeaders; /** * Create an instance that wraps the given {@link ExecutionResult}. */ - public WebOutput(WebInput input, ExecutionResult executionResult, @Nullable HttpHeaders headers) { + public WebOutput(WebInput input, ExecutionResult executionResult) { + this(input, executionResult, null); + } + + private WebOutput(WebInput input, ExecutionResult executionResult, @Nullable HttpHeaders responseHeaders) { Assert.notNull(input, "WebInput is required."); Assert.notNull(executionResult, "ExecutionResult is required."); this.input = input; this.executionResult = executionResult; - this.headers = headers; + this.responseHeaders = responseHeaders; } @@ -88,11 +93,15 @@ public class WebOutput implements ExecutionResult { } /** - * Return headers to be added to the HTTP response. + * Return a read-only view of any custom headers to be added to the HTTP + * response, or {@code null} until {@link #transform(Consumer)} is used to + * add such headers. + * @see #transform(Consumer) + * @see Builder#responseHeader(String, String...) */ @Nullable - public HttpHeaders getHeaders() { - return this.headers; + public HttpHeaders getResponseHeaders() { + return (this.responseHeaders != null ? HttpHeaders.readOnlyHttpHeaders(this.responseHeaders) : null); } /** @@ -106,6 +115,9 @@ public class WebOutput implements ExecutionResult { } + /** + * Builder to transform a {@link WebOutput}. + */ public static class Builder { private final WebInput input; @@ -127,12 +139,13 @@ public class WebOutput implements ExecutionResult { this.data = output.getData(); this.errors = output.getErrors(); this.extensions = output.getExtensions(); - this.headers = output.getHeaders(); + this.headers = output.responseHeaders; } /** - * Set the execution {@link ExecutionResult#getData() data}. + * Set the {@link ExecutionResult#getData() data} of the GraphQL + * execution result. */ public Builder data(@Nullable Object data) { this.data = data; @@ -140,7 +153,8 @@ public class WebOutput implements ExecutionResult { } /** - * Set the execution {@link ExecutionResult#getErrors() errors}. + * Set the {@link ExecutionResult#getErrors() errors} of the GraphQL + * execution result. */ public Builder errors(@Nullable List errors) { this.errors = (errors != null ? errors : Collections.emptyList()); @@ -148,14 +162,22 @@ public class WebOutput implements ExecutionResult { } /** - * Set the execution {@link ExecutionResult#getExtensions() extensions}. + * Set the {@link ExecutionResult#getExtensions() extensions} of the + * GraphQL execution result. */ public Builder extensions(@Nullable Map extensions) { this.extensions = extensions; return this; } - public Builder header(String name, String... values) { + /** + * Add a custom header to be set on the HTTP response. + * + *

Note: This can be used for GraphQL over HTTP query + * requests but has no impact for queries over a WebSocket session where + * the initial handshake request completes before queries begin. + */ + public Builder responseHeader(String name, String... values) { initHeaders(); for (String value : values) { this.headers.add(name, value); @@ -163,7 +185,14 @@ public class WebOutput implements ExecutionResult { return this; } - public Builder headers(Consumer consumer) { + /** + * Consume and update the headers to be set on the HTTP response. + * + *

Note: This can be used for GraphQL over HTTP query + * requests but has no impact for queries over a WebSocket session where + * the initial handshake request completes before queries begin. + */ + public Builder responseHeaders(Consumer consumer) { initHeaders(); consumer.accept(this.headers); return this; diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/webflux/GraphQLHttpHandler.java b/spring-graphql-web/src/main/java/org/springframework/graphql/webflux/GraphQLHttpHandler.java index 4992f165..a1841114 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/webflux/GraphQLHttpHandler.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/webflux/GraphQLHttpHandler.java @@ -71,8 +71,8 @@ public class GraphQLHttpHandler { logger.debug("Execution complete"); } ServerResponse.BodyBuilder builder = ServerResponse.ok(); - if (output.getHeaders() != null) { - builder.headers(headers -> headers.putAll(output.getHeaders())); + if (output.getResponseHeaders() != null) { + builder.headers(headers -> headers.putAll(output.getResponseHeaders())); } return builder.bodyValue(spec); }); diff --git a/spring-graphql-web/src/main/java/org/springframework/graphql/webmvc/GraphQLHttpHandler.java b/spring-graphql-web/src/main/java/org/springframework/graphql/webmvc/GraphQLHttpHandler.java index 36c659fd..c19904af 100644 --- a/spring-graphql-web/src/main/java/org/springframework/graphql/webmvc/GraphQLHttpHandler.java +++ b/spring-graphql-web/src/main/java/org/springframework/graphql/webmvc/GraphQLHttpHandler.java @@ -75,8 +75,8 @@ public class GraphQLHttpHandler { logger.debug("Execution complete"); } ServerResponse.BodyBuilder builder = ServerResponse.ok(); - if (output.getHeaders() != null) { - builder.headers(headers -> headers.putAll(output.getHeaders())); + if (output.getResponseHeaders() != null) { + builder.headers(headers -> headers.putAll(output.getResponseHeaders())); } return builder.body(output.toSpecification()); }); diff --git a/spring-graphql-web/src/test/java/org/springframework/graphql/DefaultWebGraphQLServiceTests.java b/spring-graphql-web/src/test/java/org/springframework/graphql/DefaultWebGraphQLServiceTests.java index 3303edc2..dca79ea4 100644 --- a/spring-graphql-web/src/test/java/org/springframework/graphql/DefaultWebGraphQLServiceTests.java +++ b/spring-graphql-web/src/test/java/org/springframework/graphql/DefaultWebGraphQLServiceTests.java @@ -71,7 +71,7 @@ public class DefaultWebGraphQLServiceTests { assertThat(sb.toString()).isEqualTo(":pre1:pre2:pre3:post3:post2:post1"); assertThat(webOutput.isDataPresent()).isTrue(); - assertThat(webOutput.getHeaders().get("MyHeader")).containsExactly("MyValue3", "MyValue2", "MyValue1"); + assertThat(webOutput.getResponseHeaders().get("MyHeader")).containsExactly("MyValue3", "MyValue2", "MyValue1"); } @@ -107,12 +107,11 @@ public class DefaultWebGraphQLServiceTests { } @Override - public Mono postHandle(WebOutput webOutput) { + public Mono postHandle(WebOutput output) { this.output.append(":post").append(this.index); return Mono.delay(Duration.ofMillis(50)) - .map(aLong -> webOutput.transform(builder -> { - builder.header("myHeader", "MyValue" + this.index); - })); + .map(aLong -> output.transform(builder -> + builder.responseHeader("myHeader", "MyValue" + this.index))); } }