Skip to content

Commit

Permalink
Merge pull request #2128 from Netflix/kavitha/apq-enhancements
Browse files Browse the repository at this point in the history
User configured PreParsedDocumentProvider should be used for regular queries when APQ feature is enabled.
  • Loading branch information
srinivasankavitha authored Feb 24, 2025
2 parents 60667e0 + 70c84af commit 0317b08
Show file tree
Hide file tree
Showing 8 changed files with 271 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties
import org.springframework.cache.CacheManager
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.Ordered
import org.springframework.core.annotation.Order
import java.util.Optional

/**
Expand All @@ -38,6 +40,7 @@ open class DgsGraphQLMicrometerAutoConfiguration {
}

@Bean
@Order(Ordered.LOWEST_PRECEDENCE)
@ConditionalOnProperty(
prefix = "$AUTO_CONF_PREFIX.instrumentation",
name = ["enabled"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ class MicrometerServletSmokeTest {
.and("gql.operation.name", "anonymous")
.and("gql.query.complexity", "none")
.and("gql.query.sig.hash", "none")
.and("gql.errorDetail", "none")
.and("gql.errorDetail", "INVALID_ARGUMENT")
.and("gql.errorCode", "BAD_REQUEST")
.and("gql.path", "[hello]")
.and("outcome", "failure"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,8 @@ management:
exposure:
include:
- metrics
debug: true
debug: true
dgs:
graphql:
apq:
enabled: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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.apq

import graphql.ExecutionInput
import graphql.execution.preparsed.PreparsedDocumentEntry
import graphql.execution.preparsed.PreparsedDocumentProvider
import graphql.execution.preparsed.persisted.ApolloPersistedQuerySupport
import graphql.execution.preparsed.persisted.PersistedQueryCache
import java.util.*
import java.util.concurrent.CompletableFuture
import java.util.function.Function

class DgsAPQPreParsedDocumentProviderWrapper(
persistedQueryCache: PersistedQueryCache,
private val preparsedDocumentProvider: Optional<PreparsedDocumentProvider>,
) : ApolloPersistedQuerySupport(persistedQueryCache) {
override fun getDocumentAsync(
executionInput: ExecutionInput,
parseAndValidateFunction: Function<ExecutionInput, PreparsedDocumentEntry>,
): CompletableFuture<PreparsedDocumentEntry> {
val queryId = getPersistedQueryId(executionInput)
if (queryId.isPresent) {
return super.getDocumentAsync(executionInput, parseAndValidateFunction)
}

if (preparsedDocumentProvider.isPresent) {
return preparsedDocumentProvider.get().getDocumentAsync(executionInput, parseAndValidateFunction)
}

return CompletableFuture.completedFuture(parseAndValidateFunction.apply(executionInput))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,28 @@ package com.netflix.graphql.dgs.apq
import com.github.benmanes.caffeine.cache.Cache
import com.github.benmanes.caffeine.cache.Caffeine
import com.github.benmanes.caffeine.cache.CaffeineSpec
import com.netflix.graphql.dgs.internal.QueryValueCustomizer
import com.netflix.graphql.dgs.springgraphql.autoconfig.DgsSpringGraphQLAutoConfiguration
import graphql.execution.preparsed.PreparsedDocumentEntry
import graphql.execution.preparsed.persisted.ApolloPersistedQuerySupport
import graphql.execution.preparsed.PreparsedDocumentProvider
import graphql.execution.preparsed.persisted.PersistedQueryCache
import io.micrometer.core.instrument.MeterRegistry
import io.micrometer.core.instrument.binder.cache.CaffeineCacheMetrics
import org.springframework.beans.factory.annotation.Qualifier
import org.springframework.boot.autoconfigure.AutoConfiguration
import org.springframework.boot.autoconfigure.AutoConfigureAfter
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
import org.springframework.boot.autoconfigure.graphql.GraphQlSourceBuilderCustomizer
import org.springframework.boot.context.properties.EnableConfigurationProperties
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import java.time.Duration
import java.util.*

@AutoConfiguration
@AutoConfigureAfter(DgsSpringGraphQLAutoConfiguration::class)
@ConditionalOnProperty(
prefix = DgsAPQSupportProperties.PREFIX,
name = ["enabled"],
Expand All @@ -46,18 +50,17 @@ import java.time.Duration
@EnableConfigurationProperties(DgsAPQSupportProperties::class)
open class DgsAPQSupportAutoConfiguration {
@Bean
@ConditionalOnBean(PersistedQueryCache::class)
open fun apolloPersistedQuerySupport(persistedQueryCache: PersistedQueryCache): ApolloPersistedQuerySupport =
ApolloPersistedQuerySupport(persistedQueryCache)

@Bean
@ConditionalOnBean(ApolloPersistedQuerySupport::class)
open fun apolloAPQQueryValueCustomizer(): QueryValueCustomizer =
QueryValueCustomizer { query ->
if (query.isNullOrBlank()) {
ApolloPersistedQuerySupport.PERSISTED_QUERY_MARKER
} else {
query
open fun apqSourceBuilderCustomizer(
preparsedDocumentProvider: Optional<PreparsedDocumentProvider>,
persistedQueryCache: Optional<PersistedQueryCache>,
): GraphQlSourceBuilderCustomizer =
GraphQlSourceBuilderCustomizer { builder ->
builder.configureGraphQl { graphQlBuilder ->
// For non-APQ queries, the user specified PreparsedDocumentProvider should be used, so we configure the DgsAPQPreparsedDocumentProvider to
// wrap the user specified one and delegate appropriately since we can only have one PreParsedDocumentProvider bean
val apqPreParsedDocumentProvider =
DgsAPQPreParsedDocumentProviderWrapper(persistedQueryCache.get(), preparsedDocumentProvider)
graphQlBuilder.preparsedDocumentProvider(apqPreParsedDocumentProvider)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ import org.springframework.context.ApplicationContext
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.DefaultParameterNameDiscoverer
import org.springframework.core.Ordered
import org.springframework.core.PriorityOrdered
import org.springframework.core.ReactiveAdapterRegistry
import org.springframework.core.annotation.Order
Expand Down Expand Up @@ -171,7 +172,9 @@ open class DgsSpringGraphQLAutoConfiguration(
graphQLContextContributors: ObjectProvider<GraphQLContextContributor>,
): Instrumentation = GraphQLContextContributorInstrumentation(graphQLContextContributors.orderedStream().toList())

// This instrumentation needs to run before MetricsInstrumentation
@Bean
@Order(Ordered.LOWEST_PRECEDENCE - 1)
@ConditionalOnProperty(
prefix = "${AUTO_CONF_PREFIX}.errors.classification",
name = ["enabled"],
Expand Down Expand Up @@ -491,12 +494,14 @@ open class DgsSpringGraphQLAutoConfiguration(
@Qualifier("query") providedQueryExecutionStrategy: Optional<ExecutionStrategy>,
@Qualifier("mutation") providedMutationExecutionStrategy: Optional<ExecutionStrategy>,
dataFetcherExceptionHandler: DataFetcherExceptionHandler,
environment: Environment,
): GraphQlSourceBuilderCustomizer =
GraphQlSourceBuilderCustomizer { builder ->
builder.configureGraphQl { graphQlBuilder ->
if (preparsedDocumentProvider.isPresent) {
graphQlBuilder
.preparsedDocumentProvider(preparsedDocumentProvider.get())
val apqEnabled = environment.getProperty("dgs.graphql.apq.enabled", Boolean::class.java, false)
// If apq is enabled, we will not use this preparsedDocumentProvider and use DgsAPQPreparsedDocumentProviderWrapper instead
if (preparsedDocumentProvider.isPresent && !apqEnabled) {
graphQlBuilder.preparsedDocumentProvider(preparsedDocumentProvider.get())
}

if (providedQueryExecutionStrategy.isPresent) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
* 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.apq

import com.netflix.graphql.dgs.apq.DgsAPQPreParsedDocumentProviderWrapper
import graphql.ExecutionInput
import graphql.execution.preparsed.PreparsedDocumentEntry
import graphql.execution.preparsed.PreparsedDocumentProvider
import graphql.execution.preparsed.persisted.PersistedQueryCache
import graphql.execution.preparsed.persisted.PersistedQuerySupport.PERSISTED_QUERY_MARKER
import graphql.language.Document
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.junit5.MockKExtension
import io.mockk.mockk
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.springframework.beans.factory.annotation.Autowired
import java.util.*
import java.util.concurrent.CompletableFuture

@ExtendWith(MockKExtension::class)
class DgsAPQPreParsedDocumentProviderWrapperTest {
@Autowired
private lateinit var dgsAPQPreParsedDocumentProvider: DgsAPQPreParsedDocumentProviderWrapper

@MockK
lateinit var preparsedDocumentProvider: PreparsedDocumentProvider

@MockK
lateinit var persistedQueryCache: PersistedQueryCache

@BeforeEach
fun setUp() {
dgsAPQPreParsedDocumentProvider =
DgsAPQPreParsedDocumentProviderWrapper(persistedQueryCache, Optional.of(preparsedDocumentProvider))
}

@Test
fun `APQ only queries with just the hash use the persisted query cache`() {
var count = 0
val document = mockk<Document>()
val computeFunction = { _: ExecutionInput ->
count++
PreparsedDocumentEntry(document)
}
val extensions: MutableMap<String, Any> = HashMap()
extensions["persistedQuery"] =
mapOf("version" to "1", "sha256Hash" to "ecf4edb46db40b5132295c0291d62fb65d6759a9eedfa4d5d612dd5ec54a6b38")
val executionInput =
ExecutionInput
.Builder()
.query(PERSISTED_QUERY_MARKER)
.extensions(extensions)
.build()

every {
persistedQueryCache.getPersistedQueryDocumentAsync(any(), any(), any())
}.returns(CompletableFuture.completedFuture(computeFunction(executionInput)))

dgsAPQPreParsedDocumentProvider.getDocumentAsync(executionInput, computeFunction)
assertThat(count).isEqualTo(1)
}

@Test
fun `APQ queries with query and hash use the persisted query cache`() {
var count = 0
val document = mockk<Document>()
val computeFunction = { _: ExecutionInput ->
count++
PreparsedDocumentEntry(document)
}
val extensions: MutableMap<String, Any> = HashMap()
extensions["persistedQuery"] =
mapOf("version" to "1", "sha256Hash" to "ecf4edb46db40b5132295c0291d62fb65d6759a9eedfa4d5d612dd5ec54a6b38")
val executionInput =
ExecutionInput
.Builder()
.query("{__typename}")
.extensions(extensions)
.build()

every {
persistedQueryCache.getPersistedQueryDocumentAsync(any(), any(), any())
}.returns(CompletableFuture.completedFuture(computeFunction(executionInput)))

dgsAPQPreParsedDocumentProvider.getDocumentAsync(executionInput, computeFunction)
assertThat(count).isEqualTo(1)
}

@Test
fun `Plain queries (non-APQ) use the user specified preparsed document provider`() {
var count = 0
val document = mockk<Document>()
val computeFunction = { _: ExecutionInput ->
count++
PreparsedDocumentEntry(document)
}
val extensions: MutableMap<String, Any> = HashMap()
extensions["persistedQuery"] =
mapOf("version" to "1", "sha256Hash" to "ecf4edb46db40b5132295c0291d62fb65d6759a9eedfa4d5d612dd5ec54a6b38")
val executionInput = ExecutionInput.Builder().query("{__typename}").build()

every {
preparsedDocumentProvider.getDocumentAsync(executionInput, any())
}.returns(CompletableFuture.completedFuture(computeFunction(executionInput)))

dgsAPQPreParsedDocumentProvider.getDocumentAsync(executionInput, computeFunction)
assertThat(count).isEqualTo(1)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright 2024 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.netflix.graphql.dgs.apq.DgsAPQSupportAutoConfiguration
import graphql.execution.preparsed.persisted.PersistedQueryCache
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.springframework.boot.autoconfigure.AutoConfigurations
import org.springframework.boot.autoconfigure.graphql.GraphQlAutoConfiguration
import org.springframework.boot.test.context.runner.ApplicationContextRunner

class DgsAPQAutoConfigurationTest {
private val autoConfigurations =
AutoConfigurations.of(
DgsSpringGraphQLAutoConfiguration::class.java,
DgsAPQSupportAutoConfiguration::class.java,
GraphQlAutoConfiguration::class.java,
)

@Test
fun shouldContributeAPQBeans() {
val contextRunner =
ApplicationContextRunner()
.withConfiguration(autoConfigurations)
.withPropertyValues("dgs.graphql.apq.enabled=true")

contextRunner.run { context ->
assertThat(context)
.hasBean("apqSourceBuilderCustomizer")
.hasBean("sourceBuilderCustomizer")
.hasSingleBean(PersistedQueryCache::class.java)
}
}

@Test
fun shouldNotContributeAPQBeans() {
val contextRunner =
ApplicationContextRunner()
.withConfiguration(autoConfigurations)

contextRunner.run { context ->
assertThat(context)
.doesNotHaveBean("apqSourceBuilderCustomizer")
.hasBean("sourceBuilderCustomizer")
.doesNotHaveBean(PersistedQueryCache::class.java)
}
}
}

0 comments on commit 0317b08

Please sign in to comment.