From 5800007b7c38bf707b6083986fac304b0e05d9ef Mon Sep 17 00:00:00 2001 From: Franklin Wang <9077461+gnawf@users.noreply.github.com> Date: Fri, 23 Jun 2023 16:48:02 +1200 Subject: [PATCH] Performance improvements around result transforming (#448) * Use computed nodes if flatten is true * Stop constructing JSON node paths * More performance improvements to avoid extra memory allocations * Delete JsonNodePath from main module * Add Nadel prefix and more type safety * Avoid list allocation --- .../java/graphql/nadel/NadelResultMerger.kt | 10 +- .../transform/NadelDeepRenameTransform.kt | 14 +- .../engine/transform/NadelRenameTransform.kt | 8 +- .../engine/transform/NadelTransformUtil.kt | 39 --- .../NadelTypeRenameResultTransform.kt | 21 +- .../hydration/NadelHydrationTransform.kt | 19 +- .../batch/NadelBatchHydrationByIndex.kt | 21 +- .../batch/NadelBatchHydrationByObjectId.kt | 6 +- .../hydration/batch/NadelBatchHydrator.kt | 4 +- .../result/NadelResultInstruction.kt | 32 +- .../result/NadelResultTransformer.kt | 292 ++---------------- .../engine/transform/result/json/JsonNode.kt | 17 +- .../result/json/JsonNodeExtractor.kt | 60 +--- .../engine/transform/result/json/JsonNodes.kt | 50 +-- .../graphql/nadel/engine/util/GraphQLUtil.kt | 29 +- .../transform/NadelTransformJavaCompatTest.kt | 7 +- ...nt-in-renamed-object-input-in-hydration.kt | 7 +- .../hooks/polymorphic-hydration-hooks.kt | 9 +- .../hooks/transforms-can-copy-array-value.kt | 81 ----- .../hooks/transforms-can-set-array-value.kt | 76 ----- .../transforms/RemoveFieldTestTransform.kt | 5 +- .../graphql/nadel/tests/yaml}/JsonNodePath.kt | 2 +- .../graphql/nadel/tests/yaml/LowLevelYaml.kt | 2 - .../basic/transforms-can-copy-array-value.yml | 63 ---- .../basic/transforms-can-set-array-value.yml | 63 ---- 25 files changed, 186 insertions(+), 751 deletions(-) delete mode 100644 test/src/test/kotlin/graphql/nadel/tests/hooks/transforms-can-copy-array-value.kt delete mode 100644 test/src/test/kotlin/graphql/nadel/tests/hooks/transforms-can-set-array-value.kt rename {lib/src/main/java/graphql/nadel/engine/transform/result/json => test/src/test/kotlin/graphql/nadel/tests/yaml}/JsonNodePath.kt (96%) delete mode 100644 test/src/test/resources/fixtures/basic/transforms-can-copy-array-value.yml delete mode 100644 test/src/test/resources/fixtures/basic/transforms-can-set-array-value.yml diff --git a/lib/src/main/java/graphql/nadel/NadelResultMerger.kt b/lib/src/main/java/graphql/nadel/NadelResultMerger.kt index 4e99f8d82..4567f541a 100644 --- a/lib/src/main/java/graphql/nadel/NadelResultMerger.kt +++ b/lib/src/main/java/graphql/nadel/NadelResultMerger.kt @@ -5,6 +5,7 @@ import graphql.ExecutionResultImpl import graphql.GraphQLError import graphql.introspection.Introspection import graphql.nadel.engine.transform.query.NadelFieldAndService +import graphql.nadel.engine.transform.result.NadelResultKey import graphql.nadel.engine.util.AnyMap import graphql.nadel.engine.util.JsonMap import graphql.nadel.engine.util.MutableJsonMap @@ -16,9 +17,6 @@ import graphql.nadel.util.NamespacedUtil.isNamespacedField import graphql.normalized.ExecutableNormalizedField import graphql.schema.GraphQLSchema -@JvmInline -private value class ResultKey(val value: String) - internal object NadelResultMerger { fun mergeResults( fields: List, @@ -118,14 +116,14 @@ internal object NadelResultMerger { private fun buildRequiredFieldMap( fields: List, engineSchema: GraphQLSchema, - ): MutableMap> { - val requiredFields = mutableMapOf>() + ): MutableMap> { + val requiredFields = mutableMapOf>() // NOTE: please ensure all fields are from object types and will NOT have multiple field defs // Other code in this file relies on this contract for ((field) in fields) { val requiredChildFields = requiredFields - .computeIfAbsent(ResultKey(field.resultKey)) { + .computeIfAbsent(NadelResultKey(field.resultKey)) { mutableListOf() } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/NadelDeepRenameTransform.kt b/lib/src/main/java/graphql/nadel/engine/transform/NadelDeepRenameTransform.kt index 0aca09826..541d87fb3 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/NadelDeepRenameTransform.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/NadelDeepRenameTransform.kt @@ -12,6 +12,8 @@ import graphql.nadel.engine.transform.query.NFUtil import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.transform.query.NadelQueryTransformer import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey +import graphql.nadel.engine.transform.result.json.JsonNode import graphql.nadel.engine.transform.result.json.JsonNodeExtractor import graphql.nadel.engine.transform.result.json.JsonNodes import graphql.nadel.engine.util.emptyOrSingle @@ -310,15 +312,17 @@ internal class NadelDeepRenameTransform : NadelTransform NadelResultInstruction.Set( - subjectPath = destinationPath, + subject = parentNode, + key = NadelResultKey(overallField.resultKey), newValue = null, ) - else -> NadelResultInstruction.Copy( - subjectPath = sourceFieldNode.resultPath, - destinationPath = destinationPath, + + else -> NadelResultInstruction.Set( + subject = parentNode, + key = NadelResultKey(overallField.resultKey), + newValue = sourceFieldNode, ) } } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/NadelRenameTransform.kt b/lib/src/main/java/graphql/nadel/engine/transform/NadelRenameTransform.kt index 1fde2fe9f..539665979 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/NadelRenameTransform.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/NadelRenameTransform.kt @@ -13,6 +13,7 @@ import graphql.nadel.engine.transform.query.NFUtil.createField import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.transform.query.NadelQueryTransformer import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey import graphql.nadel.engine.transform.result.json.JsonNode import graphql.nadel.engine.transform.result.json.JsonNodeExtractor import graphql.nadel.engine.transform.result.json.JsonNodes @@ -196,9 +197,10 @@ internal class NadelRenameTransform : NadelTransform { val sourceFieldNode = JsonNodeExtractor.getNodesAt(parentNode, queryPathForSourceField) .emptyOrSingle() ?: return@instruction null - NadelResultInstruction.Copy( - subjectPath = sourceFieldNode.resultPath, - destinationPath = parentNode.resultPath + overallField.resultKey, + NadelResultInstruction.Set( + subject = parentNode, + key = NadelResultKey(overallField.resultKey), + newValue = sourceFieldNode, ) } } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/NadelTransformUtil.kt b/lib/src/main/java/graphql/nadel/engine/transform/NadelTransformUtil.kt index 4167f9153..52570b4ed 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/NadelTransformUtil.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/NadelTransformUtil.kt @@ -2,18 +2,13 @@ package graphql.nadel.engine.transform import graphql.introspection.Introspection.TypeNameMetaFieldDef import graphql.nadel.Service -import graphql.nadel.ServiceExecutionResult import graphql.nadel.engine.blueprint.NadelFieldInstruction import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint import graphql.nadel.engine.transform.artificial.NadelAliasHelper -import graphql.nadel.engine.transform.query.NadelQueryPath -import graphql.nadel.engine.transform.result.NadelResultInstruction import graphql.nadel.engine.transform.result.json.JsonNode -import graphql.nadel.engine.transform.result.json.JsonNodes import graphql.nadel.engine.util.JsonMap import graphql.nadel.engine.util.getField import graphql.nadel.engine.util.makeFieldCoordinates -import graphql.nadel.engine.util.queryPath import graphql.normalized.ExecutableNormalizedField import graphql.normalized.ExecutableNormalizedField.newNormalizedField import graphql.schema.GraphQLFieldDefinition @@ -75,40 +70,6 @@ object NadelTransformUtil { val coordinates = makeFieldCoordinates(overallTypeName, overallField.name) return executionBlueprint.engineSchema.getField(coordinates) } - - /** - * Creates a list of Set instructions that will be used to modified the values of the result data - * for a particular field. - * - * This function will call the transformerFunction for all result nodes associated with the overallField. So the - * caller doesn't need to worry about whether the overallField is a collection or an individual field. - * - * Note that the transformer will only be called for non-null values. - */ - fun createSetInstructions( - nodes: JsonNodes, - underlyingParentField: ExecutableNormalizedField?, - result: ServiceExecutionResult, - overallField: ExecutableNormalizedField, - transformerFunction: (Any) -> Any?, - ): List { - val parentQueryPath = underlyingParentField?.queryPath ?: NadelQueryPath.root - - val valueNodes: List = nodes.getNodesAt( - queryPath = parentQueryPath + overallField.resultKey, - flatten = true - ) - - return valueNodes - .mapNotNull { valueNode -> - nodes.getNodeAt(valueNode.resultPath)?.let { jsonNode -> - NadelResultInstruction.Set( - valueNode.resultPath, - jsonNode.value?.let { value -> transformerFunction(value) }, - ) - } - } - } } /** diff --git a/lib/src/main/java/graphql/nadel/engine/transform/NadelTypeRenameResultTransform.kt b/lib/src/main/java/graphql/nadel/engine/transform/NadelTypeRenameResultTransform.kt index 3a9489ef5..e44a2e09b 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/NadelTypeRenameResultTransform.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/NadelTypeRenameResultTransform.kt @@ -10,7 +10,10 @@ import graphql.nadel.engine.transform.NadelTypeRenameResultTransform.State import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.transform.query.NadelQueryTransformer import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey +import graphql.nadel.engine.transform.result.json.JsonNode import graphql.nadel.engine.transform.result.json.JsonNodes +import graphql.nadel.engine.util.JsonMap import graphql.nadel.engine.util.queryPath import graphql.normalized.ExecutableNormalizedField @@ -57,21 +60,27 @@ internal class NadelTypeRenameResultTransform : NadelTransform { state: State, nodes: JsonNodes, ): List { - val typeNameNodes = nodes.getNodesAt( - (underlyingParentField?.queryPath ?: NadelQueryPath.root) + overallField.resultKey, + val parentNodes = nodes.getNodesAt( + underlyingParentField?.queryPath ?: NadelQueryPath.root, flatten = true, ) - return typeNameNodes.mapNotNull { typeNameNode -> - val underlyingTypeName = typeNameNode.value as String? + return parentNodes.mapNotNull { parentNode -> + @Suppress("UNCHECKED_CAST") + val parentMap = parentNode.value as? JsonMap ?: return@mapNotNull null + val underlyingTypeName = parentMap[overallField.resultKey] as String? + ?: return@mapNotNull null + val overallTypeName = executionBlueprint.getOverallTypeName( service = service, underlyingTypeName = underlyingTypeName, ) + NadelResultInstruction.Set( - subjectPath = typeNameNode.resultPath, - newValue = overallTypeName, + subject = parentNode, + key = NadelResultKey(overallField.resultKey), + newValue = JsonNode(overallTypeName), ) } } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationTransform.kt b/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationTransform.kt index c0c47ad2b..e1678c501 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationTransform.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationTransform.kt @@ -22,6 +22,7 @@ import graphql.nadel.engine.transform.hydration.NadelHydrationUtil.getInstructio import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.transform.query.NadelQueryTransformer import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey import graphql.nadel.engine.transform.result.json.JsonNode import graphql.nadel.engine.transform.result.json.JsonNodeExtractor import graphql.nadel.engine.transform.result.json.JsonNodes @@ -191,7 +192,13 @@ internal class NadelHydrationTransform( } val instruction = getHydrationFieldInstruction(state, instructions, executionContext.hooks, parentNode) - ?: return listOf(NadelResultInstruction.Set(parentNode.resultPath + state.hydratedField.fieldName, null)) + ?: return listOf( + NadelResultInstruction.Set( + subject = parentNode, + key = NadelResultKey(state.hydratedField.resultKey), + newValue = null, + ), + ) val actorQueryResults = coroutineScope { NadelHydrationFieldsBuilder.makeActorQueries( @@ -238,8 +245,9 @@ internal class NadelHydrationTransform( return listOf( NadelResultInstruction.Set( - subjectPath = parentNode.resultPath + fieldToHydrate.resultKey, - newValue = data?.value, + subject = parentNode, + key = NadelResultKey(fieldToHydrate.resultKey), + newValue = JsonNode(data?.value), ), ) + errors } @@ -256,8 +264,9 @@ internal class NadelHydrationTransform( return listOf( NadelResultInstruction.Set( - subjectPath = parentNode.resultPath + fieldToHydrate.resultKey, - newValue = data, + subject = parentNode, + key = NadelResultKey(fieldToHydrate.resultKey), + newValue = JsonNode(data), ), ) + addErrors } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByIndex.kt b/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByIndex.kt index 3be30054b..d49b25837 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByIndex.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByIndex.kt @@ -7,6 +7,7 @@ import graphql.nadel.engine.transform.artificial.NadelAliasHelper import graphql.nadel.engine.transform.hydration.NadelHydrationUtil import graphql.nadel.engine.transform.hydration.batch.NadelBatchHydrationInputBuilder.getBatchInputDef import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey import graphql.nadel.engine.transform.result.json.JsonNode import graphql.nadel.engine.util.AnyList import graphql.nadel.engine.util.emptyOrSingle @@ -48,14 +49,17 @@ internal class NadelBatchHydrationByIndex private constructor( val inputValues = getInputValues(parentNode) NadelResultInstruction.Set( - subjectPath = parentNode.resultPath + fieldToHydrate.resultKey, - newValue = if (isManyInputNodesToParentNodes) { - chunker.take(inputValues) - } else { - chunker.takeOne( - inputValue = inputValues.emptyOrSingle(), - ) - }, + subject = parentNode, + key = NadelResultKey(fieldToHydrate.resultKey), + newValue = JsonNode( + if (isManyInputNodesToParentNodes) { + chunker.take(inputValues) + } else { + chunker.takeOne( + inputValue = inputValues.emptyOrSingle(), + ) + }, + ), ) } } @@ -145,6 +149,7 @@ internal class NadelBatchHydrationByIndex private constructor( badCount() } } + else -> error("Unsupported batch result type") } } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByObjectId.kt b/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByObjectId.kt index 04a7dd07a..167cee9a4 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByObjectId.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByObjectId.kt @@ -7,6 +7,7 @@ import graphql.nadel.engine.blueprint.hydration.NadelBatchHydrationMatchStrategy import graphql.nadel.engine.transform.NadelTransformUtil import graphql.nadel.engine.transform.hydration.NadelHydrationUtil.getHydrationActorNodes import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey import graphql.nadel.engine.transform.result.asMutable import graphql.nadel.engine.transform.result.json.JsonNode import graphql.nadel.engine.transform.result.json.JsonNodeExtractor @@ -160,8 +161,9 @@ internal object NadelBatchHydrationByObjectId { } return NadelResultInstruction.Set( - subjectPath = sourceNode.resultPath + state.hydratedField.resultKey, - newValue = newValue, + subject = sourceNode, + key = NadelResultKey(state.hydratedField.resultKey), + newValue = JsonNode(newValue), ) } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrator.kt b/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrator.kt index 3407296ef..453e5510e 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrator.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrator.kt @@ -15,6 +15,7 @@ import graphql.nadel.engine.transform.hydration.batch.NadelBatchHydrationByObjec import graphql.nadel.engine.transform.hydration.batch.NadelBatchHydrationByObjectId.getHydrateInstructionsMatchingObjectIds import graphql.nadel.engine.transform.hydration.batch.NadelBatchHydrationTransform.State import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey import graphql.nadel.engine.transform.result.json.JsonNode import graphql.schema.FieldCoordinates import kotlinx.coroutines.Deferred @@ -61,7 +62,8 @@ internal class NadelBatchHydrator( when (instruction) { null -> parentNodes.map { NadelResultInstruction.Set( - it.resultPath + state.hydratedField.fieldName, + subject = it, + key = NadelResultKey(state.hydratedField.resultKey), newValue = null, ) } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultInstruction.kt b/lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultInstruction.kt index 0326d6ff2..7a975b665 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultInstruction.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultInstruction.kt @@ -1,34 +1,24 @@ package graphql.nadel.engine.transform.result import graphql.GraphQLError -import graphql.nadel.engine.transform.result.json.JsonNodePath +import graphql.nadel.engine.transform.result.json.JsonNode + +// todo: should be a value class one day… can't because of Java interop +data class NadelResultKey(val value: String) sealed class NadelResultInstruction { data class Set( - override val subjectPath: JsonNodePath, - val newValue: Any?, - ) : NadelResultInstruction(), NadelResultInstructionWithSubject + val subject: JsonNode, + val key: NadelResultKey, + val newValue: JsonNode?, + ) : NadelResultInstruction() data class Remove( - override val subjectPath: JsonNodePath, - ) : NadelResultInstruction(), NadelResultInstructionWithSubject - - data class Copy( - override val subjectPath: JsonNodePath, - val destinationPath: JsonNodePath, - ) : NadelResultInstruction(), NadelResultInstructionWithSubject { - val destinationKey: String - get() = destinationPath.segments.last().value as String - } + val subject: JsonNode, + val key: NadelResultKey, + ) : NadelResultInstruction() data class AddError( val error: GraphQLError, ) : NadelResultInstruction() } - -interface NadelResultInstructionWithSubject { - val subjectPath: JsonNodePath - - val subjectKey: String - get() = subjectPath.segments.last().value as String -} diff --git a/lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultTransformer.kt b/lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultTransformer.kt index 7c65b3b2a..b42f239db 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultTransformer.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultTransformer.kt @@ -5,26 +5,15 @@ import graphql.nadel.ServiceExecutionResult import graphql.nadel.engine.NadelExecutionContext import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint import graphql.nadel.engine.plan.NadelExecutionPlan -import graphql.nadel.engine.transform.result.NadelResultTransformer.DataMutation -import graphql.nadel.engine.transform.result.json.AnyJsonNodePathSegment -import graphql.nadel.engine.transform.result.json.JsonNode -import graphql.nadel.engine.transform.result.json.JsonNodePath -import graphql.nadel.engine.transform.result.json.JsonNodePathSegment import graphql.nadel.engine.transform.result.json.JsonNodes -import graphql.nadel.engine.util.AnyList -import graphql.nadel.engine.util.AnyMap -import graphql.nadel.engine.util.AnyMutableList -import graphql.nadel.engine.util.AnyMutableMap import graphql.nadel.engine.util.JsonMap import graphql.nadel.engine.util.MutableJsonMap -import graphql.nadel.engine.util.asMutableJsonMap import graphql.nadel.engine.util.queryPath import graphql.normalized.ExecutableNormalizedField import kotlinx.coroutines.Deferred import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.coroutineScope -import kotlin.reflect.KClass internal class NadelResultTransformer(private val executionBlueprint: NadelOverallExecutionBlueprint) { suspend fun transform( @@ -43,7 +32,7 @@ internal class NadelResultTransformer(private val executionBlueprint: NadelOvera for ((field, steps) in executionPlan.transformationSteps) { // This can be null if we did not end up sending the field e.g. for hydration val underlyingFields = overallToUnderlyingFields[field] - if (underlyingFields == null || underlyingFields.isEmpty()) { + if (underlyingFields.isNullOrEmpty()) { continue } @@ -82,128 +71,40 @@ internal class NadelResultTransformer(private val executionBlueprint: NadelOvera } private fun mutate(result: ServiceExecutionResult, instructions: List) { - // For now, we don't have any instructions to modify anything other than data, so return early - if (result.data == null) { - return - } - - val mutations = instructions.mapNotNull { transformation -> + instructions.forEach { transformation -> when (transformation) { - is NadelResultInstruction.Set -> prepareSet(result.data, transformation) - is NadelResultInstruction.Remove -> prepareRemove(result.data, transformation) - is NadelResultInstruction.Copy -> prepareCopy(result.data, transformation) - is NadelResultInstruction.AddError -> prepareAddError(result.errors, transformation) + is NadelResultInstruction.Set -> process(transformation) + is NadelResultInstruction.Remove -> process(transformation) + is NadelResultInstruction.AddError -> process(transformation, result.errors) } } - - val transformContext = TransformContext() - mutations.forEach { - it.run(context = transformContext) - } } - private fun prepareSet( - data: JsonMap, + private fun process( instruction: NadelResultInstruction.Set, - ): DataMutation? { - return prepareSet( - data, - destPath = instruction.subjectPath, - newValue = instruction.newValue, - ) + ) { + @Suppress("UNCHECKED_CAST") + val map = instruction.subject.value as? MutableJsonMap ?: return + map[instruction.key.value] = instruction.newValue?.value } - private fun prepareRemove( - data: JsonMap, + private fun process( instruction: NadelResultInstruction.Remove, - ): DataMutation? { - val parentPath = instruction.subjectPath.dropLast(1) - val parentMap = data.getJsonMapAt(parentPath) ?: return null - val subjectKey = instruction.subjectKey - - val dataToDelete = parentMap[subjectKey] - - return DataMutation { context -> - // Ensure that the node is still the same before removing it, ensures we don't remove the wrong thing - if (parentMap[subjectKey] === dataToDelete) { - modifyMap(context, parentPath, parentMap) { - it.remove(subjectKey) - } - } - } - } - - private fun prepareCopy( - data: JsonMap, - instruction: NadelResultInstruction.Copy, - ): DataMutation? { - val dataToCopy = data.getAt(instruction.subjectPath) - return prepareSet( - data, - destPath = instruction.destinationPath, - newValue = dataToCopy.value, - ) - } - - private fun prepareSet( - data: JsonMap, - destPath: JsonNodePath, - newValue: Any?, - ): DataMutation? { - val destParentPath = destPath.dropLast(1) - val destParentValue = data.getAt(destParentPath).value - val destKey = destPath.segments.last().value + ) { + @Suppress("UNCHECKED_CAST") + val map = instruction.subject.value as? MutableJsonMap ?: return - return when (destParentValue) { - is AnyMap -> DataMutation { - when (destKey) { - is String -> destParentValue.asMutableJsonMap()[destKey] = newValue - else -> error("Set instruction expected string key for JSON object") - } - } - is AnyList -> DataMutation { - when (destKey) { - is Int -> destParentValue.asMutable()[destKey] = newValue - else -> error("Set instruction expected integer key for JSON array") - } - } - null -> null - else -> error("Set instruction had leaf value as parent of destination path") - } + map.remove(instruction.key.value) } - private fun prepareAddError( - errors: List, + private fun process( instruction: NadelResultInstruction.AddError, - ): DataMutation { - val newError = instruction.error.toSpecification() - - return DataMutation { - val mutableErrors = errors.asMutable() - mutableErrors.add(newError) - } - } - - private inline fun modifyMap( - context: TransformContext, - mapPath: JsonNodePath, - map: JsonMap, - modification: (MutableJsonMap) -> Unit, + errors: List, ) { - val wasEmpty = map.isEmpty() - modification(map.asMutable()) - - if (map.isEmpty()) { - if (!wasEmpty) { - context.emptyPaths.add(mapPath) - } - } else if (wasEmpty) { - context.emptyPaths.remove(mapPath) - } - } + val newError = instruction.error.toSpecification() - private fun interface DataMutation { - fun run(context: TransformContext) + val mutableErrors = errors.asMutable() + mutableErrors.add(newError) } private fun getRemoveArtificialFieldInstructions( @@ -214,19 +115,17 @@ internal class NadelResultTransformer(private val executionBlueprint: NadelOvera .asSequence() .flatMap { field -> nodes.getNodesAt( - queryPath = field.queryPath, - ).map { jsonNode -> + queryPath = field.queryPath.dropLast(1), + flatten = true, + ).map { parentNode -> NadelResultInstruction.Remove( - subjectPath = jsonNode.resultPath, + subject = parentNode, + key = NadelResultKey(field.resultKey), ) } } .toList() } - - private data class TransformContext( - val emptyPaths: HashSet = hashSetOf(), - ) } internal fun Map.asMutable(): MutableMap { @@ -237,145 +136,4 @@ private fun List.asMutable(): MutableList { return this as? MutableList ?: throw NotMutableError() } -/** - * This gets the object right before the last segment of the [node], aka the parent. - */ -private fun JsonMap.getParentOf(node: JsonNodePath): JsonMap? { - val pathToParent = node.dropLast(1) - return getJsonMapAt(pathToParent) -} - -private fun JsonMap.getJsonMapAt(path: JsonNodePath): JsonMap? { - @Suppress("UNCHECKED_CAST") - return getAt(path).value as JsonMap? -} - -private fun JsonMap.getAt(path: JsonNodePath): JsonNode { - var cursor: Any = this - - for (segment in path.segments) { - cursor = when (segment) { - is JsonNodePathSegment.Int -> { - if (cursor is AnyList) { - cursor[segment.value] - } else { - error("Requires List") - } - } - is JsonNodePathSegment.String -> { - if (cursor is AnyMap) { - cursor[segment.value] - } else { - error("Requires List") - } - } - } ?: return JsonNode(path, null) - } - - return JsonNode( - path, - cursor, - ) - // return path.segments.fold(JsonNode(JsonNodePath.root, this), JsonNode::get) -} - -/** - * This cleans up a the GraphQL response when a field is removed. - * - * If we remove a field from the tree and it is has no siblings, then we need to remove - * the parent field too, and so on. - */ -private fun JsonMap.cleanup(path: JsonNodePath) { - // This gets the node at every path segment up until the node specified by the path - val items = path.segments.runningFold(JsonNode(JsonNodePath.root, this), JsonNode::get) - - // We work backwards here and see if a child has been deleted from the Map - // If the map is empty, we want to remove it from the - var childKeyToDelete: AnyJsonNodePathSegment? = null - for (item in items.asReversed()) { - when (val value = item.value) { - is AnyMutableMap -> { - when (childKeyToDelete) { - null -> { - } - is JsonNodePathSegment.String -> { - value.remove(key = childKeyToDelete.value) - } - else -> throw UnsupportedOperationException("Unsupported key to delete from Map") - } - if (value.isEmpty()) { - childKeyToDelete = item.resultPath.segments.lastOrNull() - } else { - break - } - } - is AnyMutableList -> { - when (childKeyToDelete) { - null -> { - } - is JsonNodePathSegment.Int -> break - else -> throw UnsupportedOperationException("Unsupported key to delete from List") - } - } - is AnyMap, is AnyList -> { - throw NotMutableError() - } - null -> { - } - else -> break - } - } -} - -private fun JsonNode.get(segment: AnyJsonNodePathSegment): JsonNode { - return getNextNode(this, segment) -} - -private fun getNextNode(current: JsonNode, segment: AnyJsonNodePathSegment): JsonNode { - return when (segment) { - is JsonNodePathSegment.String -> { - val path = current.resultPath + segment.value - when (val value = current.value) { - is AnyMap? -> JsonNode( - resultPath = path, - value = value?.get(segment.value), - ) - else -> throw UnexpectedDataType( - path, - expected = Map::class, - actual = value?.javaClass, - ) - } - } - is JsonNodePathSegment.Int -> { - val path = current.resultPath + segment.value - when (val value = current.value) { - is AnyList? -> JsonNode( - resultPath = path, - value = value?.get(segment.value), - ) - else -> throw UnexpectedDataType( - path, - expected = List::class, - actual = value?.javaClass, - ) - } - } - } -} - private class NotMutableError : RuntimeException("Data was required to be mutable but was not") - -private class UnexpectedDataType private constructor(message: String) : RuntimeException(message) { - companion object { - operator fun invoke(path: JsonNodePath, expected: KClass<*>, actual: Class<*>?): UnexpectedDataType { - val pathStr = path.segments.joinToString("/") { - it.value.toString() - } - val expectedStr = expected.qualifiedName - val actualStr = actual?.name ?: "null" - val message = "Expected data at '$pathStr' to be of type '$expectedStr' but got '$actualStr'" - return UnexpectedDataType(message) - } - } -} diff --git a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNode.kt b/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNode.kt index ad3eb1f53..b90e80831 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNode.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNode.kt @@ -1,6 +1,15 @@ package graphql.nadel.engine.transform.result.json -data class JsonNode( - val resultPath: JsonNodePath, - val value: Any?, -) +import graphql.nadel.engine.util.AnyList +import graphql.nadel.engine.util.AnyMap + +/** + * todo: one day split this into a sealed class that acts like a union type for each possible type + * + * This union type will give us better type safety + */ +data class JsonNode(val value: Any?) { + init { + require(value == null || value is AnyMap || value is AnyList || value is Number || value is Boolean || value is String) + } +} diff --git a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodeExtractor.kt b/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodeExtractor.kt index fa33506d5..db5d857fe 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodeExtractor.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodeExtractor.kt @@ -4,7 +4,6 @@ import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.util.AnyList import graphql.nadel.engine.util.AnyMap import graphql.nadel.engine.util.JsonMap -import graphql.nadel.engine.util.foldWhileNotNull @Deprecated("Start moving to JsonNodes for performance reasons") object JsonNodeExtractor { @@ -12,7 +11,7 @@ object JsonNodeExtractor { * Extracts the nodes at the given query selection path. */ fun getNodesAt(data: JsonMap, queryPath: NadelQueryPath, flatten: Boolean = false): List { - val rootNode = JsonNode(JsonNodePath.root, data) + val rootNode = JsonNode(data) return getNodesAt(rootNode, queryPath, flatten) } @@ -20,6 +19,10 @@ object JsonNodeExtractor { * Extracts the nodes at the given query selection path. */ fun getNodesAt(rootNode: JsonNode, queryPath: NadelQueryPath, flatten: Boolean = false): List { + if (queryPath.size == 1) { + return getNodes(rootNode, queryPath.last(), flattenLists = flatten) + } + // This is a breadth-first search return queryPath.segments.foldIndexed(listOf(rootNode)) { index, queue, pathSegment -> val atEnd = index == queryPath.segments.lastIndex @@ -32,57 +35,27 @@ object JsonNodeExtractor { } } - /** - * Extract the node at the given json node path. - */ - fun getNodeAt(data: JsonMap, path: JsonNodePath): JsonNode? { - val rootNode = JsonNode(JsonNodePath.root, data) - return getNodeAt(rootNode, path) - } - - /** - * Extract the node at the given json node path. - */ - fun getNodeAt(rootNode: JsonNode, path: JsonNodePath): JsonNode? { - return path.segments.foldWhileNotNull(rootNode as JsonNode?) { currentNode, segment -> - when (currentNode?.value) { - is AnyMap -> currentNode.value[segment.value]?.let { - JsonNode(currentNode.resultPath + segment, it) - } - is AnyList -> when (segment) { - is JsonNodePathSegment.Int -> currentNode.value.getOrNull(segment.value)?.let { - JsonNode(currentNode.resultPath + segment, it) - } - else -> null - } - else -> null - } - } - } - private fun getNodes(node: JsonNode, segment: String, flattenLists: Boolean): List { return when (node.value) { - is AnyMap -> getNodes(node.resultPath, node.value, segment, flattenLists) + is AnyMap -> getNodes(node.value, segment, flattenLists) null -> emptyList() else -> throw IllegalNodeTypeException(node) } } private fun getNodes( - parentPath: JsonNodePath, map: AnyMap, segment: String, flattenLists: Boolean, ): List { - val newPath = parentPath + segment val value = map[segment] // We flatten lists as these nodes contribute to the BFS queue if (value is AnyList && flattenLists) { - return getFlatNodes(parentPath = newPath, value) + return getFlatNodes(value) } - return listOf(JsonNode(newPath, value)) + return listOf(JsonNode(value)) } /** @@ -97,12 +70,11 @@ object JsonNodeExtractor { * * etc. */ - private fun getFlatNodes(parentPath: JsonNodePath, values: AnyList): List { - return values.flatMapIndexed { index, value -> - val newPath = parentPath + index + private fun getFlatNodes(values: AnyList): List { + return values.flatMap { value -> when (value) { - is AnyList -> getFlatNodes(newPath, value) - else -> listOf(JsonNode(newPath, value)) + is AnyList -> getFlatNodes(value) + else -> listOf(JsonNode(value)) } } } @@ -112,11 +84,11 @@ class IllegalNodeTypeException private constructor(message: String) : RuntimeExc companion object { operator fun invoke(node: JsonNode): IllegalNodeTypeException { val nodeType = node.value?.javaClass?.name ?: "null" - val pathString = node.resultPath.segments.joinToString("/") { - it.value.toString() - } + // val pathString = node.resultPath.segments.joinToString("/") { + // it.value.toString() + // } - return IllegalNodeTypeException("Unknown node type '$nodeType' at '$pathString' was not a map") + return IllegalNodeTypeException("Unknown node type '$nodeType' was not a map") } } } diff --git a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodes.kt b/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodes.kt index 55e5c7d97..a19854eb5 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodes.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodes.kt @@ -5,7 +5,6 @@ import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.util.AnyList import graphql.nadel.engine.util.AnyMap import graphql.nadel.engine.util.JsonMap -import graphql.nadel.engine.util.foldWhileNotNull import java.util.Collections /** @@ -23,18 +22,10 @@ class JsonNodes( * Extracts the nodes at the given query selection path. */ fun getNodesAt(queryPath: NadelQueryPath, flatten: Boolean = false): List { - val rootNode = JsonNode(JsonNodePath.root, data) + val rootNode = JsonNode(data) return getNodesAt(rootNode, queryPath, flatten) } - /** - * Extract the node at the given json node path. - */ - fun getNodeAt(path: JsonNodePath): JsonNode? { - val rootNode = JsonNode(JsonNodePath.root, data) - return getNodeAt(rootNode, path) - } - /** * Extracts the nodes at the given query selection path. */ @@ -49,7 +40,7 @@ class JsonNodes( val hasMore = index < queryPath.segments.lastIndex val pathSegment = queryPath.segments[index] - queue = if (hasMore) { + queue = if (hasMore || flatten) { nodes.computeIfAbsent(subPath) { queue.flatMap { node -> getNodes(node, pathSegment, flattenLists = true) @@ -65,50 +56,28 @@ class JsonNodes( return queue } - /** - * Extract the node at the given json node path. - */ - private fun getNodeAt(rootNode: JsonNode, path: JsonNodePath): JsonNode? { - return path.segments.foldWhileNotNull(rootNode as JsonNode?) { currentNode, segment -> - when (currentNode?.value) { - is AnyMap -> currentNode.value[segment.value]?.let { - JsonNode(currentNode.resultPath + segment, it) - } - is AnyList -> when (segment) { - is JsonNodePathSegment.Int -> currentNode.value.getOrNull(segment.value)?.let { - JsonNode(currentNode.resultPath + segment, it) - } - else -> null - } - else -> null - } - } - } - private fun getNodes(node: JsonNode, segment: String, flattenLists: Boolean): Sequence { return when (node.value) { - is AnyMap -> getNodes(node.resultPath, node.value, segment, flattenLists) + is AnyMap -> getNodes(node.value, segment, flattenLists) null -> emptySequence() else -> throw IllegalNodeTypeException(node) } } private fun getNodes( - parentPath: JsonNodePath, map: AnyMap, segment: String, flattenLists: Boolean, ): Sequence { - val newPath = parentPath + segment val value = map[segment] // We flatten lists as these nodes contribute to the BFS queue if (value is AnyList && flattenLists) { - return getFlatNodes(parentPath = newPath, value) + return getFlatNodes(value) } return sequenceOf( - JsonNode(newPath, value), + JsonNode(value), ) } @@ -124,15 +93,14 @@ class JsonNodes( * * etc. */ - private fun getFlatNodes(parentPath: JsonNodePath, values: AnyList): Sequence { + private fun getFlatNodes(values: AnyList): Sequence { return values .asSequence() - .flatMapIndexed { index, value -> - val newPath = parentPath + index + .flatMap { value -> when (value) { - is AnyList -> getFlatNodes(newPath, value) + is AnyList -> getFlatNodes(value) else -> sequenceOf( - JsonNode(newPath, value), + JsonNode(value), ) } } diff --git a/lib/src/main/java/graphql/nadel/engine/util/GraphQLUtil.kt b/lib/src/main/java/graphql/nadel/engine/util/GraphQLUtil.kt index c0c0358f1..af59141d4 100644 --- a/lib/src/main/java/graphql/nadel/engine/util/GraphQLUtil.kt +++ b/lib/src/main/java/graphql/nadel/engine/util/GraphQLUtil.kt @@ -62,6 +62,7 @@ import graphql.schema.GraphQLUnionType import graphql.schema.GraphQLUnmodifiedType import graphql.schema.idl.TypeUtil import kotlinx.coroutines.future.asDeferred +import java.util.Arrays internal typealias AnyAstValue = Value<*> internal typealias AnyAstNode = Node<*> @@ -225,7 +226,33 @@ fun ExecutableNormalizedField.copyWithChildren(children: List(count) + run { + var cursor: ExecutableNormalizedField = this + var index = count - 1 + while (true) { + array[index] = cursor.resultKey + index-- + cursor = cursor.parent ?: break + } + } + + @Suppress("UNCHECKED_CAST") + array.asList() as List + }, + ) inline fun Document.getDefinitionsOfType(): List { return getDefinitionsOfType(T::class.java) diff --git a/lib/src/test/kotlin/graphql/nadel/transform/NadelTransformJavaCompatTest.kt b/lib/src/test/kotlin/graphql/nadel/transform/NadelTransformJavaCompatTest.kt index f704bc975..114a78179 100644 --- a/lib/src/test/kotlin/graphql/nadel/transform/NadelTransformJavaCompatTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/transform/NadelTransformJavaCompatTest.kt @@ -10,6 +10,8 @@ import graphql.nadel.engine.transform.NadelTransformJavaCompat import graphql.nadel.engine.transform.query.NadelQueryTransformer import graphql.nadel.engine.transform.query.NadelQueryTransformerJavaCompat import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey +import graphql.nadel.engine.transform.result.json.JsonNode import graphql.nadel.engine.transform.result.json.JsonNodes import graphql.nadel.test.NadelTransformJavaCompatAdapter import graphql.nadel.test.mock @@ -155,8 +157,9 @@ class NadelTransformJavaCompatTest : DescribeSpec({ // given val expectedResult = listOf( NadelResultInstruction.Set( - mock(), - newValue = 1, + subject = JsonNode(mutableMapOf()), + key = NadelResultKey("hello"), + newValue = JsonNode(1), ) ) val compat = spy(object : NadelTransformJavaCompatAdapter { diff --git a/test/src/test/kotlin/graphql/nadel/tests/hooks/ari-argument-in-renamed-object-input-in-hydration.kt b/test/src/test/kotlin/graphql/nadel/tests/hooks/ari-argument-in-renamed-object-input-in-hydration.kt index 864c33d93..8f0ad75aa 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/hooks/ari-argument-in-renamed-object-input-in-hydration.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/hooks/ari-argument-in-renamed-object-input-in-hydration.kt @@ -16,6 +16,8 @@ import graphql.nadel.engine.transform.NadelTransformFieldResult import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.transform.query.NadelQueryTransformer import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey +import graphql.nadel.engine.transform.result.json.JsonNode import graphql.nadel.engine.transform.result.json.JsonNodes import graphql.nadel.engine.util.JsonMap import graphql.nadel.engine.util.queryPath @@ -160,8 +162,9 @@ class `ari-argument-in-renamed-object-input-in-hydration` : EngineTestHook { val owner = (state.getArgument("owner").argumentValue.value as StringValue).value NadelResultInstruction.Set( - subjectPath = parentNode.resultPath + overallField.resultKey, - newValue = "ari:cloud:$owner::$type/$value", + subject = parentNode, + key = NadelResultKey(overallField.resultKey), + newValue = JsonNode("ari:cloud:$owner::$type/$value"), ) } } diff --git a/test/src/test/kotlin/graphql/nadel/tests/hooks/polymorphic-hydration-hooks.kt b/test/src/test/kotlin/graphql/nadel/tests/hooks/polymorphic-hydration-hooks.kt index cd8b41aca..11c2c6ea9 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/hooks/polymorphic-hydration-hooks.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/hooks/polymorphic-hydration-hooks.kt @@ -6,8 +6,7 @@ import graphql.nadel.engine.blueprint.NadelGenericHydrationInstruction import graphql.nadel.engine.blueprint.NadelHydrationFieldInstruction import graphql.nadel.engine.transform.artificial.NadelAliasHelper import graphql.nadel.engine.transform.result.json.JsonNode -import graphql.nadel.engine.transform.result.json.JsonNodeExtractor -import graphql.nadel.engine.transform.result.json.JsonNodePath +import graphql.nadel.engine.util.JsonMap import graphql.nadel.tests.EngineTestHook import graphql.nadel.tests.UseHook @@ -16,15 +15,13 @@ private class PolymorphicHydrationHooks : NadelEngineExecutionHooks { instructions: List, parentNode: JsonNode, aliasHelper: NadelAliasHelper, - userContext: Any? + userContext: Any?, ): T? { - val dataIdFieldName = if (instructions.any { it is NadelHydrationFieldInstruction }) "hydration__data__dataId" else "batch_hydration__data__dataId" - val dataIdValue = JsonNodeExtractor.getNodeAt(parentNode, JsonNodePath.root + dataIdFieldName)!! - .value as String + val dataIdValue = (parentNode.value as JsonMap)[dataIdFieldName] as String val actorFieldName = when { dataIdValue.startsWith("human", ignoreCase = true) -> "humanById" dataIdValue.startsWith("null", ignoreCase = true) -> null diff --git a/test/src/test/kotlin/graphql/nadel/tests/hooks/transforms-can-copy-array-value.kt b/test/src/test/kotlin/graphql/nadel/tests/hooks/transforms-can-copy-array-value.kt deleted file mode 100644 index b17517c96..000000000 --- a/test/src/test/kotlin/graphql/nadel/tests/hooks/transforms-can-copy-array-value.kt +++ /dev/null @@ -1,81 +0,0 @@ -package graphql.nadel.tests.hooks - -import graphql.nadel.Service -import graphql.nadel.ServiceExecutionHydrationDetails -import graphql.nadel.ServiceExecutionResult -import graphql.nadel.engine.NadelExecutionContext -import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint -import graphql.nadel.engine.transform.NadelTransform -import graphql.nadel.engine.transform.NadelTransformFieldResult -import graphql.nadel.engine.transform.query.NadelQueryTransformer -import graphql.nadel.engine.transform.result.NadelResultInstruction -import graphql.nadel.engine.transform.result.json.JsonNodeExtractor -import graphql.nadel.engine.transform.result.json.JsonNodes -import graphql.nadel.engine.util.queryPath -import graphql.nadel.tests.EngineTestHook -import graphql.nadel.tests.UseHook -import graphql.normalized.ExecutableNormalizedField - -@UseHook -class `transforms-can-copy-array-value` : EngineTestHook { - override val customTransforms: List> - get() = listOf( - object : NadelTransform { - override suspend fun isApplicable( - executionContext: NadelExecutionContext, - executionBlueprint: NadelOverallExecutionBlueprint, - services: Map, - service: Service, - overallField: ExecutableNormalizedField, - hydrationDetails: ServiceExecutionHydrationDetails?, - ): Any? { - return overallField.name.takeIf { it == "ids" } - } - - override suspend fun transformField( - executionContext: NadelExecutionContext, - transformer: NadelQueryTransformer, - executionBlueprint: NadelOverallExecutionBlueprint, - service: Service, - field: ExecutableNormalizedField, - state: Any, - ): NadelTransformFieldResult { - return NadelTransformFieldResult.unmodified(field) - } - - override suspend fun getResultInstructions( - executionContext: NadelExecutionContext, - executionBlueprint: NadelOverallExecutionBlueprint, - service: Service, - overallField: ExecutableNormalizedField, - underlyingParentField: ExecutableNormalizedField?, - result: ServiceExecutionResult, - state: Any, - nodes: JsonNodes, - ): List { - val nodes = JsonNodeExtractor.getNodesAt( - data = result.data, - queryPath = underlyingParentField!!.queryPath + overallField.resultKey, - flatten = true, - ) - - return nodes - .mapIndexed { index, node -> - val copyFromIndex = (index - 1) - .takeIf { it >= 0 } - ?: nodes.lastIndex - - NadelResultInstruction.Copy( - subjectPath = node.resultPath.dropLast(1) + copyFromIndex, - destinationPath = node.resultPath - ) - } - .onEach { - // Ensure we are operating an array elements - require(it.subjectPath.segments.last().value is Int) - require(it.destinationPath.segments.last().value is Int) - } - } - } - ) -} diff --git a/test/src/test/kotlin/graphql/nadel/tests/hooks/transforms-can-set-array-value.kt b/test/src/test/kotlin/graphql/nadel/tests/hooks/transforms-can-set-array-value.kt deleted file mode 100644 index 1f3c20b55..000000000 --- a/test/src/test/kotlin/graphql/nadel/tests/hooks/transforms-can-set-array-value.kt +++ /dev/null @@ -1,76 +0,0 @@ -package graphql.nadel.tests.hooks - -import graphql.nadel.Service -import graphql.nadel.ServiceExecutionHydrationDetails -import graphql.nadel.ServiceExecutionResult -import graphql.nadel.engine.NadelExecutionContext -import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint -import graphql.nadel.engine.transform.NadelTransform -import graphql.nadel.engine.transform.NadelTransformFieldResult -import graphql.nadel.engine.transform.query.NadelQueryTransformer -import graphql.nadel.engine.transform.result.NadelResultInstruction -import graphql.nadel.engine.transform.result.json.JsonNodeExtractor -import graphql.nadel.engine.transform.result.json.JsonNodes -import graphql.nadel.engine.util.queryPath -import graphql.nadel.tests.EngineTestHook -import graphql.nadel.tests.UseHook -import graphql.normalized.ExecutableNormalizedField - -@UseHook -class `transforms-can-set-array-value` : EngineTestHook { - override val customTransforms: List> - get() = listOf( - object : NadelTransform { - override suspend fun isApplicable( - executionContext: NadelExecutionContext, - executionBlueprint: NadelOverallExecutionBlueprint, - services: Map, - service: Service, - overallField: ExecutableNormalizedField, - hydrationDetails: ServiceExecutionHydrationDetails?, - ): Any? { - return overallField.name.takeIf { it == "ids" } - } - - override suspend fun transformField( - executionContext: NadelExecutionContext, - transformer: NadelQueryTransformer, - executionBlueprint: NadelOverallExecutionBlueprint, - service: Service, - field: ExecutableNormalizedField, - state: Any, - ): NadelTransformFieldResult { - return NadelTransformFieldResult.unmodified(field) - } - - override suspend fun getResultInstructions( - executionContext: NadelExecutionContext, - executionBlueprint: NadelOverallExecutionBlueprint, - service: Service, - overallField: ExecutableNormalizedField, - underlyingParentField: ExecutableNormalizedField?, - result: ServiceExecutionResult, - state: Any, - nodes: JsonNodes, - ): List { - val nodes = JsonNodeExtractor.getNodesAt( - data = result.data, - queryPath = underlyingParentField!!.queryPath + overallField.resultKey, - flatten = true, - ) - - return nodes - .mapIndexed { index, node -> - NadelResultInstruction.Set( - subjectPath = node.resultPath, - newValue = "$index-${node.value}", - ) - } - .onEach { - // Ensure we are setting an array element - require(it.subjectPath.segments.last().value is Int) - } - } - } - ) -} diff --git a/test/src/test/kotlin/graphql/nadel/tests/transforms/RemoveFieldTestTransform.kt b/test/src/test/kotlin/graphql/nadel/tests/transforms/RemoveFieldTestTransform.kt index 07648c7fe..b2f42f962 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/transforms/RemoveFieldTestTransform.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/transforms/RemoveFieldTestTransform.kt @@ -12,6 +12,7 @@ import graphql.nadel.engine.transform.NadelTransformFieldResult import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.transform.query.NadelQueryTransformer import graphql.nadel.engine.transform.result.NadelResultInstruction +import graphql.nadel.engine.transform.result.NadelResultKey import graphql.nadel.engine.transform.result.json.JsonNodeExtractor import graphql.nadel.engine.transform.result.json.JsonNodes import graphql.nadel.engine.util.queryPath @@ -83,9 +84,9 @@ class RemoveFieldTestTransform : NadelTransform { ) return parentNodes.map { parentNode -> - val destinationPath = parentNode.resultPath + overallField.resultKey NadelResultInstruction.Set( - subjectPath = destinationPath, + subject = parentNode, + key = NadelResultKey(overallField.resultKey), newValue = null, ) } + NadelResultInstruction.AddError(state) diff --git a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodePath.kt b/test/src/test/kotlin/graphql/nadel/tests/yaml/JsonNodePath.kt similarity index 96% rename from lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodePath.kt rename to test/src/test/kotlin/graphql/nadel/tests/yaml/JsonNodePath.kt index fe433b475..aa05ba9e5 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodePath.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/yaml/JsonNodePath.kt @@ -1,4 +1,4 @@ -package graphql.nadel.engine.transform.result.json +package graphql.nadel.tests.yaml typealias AnyJsonNodePathSegment = JsonNodePathSegment<*> diff --git a/test/src/test/kotlin/graphql/nadel/tests/yaml/LowLevelYaml.kt b/test/src/test/kotlin/graphql/nadel/tests/yaml/LowLevelYaml.kt index df3151b55..21343650a 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/yaml/LowLevelYaml.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/yaml/LowLevelYaml.kt @@ -1,7 +1,5 @@ package graphql.nadel.tests.yaml -import graphql.nadel.engine.transform.result.json.JsonNodePath -import graphql.nadel.engine.transform.result.json.JsonNodePathSegment import org.yaml.snakeyaml.DumperOptions import org.yaml.snakeyaml.comments.CommentLine import org.yaml.snakeyaml.composer.Composer diff --git a/test/src/test/resources/fixtures/basic/transforms-can-copy-array-value.yml b/test/src/test/resources/fixtures/basic/transforms-can-copy-array-value.yml deleted file mode 100644 index 95667e3ff..000000000 --- a/test/src/test/resources/fixtures/basic/transforms-can-copy-array-value.yml +++ /dev/null @@ -1,63 +0,0 @@ -name: "transforms can copy array value" -enabled: true -overallSchema: - service: | - type Query { - foo: Foo - } - type Foo { - ids: [ID] - } -underlyingSchema: - service: | - type Query { - foo: Foo - } - type Foo { - ids: [ID] - } -query: | - query { - foo { - ids - } - } -variables: { } -serviceCalls: - - serviceName: "service" - request: - query: | - query { - foo { - ids - } - } - variables: { } - # language=JSON - response: |- - { - "data": { - "foo": { - "ids": [ - "FOO-ONE", - "FOO-TWO", - "FOO-THREE" - ] - } - }, - "extensions": {} - } -# language=JSON -response: |- - { - "data": { - "foo": { - "ids": [ - "FOO-THREE", - "FOO-ONE", - "FOO-TWO" - ] - } - }, - "extensions": {} - } diff --git a/test/src/test/resources/fixtures/basic/transforms-can-set-array-value.yml b/test/src/test/resources/fixtures/basic/transforms-can-set-array-value.yml deleted file mode 100644 index 4d69dcfcc..000000000 --- a/test/src/test/resources/fixtures/basic/transforms-can-set-array-value.yml +++ /dev/null @@ -1,63 +0,0 @@ -name: "transforms can set array value" -enabled: true -overallSchema: - service: | - type Query { - foo: Foo - } - type Foo { - ids: [ID] - } -underlyingSchema: - service: | - type Query { - foo: Foo - } - type Foo { - ids: [ID] - } -query: | - query { - foo { - ids - } - } -variables: { } -serviceCalls: - - serviceName: "service" - request: - query: | - query { - foo { - ids - } - } - variables: { } - # language=JSON - response: |- - { - "data": { - "foo": { - "ids": [ - "FOO-ONE", - "FOO-TWO", - "FOO-THREE" - ] - } - }, - "extensions": {} - } -# language=JSON -response: |- - { - "data": { - "foo": { - "ids": [ - "0-FOO-ONE", - "1-FOO-TWO", - "2-FOO-THREE" - ] - } - }, - "extensions": {} - }