Delete old flags

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

@@ -13,6 +13,7 @@ import graphql.execution.preparsed.NoOpPreparsedDocumentProvider
 import graphql.execution.preparsed.PreparsedDocumentEntry
 import graphql.execution.preparsed.PreparsedDocumentProvider
 import graphql.language.Document
+import graphql.nadel.engine.instrumentation.NadelInstrumentationTimer
 import graphql.nadel.hooks.ServiceExecutionHooks
 import graphql.nadel.instrumentation.NadelInstrumentation
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationCreateStateParameters

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

@@ -1,13 +1,9 @@
 package graphql.nadel
 
-import graphql.nadel.hints.AllDocumentVariablesHint
 import graphql.nadel.hints.LegacyOperationNamesHint
-import graphql.nadel.hints.NewResultMergerAndNamespacedTypename
 
-data class NadelExecutionHints constructor(
+data class NadelExecutionHints(
     val legacyOperationNames: LegacyOperationNamesHint,
-    val allDocumentVariablesHint: AllDocumentVariablesHint,
-    val newResultMergerAndNamespacedTypename: NewResultMergerAndNamespacedTypename,
 ) {
     /**
      * Returns a builder with the same field values as this object.
@@ -21,37 +17,21 @@ data class NadelExecutionHints constructor(
 
     class Builder {
         private var legacyOperationNames: LegacyOperationNamesHint = LegacyOperationNamesHint { false }
-        private var allDocumentVariablesHint: AllDocumentVariablesHint = AllDocumentVariablesHint { false }
-        private var newResultMergerAndNamespacedTypename: NewResultMergerAndNamespacedTypename = NewResultMergerAndNamespacedTypename { false }
 
         constructor()
 
         constructor(nadelExecutionHints: NadelExecutionHints) {
             legacyOperationNames = nadelExecutionHints.legacyOperationNames
-            allDocumentVariablesHint = nadelExecutionHints.allDocumentVariablesHint
-            newResultMergerAndNamespacedTypename = nadelExecutionHints.newResultMergerAndNamespacedTypename
         }
 
         fun legacyOperationNames(flag: LegacyOperationNamesHint): Builder {
             legacyOperationNames = flag
             return this
         }
 
-        fun allDocumentVariablesHint(flag: AllDocumentVariablesHint): Builder {
-            allDocumentVariablesHint = flag
-            return this
-        }
-
-        fun newResultMergerAndNamespacedTypename(flag: NewResultMergerAndNamespacedTypename): Builder {
-            newResultMergerAndNamespacedTypename = flag
-            return this
-        }
-
         fun build(): NadelExecutionHints {
             return NadelExecutionHints(
                 legacyOperationNames,
-                allDocumentVariablesHint,
-                newResultMergerAndNamespacedTypename,
             )
         }
     }

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

@@ -20,6 +20,7 @@ import graphql.nadel.engine.transform.query.NadelFieldToService
 import graphql.nadel.engine.transform.query.NadelQueryTransformer
 import graphql.nadel.engine.transform.result.NadelResultTransformer
 import graphql.nadel.engine.util.beginExecute
+import graphql.nadel.engine.util.compileToDocument
 import graphql.nadel.engine.util.copy
 import graphql.nadel.engine.util.getOperationKind
 import graphql.nadel.engine.util.newExecutionResult
@@ -33,12 +34,12 @@ import graphql.nadel.hooks.ServiceExecutionHooks
 import graphql.nadel.instrumentation.parameters.ErrorData
 import graphql.nadel.instrumentation.parameters.ErrorType.ServiceExecutionError
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationOnErrorParameters
+import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParameters.ChildStep.Companion.DocumentCompilation
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParameters.RootStep
+import graphql.nadel.instrumentation.parameters.child
 import graphql.nadel.util.OperationNameUtil
 import graphql.normalized.ExecutableNormalizedField
 import graphql.normalized.ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables
-import graphql.normalized.ExecutableNormalizedOperationToAstCompiler.compileToDocument
-import graphql.normalized.VariablePredicate
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
@@ -121,20 +122,23 @@ class NextgenEngine @JvmOverloads constructor(
         executionHints: NadelExecutionHints,
     ): ExecutionResult {
         try {
-            val query = createExecutableNormalizedOperationWithRawVariables(
-                querySchema,
-                queryDocument,
-                executionInput.operationName,
-                executionInput.rawVariables,
-                executionInput.graphQLContext,
-                Locale.getDefault()
-            )
-
             val timer = NadelInstrumentationTimer(
                 instrumentation,
                 userContext = executionInput.context,
                 instrumentationState,
             )
+
+            val query = timer.time(step = RootStep.ExecutableOperationParsing) {
+                createExecutableNormalizedOperationWithRawVariables(
+                    querySchema,
+                    queryDocument,
+                    executionInput.operationName,
+                    executionInput.rawVariables,
+                    executionInput.graphQLContext,
+                    Locale.getDefault()
+                )
+            }
+
             val executionContext = NadelExecutionContext(
                 executionInput,
                 query,
@@ -174,11 +178,7 @@ class NextgenEngine @JvmOverloads constructor(
                         }
                 }.awaitAll()
 
-                if (executionHints.newResultMergerAndNamespacedTypename()) {
-                    NadelResultMerger.mergeResults(fields, engineSchema, results)
-                } else {
-                    graphql.nadel.engine.util.mergeResults(results)
-                }
+                NadelResultMerger.mergeResults(fields, engineSchema, results)
             } catch (e: Throwable) {
                 beginExecuteContext?.onCompleted(null, e)
                 throw e
@@ -214,12 +214,14 @@ class NextgenEngine @JvmOverloads constructor(
             transformQuery(service, executionContext, executionPlan, topLevelField)
         }
         val transformedQuery = queryTransform.result.single()
-        val result: ServiceExecutionResult = executeService(
-            service = service,
-            transformedQuery = transformedQuery,
-            executionContext = executionContext,
-            executionHydrationDetails = serviceHydrationDetails,
-        )
+        val result: ServiceExecutionResult = timer.time(step = RootStep.ServiceExecution.child(service.name)) {
+            executeService(
+                service = service,
+                transformedQuery = transformedQuery,
+                executionContext = executionContext,
+                executionHydrationDetails = serviceHydrationDetails,
+            )
+        }
         val transformedResult: ServiceExecutionResult = when {
             topLevelField.name.startsWith("__") -> result
             else -> timer.time(step = RootStep.ResultTransforming) {
@@ -243,24 +245,25 @@ class NextgenEngine @JvmOverloads constructor(
         executionContext: NadelExecutionContext,
         executionHydrationDetails: ServiceExecutionHydrationDetails? = null,
     ): ServiceExecutionResult {
-        val executionInput = executionContext.executionInput
+        val timer = executionContext.timer
 
-        val jsonPredicate: VariablePredicate = getDocumentVariablePredicate(executionContext.hints, service)
+        val executionInput = executionContext.executionInput
 
-        val compileResult = compileToDocument(
-            service.underlyingSchema,
-            transformedQuery.getOperationKind(engineSchema),
-            getOperationName(service, executionContext),
-            listOf(transformedQuery),
-            jsonPredicate
-        )
+        val compileResult = timer.time(step = DocumentCompilation) {
+            compileToDocument(
+                schema = service.underlyingSchema,
+                operationKind = transformedQuery.getOperationKind(engineSchema),
+                operationName = getOperationName(service, executionContext),
+                topLevelFields = listOf(transformedQuery),
+                variablePredicate = DocumentPredicates.allVariablesPredicate,
+            )
+        }
 
         val serviceExecParams = ServiceExecutionParameters(
             query = compileResult.document,
             context = executionInput.context,
             graphQLContext = executionInput.graphQLContext,
             executionId = executionInput.executionId ?: executionIdProvider.provide(executionInput),
-            cacheControl = executionInput.cacheControl,
             variables = compileResult.variables,
             operationDefinition = compileResult.document.definitions.singleOfType(),
             serviceContext = executionContext.getContextForService(service).await(),
@@ -312,14 +315,6 @@ class NextgenEngine @JvmOverloads constructor(
         )
     }
 
-    private fun getDocumentVariablePredicate(hints: NadelExecutionHints, service: Service): VariablePredicate {
-        return if (hints.allDocumentVariablesHint.invoke(service)) {
-            DocumentPredicates.allVariablesPredicate
-        } else {
-            DocumentPredicates.jsonPredicate
-        }
-    }
-
     private fun getOperationName(service: Service, executionContext: NadelExecutionContext): String? {
         val originalOperationName = executionContext.query.operationName
         return if (executionContext.hints.legacyOperationNames(service)) {

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

@@ -1,7 +1,6 @@
 package graphql.nadel
 
 import graphql.GraphQLContext
-import graphql.cachecontrol.CacheControl
 import graphql.execution.ExecutionId
 import graphql.language.Document
 import graphql.language.OperationDefinition
@@ -14,9 +13,7 @@ class ServiceExecutionParameters internal constructor(
     val variables: Map<String, Any>,
     val operationDefinition: OperationDefinition,
     val executionId: ExecutionId,
-    val cacheControl: CacheControl?,
     private val serviceContext: Any?,
-
     /**
      * @return details abut this service hydration or null if it's not a hydration call
      */

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

@@ -711,7 +711,7 @@ private class SharedTypesAnalysis(
 
         val renameInstruction = if (overallOutputTypeName !in serviceDefinedTypes) {
             // Service does not own type, it is shared
-            // If the name is  different than the overall type, then we mark the rename
+            // If the name is different than the overall type, then we mark the rename
             when (val underlyingOutputTypeName = underlyingField.type.unwrapAll().name) {
                 overallOutputTypeName -> null
                 in scalarTypeNames -> null

lib/src/main/java/graphql/nadel/engine/instrumentation/NadelInstrumentationTimer.kt

@@ -5,6 +5,7 @@ import graphql.nadel.instrumentation.NadelInstrumentation
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParameters
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParameters.Step
 import java.time.Duration
+import java.time.Instant
 
 internal class NadelInstrumentationTimer(
     private val instrumentation: NadelInstrumentation,
@@ -15,13 +16,14 @@ internal class NadelInstrumentationTimer(
         step: Step,
         function: () -> T,
     ): T {
-        val start = System.nanoTime()
+        val start = Instant.now()
+        val startNs = System.nanoTime()
 
         val result = try {
             function()
         } catch (e: Throwable) {
             try {
-                emit(step, startNs = start, exception = e)
+                emit(step, start = start, startNs = startNs, exception = e)
             } catch (e2: Throwable) {
                 e2.addSuppressed(e)
                 throw e2
@@ -30,7 +32,7 @@ internal class NadelInstrumentationTimer(
             throw e
         }
 
-        emit(step, startNs = start)
+        emit(step, start, startNs = startNs)
 
         return result
     }
@@ -44,28 +46,31 @@ internal class NadelInstrumentationTimer(
     }
 
     @Suppress("NOTHING_TO_INLINE") // inline anyway
-    private inline fun emit(step: Step, startNs: Long, exception: Throwable? = null) {
+    private inline fun emit(step: Step, start: Instant, startNs: Long, exception: Throwable? = null) {
         val endNs = System.nanoTime()
         val duration = Duration.ofNanos(endNs - startNs)
-        emit(step, duration, exception)
+
+        instrumentation.onStepTimed(newParameters(step, start, duration, exception))
     }
 
     @Suppress("NOTHING_TO_INLINE") // inline anyway
     private inline fun emit(step: Step, duration: Duration, exception: Throwable? = null) {
-        instrumentation.onStepTimed(newParameters(step, duration, exception))
+        instrumentation.onStepTimed(newParameters(step, null, duration, exception))
     }
 
     private fun newParameters(
         step: Step,
+        startedAt: Instant?,
         duration: Duration,
         exception: Throwable? = null,
     ): NadelInstrumentationTimingParameters {
         return NadelInstrumentationTimingParameters(
-            step,
-            duration,
+            step = step,
+            startedAt = startedAt,
+            duration = duration,
             exception = exception,
-            userContext,
-            instrumentationState
+            context = userContext,
+            instrumentationState = instrumentationState,
         )
     }
 

lib/src/main/java/graphql/nadel/engine/util/GraphQLUtil.kt

@@ -41,10 +41,12 @@ import graphql.nadel.ServiceExecutionResult
 import graphql.nadel.engine.transform.query.NadelQueryPath
 import graphql.nadel.instrumentation.NadelInstrumentation
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationExecuteOperationParameters
-import graphql.nadel.util.ErrorUtil.createGraphQLErrorsFromRawErrors
 import graphql.normalized.ExecutableNormalizedField
 import graphql.normalized.ExecutableNormalizedOperation
+import graphql.normalized.ExecutableNormalizedOperationToAstCompiler
+import graphql.normalized.ExecutableNormalizedOperationToAstCompiler.CompilerResult
 import graphql.normalized.NormalizedInputValue
+import graphql.normalized.VariablePredicate
 import graphql.schema.FieldCoordinates
 import graphql.schema.GraphQLCodeRegistry
 import graphql.schema.GraphQLFieldDefinition
@@ -270,48 +272,6 @@ val AnyAstType.isNonNull: Boolean get() = TypeUtil.isNonNull(this)
 val AnyAstType.isWrapped: Boolean get() = TypeUtil.isWrapped(this)
 val AnyAstType.isNotWrapped: Boolean get() = !isWrapped
 
-@Deprecated("Use NadelResultMerger instead")
-internal fun mergeResults(results: List<ServiceExecutionResult>): ExecutionResult {
-    val data: MutableJsonMap = mutableMapOf()
-    val extensions: MutableJsonMap = mutableMapOf()
-    val errors: MutableList<GraphQLError> = mutableListOf()
-
-    fun putAndMergeTopLevelData(oneData: JsonMap) {
-        for ((topLevelFieldName: String, newTopLevelFieldValue: Any?) in oneData) {
-            if (topLevelFieldName in data) {
-                val existingValue = data[topLevelFieldName]
-                if (existingValue == null) {
-                    data[topLevelFieldName] = newTopLevelFieldValue
-                } else if (existingValue is AnyMap && newTopLevelFieldValue is AnyMap) {
-                    existingValue.asMutableJsonMap().putAll(
-                        newTopLevelFieldValue.asJsonMap(),
-                    )
-                }
-            } else {
-                data[topLevelFieldName] = newTopLevelFieldValue
-            }
-        }
-    }
-
-    for (result in results) {
-        val resultData = result.data
-        putAndMergeTopLevelData(resultData)
-        errors.addAll(createGraphQLErrorsFromRawErrors(result.errors))
-        extensions.putAll(result.extensions)
-    }
-
-    return newExecutionResult()
-        .data(data)
-        .extensions(extensions.let {
-            @Suppress("UNCHECKED_CAST") // .extensions should take in a Map<*, *> instead of strictly Map<Any?, Any?>
-            it as Map<Any?, Any?>
-        }.takeIf {
-            it.isNotEmpty()
-        })
-        .errors(errors)
-        .build()
-}
-
 fun makeFieldCoordinates(typeName: String, fieldName: String): FieldCoordinates {
     return FieldCoordinates.coordinates(typeName, fieldName)
 }
@@ -549,3 +509,19 @@ internal fun javaValueToAstValue(value: Any?): AnyAstValue {
 
 val GraphQLSchema.operationTypes
     get() = listOfNotNull(queryType, mutationType, subscriptionType)
+
+fun compileToDocument(
+    schema: GraphQLSchema,
+    operationKind: OperationDefinition.Operation,
+    operationName: String?,
+    topLevelFields: List<ExecutableNormalizedField>,
+    variablePredicate: VariablePredicate?,
+): CompilerResult {
+    return ExecutableNormalizedOperationToAstCompiler.compileToDocument(
+        schema,
+        operationKind,
+        operationName,
+        topLevelFields,
+        variablePredicate,
+    )
+}