Add max query depth (#458)

* Bump GraphQL Java and add max query depth option

* Add test for maximum query depth

* Add another test

lib/build.gradle.kts

@@ -7,7 +7,7 @@ plugins {
     id("com.bnorm.power.kotlin-power-assert")
 }
 
-val graphqlJavaVersion = "0.0.0-2023-06-07T00-38-50-5b19375"
+val graphqlJavaVersion = "0.0.0-2023-07-12T23-55-39-fa8cf1b"
 val slf4jVersion = "1.7.25"
 
 dependencies {

lib/src/main/java/graphql/nadel/Nadel.kt

@@ -13,6 +13,9 @@ import graphql.execution.preparsed.NoOpPreparsedDocumentProvider
 import graphql.execution.preparsed.PreparsedDocumentEntry
 import graphql.execution.preparsed.PreparsedDocumentProvider
 import graphql.language.Document
+import graphql.nadel.engine.blueprint.NadelDefaultIntrospectionRunner
+import graphql.nadel.engine.blueprint.NadelIntrospectionRunnerFactory
+import graphql.nadel.engine.transform.NadelTransform
 import graphql.nadel.hooks.ServiceExecutionHooks
 import graphql.nadel.instrumentation.NadelInstrumentation
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationCreateStateParameters
@@ -38,15 +41,12 @@ import java.util.concurrent.atomic.AtomicReference
 import java.util.function.Function
 
 class Nadel private constructor(
-    private val engine: NadelExecutionEngine,
+    private val engine: NextgenEngine,
     val services: List<Service>,
     val engineSchema: GraphQLSchema,
     val querySchema: GraphQLSchema,
-    internal val serviceExecutionFactory: ServiceExecutionFactory,
-    internal val instrumentation: NadelInstrumentation,
-    internal val serviceExecutionHooks: ServiceExecutionHooks,
-    internal val preparsedDocumentProvider: PreparsedDocumentProvider,
-    internal val executionIdProvider: ExecutionIdProvider,
+    private val instrumentation: NadelInstrumentation,
+    private val preparsedDocumentProvider: PreparsedDocumentProvider,
 ) {
     fun execute(nadelExecutionInput: NadelExecutionInput): CompletableFuture<ExecutionResult> {
         val executionInput: ExecutionInput = newExecutionInput()
@@ -226,15 +226,17 @@ class Nadel private constructor(
     }
 
     class Builder {
-        private var serviceExecutionFactory: ServiceExecutionFactory? = null
         private var instrumentation: NadelInstrumentation = object : NadelInstrumentation {}
         private var serviceExecutionHooks: ServiceExecutionHooks = object : ServiceExecutionHooks {}
         private var preparsedDocumentProvider: PreparsedDocumentProvider = NoOpPreparsedDocumentProvider.INSTANCE
         private var executionIdProvider = ExecutionIdProvider.DEFAULT_EXECUTION_ID_PROVIDER
-        private var engineFactory: NadelExecutionEngineFactory = NadelExecutionEngineFactory(::NextgenEngine)
+        private var transforms = emptyList<NadelTransform<out Any>>()
+        private var introspectionRunnerFactory = NadelIntrospectionRunnerFactory(::NadelDefaultIntrospectionRunner)
 
         private var schemaBuilder = NadelSchemas.Builder()
 
+        private var maxQueryDepth = Integer.MAX_VALUE
+
         fun overallSchema(serviceName: String, nsdl: Reader): Builder {
             schemaBuilder.overallSchema(serviceName, nsdl)
             return this
@@ -306,7 +308,6 @@ class Nadel private constructor(
         }
 
         fun serviceExecutionFactory(serviceExecutionFactory: ServiceExecutionFactory): Builder {
-            this.serviceExecutionFactory = serviceExecutionFactory
             schemaBuilder.serviceExecutionFactory(serviceExecutionFactory)
             return this
         }
@@ -331,50 +332,44 @@ class Nadel private constructor(
             return this
         }
 
-        fun engineFactory(engineFactory: NadelExecutionEngineFactory): Builder {
-            this.engineFactory = engineFactory
+        fun transforms(transforms: List<NadelTransform<out Any>>): Builder {
+            this.transforms = transforms
             return this
         }
 
-        fun build(): Nadel {
-            val serviceExecutionFactory = requireNotNull(serviceExecutionFactory) {
-                "Must set serviceExecutionFactory on the builder"
-            }
+        fun introspectionRunnerFactory(introspectionRunnerFactory: NadelIntrospectionRunnerFactory): Builder {
+            this.introspectionRunnerFactory = introspectionRunnerFactory
+            return this
+        }
 
+        fun maxQueryDepth(maxQueryDepth: Int): Builder {
+            this.maxQueryDepth = maxQueryDepth
+            return this
+        }
+
+        fun build(): Nadel {
             val (engineSchema, services) = schemaBuilder.build()
 
             val querySchema = QuerySchemaGenerator.generateQuerySchema(engineSchema)
 
-            lateinit var nadel: Nadel
             return Nadel(
-                engine = object : NadelExecutionEngine {
-                    val real by lazy {
-                        // Dumb hack because the engine factory takes in a Nadel object, but we haven't created one yet
-                        // This used to work because we created two Nadel instances but now we only create one
-                        // Todo: we can remove this at some point with some refactoring
-                        engineFactory.create(nadel)
-                    }
-
-                    override fun execute(
-                        executionInput: ExecutionInput,
-                        queryDocument: Document,
-                        instrumentationState: InstrumentationState?,
-                        nadelExecutionParams: NadelExecutionParams,
-                    ): CompletableFuture<ExecutionResult> {
-                        return real.execute(executionInput, queryDocument, instrumentationState, nadelExecutionParams)
-                    }
-                },
-                serviceExecutionFactory = serviceExecutionFactory,
+                engine = NextgenEngine(
+                    engineSchema = engineSchema,
+                    querySchema = querySchema,
+                    instrumentation = instrumentation,
+                    serviceExecutionHooks = serviceExecutionHooks,
+                    executionIdProvider = executionIdProvider,
+                    services = services,
+                    transforms = transforms,
+                    introspectionRunnerFactory = introspectionRunnerFactory,
+                    maxQueryDepth = maxQueryDepth,
+                ),
                 services = services,
                 engineSchema = engineSchema,
                 querySchema = querySchema,
                 instrumentation = instrumentation,
-                serviceExecutionHooks = serviceExecutionHooks,
                 preparsedDocumentProvider = preparsedDocumentProvider,
-                executionIdProvider = executionIdProvider,
-            ).also {
-                nadel = it
-            }
+            )
         }
 
         @Deprecated("Use overallSchema instead", replaceWith = ReplaceWith("overallSchema(serviceName, nsdl)"))

lib/src/main/java/graphql/nadel/NextgenEngine.kt

@@ -4,6 +4,7 @@ import graphql.ErrorType
 import graphql.ExecutionInput
 import graphql.ExecutionResult
 import graphql.GraphQLError
+import graphql.execution.ExecutionIdProvider
 import graphql.execution.instrumentation.InstrumentationState
 import graphql.language.Document
 import graphql.nadel.engine.NadelExecutionContext
@@ -31,6 +32,7 @@ import graphql.nadel.engine.util.provide
 import graphql.nadel.engine.util.singleOfType
 import graphql.nadel.engine.util.strictAssociateBy
 import graphql.nadel.hooks.ServiceExecutionHooks
+import graphql.nadel.instrumentation.NadelInstrumentation
 import graphql.nadel.instrumentation.parameters.ErrorData
 import graphql.nadel.instrumentation.parameters.ErrorType.ServiceExecutionError
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationOnErrorParameters
@@ -41,6 +43,7 @@ import graphql.nadel.util.OperationNameUtil
 import graphql.normalized.ExecutableNormalizedField
 import graphql.normalized.ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables
 import graphql.normalized.VariablePredicate
+import graphql.schema.GraphQLSchema
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
@@ -53,46 +56,49 @@ import kotlinx.coroutines.future.asCompletableFuture
 import kotlinx.coroutines.future.asDeferred
 import kotlinx.coroutines.future.await
 import kotlinx.coroutines.launch
-import java.util.Locale
 import java.util.concurrent.CompletableFuture
+import graphql.normalized.ExecutableNormalizedOperationFactory.Options.defaultOptions as executableNormalizedOperationFactoryOptions
 
-class NextgenEngine @JvmOverloads constructor(
-    nadel: Nadel,
+internal class NextgenEngine(
+    private val engineSchema: GraphQLSchema,
+    private val querySchema: GraphQLSchema,
+    private val instrumentation: NadelInstrumentation,
+    private val serviceExecutionHooks: ServiceExecutionHooks,
+    private val executionIdProvider: ExecutionIdProvider,
+    maxQueryDepth: Int,
+    services: List<Service>,
     transforms: List<NadelTransform<out Any>> = emptyList(),
     introspectionRunnerFactory: NadelIntrospectionRunnerFactory = NadelIntrospectionRunnerFactory(::NadelDefaultIntrospectionRunner),
-) : NadelExecutionEngine {
-
+) {
     private val coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
-    private val services: Map<String, Service> = nadel.services.strictAssociateBy { it.name }
-    private val engineSchema = nadel.engineSchema
-    private val querySchema = nadel.querySchema
-    private val serviceExecutionHooks: ServiceExecutionHooks = nadel.serviceExecutionHooks
+    private val services: Map<String, Service> = services.strictAssociateBy { it.name }
     private val overallExecutionBlueprint = NadelExecutionBlueprintFactory.create(
-        engineSchema = nadel.engineSchema,
-        services = nadel.services,
+        engineSchema = engineSchema,
+        services = services,
     )
     private val executionPlanner = NadelExecutionPlanFactory.create(
         executionBlueprint = overallExecutionBlueprint,
         engine = this,
         transforms = transforms,
     )
     private val resultTransformer = NadelResultTransformer(overallExecutionBlueprint)
-    private val instrumentation = nadel.instrumentation
     private val dynamicServiceResolution = DynamicServiceResolution(
         engineSchema = engineSchema,
         serviceExecutionHooks = serviceExecutionHooks,
-        services = nadel.services,
+        services = services,
     )
     private val fieldToService = NadelFieldToService(
-        querySchema = nadel.querySchema,
+        querySchema = querySchema,
         overallExecutionBlueprint = overallExecutionBlueprint,
         introspectionRunnerFactory = introspectionRunnerFactory,
         dynamicServiceResolution = dynamicServiceResolution,
-        services,
+        services = this.services,
     )
-    private val executionIdProvider = nadel.executionIdProvider
 
-    override fun execute(
+    private val operationParseOptions = executableNormalizedOperationFactoryOptions()
+        .maxChildrenDepth(maxQueryDepth)
+
+    fun execute(
         executionInput: ExecutionInput,
         queryDocument: Document,
         instrumentationState: InstrumentationState?,
@@ -108,7 +114,7 @@ class NextgenEngine @JvmOverloads constructor(
         }.asCompletableFuture()
     }
 
-    override fun close() {
+    fun close() {
         // Closes the scope after letting in flight requests go through
         coroutineScope.launch {
             delay(60_000) // Wait a minute
@@ -135,8 +141,8 @@ class NextgenEngine @JvmOverloads constructor(
                     queryDocument,
                     executionInput.operationName,
                     executionInput.rawVariables,
-                    executionInput.graphQLContext,
-                    Locale.getDefault()
+                    operationParseOptions
+                        .graphQLContext(executionInput.graphQLContext),
                 )
             }
 
@@ -353,11 +359,4 @@ class NextgenEngine @JvmOverloads constructor(
             field,
         )
     }
-
-    companion object {
-        @JvmStatic
-        fun newNadel(): Nadel.Builder {
-            return Nadel.Builder().engineFactory(::NextgenEngine)
-        }
-    }
 }

lib/src/main/java/graphql/nadel/engine/blueprint/IntrospectionService.kt

@@ -20,7 +20,7 @@ import graphql.schema.GraphQLObjectType
 import graphql.schema.GraphQLSchema
 import java.util.concurrent.CompletableFuture
 
-internal class IntrospectionService constructor(
+internal class IntrospectionService(
     schema: GraphQLSchema,
     introspectionRunnerFactory: NadelIntrospectionRunnerFactory,
 ) : Service(name, schema, introspectionRunnerFactory.make(schema), NadelDefinitionRegistry()) {

test/src/test/kotlin/graphql/nadel/tests/CentralSchemaTesting.kt

@@ -7,7 +7,6 @@ import graphql.language.AstPrinter
 import graphql.nadel.Nadel
 import graphql.nadel.NadelExecutionInput.Companion.newNadelExecutionInput
 import graphql.nadel.NadelSchemas
-import graphql.nadel.NextgenEngine
 import graphql.nadel.ServiceExecution
 import graphql.nadel.ServiceExecutionFactory
 import graphql.nadel.ServiceExecutionResult
@@ -83,9 +82,6 @@ suspend fun main() {
         }
 
     val nadel = Nadel.newNadel()
-        .engineFactory { nadel ->
-            NextgenEngine(nadel)
-        }
         .overallSchemas(overallSchemas)
         .underlyingSchemas(underlyingSchemas)
         .serviceExecutionFactory(object : ServiceExecutionFactory {

test/src/test/kotlin/graphql/nadel/tests/EngineTests.kt

@@ -7,7 +7,6 @@ import graphql.nadel.Nadel
 import graphql.nadel.NadelExecutionHints
 import graphql.nadel.NadelExecutionInput.Companion.newNadelExecutionInput
 import graphql.nadel.NadelSchemas
-import graphql.nadel.NextgenEngine
 import graphql.nadel.ServiceExecution
 import graphql.nadel.ServiceExecutionFactory
 import graphql.nadel.ServiceExecutionResult
@@ -149,12 +148,7 @@ private suspend fun execute(
 
         val nadel: Nadel = Nadel.newNadel()
             .schemaTransformationHook(testHook.schemaTransformationHook)
-            .engineFactory { nadel ->
-                NextgenEngine(
-                    nadel = nadel,
-                    transforms = testHook.customTransforms,
-                )
-            }
+            .transforms(testHook.customTransforms)
             .overallSchemas(fixture.overallSchema)
             .underlyingSchemas(fixture.underlyingSchema)
             .overallWiringFactory(testHook.wiringFactory)

test/src/test/kotlin/graphql/nadel/tests/TestFixture.kt

@@ -8,8 +8,6 @@ import com.fasterxml.jackson.annotation.JsonProperty
 import com.fasterxml.jackson.module.kotlin.readValue
 import graphql.language.AstSorter
 import graphql.language.Document
-import graphql.nadel.Nadel
-import graphql.nadel.NadelExecutionEngine
 import graphql.nadel.engine.util.JsonMap
 import graphql.parser.Parser
 
@@ -40,10 +38,6 @@ data class TestFixture(
     }
 }
 
-fun interface TestEngineFactory {
-    fun make(nadel: Nadel, testHook: EngineTestHook): NadelExecutionEngine
-}
-
 data class ServiceCall(
     val serviceName: String,
     val request: Request,

test/src/test/kotlin/graphql/nadel/tests/hooks/`large-query-but-not-deep`.kt

@@ -0,0 +1,13 @@
+package graphql.nadel.tests.hooks
+
+import graphql.nadel.Nadel
+import graphql.nadel.tests.EngineTestHook
+import graphql.nadel.tests.UseHook
+
+@UseHook
+class `large-query-but-not-deep` : EngineTestHook {
+    override fun makeNadel(builder: Nadel.Builder): Nadel.Builder {
+        return super.makeNadel(builder)
+            .maxQueryDepth(10)
+    }
+}

test/src/test/kotlin/graphql/nadel/tests/hooks/operation-depth-limit.kt

@@ -0,0 +1,13 @@
+package graphql.nadel.tests.hooks
+
+import graphql.nadel.Nadel
+import graphql.nadel.tests.EngineTestHook
+import graphql.nadel.tests.UseHook
+
+@UseHook
+class `operation-depth-limit` : EngineTestHook {
+    override fun makeNadel(builder: Nadel.Builder): Nadel.Builder {
+        return super.makeNadel(builder)
+            .maxQueryDepth(10)
+    }
+}