DGS Support for Apollo Persisted Query

[jregistr]

Hello, It'd be really nice if DGS had built-in support for Apollo Persisted Queries as it's something we use in production.

TLDR: APQ allows a client to pass a smaller request body containing a hash as an identifier for a Query the server has stored. It is great when some clients have large queries that are too big for CDN's cache.
e.g.

{
   "variables": {"id": 100012},
   "extensions": {
      "persistedQuery":  {
           "version": 1,
           "sha256Hash": "ecf4edb46db40b5132295c0291d62fb65d6759a9eedfa4d5d612dd5ec54a6b38"
      }
    }
}

Btw, this is related to #50.

In the initial investigation on adding this, we can see that both parsing&validation caching and an APQ cache are both done via the PreparsedDocumentProvider. See PersistedQuerySupport.java for example.

In our in-house implementation and apollo's (Query String Cache, Ast Document Store), caching the query for a hash and the validation cache are separate. This allows us to put the Query cache in redis. The document cache must be in memory as it's entire purpose is to avoid de-serializing the query multiple times.

I've been experimenting with this and my initial ideation is to add an implementation of PersistedQueryCache that uses the spring boot caching annotations (@Cacheable, @CacheEvict) for the Query cache and a Concurrent HashMap for the document store.

https://github.com/jregistr/dgs-framework/tree/feature/experimenting-with-apq/graphql-apollo-apq/src/main/kotlin/com/netflix/graphql/dgs/apolloapq

Anyone else using APQ?
And I'd greatly appreciate some feedback and thoughts on APQ support with DGS.

Thank You!

[golfingal72]

Hi All, I turned on the persisted query support in the Apollo graphQL Federation and there are some issues in the DGS framework
There are 2 issues I see:

1. I had to add some debug to the existing "master branch" (of DGS which I pulled yesterday) to actually trap the error. Below is a trace of what came in and the error that is happening:

2021-10-08 11:27:41,158 [http-nio-9080-exec-1] DEBUG com.netflix.graphql.dgs.mvc.DgsRestController - Starting /graphql handling 2021-10-08 11:27:41,171 [http-nio-9080-exec-1] DEBUG com.netflix.graphql.dgs.mvc.DgsRestController - Reading input value: '{"extensions":{"persistedQuery":{"version":1,"sha256Hash":"bfea2c553b5b1ed32e1d97b5166ad2dd917f68c67d7c8daef11dae402af4dda2"}}}' 2021-10-08 11:27:41,190 [http-nio-9080-exec-1] DEBUG com.netflix.graphql.dgs.mvc.DgsRestController - Parsed variables: {} 2021-10-08 11:27:41,190 [http-nio-9080-exec-1] DEBUG com.netflix.graphql.dgs.mvc.DgsRestController - Extension variables: {persistedQuery={version=1, sha256Hash=bfea2c553b5b1ed32e1d97b5166ad2dd917f68c67d7c8daef11dae402af4dda2}} 2021-10-08 11:27:41,190 [http-nio-9080-exec-1] DEBUG com.netflix.graphql.dgs.mvc.DgsRestController - Starting dgsQueryExecutor 2021-10-08 11:27:41,197 [http-nio-9080-exec-1] ERROR com.netflix.graphql.dgs.mvc.DgsRestController - Error RUNNING java.lang.NullPointerException: null cannot be cast to non-null type kotlin.String at com.netflix.graphql.dgs.mvc.DgsRestController$graphql$executionResult$1.invoke(DgsRestController.kt:166) at com.netflix.graphql.dgs.mvc.DgsRestController$graphql$executionResult$1.invoke(DgsRestController.kt:164) at com.netflix.graphql.dgs.internal.utils.TimeTracer.logTime(TimeTracer.kt:24) at com.netflix.graphql.dgs.mvc.DgsRestController.graphql(DgsRestController.kt:163) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:190)

2. In the DefaultDgsExecutor: the BaseExecutor does NOT get the extensions as a parameter, therefore in the creation of the ExecutionInput, it will not get those extensions...

I'm gonna try and patch the code to see if I can get it into the logic like described above... thus far, I can't even get to it.

[paulbakker]

This would be great to support. We've discussed it recently, but it didn't make it in our planning yet. That just means any help would be much appreciated!

[golfingal72]

@paulbakker do you think you could get these changes in the next release?

[golfingal72]

@paulbakker
Here are the changes you need to make: Obviously, the user has to implement the PreParsedDocument bean as described above to get it work, But I do have it working via the ApolloGateway.. if there is any way you could get this in, it would be great...

in DefaultDgsQueryExecutor. add extensions to the BaseExecutor

val executionResult = BaseDgsQueryExecutor.baseExecute(
query,
variables,
extensions, // THIS IS NEW
operationName,
dgsContext,
graphQLSchema,
dataLoaderProvider,
chainedInstrumentation,
queryExecutionStrategy,
mutationExecutionStrategy,
idProvider,
preparsedDocumentProvider,
)

BaseExecutor

fun baseExecute(
query: String,
variables: Map<String, Any>?,
extensions: Map<String, Any>?, // THIS IS NEW
operationName: String?,
dgsContext: DgsContext,
graphQLSchema: GraphQLSchema,
dataLoaderProvider: DgsDataLoaderProvider,
chainedInstrumentation: ChainedInstrumentation,
queryExecutionStrategy: ExecutionStrategy,
mutationExecutionStrategy: ExecutionStrategy,
idProvider: Optional,
preparsedDocumentProvider: PreparsedDocumentProvider,
): CompletableFuture {
val graphQLBuilder =
GraphQL.newGraphQL(graphQLSchema)
.preparsedDocumentProvider(preparsedDocumentProvider)
.instrumentation(chainedInstrumentation)
.queryExecutionStrategy(queryExecutionStrategy)
.mutationExecutionStrategy(mutationExecutionStrategy)
.subscriptionExecutionStrategy(SubscriptionExecutionStrategy())
if (idProvider.isPresent) {
graphQLBuilder.executionIdProvider(idProvider.get())
}
val graphQL = graphQLBuilder.build()

    val dataLoaderRegistry = dataLoaderProvider.buildRegistryWithContextSupplier { dgsContext }
    val executionInput: ExecutionInput = ExecutionInput.newExecutionInput()
        .query(query)
        .extensions(extensions)  **// THIS IS NEW** 
        .dataLoaderRegistry(dataLoaderRegistry)
        .variables(variables)
        .operationName(operationName)
        .context(dgsContext)
        .build()
    return try {
        graphQL.executeAsync(executionInput)
    } catch(pqnf: PersistedQueryNotFound) {

  **// THIS IS NEW, the PreparsedDocumentProvider needs to throw this error, Gateway looks specifically for the message..don't dump this is a full blown stack trace cause it can happen all the time and not worth littering the logs** 
        logger.info("PersistedQueryNotFound Error", pqnf.getMessage());

        CompletableFuture.completedFuture(ExecutionResultImpl(null, listOf<GraphQLError>(
                TypedGraphQLError.newInternalErrorBuilder()
                                .message("PersistedQueryNotFound")
                                .build())))

    } catch (e: Exception) {
        logger.error("Encountered an exception while handling query $query", e)
        val errors: List<GraphQLError> = if (e is GraphQLError) listOf<GraphQLError>(e) else emptyList()
        CompletableFuture.completedFuture(ExecutionResultImpl(null, errors))
    }

DefaultDgsReactiveQueryExecutor.kt
BaseDgsQueryExecutor.baseExecute(
query,
variables,
extensions, // THIS IS NEW
operationName,
it.t2,
.....);

DgsRestController.kt
it would be nice to add:

logger.debug("Parsed variables: {}", queryVariables)
logger.debug("Extension variables: {}", extensions) //THIS IS NEW

I am NOT a kotlin developer, but you get the gist... find a way to send an empty query string to the execution,
my naive coding of it
This change might apply to any other REST endpoints you have (reactive?)

           val queryArg = inputQuery["query"];
            val queryString = if (queryArg != null && queryArg is String?) {
               queryArg
            } else {
               logger.debug(" Setting queryString to empty string")
               ""
            }

            logger.debug("Query String: '{}'", queryString);
             dgsQueryExecutor.execute(
               //inputQuery["query"] as String,
               queryString as String,  **//THIS IS NEW**
                 queryVariables,
                 extensions,
                 headers,

@@ -179,6 +199,7 @@ open class DgsRestController(open val dgsQueryExecutor: DgsQueryExecutor) {
.body("Trying to execute subscription on /graphql. Use /subscriptions instead!")

[golfingal72]

In BaseQueryExecutor where it catches PersistedQueryNotFound, it's not worth logging an error and stack trace because this is
normal operation and it will litter the logs

catch(pqnf: PersistedQueryNotFound) {

// THIS IS NEW, the PreparsedDocumentProvider needs to throw this error, Gateway looks specifically for the message..don't dump this is a full blown stack trace cause it can happen all the time and not worth littering the logs
logger.debug("PersistedQueryNotFound Error");

    CompletableFuture.completedFuture(ExecutionResultImpl(null, listOf<GraphQLError>(
            TypedGraphQLError.newInternalErrorBuilder()
                            .message("PersistedQueryNotFound")
                            .build())))

}

[saravind-hotstar]

@golfingal72 Please can you elaborate steps to enable this feature happy to get the APQ working

[saravind-hotstar]

@jregistr Please can you let me know where you able to get APQ to work in dgs in some form.

[berngp]

Created PR #829 with an initial implementation of this feature. The PR is up for review and cleanup but describes some ergonomics that you, as a user, will have available. Feedback is welcomed since at this stage is easy to influence the ergonomics the framework will provide you.