Skip to content

Commit

Permalink
Address review feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
srinivasankavitha committed Feb 21, 2025
1 parent 4b1afe2 commit b2dc0cc
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@ import org.springframework.boot.actuate.autoconfigure.metrics.CompositeMeterRegi
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration
import org.springframework.boot.actuate.autoconfigure.metrics.PropertiesAutoTimer
import org.springframework.boot.autoconfigure.AutoConfiguration
import org.springframework.boot.autoconfigure.AutoConfigureAfter
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.context.properties.EnableConfigurationProperties
import org.springframework.cache.CacheManager
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.PriorityOrdered
import org.springframework.core.annotation.Order
import java.util.Optional

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import java.util.*
import java.util.concurrent.CompletableFuture
import java.util.function.Function

class DgsAPQPreParsedDocumentProvider(
class DgsAPQPreParsedDocumentProviderWrapper(
persistedQueryCache: PersistedQueryCache,
private val preparsedDocumentProvider: Optional<PreparsedDocumentProvider>,
) : ApolloPersistedQuerySupport(persistedQueryCache) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ open class DgsAPQSupportAutoConfiguration {
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 = DgsAPQPreParsedDocumentProvider(persistedQueryCache.get(), preparsedDocumentProvider)
val apqPreParsedDocumentProvider =
DgsAPQPreParsedDocumentProviderWrapper(persistedQueryCache.get(), preparsedDocumentProvider)
graphQlBuilder.preparsedDocumentProvider(apqPreParsedDocumentProvider)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ open class DgsSpringGraphQLAutoConfiguration(
GraphQlSourceBuilderCustomizer { builder ->
builder.configureGraphQl { graphQlBuilder ->
val apqEnabled = environment.getProperty("dgs.graphql.apq.enabled", Boolean::class.java, false)
// if apq is enabled use the APQPreparsedDocumentProvider instead
// If apq is enabled, we will not use this preparsedDocumentProvider and use DgsAPQPreparsedDocumentProviderWrapper instead
if (preparsedDocumentProvider.isPresent && !apqEnabled) {
graphQlBuilder.preparsedDocumentProvider(preparsedDocumentProvider.get())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.netflix.graphql.dgs.springgraphql.apq

import com.netflix.graphql.dgs.apq.DgsAPQPreParsedDocumentProvider
import com.netflix.graphql.dgs.apq.DgsAPQPreParsedDocumentProviderWrapper
import graphql.ExecutionInput
import graphql.execution.preparsed.PreparsedDocumentEntry
import graphql.execution.preparsed.PreparsedDocumentProvider
Expand All @@ -36,9 +36,9 @@ import java.util.*
import java.util.concurrent.CompletableFuture

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

@MockK
lateinit var preparsedDocumentProvider: PreparsedDocumentProvider
Expand All @@ -48,7 +48,8 @@ class DgsAPQPreParsedDocumentProviderTest {

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

@Test
Expand All @@ -74,7 +75,7 @@ class DgsAPQPreParsedDocumentProviderTest {
}.returns(CompletableFuture.completedFuture(computeFunction(executionInput)))

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

@Test
Expand All @@ -100,7 +101,7 @@ class DgsAPQPreParsedDocumentProviderTest {
}.returns(CompletableFuture.completedFuture(computeFunction(executionInput)))

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

@Test
Expand All @@ -121,6 +122,6 @@ class DgsAPQPreParsedDocumentProviderTest {
}.returns(CompletableFuture.completedFuture(computeFunction(executionInput)))

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

0 comments on commit b2dc0cc

Please sign in to comment.