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

HTTP Headers: remove X-APOLLO-OPERATION-NAME, X-APOLLO-OPERATION-ID and the multipart boundary #5533

Merged
merged 5 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.apollographql.apollo3.api.http

import com.apollographql.apollo3.annotations.ApolloDeprecatedSince
import com.apollographql.apollo3.annotations.ApolloInternal
import com.apollographql.apollo3.api.ApolloRequest
import com.apollographql.apollo3.api.CustomScalarAdapters
Expand Down Expand Up @@ -38,8 +39,6 @@ class DefaultHttpRequestComposer(
val customScalarAdapters = apolloRequest.executionContext[CustomScalarAdapters] ?: CustomScalarAdapters.Empty

val requestHeaders = mutableListOf<HttpHeader>().apply {
add(HttpHeader(HEADER_APOLLO_OPERATION_ID, operation.id()))
add(HttpHeader(HEADER_APOLLO_OPERATION_NAME, operation.name()))
if (apolloRequest.operation is Subscription<*>) {
add(HttpHeader(HEADER_ACCEPT_NAME, HEADER_ACCEPT_VALUE_MULTIPART))
} else {
Expand All @@ -58,15 +57,23 @@ class DefaultHttpRequestComposer(
HttpRequest.Builder(
method = HttpMethod.Get,
url = buildGetUrl(serverUrl, operation, customScalarAdapters, sendApqExtensions, sendDocument),
)
).addHeader(HEADER_APOLLO_REQUIRE_PREFLIGHT, "true")
}

HttpMethod.Post -> {
val query = if (sendDocument) operation.document() else null
val body = buildPostBody(operation, customScalarAdapters, sendApqExtensions, query)
HttpRequest.Builder(
method = HttpMethod.Post,
url = serverUrl,
).body(buildPostBody(operation, customScalarAdapters, sendApqExtensions, query))
).body(body)
.let {
if (body.contentType.startsWith("multipart/form-data")) {
it.addHeader(HEADER_APOLLO_REQUIRE_PREFLIGHT, "true")
} else {
it
}
}
}
}

Expand All @@ -77,26 +84,28 @@ class DefaultHttpRequestComposer(
}

companion object {
@Deprecated("If needed, add this header with ApolloCall.addHttpHeader() instead", level = DeprecationLevel.ERROR)
@ApolloDeprecatedSince(ApolloDeprecatedSince.Version.v4_0_0)
val HEADER_APOLLO_OPERATION_ID = "X-APOLLO-OPERATION-ID"
@Deprecated("If needed, add this header with ApolloCall.addHttpHeader() instead", level = DeprecationLevel.ERROR)
@ApolloDeprecatedSince(ApolloDeprecatedSince.Version.v4_0_0)
val HEADER_APOLLO_OPERATION_NAME = "X-APOLLO-OPERATION-NAME"

// Note: in addition to this being a generally useful header to send, Apollo
// Server's CSRF prevention feature (introduced in AS3.7 and intended to be
// Note: Apollo Server's CSRF prevention feature (introduced in AS3.7 and intended to be
// the default in AS4) includes this in the set of headers that indicate
// that a GET request couldn't have been a non-preflighted simple request
// and thus is safe to execute. If this project is changed to not always
// send this header, its GET requests may be blocked by Apollo Server with
// CSRF prevention enabled. See
// https://www.apollographql.com/docs/apollo-server/security/cors/#preventing-cross-site-request-forgery-csrf
// and thus is safe to execute.
// See https://www.apollographql.com/docs/apollo-server/security/cors/#preventing-cross-site-request-forgery-csrf
// for details.
val HEADER_APOLLO_OPERATION_NAME = "X-APOLLO-OPERATION-NAME"
internal val HEADER_APOLLO_REQUIRE_PREFLIGHT = "Apollo-Require-Preflight"

val HEADER_ACCEPT_NAME = "Accept"

// TODO The deferSpec=20220824 part is a temporary measure so early backend implementations of the @defer directive
// can recognize early client implementations and potentially reply in a compatible way.
// This should be removed in later versions.
val HEADER_ACCEPT_VALUE_DEFER = "multipart/mixed; deferSpec=20220824, application/json"
val HEADER_ACCEPT_VALUE_MULTIPART = "multipart/mixed; boundary=\"graphql\"; subscriptionSpec=1.0, application/json"
val HEADER_ACCEPT_VALUE_DEFER = "multipart/mixed;deferSpec=20220824, application/json"
val HEADER_ACCEPT_VALUE_MULTIPART = "multipart/mixed;subscriptionSpec=1.0, application/json"

private fun <D : Operation.Data> buildGetUrl(
serverUrl: String,
Expand Down Expand Up @@ -233,8 +242,8 @@ class DefaultHttpRequestComposer(
)
}

if (uploads.isEmpty()) {
return object : HttpBody {
return if (uploads.isEmpty()) {
object : HttpBody {
override val contentType = "application/json"
override val contentLength = operationByteString.size.toLong()

Expand All @@ -243,7 +252,7 @@ class DefaultHttpRequestComposer(
}
}
} else {
return UploadsHttpBody(uploads, operationByteString)
UploadsHttpBody(uploads, operationByteString)
}
}

Expand Down
12 changes: 0 additions & 12 deletions libraries/apollo-http-cache/api/apollo-http-cache.api
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,7 @@ public abstract interface class com/apollographql/apollo3/cache/http/ApolloHttpC
}

public final class com/apollographql/apollo3/cache/http/CachingHttpInterceptor : com/apollographql/apollo3/network/http/HttpInterceptor {
public static final field CACHE_DO_NOT_STORE Ljava/lang/String;
public static final field CACHE_EXPIRE_AFTER_READ_HEADER Ljava/lang/String;
public static final field CACHE_EXPIRE_TIMEOUT_HEADER Ljava/lang/String;
public static final field CACHE_FETCH_POLICY_HEADER Ljava/lang/String;
public static final field CACHE_FIRST Ljava/lang/String;
public static final field CACHE_KEY_HEADER Ljava/lang/String;
public static final field CACHE_ONLY Ljava/lang/String;
public static final field CACHE_OPERATION_TYPE_HEADER Ljava/lang/String;
public static final field CACHE_SERVED_DATE_HEADER Ljava/lang/String;
public static final field Companion Lcom/apollographql/apollo3/cache/http/CachingHttpInterceptor$Companion;
public static final field FROM_CACHE Ljava/lang/String;
public static final field NETWORK_FIRST Ljava/lang/String;
public static final field NETWORK_ONLY Ljava/lang/String;
Comment on lines -9 to -21
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Breaking change but this should have been internal from day 1

public fun <init> (Ljava/io/File;JLokio/FileSystem;)V
public synthetic fun <init> (Ljava/io/File;JLokio/FileSystem;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun getCache ()Lcom/apollographql/apollo3/cache/http/ApolloHttpCache;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.apollographql.apollo3.cache.http

import com.apollographql.apollo3.api.http.DefaultHttpRequestComposer
import com.apollographql.apollo3.api.http.HttpHeader
import com.apollographql.apollo3.api.http.HttpMethod
import com.apollographql.apollo3.api.http.HttpRequest
Expand Down Expand Up @@ -125,7 +124,7 @@ class CachingHttpInterceptor(
}

private fun cacheMightThrow(request: HttpRequest, cacheKey: String): HttpResponse {
val operationName = request.headers.valueOf(DefaultHttpRequestComposer.HEADER_APOLLO_OPERATION_NAME)
val operationName = request.headers.valueOf(OPERATION_NAME_HEADER)
val response = try {
lruHttpCache.read(cacheKey)
.newBuilder()
Expand Down Expand Up @@ -177,48 +176,49 @@ class CachingHttpInterceptor(
/**
*
*/
const val CACHE_KEY_HEADER = "X-APOLLO-CACHE-KEY"
internal const val CACHE_KEY_HEADER = "X-APOLLO-CACHE-KEY"
internal const val OPERATION_NAME_HEADER = "X-APOLLO-OPERATION-NAME"

internal const val REQUEST_UUID_HEADER = "X-APOLLO-REQUEST-UUID"

/**
* Cache fetch strategy http header
*/
const val CACHE_FETCH_POLICY_HEADER = "X-APOLLO-CACHE-FETCH-POLICY"
internal const val CACHE_FETCH_POLICY_HEADER = "X-APOLLO-CACHE-FETCH-POLICY"

/**
* Cache operation type http header
*/
const val CACHE_OPERATION_TYPE_HEADER = "X-APOLLO-CACHE-OPERATION-TYPE"
internal const val CACHE_OPERATION_TYPE_HEADER = "X-APOLLO-CACHE-OPERATION-TYPE"

const val CACHE_ONLY = "CACHE_ONLY"
const val NETWORK_ONLY = "NETWORK_ONLY"
const val CACHE_FIRST = "CACHE_FIRST"
const val NETWORK_FIRST = "NETWORK_FIRST"
internal const val CACHE_ONLY = "CACHE_ONLY"
internal const val NETWORK_ONLY = "NETWORK_ONLY"
internal const val CACHE_FIRST = "CACHE_FIRST"
internal const val NETWORK_FIRST = "NETWORK_FIRST"

/**
* Request served Date/time http header
*/
const val CACHE_SERVED_DATE_HEADER = "X-APOLLO-SERVED-DATE"
internal const val CACHE_SERVED_DATE_HEADER = "X-APOLLO-SERVED-DATE"

/**
* Cached response expiration timeout http header (in millisecond)
*/
const val CACHE_EXPIRE_TIMEOUT_HEADER = "X-APOLLO-EXPIRE-TIMEOUT"
internal const val CACHE_EXPIRE_TIMEOUT_HEADER = "X-APOLLO-EXPIRE-TIMEOUT"

/**
* Expire cached response flag http header
*/
const val CACHE_EXPIRE_AFTER_READ_HEADER = "X-APOLLO-EXPIRE-AFTER-READ"
internal const val CACHE_EXPIRE_AFTER_READ_HEADER = "X-APOLLO-EXPIRE-AFTER-READ"

/**
* Do not store the http response
*/
const val CACHE_DO_NOT_STORE = "X-APOLLO-CACHE-DO-NOT-STORE"
internal const val CACHE_DO_NOT_STORE = "X-APOLLO-CACHE-DO-NOT-STORE"

/**
* Signals that HTTP response comes from the local cache
*/
const val FROM_CACHE = "X-APOLLO-FROM-CACHE"
internal const val FROM_CACHE = "X-APOLLO-FROM-CACHE"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import com.apollographql.apollo3.api.Subscription
import com.apollographql.apollo3.api.http.HttpRequest
import com.apollographql.apollo3.api.http.HttpResponse
import com.apollographql.apollo3.api.http.valueOf
import com.apollographql.apollo3.cache.http.CachingHttpInterceptor.Companion.OPERATION_NAME_HEADER
import com.apollographql.apollo3.interceptor.ApolloInterceptor
import com.apollographql.apollo3.interceptor.ApolloInterceptorChain
import com.apollographql.apollo3.network.http.HttpInfo
Expand Down Expand Up @@ -115,6 +116,7 @@ fun ApolloClient.Builder.httpCache(
)
.addHttpHeader(CachingHttpInterceptor.CACHE_FETCH_POLICY_HEADER, policyStr)
.addHttpHeader(CachingHttpInterceptor.REQUEST_UUID_HEADER, request.requestUuid.toString())
.addHttpHeader(OPERATION_NAME_HEADER, request.operation.name())
.build()
)
.run {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ internal class MockServerImpl(
}

is ConnectionClosed -> {
println("Connection Closed")
println("Connection Closed (${e.message})")
// Nothing, ignore those
}

Expand Down
20 changes: 20 additions & 0 deletions tests/http-headers/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
plugins {
id("org.jetbrains.kotlin.jvm")
id("com.apollographql.apollo3")
}

apolloTest()

dependencies {
implementation(libs.apollo.runtime)
implementation(libs.apollo.mockserver)
testImplementation(libs.kotlin.test.junit)
testImplementation(libs.apollo.testingsupport)
}

apollo {
service("service") {
packageName.set("httpheaders")
mapScalarToUpload("Upload")
}
}
7 changes: 7 additions & 0 deletions tests/http-headers/src/main/graphql/operations.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
query GetRandom {
random
}

mutation Upload($upload: Upload) {
upload(upload: $upload)
}
9 changes: 9 additions & 0 deletions tests/http-headers/src/main/graphql/schema.graphqls
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
type Query {
random: Int!
}

type Mutation {
upload(upload: Upload): String
}

scalar Upload
56 changes: 56 additions & 0 deletions tests/http-headers/src/test/kotlin/HttpHeaderTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@

import com.apollographql.apollo3.api.DefaultUpload
import com.apollographql.apollo3.api.Optional
import com.apollographql.apollo3.mockserver.enqueueString
import com.apollographql.apollo3.mockserver.headerValueOf
import com.apollographql.apollo3.testing.mockServerTest
import httpheaders.GetRandomQuery
import httpheaders.UploadMutation
import kotlin.test.Test
import kotlin.test.assertEquals

class HttpHeadersTest {
@Test
fun postRequestsDoNotSendPreflightHeader() = mockServerTest {

mockServer.enqueueString("")
apolloClient.query(GetRandomQuery()).execute()

mockServer.takeRequest().apply {
assertEquals(null, headers.headerValueOf("apollo-require-preflight"))
}
}

@Test
fun getRequestsSendPreflightHeader() = mockServerTest(
clientBuilder = { autoPersistedQueries() }
){

mockServer.enqueueString("")
apolloClient.query(GetRandomQuery()).enableAutoPersistedQueries(true).execute()

mockServer.takeRequest().apply {
assertEquals("GET", method)
assertEquals("true", headers.headerValueOf("apollo-require-preflight"))
}
}

@Test
fun uploadRequestsSendPreflightHeader() = mockServerTest {

mockServer.enqueueString("")
val upload = DefaultUpload.Builder()
.content("hello")
.contentLength(5)
.contentType("text/plain")
.fileName("hello")
.build()

apolloClient.mutation(UploadMutation(Optional.present(upload))).execute()

mockServer.takeRequest().apply {
assertEquals("POST", method)
assertEquals("true", headers.headerValueOf("apollo-require-preflight"))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,7 @@ class LoggingInterceptorTest {
client.query(HeroNameQuery()).execute()
logger.assertLog("""
Post http://0.0.0.0/
X-APOLLO-OPERATION-ID: 7e7c85cbf5ef3af5641552c55965608a4e5d7243f3116a486d21c3a958d34235
X-APOLLO-OPERATION-NAME: HeroName
accept: multipart/mixed; deferspec=20220824, application/json
accept: multipart/mixed;deferspec=20220824, application/json
[end of headers]

HTTP: 200
Expand All @@ -142,9 +140,7 @@ class LoggingInterceptorTest {
client.query(HeroNameQuery()).execute()
logger.assertLog("""
Post http://0.0.0.0/
X-APOLLO-OPERATION-ID: 7e7c85cbf5ef3af5641552c55965608a4e5d7243f3116a486d21c3a958d34235
X-APOLLO-OPERATION-NAME: HeroName
accept: multipart/mixed; deferspec=20220824, application/json
accept: multipart/mixed;deferspec=20220824, application/json
[end of headers]
{"operationName":"HeroName","variables":{},"query":"query HeroName { hero { name } }"}

Expand Down Expand Up @@ -185,9 +181,7 @@ class LoggingInterceptorTest {
client.query(HeroNameQuery()).execute()
logger.assertLog("""
Post http://0.0.0.0/
X-APOLLO-OPERATION-ID: 7e7c85cbf5ef3af5641552c55965608a4e5d7243f3116a486d21c3a958d34235
X-APOLLO-OPERATION-NAME: HeroName
accept: multipart/mixed; deferspec=20220824, application/json
accept: multipart/mixed;deferspec=20220824, application/json
[end of headers]
{"operationName":"HeroName","variables":{},"query":"query HeroName { hero { name } }"}

Expand Down