From 5d615eb5cdbc73a5b391348733ebf5f5e5cdd77a Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 13 Feb 2025 16:52:23 +0100 Subject: [PATCH] Update config flag for legacy HTTP behavior This commit renames and changes the behavior of the configuration option on `GraphQlHttpHandler` implementations. The `setHttpOkOnValidationErrors` option is `false` by default and is introduced as a deprecated method right away. Our goal here is to fade out this option as soon as possible the traditional behavior for "application/graphql-response+json" media types. See gh-1117 --- .../server/webflux/GraphQlHttpHandler.java | 32 ++++++++++--------- .../server/webmvc/GraphQlHttpHandler.java | 32 ++++++++++--------- .../webflux/GraphQlHttpProtocolTests.java | 1 - .../webmvc/GraphQlHttpProtocolTests.java | 1 - 4 files changed, 34 insertions(+), 32 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlHttpHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlHttpHandler.java index c9c14425..56b6e262 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlHttpHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlHttpHandler.java @@ -44,7 +44,7 @@ public class GraphQlHttpHandler extends AbstractGraphQlHttpHandler { private static final List SUPPORTED_MEDIA_TYPES = List.of( MediaTypes.APPLICATION_GRAPHQL_RESPONSE, MediaType.APPLICATION_JSON, APPLICATION_GRAPHQL); - private boolean isStandardMode = false; + private boolean httpOkOnValidationErrors = false; /** @@ -65,29 +65,31 @@ public class GraphQlHttpHandler extends AbstractGraphQlHttpHandler { } /** - * Return whether this HTTP handler should conform to the "GraphQL over HTTP specification" - * when the {@link MediaTypes#APPLICATION_GRAPHQL_RESPONSE} is selected. - *

When enabled, this mode will use 4xx/5xx HTTP response status if an error occurs before + * Return whether this HTTP handler should use HTTP 200 OK responses if an error occurs before * the GraphQL request execution phase starts; for example, if JSON parsing, GraphQL document parsing, - * or GraphQL document validation fails. When disabled, behavior will remain consistent with the - * "application/json" response content type. + * or GraphQL document validation fail. + *

This option only applies to {@link MediaTypes#APPLICATION_GRAPHQL_RESPONSE} responses, + * as legacy {@link MediaType#APPLICATION_JSON} responses always use HTTP 200 OK in such cases. + * Enabling this option means the server will not conform to the "GraphQL over HTTP specification". *

By default, this is set to {@code false}. * @since 1.4.0 * @see GraphQL over HTTP specification */ - public boolean isStandardMode() { - return this.isStandardMode; + public boolean isHttpOkOnValidationErrors() { + return this.httpOkOnValidationErrors; } /** - * Set whether this HTTP handler should conform to the "GraphQL over HTTP specification" - * when the {@link MediaTypes#APPLICATION_GRAPHQL_RESPONSE} is selected. - * @param standardMode whether the "standard mode" should be enabled + * Set whether this HTTP handler should use HTTP 200 OK responses if an error occurs before + * the GraphQL request execution phase starts. + * @param httpOkOnValidationErrors whether "HTTP 200 OK" responses should always be used * @since 1.4.0 - * @see #isStandardMode + * @deprecated since 1.4, will be made {@code false} permanently in a future release + * @see #isHttpOkOnValidationErrors */ - public void setStandardMode(boolean standardMode) { - this.isStandardMode = standardMode; + @Deprecated(since = "1.4.0", forRemoval = true) + public void setHttpOkOnValidationErrors(boolean httpOkOnValidationErrors) { + this.httpOkOnValidationErrors = httpOkOnValidationErrors; } protected Mono prepareResponse(ServerRequest request, WebGraphQlResponse response) { @@ -100,7 +102,7 @@ public class GraphQlHttpHandler extends AbstractGraphQlHttpHandler { } protected HttpStatus selectResponseStatus(WebGraphQlResponse response, MediaType responseMediaType) { - if (this.isStandardMode + if (!isHttpOkOnValidationErrors() && !response.getExecutionResult().isDataPresent() && MediaTypes.APPLICATION_GRAPHQL_RESPONSE.equals(responseMediaType)) { return HttpStatus.BAD_REQUEST; diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandler.java index 071dade0..077c7c67 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandler.java @@ -49,7 +49,7 @@ public class GraphQlHttpHandler extends AbstractGraphQlHttpHandler { private static final List SUPPORTED_MEDIA_TYPES = List.of( MediaTypes.APPLICATION_GRAPHQL_RESPONSE, MediaType.APPLICATION_JSON, APPLICATION_GRAPHQL); - private boolean isStandardMode = false; + private boolean httpOkOnValidationErrors = false; /** * Create a new instance. @@ -72,29 +72,31 @@ public class GraphQlHttpHandler extends AbstractGraphQlHttpHandler { } /** - * Return whether this HTTP handler should conform to the "GraphQL over HTTP specification" - * when the {@link MediaTypes#APPLICATION_GRAPHQL_RESPONSE} is selected. - *

When enabled, this mode will use 4xx/5xx HTTP response status if an error occurs before + * Return whether this HTTP handler should use HTTP 200 OK responses if an error occurs before * the GraphQL request execution phase starts; for example, if JSON parsing, GraphQL document parsing, - * or GraphQL document validation fails. When disabled, behavior will remain consistent with the - * "application/json" response content type. + * or GraphQL document validation fail. + *

This option only applies to {@link MediaTypes#APPLICATION_GRAPHQL_RESPONSE} responses, + * as legacy {@link MediaType#APPLICATION_JSON} responses always use HTTP 200 OK in such cases. + * Enabling this option means the server will not conform to the "GraphQL over HTTP specification". *

By default, this is set to {@code false}. * @since 1.4.0 * @see GraphQL over HTTP specification */ - public boolean isStandardMode() { - return this.isStandardMode; + public boolean isHttpOkOnValidationErrors() { + return this.httpOkOnValidationErrors; } /** - * Set whether this HTTP handler should conform to the "GraphQL over HTTP specification" - * when the {@link MediaTypes#APPLICATION_GRAPHQL_RESPONSE} is selected. - * @param standardMode whether the "standard mode" should be enabled + * Set whether this HTTP handler should use HTTP 200 OK responses if an error occurs before + * the GraphQL request execution phase starts. + * @param httpOkOnValidationErrors whether "HTTP 200 OK" responses should always be used * @since 1.4.0 - * @see #isStandardMode + * @deprecated since 1.4, will be made {@code false} permanently in a future release + * @see #isHttpOkOnValidationErrors */ - public void setStandardMode(boolean standardMode) { - this.isStandardMode = standardMode; + @Deprecated(since = "1.4.0", forRemoval = true) + public void setHttpOkOnValidationErrors(boolean httpOkOnValidationErrors) { + this.httpOkOnValidationErrors = httpOkOnValidationErrors; } @@ -129,7 +131,7 @@ public class GraphQlHttpHandler extends AbstractGraphQlHttpHandler { } protected HttpStatus selectResponseStatus(WebGraphQlResponse response, MediaType responseMediaType) { - if (this.isStandardMode + if (!isHttpOkOnValidationErrors() && !response.getExecutionResult().isDataPresent() && MediaTypes.APPLICATION_GRAPHQL_RESPONSE.equals(responseMediaType)) { return HttpStatus.BAD_REQUEST; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlHttpProtocolTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlHttpProtocolTests.java index e1c35aa3..5def941e 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlHttpProtocolTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlHttpProtocolTests.java @@ -229,7 +229,6 @@ public class GraphQlHttpProtocolTests { AnnotatedBeanDefinitionReader reader = new AnnotatedBeanDefinitionReader(context); reader.register(WebFluxTestConfig.class); GraphQlHttpHandler httpHandler = graphQlSetup.toHttpHandlerWebFlux(); - httpHandler.setStandardMode(true); RouterFunction routerFunction = RouterFunctions .route() .POST("/graphql", RequestPredicates.accept(MediaType.APPLICATION_JSON, MediaTypes.APPLICATION_GRAPHQL_RESPONSE), diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlHttpProtocolTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlHttpProtocolTests.java index 657ddbc2..c2591122 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlHttpProtocolTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlHttpProtocolTests.java @@ -226,7 +226,6 @@ public class GraphQlHttpProtocolTests { reader.register(MvcTestConfig.class); context.setServletContext(new MockServletContext()); GraphQlHttpHandler httpHandler = graphQlSetup.toHttpHandler(); - httpHandler.setStandardMode(true); RouterFunction routerFunction = RouterFunctions .route() .POST("/graphql", RequestPredicates.accept(MediaType.APPLICATION_JSON, MediaTypes.APPLICATION_GRAPHQL_RESPONSE),