From dc631eacfea262f15d1e6e8e8d52c47e8579a60e Mon Sep 17 00:00:00 2001 From: "$(git --no-pager log --format=format:'%an' -n 1)" Date: Tue, 7 Jan 2025 15:24:56 -0800 Subject: [PATCH 1/2] Added backward compatibility support for setting headers through either a field in "extensions" or using DgsExecutionResult. While users can now just use WebGraphQlInterceptor, this adds compatibility with the old mechanism. --- .../DgsSpringGraphQLAutoConfiguration.kt | 28 +++--- .../netflix/graphql/dgs/DgsExecutionResult.kt | 73 +-------------- .../dgs/internal/DgsExecutionResultTest.kt | 93 ------------------- 3 files changed, 18 insertions(+), 176 deletions(-) delete mode 100644 graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResultTest.kt diff --git a/graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt b/graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt index 411e82483..e927339ed 100644 --- a/graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt +++ b/graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt @@ -16,17 +16,7 @@ package com.netflix.graphql.dgs.springgraphql.autoconfig -import com.netflix.graphql.dgs.DataLoaderInstrumentationExtensionProvider -import com.netflix.graphql.dgs.DgsComponent -import com.netflix.graphql.dgs.DgsDataLoaderCustomizer -import com.netflix.graphql.dgs.DgsDataLoaderInstrumentation -import com.netflix.graphql.dgs.DgsDataLoaderOptionsProvider -import com.netflix.graphql.dgs.DgsDefaultPreparsedDocumentProvider -import com.netflix.graphql.dgs.DgsFederationResolver -import com.netflix.graphql.dgs.DgsQueryExecutor -import com.netflix.graphql.dgs.DgsRuntimeWiring -import com.netflix.graphql.dgs.DgsTypeDefinitionRegistry -import com.netflix.graphql.dgs.ReloadSchemaIndicator +import com.netflix.graphql.dgs.* import com.netflix.graphql.dgs.autoconfig.DgsConfigurationProperties import com.netflix.graphql.dgs.autoconfig.DgsDataloaderConfigurationProperties import com.netflix.graphql.dgs.autoconfig.DgsInputArgumentConfiguration @@ -102,6 +92,8 @@ import org.springframework.graphql.execution.RuntimeWiringConfigurer import org.springframework.graphql.execution.SchemaReport import org.springframework.graphql.execution.SelfDescribingDataFetcher import org.springframework.graphql.execution.SubscriptionExceptionResolver +import org.springframework.graphql.server.WebGraphQlInterceptor +import org.springframework.graphql.server.WebGraphQlResponse import org.springframework.http.HttpHeaders import org.springframework.mock.web.MockHttpServletRequest import org.springframework.web.bind.support.WebDataBinderFactory @@ -520,6 +512,20 @@ open class DgsSpringGraphQLAutoConfiguration( graphQLContextContributors, ) + @Bean + open fun dgsHeadersInterceptor(): WebGraphQlInterceptor = + WebGraphQlInterceptor { request, chain -> + chain.next(request).doOnNext { response: WebGraphQlResponse -> + val responseHeadersExtension = response.extensions["dgs-response-headers"] + if (responseHeadersExtension is HttpHeaders) { + response.responseHeaders.addAll(responseHeadersExtension) + } + if (response.executionResult is DgsExecutionResult) { + response.responseHeaders.addAll((response.executionResult as DgsExecutionResult).headers) + } + } + } + @Configuration(proxyBeanMethods = false) @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET) open class WebMvcConfiguration( diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsExecutionResult.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsExecutionResult.kt index e8773344b..053b98308 100644 --- a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsExecutionResult.kt +++ b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsExecutionResult.kt @@ -18,81 +18,14 @@ package com.netflix.graphql.dgs import graphql.ExecutionResult import graphql.ExecutionResultImpl -import org.slf4j.Logger -import org.slf4j.LoggerFactory import org.springframework.http.HttpHeaders import org.springframework.http.HttpStatus -import org.springframework.http.ResponseEntity class DgsExecutionResult( private val executionResult: ExecutionResult, - private var headers: HttpHeaders, + val headers: HttpHeaders, val status: HttpStatus, ) : ExecutionResult by executionResult { - init { - addExtensionsHeaderKeyToHeader() - } - - /** Read-Only reference to the HTTP Headers. */ - fun headers(): HttpHeaders = HttpHeaders.readOnlyHttpHeaders(headers) - - fun toSpringResponse(): ResponseEntity = - ResponseEntity( - toSpecification(), - headers, - status, - ) - - // Refer to https://github.com/Netflix/dgs-framework/pull/1261 for further details. - override fun toSpecification(): MutableMap { - val spec = executionResult.toSpecification() - - val extensions = - spec["extensions"] as Map<*, *>? - ?: return spec - - if (DGS_RESPONSE_HEADERS_KEY in extensions) { - if (extensions.size == 1) { - spec -= "extensions" - } else { - spec["extensions"] = extensions - DGS_RESPONSE_HEADERS_KEY - } - } - - return spec - } - - // Refer to https://github.com/Netflix/dgs-framework/pull/1261 for further details. - private fun addExtensionsHeaderKeyToHeader() { - val extensions = - executionResult.extensions - ?: return - - val dgsResponseHeaders = - extensions[DGS_RESPONSE_HEADERS_KEY] - ?: return - - if (dgsResponseHeaders is Map<*, *> && dgsResponseHeaders.isNotEmpty()) { - // If the HttpHeaders are empty/read-only we need to switch to a new instance that allows us - // to store the headers that are part of the GraphQL response _extensions_. - - val updatedHeaders = HttpHeaders.writableHttpHeaders(headers) - - dgsResponseHeaders.forEach { (key, value) -> - if (key != null) { - updatedHeaders.add(key.toString(), value?.toString()) - } - } - headers = HttpHeaders.readOnlyHttpHeaders(updatedHeaders) - } else { - logger.warn( - "{} must be of type java.util.Map, but was {}", - DGS_RESPONSE_HEADERS_KEY, - dgsResponseHeaders.javaClass.name, - ) - } - } - /** * Facilitate the construction of a [DgsExecutionResult] instance. */ @@ -128,10 +61,6 @@ class DgsExecutionResult( } companion object { - // defined in here and DgsRestController, for backwards compatibility. Keep these two variables synced. - const val DGS_RESPONSE_HEADERS_KEY = "dgs-response-headers" - private val logger: Logger = LoggerFactory.getLogger(DgsExecutionResult::class.java) - @JvmStatic fun builder(): Builder = Builder() } diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResultTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResultTest.kt deleted file mode 100644 index fecf43040..000000000 --- a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResultTest.kt +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Copyright 2022 Netflix, Inc. - * - * 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 - * - * http://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 com.netflix.graphql.dgs.internal - -import com.netflix.graphql.dgs.DgsExecutionResult -import graphql.ExecutionResultImpl -import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.Assertions.entry -import org.junit.jupiter.api.Nested -import org.junit.jupiter.api.Test -import org.springframework.http.HttpHeaders -import org.springframework.http.HttpStatus - -class DgsExecutionResultTest { - @Test - fun `should be able to pass null for data`() { - assertThat( - DgsExecutionResult - .builder() - .executionResult(ExecutionResultImpl.newExecutionResult().data(null)) - .build() - .toSpecification(), - ).contains(entry("data", null)) - } - - @Test - fun `should default to not having data`() { - assertThat( - DgsExecutionResult.builder().build().toSpecification(), - ).doesNotContainKey("data") - } - - @Test - fun `should be able to pass in custom data`() { - val data = "Check under your chair" - - assertThat( - DgsExecutionResult - .builder() - .executionResult(ExecutionResultImpl.newExecutionResult().data(data)) - .build() - .toSpecification(), - ).contains(entry("data", data)) - } - - @Nested - inner class ToSpringResponse { - @Test - fun `should create a spring response with specified headers`() { - val headers = HttpHeaders() - headers.add("We can add headers now??", "Yes we can") - - assertThat( - DgsExecutionResult - .builder() - .headers(headers) - .build() - .toSpringResponse() - .headers - .toMap(), - ).containsAllEntriesOf(headers.toMap()) - } - - @Test - fun `should create a spring response with specified http status code`() { - val httpStatusCode = HttpStatus.ALREADY_REPORTED - - assertThat( - DgsExecutionResult - .builder() - .status(httpStatusCode) - .build() - .toSpringResponse() - .statusCode - .value(), - ).isEqualTo(httpStatusCode.value()) - } - } -} From 06bb0d7af4a164d6e5aab7323bb9521ce49981ff Mon Sep 17 00:00:00 2001 From: "$(git --no-pager log --format=format:'%an' -n 1)" Date: Tue, 7 Jan 2025 16:38:50 -0800 Subject: [PATCH 2/2] Added test for setting headers through extensions and DgsExecutionResult --- .../DgsSpringGraphQLAutoConfiguration.kt | 23 +++- .../autoconfig/DgsHeadersSmokeTest.kt | 126 ++++++++++++++++++ .../netflix/graphql/dgs/DgsExecutionResult.kt | 2 +- 3 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 graphql-dgs-spring-graphql/src/test/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsHeadersSmokeTest.kt diff --git a/graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt b/graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt index e927339ed..73be9a296 100644 --- a/graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt +++ b/graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt @@ -16,7 +16,18 @@ package com.netflix.graphql.dgs.springgraphql.autoconfig -import com.netflix.graphql.dgs.* +import com.netflix.graphql.dgs.DataLoaderInstrumentationExtensionProvider +import com.netflix.graphql.dgs.DgsComponent +import com.netflix.graphql.dgs.DgsDataLoaderCustomizer +import com.netflix.graphql.dgs.DgsDataLoaderInstrumentation +import com.netflix.graphql.dgs.DgsDataLoaderOptionsProvider +import com.netflix.graphql.dgs.DgsDefaultPreparsedDocumentProvider +import com.netflix.graphql.dgs.DgsExecutionResult +import com.netflix.graphql.dgs.DgsFederationResolver +import com.netflix.graphql.dgs.DgsQueryExecutor +import com.netflix.graphql.dgs.DgsRuntimeWiring +import com.netflix.graphql.dgs.DgsTypeDefinitionRegistry +import com.netflix.graphql.dgs.ReloadSchemaIndicator import com.netflix.graphql.dgs.autoconfig.DgsConfigurationProperties import com.netflix.graphql.dgs.autoconfig.DgsDataloaderConfigurationProperties import com.netflix.graphql.dgs.autoconfig.DgsInputArgumentConfiguration @@ -512,7 +523,17 @@ open class DgsSpringGraphQLAutoConfiguration( graphQLContextContributors, ) + /** + * Backward compatibility for setting response headers through a "dgs-response-headers" field in extensions, or using DgsExecutionResult. + * While this can easily be done through a custom WebGraphQlInterceptor, this bean provides backward compatibility with older code. + */ @Bean + @ConditionalOnProperty( + prefix = "${AUTO_CONF_PREFIX}.dgs-response-headers", + name = ["enabled"], + havingValue = "true", + matchIfMissing = true, + ) open fun dgsHeadersInterceptor(): WebGraphQlInterceptor = WebGraphQlInterceptor { request, chain -> chain.next(request).doOnNext { response: WebGraphQlResponse -> diff --git a/graphql-dgs-spring-graphql/src/test/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsHeadersSmokeTest.kt b/graphql-dgs-spring-graphql/src/test/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsHeadersSmokeTest.kt new file mode 100644 index 000000000..8e227ab17 --- /dev/null +++ b/graphql-dgs-spring-graphql/src/test/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsHeadersSmokeTest.kt @@ -0,0 +1,126 @@ +/* + * Copyright 2025 Netflix, Inc. + * + * 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 + * + * http://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 com.netflix.graphql.dgs.springgraphql.autoconfig + +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.netflix.graphql.dgs.DgsComponent +import com.netflix.graphql.dgs.DgsExecutionResult +import com.netflix.graphql.dgs.DgsQuery +import graphql.ExecutionResult +import graphql.execution.instrumentation.InstrumentationState +import graphql.execution.instrumentation.SimplePerformantInstrumentation +import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.autoconfigure.graphql.GraphQlAutoConfiguration +import org.springframework.boot.autoconfigure.graphql.servlet.GraphQlWebMvcAutoConfiguration +import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.boot.test.context.TestConfiguration +import org.springframework.http.HttpHeaders +import org.springframework.http.MediaType +import org.springframework.stereotype.Component +import org.springframework.test.web.servlet.MockMvc +import org.springframework.test.web.servlet.post +import java.util.concurrent.CompletableFuture + +@SpringBootTest( + classes = [ + DgsHeadersSmokeTest.TestApp::class, + DgsSpringGraphQLAutoConfiguration::class, + GraphQlAutoConfiguration::class, + GraphQlWebMvcAutoConfiguration::class, + WebMvcAutoConfiguration::class, + ], + properties = [ + "dgs.graphql.schema-locations=classpath:/dgs-spring-graphql-smoke-test.graphqls", + ], +) +@AutoConfigureMockMvc +class DgsHeadersSmokeTest { + @Autowired + lateinit var mockMvc: MockMvc + + @Test + fun `Response headers can be set by using DgsExecutionResult`() { + val query = + """ + query { + dgsField + } + """.trimIndent() + + data class GraphQlRequest( + val query: String, + ) + + mockMvc + .post("/graphql") { + content = jacksonObjectMapper().writeValueAsString(GraphQlRequest(query)) + accept = MediaType.APPLICATION_JSON + contentType = MediaType.APPLICATION_JSON + }.andExpect { + status { isOk() } + header { string("dgsexecutionresultheader", "set from DgsExecutionResult") } + header { string("extensionheader", "set from extensions") } + } + } + + @TestConfiguration + open class TestApp { + @DgsComponent + open class DgsTestDatafetcher { + @DgsQuery + fun dgsField(): String = "test from DGS" + + @Component + open class MyIntrospection : SimplePerformantInstrumentation() { + override fun instrumentExecutionResult( + executionResult: ExecutionResult, + parameters: InstrumentationExecutionParameters, + state: InstrumentationState?, + ): CompletableFuture { + val headers = HttpHeaders() + headers.add("dgsexecutionresultheader", "set from DgsExecutionResult") + + val extensionHeaders = HttpHeaders() + extensionHeaders.add("extensionheader", "set from extensions") + val updatedExecutionResult = + executionResult.transform { r -> + r.extensions( + mapOf( + Pair( + "dgs-response-headers", + extensionHeaders, + ), + ), + ) + } + + return CompletableFuture.completedFuture( + DgsExecutionResult + .builder() + .executionResult(updatedExecutionResult) + .headers(headers) + .build(), + ) + } + } + } + } +} diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsExecutionResult.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsExecutionResult.kt index 053b98308..e8bd28c17 100644 --- a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsExecutionResult.kt +++ b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsExecutionResult.kt @@ -24,7 +24,7 @@ import org.springframework.http.HttpStatus class DgsExecutionResult( private val executionResult: ExecutionResult, val headers: HttpHeaders, - val status: HttpStatus, + val status: HttpStatus = HttpStatus.OK, ) : ExecutionResult by executionResult { /** * Facilitate the construction of a [DgsExecutionResult] instance.