Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for application/graphql-response+json content type #1699

Merged
merged 3 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import io.ktor.server.application.plugin
import io.ktor.server.plugins.contentnegotiation.ContentNegotiation
import io.ktor.server.response.respondText
import io.ktor.server.routing.Route
import io.ktor.server.routing.accept
import io.ktor.server.routing.application
import io.ktor.server.routing.get
import io.ktor.server.routing.post
Expand Down Expand Up @@ -56,14 +57,24 @@ fun Route.graphQLGetRoute(endpoint: String = "graphql", streamingResponse: Boole
* @param endpoint GraphQL server POST endpoint, defaults to 'graphql'
* @param streamingResponse Enable streaming response body without keeping it fully in memory. If set to true (default) it will set `Transfer-Encoding: chunked` header on the responses.
* @param jacksonConfiguration Jackson Object Mapper customizations
* @param acceptContentType Only route requests with accept headers that match this parameter
* @param responseContentType Respond by this parameter
*/
fun Route.graphQLPostRoute(endpoint: String = "graphql", streamingResponse: Boolean = true, jacksonConfiguration: ObjectMapper.() -> Unit = {}): Route {
fun Route.graphQLPostRoute(
endpoint: String = "graphql",
streamingResponse: Boolean = true,
jacksonConfiguration: ObjectMapper.() -> Unit = {},
acceptContentType: ContentType = ContentType("application", "graphql-response+json"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably should wait for ktor 2.3.0 which includes ktorio/ktor@45b4ccd

how do we also provide a fallback to just use application/json if there is no match?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I removed support in ktor server from this PR.
I'll make another PR when ktor 2.3.0 is released. 👍

responseContentType: ContentType = acceptContentType
): Route {
val graphQLPlugin = this.application.plugin(GraphQL)
val route = post(endpoint) {
graphQLPlugin.server.executeRequest(call)
val route = accept(acceptContentType) {
post(endpoint) {
graphQLPlugin.server.executeRequest(call)
}
}
route.install(ContentNegotiation) {
jackson(streamRequestBody = streamingResponse) {
jackson(contentType = responseContentType, streamRequestBody = streamingResponse) {
apply(jacksonConfiguration)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.expediagroup.graphql.server.ktor
import com.expediagroup.graphql.server.operations.Query
import com.expediagroup.graphql.server.types.GraphQLRequest
import io.ktor.client.plugins.contentnegotiation.ContentNegotiation
import io.ktor.client.request.accept
import io.ktor.client.request.get
import io.ktor.client.request.parameter
import io.ktor.client.request.post
Expand Down Expand Up @@ -128,9 +129,31 @@ class GraphQLPluginTest {
}
val response = client.post("/graphql") {
contentType(ContentType.Application.Json)
accept(ContentType("application", "graphql-response+json"))
setBody(GraphQLRequest(query = "query HelloQuery(\$name: String){ hello(name: \$name) }", operationName = "HelloQuery", variables = mapOf("name" to "junit")))
}
assertEquals(HttpStatusCode.OK, response.status)
assertEquals(ContentType("application", "graphql-response+json").contentType, response.contentType()?.contentType)
assertEquals(ContentType("application", "graphql-response+json").contentSubtype, response.contentType()?.contentSubtype)
assertEquals("""{"data":{"hello":"Hello junit"}}""", response.bodyAsText().trim())
}
}

@Test
fun `server should handle valid POST requests with specific accept type`() {
testApplication {
val client = createClient {
install(ContentNegotiation) {
jackson()
}
}
val response = client.post("/graphql") {
contentType(ContentType.Application.Json)
setBody(GraphQLRequest(query = "query HelloQuery(\$name: String){ hello(name: \$name) }", operationName = "HelloQuery", variables = mapOf("name" to "junit")))
}
assertEquals(HttpStatusCode.OK, response.status)
assertEquals(ContentType.Application.Json.contentType, response.contentType()?.contentType)
assertEquals(ContentType.Application.Json.contentSubtype, response.contentType()?.contentSubtype)
assertEquals("""{"data":{"hello":"Hello junit"}}""", response.bodyAsText().trim())
}
}
Expand All @@ -156,6 +179,7 @@ fun Application.testGraphQLModule() {
install(Routing) {
graphQLGetRoute()
graphQLPostRoute()
graphQLPostRoute(acceptContentType = ContentType.Application.Json, responseContentType = ContentType.Application.Json)
graphQLSDLRoute()
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2021 Expedia, Inc
* Copyright 2023 Expedia, Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,11 +20,11 @@ import com.expediagroup.graphql.server.spring.execution.SpringGraphQLServer
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.context.annotation.Import
import org.springframework.http.MediaType
import org.springframework.web.reactive.function.server.ServerRequest
import org.springframework.web.reactive.function.server.bodyValueAndAwait
import org.springframework.web.reactive.function.server.buildAndAwait
import org.springframework.web.reactive.function.server.coRouter
import org.springframework.web.reactive.function.server.json

/**
* Default route configuration for GraphQL endpoints.
Expand All @@ -45,8 +45,9 @@ class GraphQLRoutesConfiguration(

(isEndpointRequest and isNotWebSocketRequest).invoke { serverRequest ->
val graphQLResponse = graphQLServer.execute(serverRequest)
val accept = serverRequest.headers().accept().find { it == MediaType.APPLICATION_JSON } ?: MediaType.APPLICATION_GRAPHQL_RESPONSE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since application/json is current behavior I'd keep it as default for the next release as well i.e.

Suggested change
val accept = serverRequest.headers().accept().find { it == MediaType.APPLICATION_JSON } ?: MediaType.APPLICATION_GRAPHQL_RESPONSE
val acceptMediaType = serverRequest.headers().accept().find { it.includes(MediaType.APPLICATION_GRAPHQL_RESPONSE) }?.let { MediaType.APPLICATION_GRAPHQL_RESPONSE } ?: MediaType.APPLICATION_JSON

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you your review!
I've changed as the above.

if (graphQLResponse != null) {
ok().json().bodyValueAndAwait(graphQLResponse)
ok().contentType(accept).bodyValueAndAwait(graphQLResponse)
} else {
badRequest().buildAndAwait()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2022 Expedia, Inc
* Copyright 2023 Expedia, Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -192,6 +192,19 @@ class RouteConfigurationIT(@Autowired private val testClient: WebTestClient) {
.verifyGraphQLRoute("Hello POST application/graphql!")
}

@Test
fun `verify POST graphQL request with application graphql response content type`() {
testClient.post()
.uri("/graphql")
.accept(MediaType.APPLICATION_GRAPHQL_RESPONSE)
.contentType(graphQLMediaType)
.bodyValue("query { hello(name: \"POST application/graphql\") }")
.exchange()
.expectHeader()
.contentType(MediaType.APPLICATION_GRAPHQL_RESPONSE)
.verifyGraphQLRoute("Hello POST application/graphql!")
}

@Test
fun `verify POST request with XML content type will result in HTTP 400 bad request`() {
testClient.post()
Expand Down