From 1cf8c7e0cc9c8fef824c305fef67589ed128a0f3 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Tue, 14 May 2024 11:57:51 +1000 Subject: [PATCH 01/22] Implement result path tracking --- .../graphql/nadel/NadelResponseTracker.kt | 160 ++++++++++++++++++ .../main/java/graphql/nadel/NextgenEngine.kt | 5 + .../nadel/engine/NadelExecutionContext.kt | 2 + .../hydration/NadelHydrationTransform.kt | 58 +++---- .../engine/transform/result/json/JsonNodes.kt | 3 + .../instrumentation/NadelInstrumentation.kt | 7 + .../defer/HydrationDeferIsDisabled.kt | 4 +- ...rIsDisabledForRelatedIssuesTestSnapshot.kt | 25 ++- ...RelatedIssuesForParentIssueTestSnapshot.kt | 29 +++- .../HydrationDeferIsDisabledTestSnapshot.kt | 58 +++++-- 10 files changed, 296 insertions(+), 55 deletions(-) create mode 100644 lib/src/main/java/graphql/nadel/NadelResponseTracker.kt diff --git a/lib/src/main/java/graphql/nadel/NadelResponseTracker.kt b/lib/src/main/java/graphql/nadel/NadelResponseTracker.kt new file mode 100644 index 000000000..2209ffc31 --- /dev/null +++ b/lib/src/main/java/graphql/nadel/NadelResponseTracker.kt @@ -0,0 +1,160 @@ +package graphql.nadel + +import graphql.ExecutionResult +import graphql.incremental.DelayedIncrementalPartialResult +import graphql.nadel.engine.transform.query.NadelQueryPath +import graphql.nadel.engine.transform.result.json.JsonNode +import graphql.nadel.engine.util.AnyList +import graphql.nadel.engine.util.AnyMap +import kotlinx.coroutines.CompletableDeferred + +internal sealed interface NadelResultPathSegment { + @JvmInline + value class Object(val key: String) : NadelResultPathSegment + + @JvmInline + value class Array(val index: Int) : NadelResultPathSegment +} + +/** + * todo: as delayed responses go out we need to track what objects went out + */ +/** + * note: a Flow will mean that we can GC the original result sooner + */ +class ResultListener { + fun track(delayed: DelayedIncrementalPartialResult) { + // todo: track objects in here + } +} + +/** + * todo: this needs to track multiple responses + */ +internal class NadelResponseTracker { + private val result = CompletableDeferred() + + // todo: maybe store some kind of Map to free up memory earlier? + + suspend fun getResponsePath( + queryPath: NadelQueryPath, + node: JsonNode, + ): List? { + val result = result.await() + val data = result.getData() + + val queryPathSegments = queryPath.segments + val currentResultPath = mutableListOf() + val currentQueryPathSegments = mutableListOf() + + /** + * { + * "data": { + * users: [ + * { + * friends: [] + * } + * ] + * } + * } + */ + + // todo: how do we know if we need to pop? + val queue = mutableListOf(data) + + // users[0].friend.user[1] + // [a, b, c] + // b + // [a] <-- if we hit this then we should decrement the index + + fun printState(): String? { + return """ + Current query path: ${ + currentQueryPathSegments.joinToString( + separator = ",", + prefix = "[", + postfix = "]" + ) + } + Current result path: ${currentResultPath.joinToString(separator = ",", prefix = "[", postfix = "]")} + """.replaceIndent(' '.toString().repeat(n = 4)) + } + + println( + "Trying to find $node from ${ + queryPath.segments.joinToString( + separator = ",", + prefix = "[", + postfix = "]" + ) + }" + ) + while (queue.isNotEmpty()) { + val element = queue.removeLast() + if (element === node.value) { + println("Got em $currentResultPath") + return currentResultPath + } + + when (element) { + is AnyList -> { + // todo: expand the queue here + if (element.isNotEmpty()) { + queue.addAll(element) + currentResultPath.add(NadelResultPathSegment.Array(element.lastIndex)) // DFS so we traverse last element first + continue + } + } + is AnyMap -> { + if (currentQueryPathSegments.size < queryPathSegments.size) { + val nextQueryPathSegment = queryPathSegments[currentQueryPathSegments.size] + println("Entering $nextQueryPathSegment\n${printState()}") + val nextElement = element[nextQueryPathSegment] + + queue.add(nextElement) + currentResultPath.add(NadelResultPathSegment.Object(nextQueryPathSegment)) + currentQueryPathSegments.add(nextQueryPathSegment) + + if (nextElement === node.value) { + println("Got em on next $currentResultPath") + return currentResultPath + } + } + } + } + + println("Loop end") + + // Correct the result path + var last = currentResultPath.lastOrNull() + while (last is NadelResultPathSegment.Array) { + if (last.index == 0) { + println("We're popping up\n${printState()}") + currentResultPath.removeLast() + currentQueryPathSegments.removeLast() // todo: is this always the case? what about nested lists + } else { + println("Moving to next array element ${last.index - 1}") + // We're moving to the next element + currentResultPath[currentResultPath.lastIndex] = + NadelResultPathSegment.Array(last.index - 1) + // Nothing further to fix, stop + break + } + + last = currentResultPath.lastOrNull() + } + } + + println("Didn't get em") + return null + } + + fun complete(value: ExecutionResult) { + result.complete(value) + } + + fun complete(value: DelayedIncrementalPartialResult) { + // result.complete(value) + // todo: track here + } +} diff --git a/lib/src/main/java/graphql/nadel/NextgenEngine.kt b/lib/src/main/java/graphql/nadel/NextgenEngine.kt index 1c2914e68..9d5783e00 100644 --- a/lib/src/main/java/graphql/nadel/NextgenEngine.kt +++ b/lib/src/main/java/graphql/nadel/NextgenEngine.kt @@ -160,6 +160,7 @@ internal class NextgenEngine( } val incrementalResultSupport = NadelIncrementalResultSupport() + val resultTracker = NadelResponseTracker() val executionContext = NadelExecutionContext( executionInput, query, @@ -168,6 +169,7 @@ internal class NextgenEngine( instrumentationState, timer, incrementalResultSupport, + resultTracker, ) val beginExecuteContext = instrumentation.beginExecute( @@ -214,6 +216,9 @@ internal class NextgenEngine( beginExecuteContext?.onCompleted(result, null) incrementalResultSupport.onInitialResultComplete() + // todo: maybe pass in the incremental version that's built below into here + resultTracker.complete(result) + return if (incrementalResultSupport.hasDeferredResults()) { IncrementalExecutionResultImpl.Builder() .from(result) diff --git a/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt b/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt index 636170727..24b62510c 100644 --- a/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt +++ b/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt @@ -4,6 +4,7 @@ import graphql.ExecutionInput import graphql.GraphQLContext import graphql.execution.instrumentation.InstrumentationState import graphql.nadel.NadelExecutionHints +import graphql.nadel.NadelResponseTracker import graphql.nadel.Service import graphql.nadel.ServiceExecutionHydrationDetails import graphql.nadel.engine.instrumentation.NadelInstrumentationTimer @@ -21,6 +22,7 @@ data class NadelExecutionContext internal constructor( val instrumentationState: InstrumentationState?, internal val timer: NadelInstrumentationTimer, internal val incrementalResultSupport: NadelIncrementalResultSupport, + internal val resultTracker: NadelResponseTracker, internal val hydrationDetails: ServiceExecutionHydrationDetails? = null, ) { private val serviceContexts = ConcurrentHashMap>() 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 6a51ba594..5f85a6744 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 @@ -227,36 +227,35 @@ internal class NadelHydrationTransform( .asSequence() .flatten() - val results = instructionSequence - .filterIsInstance() - .emptyOrSingle() - DelayedIncrementalPartialResultImpl.Builder() .incrementalItems( - listOf( - DeferPayload.Builder() - .label(label) - .data( - mapOf( - overallField.resultKey to results?.newValue?.value, - ), - ) - .path( - overallField.parent?.listOfResultKeys?.let { - @Suppress("USELESS_CAST") // It's not useless because Java (yay) - it as List - } ?: emptyList() - ) - .errors( - instructionSequence - .filterIsInstance() - .map { - it.error - } - .toList(), - ) - .build(), - ), + instructionSequence + .filterIsInstance() + .toList() // There can be multiple if the hydration is right under a list i.e. we are hydrating [{assigneeId: 1}, {assigneeId: 2}] + .map { result -> + DeferPayload.newDeferredItem() + .label(label) + .data( + mapOf( + overallField.resultKey to result.newValue?.value, + ), + ) + .path( + executionContext.resultTracker.getResponsePath( + overallField.queryPath.dropLast(1), + result.subject, + )!! + ) + .errors( + instructionSequence + .filterIsInstance() + .map { + it.error + } + .toList(), + ) + .build() + } ) .build() } @@ -411,7 +410,8 @@ internal class NadelHydrationTransform( return if (executionContext.hints.deferSupport() && overallField.deferredExecutions.isNotEmpty()) { // We currently don't support defer if the hydration is inside a List - return !areAnyParentFieldsOutputtingLists(overallField, executionBlueprint) + // return !areAnyParentFieldsOutputtingLists(overallField, executionBlueprint) + return true } else { false } 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 fd7d85c14..272fd2403 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 @@ -22,6 +22,9 @@ class JsonNodes( return getNodesAt(rootNode, queryPath, flatten) } + /** + * todo: can this be a sequence? + */ /** * Extracts the nodes at the given query selection path. */ diff --git a/lib/src/main/java/graphql/nadel/instrumentation/NadelInstrumentation.kt b/lib/src/main/java/graphql/nadel/instrumentation/NadelInstrumentation.kt index be001ac8c..5aa058ef5 100644 --- a/lib/src/main/java/graphql/nadel/instrumentation/NadelInstrumentation.kt +++ b/lib/src/main/java/graphql/nadel/instrumentation/NadelInstrumentation.kt @@ -5,6 +5,7 @@ import graphql.execution.instrumentation.InstrumentationContext import graphql.execution.instrumentation.InstrumentationState import graphql.execution.instrumentation.SimpleInstrumentationContext.noOp import graphql.language.Document +import graphql.nadel.engine.NadelExecutionContext import graphql.nadel.instrumentation.parameters.NadelInstrumentationCreateStateParameters import graphql.nadel.instrumentation.parameters.NadelInstrumentationExecuteOperationParameters import graphql.nadel.instrumentation.parameters.NadelInstrumentationOnErrorParameters @@ -111,4 +112,10 @@ interface NadelInstrumentation { */ fun onError(parameters: NadelInstrumentationOnErrorParameters) { } + + /** + * todo: create a root "internal" implementation so we can effectively have internal interface methods + */ + fun onExecutionContext(context: NadelExecutionContext) { + } } diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabled.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabled.kt index 28eeb316f..d13e01af6 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabled.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabled.kt @@ -24,7 +24,7 @@ class HydrationDeferIsDisabledTest : HydrationDeferIsDisabled( """.trimIndent(), ) { override fun assert(result: ExecutionResult, incrementalResults: List?) { - assertTrue(result !is IncrementalExecutionResult) + // assertTrue(result !is IncrementalExecutionResult) } } @@ -76,7 +76,7 @@ class HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTest : Hydratio """.trimIndent(), ) { override fun assert(result: ExecutionResult, incrementalResults: List?) { - assertTrue(result !is IncrementalExecutionResult) + // assertTrue(result !is IncrementalExecutionResult) } } diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledForRelatedIssuesTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledForRelatedIssuesTestSnapshot.kt index 530bdd4d7..a2400402b 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledForRelatedIssuesTestSnapshot.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledForRelatedIssuesTestSnapshot.kt @@ -131,11 +131,7 @@ public class HydrationDeferIsDisabledForRelatedIssuesTestSnapshot : TestSnapshot | "issueByKey": { | "key": "GQLGW-2", | "related": [ - | { - | "assignee": { - | "name": "Franklin" - | } - | } + | {} | ] | } | }, @@ -149,6 +145,25 @@ public class HydrationDeferIsDisabledForRelatedIssuesTestSnapshot : TestSnapshot | "incremental": [ | { | "path": [ + | "issueByKey", + | "related", + | 0 + | ], + | "data": { + | "assignee": { + | "name": "Franklin" + | } + | } + | } + | ] + | } + """.trimMargin(), + """ + | { + | "hasNext": true, + | "incremental": [ + | { + | "path": [ | "issueByKey" | ], | "data": { diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot.kt index bec6840db..b28a0cd99 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot.kt @@ -119,18 +119,35 @@ public class HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnap | "parent": null | }, | { - | "parent": { - | "assignee": { - | "name": "Franklin" - | } - | } + | "parent": {} | } | ] | } - | } + | }, + | "hasNext": true | } """.trimMargin(), delayedResults = listOfJsonStrings( + """ + | { + | "hasNext": false, + | "incremental": [ + | { + | "path": [ + | "issueByKey", + | "related", + | 1, + | "parent" + | ], + | "data": { + | "assignee": { + | "name": "Franklin" + | } + | } + | } + | ] + | } + """.trimMargin(), ), ) } diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledTestSnapshot.kt index f8ba4a777..3ecc59096 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledTestSnapshot.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledTestSnapshot.kt @@ -161,28 +161,60 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { | "data": { | "issues": [ | { - | "key": "GQLGW-1", - | "assignee": { - | "name": "Franklin" - | } + | "key": "GQLGW-1" | }, | { - | "key": "GQLGW-2", - | "assignee": { - | "name": "Tom" - | } + | "key": "GQLGW-2" | }, | { - | "key": "GQLGW-3", - | "assignee": { - | "name": "Franklin" - | } + | "key": "GQLGW-3" | } | ] - | } + | }, + | "hasNext": true | } """.trimMargin(), delayedResults = listOfJsonStrings( + """ + | { + | "hasNext": false, + | "incremental": [ + | { + | "path": [ + | "issues", + | 0 + | ], + | "data": { + | "assignee": { + | "name": "Franklin" + | } + | } + | }, + | { + | "path": [ + | "issues", + | 1 + | ], + | "data": { + | "assignee": { + | "name": "Tom" + | } + | } + | }, + | { + | "path": [ + | "issues", + | 2 + | ], + | "data": { + | "assignee": { + | "name": "Franklin" + | } + | } + | } + | ] + | } + """.trimMargin(), ), ) } From 7db2b22036532807b306c9767db6873f65f5ee37 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Sun, 5 May 2024 20:23:01 +1200 Subject: [PATCH 02/22] Execute query without defer directive and assert result is same --- .../{IncrementalResultJoiner.kt => IncrementalResultCombiner.kt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/src/test/kotlin/graphql/nadel/tests/next/{IncrementalResultJoiner.kt => IncrementalResultCombiner.kt} (100%) diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/IncrementalResultJoiner.kt b/test/src/test/kotlin/graphql/nadel/tests/next/IncrementalResultCombiner.kt similarity index 100% rename from test/src/test/kotlin/graphql/nadel/tests/next/IncrementalResultJoiner.kt rename to test/src/test/kotlin/graphql/nadel/tests/next/IncrementalResultCombiner.kt From a6c8947e9122e5a51b2f95bd12c666606df76a8c Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Sun, 5 May 2024 22:27:40 +1200 Subject: [PATCH 03/22] Fixes and 2d array test --- .run/CaptureTestData.run.xml | 10 + ...sponseTracker.kt => NadelResultTracker.kt} | 67 +---- .../main/java/graphql/nadel/NextgenEngine.kt | 2 +- .../nadel/engine/NadelExecutionContext.kt | 4 +- .../hydration/NadelHydrationTransform.kt | 137 ++++----- .../transform/hydration/NadelHydrationUtil.kt | 16 +- .../hydration/defer/HydrationDeferInList.kt | 227 ++++++++++++++ ...HydrationDeferInListNestedTestSnapshot.kt} | 9 +- ...kt => HydrationDeferInListTestSnapshot.kt} | 8 +- ...drationDeferInListTopLevelTestSnapshot.kt} | 61 +++- ...ionDeferInListTwoDimensionsTestSnapshot.kt | 279 ++++++++++++++++++ .../defer/HydrationDeferIsDisabled.kt | 72 +---- .../hydration/defer/HyrationDeferErrors.kt | 4 + 13 files changed, 680 insertions(+), 216 deletions(-) create mode 100644 .run/CaptureTestData.run.xml rename lib/src/main/java/graphql/nadel/{NadelResponseTracker.kt => NadelResultTracker.kt} (56%) create mode 100644 test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInList.kt rename test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/{HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot.kt => HydrationDeferInListNestedTestSnapshot.kt} (94%) rename test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/{HydrationDeferIsDisabledForRelatedIssuesTestSnapshot.kt => HydrationDeferInListTestSnapshot.kt} (96%) rename test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/{HydrationDeferIsDisabledTestSnapshot.kt => HydrationDeferInListTopLevelTestSnapshot.kt} (77%) create mode 100644 test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTwoDimensionsTestSnapshot.kt create mode 100644 test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HyrationDeferErrors.kt diff --git a/.run/CaptureTestData.run.xml b/.run/CaptureTestData.run.xml new file mode 100644 index 000000000..fbf78b46b --- /dev/null +++ b/.run/CaptureTestData.run.xml @@ -0,0 +1,10 @@ + + + + \ No newline at end of file diff --git a/lib/src/main/java/graphql/nadel/NadelResponseTracker.kt b/lib/src/main/java/graphql/nadel/NadelResultTracker.kt similarity index 56% rename from lib/src/main/java/graphql/nadel/NadelResponseTracker.kt rename to lib/src/main/java/graphql/nadel/NadelResultTracker.kt index 2209ffc31..285800d95 100644 --- a/lib/src/main/java/graphql/nadel/NadelResponseTracker.kt +++ b/lib/src/main/java/graphql/nadel/NadelResultTracker.kt @@ -31,12 +31,10 @@ class ResultListener { /** * todo: this needs to track multiple responses */ -internal class NadelResponseTracker { +internal class NadelResultTracker { private val result = CompletableDeferred() - // todo: maybe store some kind of Map to free up memory earlier? - - suspend fun getResponsePath( + suspend fun getResultPath( queryPath: NadelQueryPath, node: JsonNode, ): List? { @@ -44,7 +42,7 @@ internal class NadelResponseTracker { val data = result.getData() val queryPathSegments = queryPath.segments - val currentResultPath = mutableListOf() + val currentResultPathSegments = mutableListOf() val currentQueryPathSegments = mutableListOf() /** @@ -59,93 +57,58 @@ internal class NadelResponseTracker { * } */ - // todo: how do we know if we need to pop? val queue = mutableListOf(data) - // users[0].friend.user[1] - // [a, b, c] - // b - // [a] <-- if we hit this then we should decrement the index - - fun printState(): String? { - return """ - Current query path: ${ - currentQueryPathSegments.joinToString( - separator = ",", - prefix = "[", - postfix = "]" - ) - } - Current result path: ${currentResultPath.joinToString(separator = ",", prefix = "[", postfix = "]")} - """.replaceIndent(' '.toString().repeat(n = 4)) - } - - println( - "Trying to find $node from ${ - queryPath.segments.joinToString( - separator = ",", - prefix = "[", - postfix = "]" - ) - }" - ) while (queue.isNotEmpty()) { val element = queue.removeLast() if (element === node.value) { - println("Got em $currentResultPath") - return currentResultPath + return currentResultPathSegments } when (element) { is AnyList -> { - // todo: expand the queue here if (element.isNotEmpty()) { queue.addAll(element) - currentResultPath.add(NadelResultPathSegment.Array(element.lastIndex)) // DFS so we traverse last element first + currentResultPathSegments.add(NadelResultPathSegment.Array(element.lastIndex)) continue } } is AnyMap -> { if (currentQueryPathSegments.size < queryPathSegments.size) { val nextQueryPathSegment = queryPathSegments[currentQueryPathSegments.size] - println("Entering $nextQueryPathSegment\n${printState()}") val nextElement = element[nextQueryPathSegment] queue.add(nextElement) - currentResultPath.add(NadelResultPathSegment.Object(nextQueryPathSegment)) + currentResultPathSegments.add(NadelResultPathSegment.Object(nextQueryPathSegment)) currentQueryPathSegments.add(nextQueryPathSegment) if (nextElement === node.value) { - println("Got em on next $currentResultPath") - return currentResultPath + return currentResultPathSegments } } } } - println("Loop end") - - // Correct the result path - var last = currentResultPath.lastOrNull() + // Correct the result path segment i.e. move to the next array element + var last = currentResultPathSegments.lastOrNull() while (last is NadelResultPathSegment.Array) { if (last.index == 0) { - println("We're popping up\n${printState()}") - currentResultPath.removeLast() - currentQueryPathSegments.removeLast() // todo: is this always the case? what about nested lists + val removedResultPathSegment = currentResultPathSegments.removeLast() + if (removedResultPathSegment is NadelResultPathSegment.Object) { + currentQueryPathSegments.removeLast() + } } else { - println("Moving to next array element ${last.index - 1}") // We're moving to the next element - currentResultPath[currentResultPath.lastIndex] = + currentResultPathSegments[currentResultPathSegments.lastIndex] = NadelResultPathSegment.Array(last.index - 1) // Nothing further to fix, stop break } - last = currentResultPath.lastOrNull() + last = currentResultPathSegments.lastOrNull() } } - println("Didn't get em") return null } diff --git a/lib/src/main/java/graphql/nadel/NextgenEngine.kt b/lib/src/main/java/graphql/nadel/NextgenEngine.kt index 9d5783e00..7eb2279bb 100644 --- a/lib/src/main/java/graphql/nadel/NextgenEngine.kt +++ b/lib/src/main/java/graphql/nadel/NextgenEngine.kt @@ -160,7 +160,7 @@ internal class NextgenEngine( } val incrementalResultSupport = NadelIncrementalResultSupport() - val resultTracker = NadelResponseTracker() + val resultTracker = NadelResultTracker() val executionContext = NadelExecutionContext( executionInput, query, diff --git a/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt b/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt index 24b62510c..2cb08d3b6 100644 --- a/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt +++ b/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt @@ -4,7 +4,7 @@ import graphql.ExecutionInput import graphql.GraphQLContext import graphql.execution.instrumentation.InstrumentationState import graphql.nadel.NadelExecutionHints -import graphql.nadel.NadelResponseTracker +import graphql.nadel.NadelResultTracker import graphql.nadel.Service import graphql.nadel.ServiceExecutionHydrationDetails import graphql.nadel.engine.instrumentation.NadelInstrumentationTimer @@ -22,7 +22,7 @@ data class NadelExecutionContext internal constructor( val instrumentationState: InstrumentationState?, internal val timer: NadelInstrumentationTimer, internal val incrementalResultSupport: NadelIncrementalResultSupport, - internal val resultTracker: NadelResponseTracker, + internal val resultTracker: NadelResultTracker, internal val hydrationDetails: ServiceExecutionHydrationDetails? = null, ) { private val serviceContexts = ConcurrentHashMap>() 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 5f85a6744..ad8bd08be 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 @@ -19,19 +19,19 @@ import graphql.nadel.engine.transform.NadelTransformUtil.makeTypeNameField import graphql.nadel.engine.transform.artificial.NadelAliasHelper import graphql.nadel.engine.transform.getInstructionsForNode import graphql.nadel.engine.transform.hydration.NadelHydrationTransform.State -import graphql.nadel.engine.transform.hydration.NadelHydrationUtil.getInstructionsToAddErrors 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.JsonMap import graphql.nadel.engine.util.emptyOrSingle import graphql.nadel.engine.util.getFieldDefinitionSequence import graphql.nadel.engine.util.isList import graphql.nadel.engine.util.queryPath import graphql.nadel.engine.util.toBuilder +import graphql.nadel.engine.util.toGraphQLError import graphql.nadel.engine.util.unwrapNonNull import graphql.nadel.hooks.NadelExecutionHooks import graphql.normalized.ExecutableNormalizedField @@ -175,7 +175,7 @@ internal class NadelHydrationTransform( ): List { return coroutineScope { parentNodes - .map { + .mapNotNull { prepareHydration( parentNode = it, state = state, @@ -190,7 +190,25 @@ internal class NadelHydrationTransform( } } .awaitAll() - .flatten() + .flatMap { hydration -> + val setData = sequenceOf( + NadelResultInstruction.Set( + subject = hydration.parentNode, + newValue = hydration.newValue, + field = overallField, + ), + ) + val addErrors = hydration.errors + .asSequence() + .map { error -> + toGraphQLError(error) + } + .map { + NadelResultInstruction.AddError(it) + } + + setData + addErrors + } } } @@ -203,57 +221,49 @@ internal class NadelHydrationTransform( ) { // Prepare the hydrations before we go async // We need to do this because if we run it async below, we cannot guarantee that our artificial fields have not yet been removed - val hydrations = parentNodes.map { - prepareHydration( - parentNode = it, - state = state, - executionBlueprint = executionBlueprint, - fieldToHydrate = overallField, - executionContext = executionContext, - ) - } + val preparedHydrations = parentNodes + .mapNotNull { + prepareHydration( + parentNode = it, + state = state, + executionBlueprint = executionBlueprint, + fieldToHydrate = overallField, + executionContext = executionContext, + ) + } // This isn't really right… but we start with this val label = overallField.deferredExecutions.firstNotNullOfOrNull { it.label } executionContext.incrementalResultSupport.defer { - val instructionSequence = hydrations + val hydrations = preparedHydrations .map { async { it.hydrate() } } .awaitAll() - .asSequence() - .flatten() DelayedIncrementalPartialResultImpl.Builder() .incrementalItems( - instructionSequence - .filterIsInstance() - .toList() // There can be multiple if the hydration is right under a list i.e. we are hydrating [{assigneeId: 1}, {assigneeId: 2}] - .map { result -> + hydrations + .map { hydration -> // Hydration of one parent node + val data = hydration.newValue + DeferPayload.newDeferredItem() .label(label) .data( mapOf( - overallField.resultKey to result.newValue?.value, + overallField.resultKey to data?.value, ), ) .path( - executionContext.resultTracker.getResponsePath( + executionContext.resultTracker.getResultPath( overallField.queryPath.dropLast(1), - result.subject, + hydration.parentNode, )!! ) - .errors( - instructionSequence - .filterIsInstance() - .map { - it.error - } - .toList(), - ) + .errors(hydration.errors.map(::toGraphQLError)) .build() } ) @@ -267,7 +277,7 @@ internal class NadelHydrationTransform( executionBlueprint: NadelOverallExecutionBlueprint, fieldToHydrate: ExecutableNormalizedField, // Field asking for hydration from the overall query executionContext: NadelExecutionContext, - ): NadelPreparedHydration { + ): NadelPreparedHydration? { val instructions = state.instructionsByObjectTypeNames.getInstructionsForNode( executionBlueprint = executionBlueprint, service = state.hydratedFieldService, @@ -277,19 +287,15 @@ internal class NadelHydrationTransform( // Do nothing if there is no hydration instruction associated with this result if (instructions.isEmpty()) { - return NadelPreparedHydration { - emptyList() - } + return null } val instruction = getHydrationFieldInstruction(state, instructions, executionContext.hooks, parentNode) ?: return NadelPreparedHydration { - listOf( - NadelResultInstruction.Set( - subject = parentNode, - key = NadelResultKey(state.hydratedField.resultKey), - newValue = null, - ), + HydrationResult( + parentNode = parentNode, + newValue = null, + errors = emptyList(), ) } @@ -342,33 +348,26 @@ internal class NadelHydrationTransform( ).emptyOrSingle() } - val errors = result?.let(::getInstructionsToAddErrors) ?: emptyList() - - listOf( - NadelResultInstruction.Set( - subject = parentNode, - key = NadelResultKey(fieldToHydrate.resultKey), - newValue = JsonNode(data?.value), - ), - ) + errors + HydrationResult( + parentNode = parentNode, + newValue = JsonNode(data?.value), + errors = result?.errors ?: emptyList(), + ) } is NadelHydrationStrategy.ManyToOne -> { - val data = actorQueryResults.map { result -> - JsonNodeExtractor.getNodesAt( - data = result.data, - queryPath = instruction.queryPathToActorField, - ).emptyOrSingle()?.value - } - - val addErrors = getInstructionsToAddErrors(actorQueryResults) + val data = actorQueryResults + .map { result -> + JsonNodeExtractor.getNodesAt( + data = result.data, + queryPath = instruction.queryPathToActorField, + ).emptyOrSingle()?.value + } - listOf( - NadelResultInstruction.Set( - subject = parentNode, - key = NadelResultKey(fieldToHydrate.resultKey), - newValue = JsonNode(data), - ), - ) + addErrors + HydrationResult( + parentNode = parentNode, + newValue = JsonNode(data), + errors = actorQueryResults.flatMap { it.errors }, + ) } } } @@ -466,5 +465,11 @@ internal class NadelHydrationTransform( * So we "prepare" a hydration to ensure we have the value of the artificial field before it gets removed. */ private fun interface NadelPreparedHydration { - suspend fun hydrate(): List + suspend fun hydrate(): HydrationResult } + +private data class HydrationResult( + val parentNode: JsonNode, + val newValue: JsonNode?, + val errors: List, +) diff --git a/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationUtil.kt b/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationUtil.kt index eb7cdedbe..02c5172c8 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationUtil.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationUtil.kt @@ -13,35 +13,35 @@ internal object NadelHydrationUtil { @JvmName("getInstructionsToAddErrors_2") fun getInstructionsToAddErrors( results: List, - ): List { + ): List { return results .asSequence() .map(NadelResolvedObjectBatch::result) - .flatMap(::sequenceOfInstructionsToAddErrors) + .flatMap(::getInstructionsToAddErrorsSequence) .toList() } fun getInstructionsToAddErrors( results: List, - ): List { + ): List { return results .asSequence() - .flatMap(::sequenceOfInstructionsToAddErrors) + .flatMap(::getInstructionsToAddErrorsSequence) .toList() } fun getInstructionsToAddErrors( result: ServiceExecutionResult, - ): List { - return sequenceOfInstructionsToAddErrors(result).toList() + ): List { + return getInstructionsToAddErrorsSequence(result).toList() } /** * Do not expose sequences as those */ - private fun sequenceOfInstructionsToAddErrors( + private fun getInstructionsToAddErrorsSequence( result: ServiceExecutionResult, - ): Sequence { + ): Sequence { return result.errors .asSequence() .map(::toGraphQLError) diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInList.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInList.kt new file mode 100644 index 000000000..45610fa6d --- /dev/null +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInList.kt @@ -0,0 +1,227 @@ +package graphql.nadel.tests.next.fixtures.hydration.defer + +import graphql.nadel.NadelExecutionHints +import graphql.nadel.engine.util.strictAssociateBy +import graphql.nadel.tests.next.NadelIntegrationTest +import org.intellij.lang.annotations.Language + +class HydrationDeferInListTest : HydrationDeferInList( + query = """ + query { + issueByKey(key: "GQLGW-2") { # Not a list + key + ... @defer { + assignee { + name + } + } + related { # Is a list + ... @defer { + assignee { + name + } + } + } + } + } + """.trimIndent(), +) + +class HydrationDeferInListTwoDimensionsTest : HydrationDeferInList( + query = """ + query { + issueGroups { + key + ... @defer { + assignee { + name + } + } + } + } + """.trimIndent(), +) + +class HydrationDeferInListTopLevelTest : HydrationDeferInList( + query = """ + query { + issues { # List + key + ... @defer { + assignee { + name + } + } + } + } + """.trimIndent(), +) + +class HydrationDeferInListNestedTest : HydrationDeferInList( + query = """ + query { + issueByKey(key: "GQLGW-3") { # Not a list + key + related { # Is a list + parent { # Not a list + ... @defer { + assignee { + name + } + } + } + } + } + } + """.trimIndent(), +) + +abstract class HydrationDeferInList( + @Language("GraphQL") + query: String, +) : NadelIntegrationTest( + query = query, + services = listOf( + Service( + name = "issues", + overallSchema = """ + directive @defer(if: Boolean, label: String) on FRAGMENT_SPREAD | INLINE_FRAGMENT + type Query { + issues: [Issue!] + issueGroups: [[Issue]] + issueByKey(key: String!): Issue + } + type Issue { + key: String! + assigneeId: ID! + self: Issue + @hydrated( + service: "issues" + field: "issueByKey" + arguments: [{name: "key", value: "$source.key"}] + ) + assignee: User + @hydrated( + service: "users" + field: "userById" + arguments: [{name: "id", value: "$source.assigneeId"}] + ) + related: [Issue!] + parent: Issue + } + """.trimIndent(), + runtimeWiring = { wiring -> + data class Issue( + val key: String, + val assigneeId: String, + val parentKey: String? = null, + val relatedKeys: List = emptyList(), + ) + + val issues = listOf( + Issue( + key = "GQLGW-1", + assigneeId = "ari:cloud:identity::user/1", + ), + Issue( + key = "GQLGW-2", + assigneeId = "ari:cloud:identity::user/2", + parentKey = "GQLGW-1", + relatedKeys = listOf("GQLGW-1"), + ), + Issue( + key = "GQLGW-3", + assigneeId = "ari:cloud:identity::user/1", + parentKey = "GQLGW-1", + relatedKeys = listOf("GQLGW-1", "GQLGW-2"), + ), + Issue( + key = "GQLGW-4", + assigneeId = "ari:cloud:identity::user/3", + parentKey = "GQLGW-1", + relatedKeys = listOf("GQLGW-1", "GQLGW-2", "GQLGW-3"), + ), + ) + val issuesByKey = issues.strictAssociateBy { it.key } + + wiring + .type("Query") { type -> + type + .dataFetcher("issueByKey") { env -> + issuesByKey[env.getArgument("key")] + } + .dataFetcher("issues") { env -> + issues + } + .dataFetcher("issueGroups") { + issues + .groupBy { + it.key.substringAfter("-").toInt() % 2 == 0 + } + .values + } + } + .type("Issue") { type -> + type + .dataFetcher("related") { env -> + env.getSource() + .relatedKeys + .map { + issuesByKey[it]!! + } + } + .dataFetcher("parent") { env -> + issuesByKey[env.getSource().parentKey] + } + } + }, + ), + Service( + name = "users", + overallSchema = """ + type Query { + userById(id: ID!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + runtimeWiring = { wiring -> + data class User( + val id: String, + val name: String, + ) + + val users = listOf( + User( + id = "ari:cloud:identity::user/1", + name = "Frank", + ), + User( + id = "ari:cloud:identity::user/2", + name = "Tom", + ), + User( + id = "ari:cloud:identity::user/3", + name = "Lin", + ), + ) + val usersById = users.strictAssociateBy { it.id } + + wiring + .type("Query") { type -> + type + .dataFetcher("userById") { env -> + usersById[env.getArgument("id")] + } + } + }, + ), + ), +) { + override fun makeExecutionHints(): NadelExecutionHints.Builder { + return super.makeExecutionHints() + .deferSupport { true } + } +} diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListNestedTestSnapshot.kt similarity index 94% rename from test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot.kt rename to test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListNestedTestSnapshot.kt index b28a0cd99..59ad5420b 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListNestedTestSnapshot.kt @@ -19,8 +19,7 @@ private suspend fun main() { * Refer to [graphql.nadel.tests.next.UpdateTestSnapshots */ @Suppress("unused") -public class HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnapshot : - TestSnapshot() { +public class HydrationDeferInListNestedTestSnapshot : TestSnapshot() { override val calls: List = listOf( ExpectedServiceCall( service = "issues", @@ -75,7 +74,7 @@ public class HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnap | { | "data": { | "userById": { - | "name": "Franklin" + | "name": "Frank" | } | } | } @@ -98,7 +97,7 @@ public class HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnap * { * "parent": { * "assignee": { - * "name": "Franklin" + * "name": "Frank" * } * } * } @@ -141,7 +140,7 @@ public class HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTestSnap | ], | "data": { | "assignee": { - | "name": "Franklin" + | "name": "Frank" | } | } | } diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledForRelatedIssuesTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTestSnapshot.kt similarity index 96% rename from test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledForRelatedIssuesTestSnapshot.kt rename to test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTestSnapshot.kt index a2400402b..5ee873069 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledForRelatedIssuesTestSnapshot.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTestSnapshot.kt @@ -19,7 +19,7 @@ private suspend fun main() { * Refer to [graphql.nadel.tests.next.UpdateTestSnapshots */ @Suppress("unused") -public class HydrationDeferIsDisabledForRelatedIssuesTestSnapshot : TestSnapshot() { +public class HydrationDeferInListTestSnapshot : TestSnapshot() { override val calls: List = listOf( ExpectedServiceCall( service = "issues", @@ -71,7 +71,7 @@ public class HydrationDeferIsDisabledForRelatedIssuesTestSnapshot : TestSnapshot | { | "data": { | "userById": { - | "name": "Franklin" + | "name": "Frank" | } | } | } @@ -112,7 +112,7 @@ public class HydrationDeferIsDisabledForRelatedIssuesTestSnapshot : TestSnapshot * "related": [ * { * "assignee": { - * "name": "Franklin" + * "name": "Frank" * } * } * ], @@ -151,7 +151,7 @@ public class HydrationDeferIsDisabledForRelatedIssuesTestSnapshot : TestSnapshot | ], | "data": { | "assignee": { - | "name": "Franklin" + | "name": "Frank" | } | } | } diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTopLevelTestSnapshot.kt similarity index 77% rename from test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledTestSnapshot.kt rename to test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTopLevelTestSnapshot.kt index 3ecc59096..3c8be7555 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabledTestSnapshot.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTopLevelTestSnapshot.kt @@ -19,7 +19,7 @@ private suspend fun main() { * Refer to [graphql.nadel.tests.next.UpdateTestSnapshots */ @Suppress("unused") -public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { +public class HydrationDeferInListTopLevelTestSnapshot : TestSnapshot() { override val calls: List = listOf( ExpectedServiceCall( service = "issues", @@ -51,6 +51,11 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { | "key": "GQLGW-3", | "hydration__assignee__assigneeId": "ari:cloud:identity::user/1", | "__typename__hydration__assignee": "Issue" + | }, + | { + | "key": "GQLGW-4", + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/3", + | "__typename__hydration__assignee": "Issue" | } | ] | } @@ -73,7 +78,7 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { | { | "data": { | "userById": { - | "name": "Franklin" + | "name": "Frank" | } | } | } @@ -95,7 +100,7 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { | { | "data": { | "userById": { - | "name": "Franklin" + | "name": "Frank" | } | } | } @@ -125,6 +130,28 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { delayedResults = listOfJsonStrings( ), ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/3") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "userById": { + | "name": "Lin" + | } + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), ) /** @@ -135,7 +162,7 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { * { * "key": "GQLGW-1", * "assignee": { - * "name": "Franklin" + * "name": "Frank" * } * }, * { @@ -147,7 +174,13 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { * { * "key": "GQLGW-3", * "assignee": { - * "name": "Franklin" + * "name": "Frank" + * } + * }, + * { + * "key": "GQLGW-4", + * "assignee": { + * "name": "Lin" * } * } * ] @@ -168,6 +201,9 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { | }, | { | "key": "GQLGW-3" + | }, + | { + | "key": "GQLGW-4" | } | ] | }, @@ -186,7 +222,7 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { | ], | "data": { | "assignee": { - | "name": "Franklin" + | "name": "Frank" | } | } | }, @@ -208,7 +244,18 @@ public class HydrationDeferIsDisabledTestSnapshot : TestSnapshot() { | ], | "data": { | "assignee": { - | "name": "Franklin" + | "name": "Frank" + | } + | } + | }, + | { + | "path": [ + | "issues", + | 3 + | ], + | "data": { + | "assignee": { + | "name": "Lin" | } | } | } diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTwoDimensionsTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTwoDimensionsTestSnapshot.kt new file mode 100644 index 000000000..ef21a2fbb --- /dev/null +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferInListTwoDimensionsTestSnapshot.kt @@ -0,0 +1,279 @@ +// @formatter:off +package graphql.nadel.tests.next.fixtures.hydration.defer + +import graphql.nadel.tests.next.ExpectedNadelResult +import graphql.nadel.tests.next.ExpectedServiceCall +import graphql.nadel.tests.next.TestSnapshot +import graphql.nadel.tests.next.listOfJsonStrings +import kotlin.Suppress +import kotlin.collections.List +import kotlin.collections.listOf + +/** + * This class is generated. Do NOT modify. + * + * Refer to [graphql.nadel.tests.next.UpdateTestSnapshots + */ +@Suppress("unused") +public class HydrationDeferInListTwoDimensionsTestSnapshot : TestSnapshot() { + override val calls: List = listOf( + ExpectedServiceCall( + service = "issues", + query = """ + | { + | issueGroups { + | key + | hydration__assignee__assigneeId: assigneeId + | __typename__hydration__assignee: __typename + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "issueGroups": [ + | [ + | { + | "key": "GQLGW-1", + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/1", + | "__typename__hydration__assignee": "Issue" + | }, + | { + | "key": "GQLGW-3", + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/1", + | "__typename__hydration__assignee": "Issue" + | } + | ], + | [ + | { + | "key": "GQLGW-2", + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/2", + | "__typename__hydration__assignee": "Issue" + | }, + | { + | "key": "GQLGW-4", + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/3", + | "__typename__hydration__assignee": "Issue" + | } + | ] + | ] + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/1") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "userById": { + | "name": "Frank" + | } + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/1") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "userById": { + | "name": "Frank" + | } + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/2") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "userById": { + | "name": "Tom" + | } + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/3") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "userById": { + | "name": "Lin" + | } + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ) + + /** + * ```json + * { + * "data": { + * "issueGroups": [ + * [ + * { + * "key": "GQLGW-1", + * "assignee": { + * "name": "Frank" + * } + * }, + * { + * "key": "GQLGW-3", + * "assignee": { + * "name": "Frank" + * } + * } + * ], + * [ + * { + * "key": "GQLGW-2", + * "assignee": { + * "name": "Tom" + * } + * }, + * { + * "key": "GQLGW-4", + * "assignee": { + * "name": "Lin" + * } + * } + * ] + * ] + * } + * } + * ``` + */ + override val result: ExpectedNadelResult = ExpectedNadelResult( + result = """ + | { + | "data": { + | "issueGroups": [ + | [ + | { + | "key": "GQLGW-1" + | }, + | { + | "key": "GQLGW-3" + | } + | ], + | [ + | { + | "key": "GQLGW-2" + | }, + | { + | "key": "GQLGW-4" + | } + | ] + | ] + | }, + | "hasNext": true + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + """ + | { + | "hasNext": false, + | "incremental": [ + | { + | "path": [ + | "issueGroups", + | 0, + | 0 + | ], + | "data": { + | "assignee": { + | "name": "Frank" + | } + | } + | }, + | { + | "path": [ + | "issueGroups", + | 0, + | 1 + | ], + | "data": { + | "assignee": { + | "name": "Frank" + | } + | } + | }, + | { + | "path": [ + | "issueGroups", + | 1, + | 0 + | ], + | "data": { + | "assignee": { + | "name": "Tom" + | } + | } + | }, + | { + | "path": [ + | "issueGroups", + | 1, + | 1 + | ], + | "data": { + | "assignee": { + | "name": "Lin" + | } + | } + | } + | ] + | } + """.trimMargin(), + ), + ) +} diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabled.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabled.kt index d13e01af6..d22bb4919 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabled.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferIsDisabled.kt @@ -9,77 +9,6 @@ import graphql.nadel.tests.next.NadelIntegrationTest import org.intellij.lang.annotations.Language import kotlin.test.assertTrue -class HydrationDeferIsDisabledTest : HydrationDeferIsDisabled( - query = """ - query { - issues { # List - key - ... @defer { - assignee { # Should not defer - name - } - } - } - } - """.trimIndent(), -) { - override fun assert(result: ExecutionResult, incrementalResults: List?) { - // assertTrue(result !is IncrementalExecutionResult) - } -} - -/** - * There's actually two hydrations here. - * - * There's one hydration at `issueByKey.assignee` which is fine because there's no List. - * - * Then there's the hydration at `issueByKey.related.assignee` which does not defer because `Issue.related` is a List. - */ -class HydrationDeferIsDisabledForRelatedIssuesTest : HydrationDeferIsDisabled( - query = """ - query { - issueByKey(key: "GQLGW-2") { # Not a list - key - ... @defer { - assignee { # Should defer - name - } - } - related { # Is a list - ... @defer { - assignee { # Should NOT defer - name - } - } - } - } - } - """.trimIndent(), -) - -class HydrationDeferIsDisabledInListOfRelatedIssuesForParentIssueTest : HydrationDeferIsDisabled( - query = """ - query { - issueByKey(key: "GQLGW-3") { # Not a list - key - related { # Is a list - parent { # Not a list - ... @defer { - assignee { # Should NOT defer - name - } - } - } - } - } - } - """.trimIndent(), -) { - override fun assert(result: ExecutionResult, incrementalResults: List?) { - // assertTrue(result !is IncrementalExecutionResult) - } -} - class HydrationDeferIsDisabledForNestedHydrationsTest : HydrationDeferIsDisabled( query = """ query { @@ -98,6 +27,7 @@ class HydrationDeferIsDisabledForNestedHydrationsTest : HydrationDeferIsDisabled ) { override fun assert(result: ExecutionResult, incrementalResults: List?) { assertTrue(result !is IncrementalExecutionResult) + assertTrue(incrementalResults == null) } } diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HyrationDeferErrors.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HyrationDeferErrors.kt new file mode 100644 index 000000000..b245c291f --- /dev/null +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HyrationDeferErrors.kt @@ -0,0 +1,4 @@ +package graphql.nadel.tests.next.fixtures.hydration.defer + +class HyrationDeferErrors { +} From 04194952284fc9dc03a0f8b26a70498053fcb480 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Mon, 6 May 2024 13:19:29 +1200 Subject: [PATCH 04/22] Clean up code and tests --- lib/build.gradle.kts | 3 +- .../java/graphql/nadel/NadelResultTracker.kt | 123 ---- .../main/java/graphql/nadel/NextgenEngine.kt | 2 + .../nadel/engine/NadelExecutionContext.kt | 2 +- .../nadel/{ => result}/NadelResultMerger.kt | 3 +- .../nadel/result/NadelResultPathBuilder.kt | 35 ++ .../nadel/result/NadelResultPathSegment.kt | 9 + .../nadel/result/NadelResultTracker.kt | 154 +++++ lib/src/test/kotlin/graphql/nadel/Jackson.kt | 5 + .../nadel/result/NadelResultTrackerTest.kt | 558 ++++++++++++++++++ 10 files changed, 768 insertions(+), 126 deletions(-) delete mode 100644 lib/src/main/java/graphql/nadel/NadelResultTracker.kt rename lib/src/main/java/graphql/nadel/{ => result}/NadelResultMerger.kt (98%) create mode 100644 lib/src/main/java/graphql/nadel/result/NadelResultPathBuilder.kt create mode 100644 lib/src/main/java/graphql/nadel/result/NadelResultPathSegment.kt create mode 100644 lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt create mode 100644 lib/src/test/kotlin/graphql/nadel/Jackson.kt create mode 100644 lib/src/test/kotlin/graphql/nadel/result/NadelResultTrackerTest.kt diff --git a/lib/build.gradle.kts b/lib/build.gradle.kts index fbe07a171..581dd633d 100644 --- a/lib/build.gradle.kts +++ b/lib/build.gradle.kts @@ -29,7 +29,6 @@ dependencies { testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.10.1") testImplementation("org.slf4j:slf4j-simple:$slf4jVersion") - testImplementation("com.fasterxml.jackson.core:jackson-databind:2.15.3") testImplementation("org.openjdk.jmh:jmh-core:1.37") testImplementation("org.openjdk.jmh:jmh-generator-annprocess:1.37") @@ -43,6 +42,8 @@ dependencies { testImplementation("io.mockk:mockk:1.13.8") testImplementation("com.tngtech.archunit:archunit:1.2.1") + + testImplementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.17.0") } // compileJava.source file("build/generated-src"), sourceSets.main.java diff --git a/lib/src/main/java/graphql/nadel/NadelResultTracker.kt b/lib/src/main/java/graphql/nadel/NadelResultTracker.kt deleted file mode 100644 index 285800d95..000000000 --- a/lib/src/main/java/graphql/nadel/NadelResultTracker.kt +++ /dev/null @@ -1,123 +0,0 @@ -package graphql.nadel - -import graphql.ExecutionResult -import graphql.incremental.DelayedIncrementalPartialResult -import graphql.nadel.engine.transform.query.NadelQueryPath -import graphql.nadel.engine.transform.result.json.JsonNode -import graphql.nadel.engine.util.AnyList -import graphql.nadel.engine.util.AnyMap -import kotlinx.coroutines.CompletableDeferred - -internal sealed interface NadelResultPathSegment { - @JvmInline - value class Object(val key: String) : NadelResultPathSegment - - @JvmInline - value class Array(val index: Int) : NadelResultPathSegment -} - -/** - * todo: as delayed responses go out we need to track what objects went out - */ -/** - * note: a Flow will mean that we can GC the original result sooner - */ -class ResultListener { - fun track(delayed: DelayedIncrementalPartialResult) { - // todo: track objects in here - } -} - -/** - * todo: this needs to track multiple responses - */ -internal class NadelResultTracker { - private val result = CompletableDeferred() - - suspend fun getResultPath( - queryPath: NadelQueryPath, - node: JsonNode, - ): List? { - val result = result.await() - val data = result.getData() - - val queryPathSegments = queryPath.segments - val currentResultPathSegments = mutableListOf() - val currentQueryPathSegments = mutableListOf() - - /** - * { - * "data": { - * users: [ - * { - * friends: [] - * } - * ] - * } - * } - */ - - val queue = mutableListOf(data) - - while (queue.isNotEmpty()) { - val element = queue.removeLast() - if (element === node.value) { - return currentResultPathSegments - } - - when (element) { - is AnyList -> { - if (element.isNotEmpty()) { - queue.addAll(element) - currentResultPathSegments.add(NadelResultPathSegment.Array(element.lastIndex)) - continue - } - } - is AnyMap -> { - if (currentQueryPathSegments.size < queryPathSegments.size) { - val nextQueryPathSegment = queryPathSegments[currentQueryPathSegments.size] - val nextElement = element[nextQueryPathSegment] - - queue.add(nextElement) - currentResultPathSegments.add(NadelResultPathSegment.Object(nextQueryPathSegment)) - currentQueryPathSegments.add(nextQueryPathSegment) - - if (nextElement === node.value) { - return currentResultPathSegments - } - } - } - } - - // Correct the result path segment i.e. move to the next array element - var last = currentResultPathSegments.lastOrNull() - while (last is NadelResultPathSegment.Array) { - if (last.index == 0) { - val removedResultPathSegment = currentResultPathSegments.removeLast() - if (removedResultPathSegment is NadelResultPathSegment.Object) { - currentQueryPathSegments.removeLast() - } - } else { - // We're moving to the next element - currentResultPathSegments[currentResultPathSegments.lastIndex] = - NadelResultPathSegment.Array(last.index - 1) - // Nothing further to fix, stop - break - } - - last = currentResultPathSegments.lastOrNull() - } - } - - return null - } - - fun complete(value: ExecutionResult) { - result.complete(value) - } - - fun complete(value: DelayedIncrementalPartialResult) { - // result.complete(value) - // todo: track here - } -} diff --git a/lib/src/main/java/graphql/nadel/NextgenEngine.kt b/lib/src/main/java/graphql/nadel/NextgenEngine.kt index 7eb2279bb..ae4fcf085 100644 --- a/lib/src/main/java/graphql/nadel/NextgenEngine.kt +++ b/lib/src/main/java/graphql/nadel/NextgenEngine.kt @@ -44,6 +44,8 @@ import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParame import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParameters.RootStep import graphql.nadel.instrumentation.parameters.child import graphql.nadel.schema.NadelDirectives.namespacedDirectiveDefinition +import graphql.nadel.result.NadelResultMerger +import graphql.nadel.result.NadelResultTracker import graphql.nadel.util.OperationNameUtil import graphql.normalized.ExecutableNormalizedField import graphql.normalized.ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables diff --git a/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt b/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt index 2cb08d3b6..17d5c44cd 100644 --- a/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt +++ b/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt @@ -4,12 +4,12 @@ import graphql.ExecutionInput import graphql.GraphQLContext import graphql.execution.instrumentation.InstrumentationState import graphql.nadel.NadelExecutionHints -import graphql.nadel.NadelResultTracker import graphql.nadel.Service import graphql.nadel.ServiceExecutionHydrationDetails import graphql.nadel.engine.instrumentation.NadelInstrumentationTimer import graphql.nadel.hooks.CreateServiceContextParams import graphql.nadel.hooks.NadelExecutionHooks +import graphql.nadel.result.NadelResultTracker import graphql.normalized.ExecutableNormalizedOperation import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentHashMap diff --git a/lib/src/main/java/graphql/nadel/NadelResultMerger.kt b/lib/src/main/java/graphql/nadel/result/NadelResultMerger.kt similarity index 98% rename from lib/src/main/java/graphql/nadel/NadelResultMerger.kt rename to lib/src/main/java/graphql/nadel/result/NadelResultMerger.kt index 4567f541a..1fd44cd22 100644 --- a/lib/src/main/java/graphql/nadel/NadelResultMerger.kt +++ b/lib/src/main/java/graphql/nadel/result/NadelResultMerger.kt @@ -1,9 +1,10 @@ -package graphql.nadel +package graphql.nadel.result import graphql.ExecutionResult import graphql.ExecutionResultImpl import graphql.GraphQLError import graphql.introspection.Introspection +import graphql.nadel.ServiceExecutionResult import graphql.nadel.engine.transform.query.NadelFieldAndService import graphql.nadel.engine.transform.result.NadelResultKey import graphql.nadel.engine.util.AnyMap diff --git a/lib/src/main/java/graphql/nadel/result/NadelResultPathBuilder.kt b/lib/src/main/java/graphql/nadel/result/NadelResultPathBuilder.kt new file mode 100644 index 000000000..7ffaebdbd --- /dev/null +++ b/lib/src/main/java/graphql/nadel/result/NadelResultPathBuilder.kt @@ -0,0 +1,35 @@ +package graphql.nadel.result + +internal interface NadelResultPathBuilder { + fun add(key: String): NadelResultPathBuilder + fun add(index: Int): NadelResultPathBuilder + fun build(): List + + companion object { + operator fun invoke( + path: List = emptyList(), + ): NadelResultPathBuilder { + return object : NadelResultPathBuilder { + private var segments = path.toMutableList() + + override fun add(key: String): NadelResultPathBuilder { + segments.add(NadelResultPathSegment.Object(key)) + return this + } + + override fun add(index: Int): NadelResultPathBuilder { + segments.add(NadelResultPathSegment.Array(index)) + return this + } + + override fun build(): List { + return segments + } + } + } + } +} + +internal fun List.toBuilder(): NadelResultPathBuilder { + return NadelResultPathBuilder(this) +} diff --git a/lib/src/main/java/graphql/nadel/result/NadelResultPathSegment.kt b/lib/src/main/java/graphql/nadel/result/NadelResultPathSegment.kt new file mode 100644 index 000000000..16b396717 --- /dev/null +++ b/lib/src/main/java/graphql/nadel/result/NadelResultPathSegment.kt @@ -0,0 +1,9 @@ +package graphql.nadel.result + +internal sealed interface NadelResultPathSegment { + @JvmInline + value class Object(val key: String) : NadelResultPathSegment + + @JvmInline + value class Array(val index: Int) : NadelResultPathSegment +} diff --git a/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt b/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt new file mode 100644 index 000000000..8aaee080b --- /dev/null +++ b/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt @@ -0,0 +1,154 @@ +package graphql.nadel.result + +import graphql.ExecutionResult +import graphql.incremental.DelayedIncrementalPartialResult +import graphql.nadel.engine.transform.query.NadelQueryPath +import graphql.nadel.engine.transform.result.json.JsonNode +import graphql.nadel.engine.util.AnyList +import graphql.nadel.engine.util.AnyMap +import kotlinx.coroutines.CompletableDeferred + +/** + * todo: this needs to track multiple responses + */ +internal class NadelResultTracker { + private val result = CompletableDeferred() + + private enum class NavigationOutcome { + QueuedChild, + ExpandedArray, + DeadEnd, + } + + suspend fun getResultPath( + queryPath: NadelQueryPath, + node: JsonNode, + ): List? { + val result = result.await() + val data = result.getData() + + val queryPathSegments = queryPath.segments + val currentResultPathSegments = mutableListOf() + val currentQueryPathSegments = mutableListOf() + + val queue = mutableListOf(data) + + fun printState(): String { + return """ + Current query path: ${ + currentQueryPathSegments.joinToString( + separator = ",", + prefix = "[", + postfix = "]" + ) + } + Current result path: ${ + currentResultPathSegments.joinToString( + separator = ",", + prefix = "[", + postfix = "]" + ) + } + """.replaceIndent(' '.toString().repeat(n = 4)) + } + + println( + "Trying to find $node from ${ + queryPath.segments.joinToString( + separator = ",", + prefix = "[", + postfix = "]" + ) + }" + ) + + while (queue.isNotEmpty()) { + val element = queue.removeLast() + println("Visiting $element") + if (element === node.value) { + println("Got em $currentResultPathSegments") + return currentResultPathSegments + } + + val outcome: NavigationOutcome = when (element) { + is AnyList -> { + if (element.isNotEmpty()) { + queue.addAll(element) + currentResultPathSegments.add(NadelResultPathSegment.Array(element.lastIndex)) + NavigationOutcome.ExpandedArray + } else { + NavigationOutcome.DeadEnd + } + } + is AnyMap -> { + if (currentQueryPathSegments.size < queryPathSegments.size) { + val nextQueryPathSegment = queryPathSegments[currentQueryPathSegments.size] + val nextElement = element[nextQueryPathSegment] + + if (nextElement == null) { + NavigationOutcome.DeadEnd // todo: tests + } else { + queue.add(nextElement) + currentResultPathSegments.add(NadelResultPathSegment.Object(nextQueryPathSegment)) + currentQueryPathSegments.add(nextQueryPathSegment) + NavigationOutcome.QueuedChild + } + } else { + NavigationOutcome.DeadEnd // todo: tests + } + } + else -> NavigationOutcome.DeadEnd // todo: tests + } + + when (outcome) { + NavigationOutcome.QueuedChild -> { + } + NavigationOutcome.ExpandedArray -> { + } + NavigationOutcome.DeadEnd -> { + if (queue.isNotEmpty()) { // i.e. we have other array elements to visit + while (currentResultPathSegments.isNotEmpty()) { + val last = currentResultPathSegments.lastOrNull() ?: break + + when (last) { + is NadelResultPathSegment.Array -> { + if (last.index == 0) { + // Nothing more to visit in the array, remember that we traverse end -> front + println("Popping dead end array") + currentResultPathSegments.removeLast() + } else { + println("Moving to next array element ${last.index - 1}") + // We're moving to the next element + currentResultPathSegments[currentResultPathSegments.lastIndex] = + NadelResultPathSegment.Array(last.index - 1) + // Nothing further to fix, stop + break + } + } + is NadelResultPathSegment.Object -> { + println("Popping up dead end object") + currentResultPathSegments.removeLast() + currentQueryPathSegments.removeLast() + } + } + } + } + } + } + + println("Loop end\n${printState()}") + } + + println("Didn't get em") + return null + } + + fun complete(value: ExecutionResult) { + result.complete(value) + } + + fun complete(value: DelayedIncrementalPartialResult) { + // result.complete(value) + // todo: track here + } +} diff --git a/lib/src/test/kotlin/graphql/nadel/Jackson.kt b/lib/src/test/kotlin/graphql/nadel/Jackson.kt new file mode 100644 index 000000000..3789dcc16 --- /dev/null +++ b/lib/src/test/kotlin/graphql/nadel/Jackson.kt @@ -0,0 +1,5 @@ +package graphql.nadel + +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper + +val jsonObjectMapper = jacksonObjectMapper() diff --git a/lib/src/test/kotlin/graphql/nadel/result/NadelResultTrackerTest.kt b/lib/src/test/kotlin/graphql/nadel/result/NadelResultTrackerTest.kt new file mode 100644 index 000000000..a25c1deae --- /dev/null +++ b/lib/src/test/kotlin/graphql/nadel/result/NadelResultTrackerTest.kt @@ -0,0 +1,558 @@ +package graphql.nadel.result + +import com.fasterxml.jackson.module.kotlin.readValue +import graphql.ExecutionResult +import graphql.nadel.engine.transform.query.NadelQueryPath +import graphql.nadel.engine.transform.result.json.JsonNode +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 graphql.nadel.engine.util.toGraphQLError +import graphql.nadel.jsonObjectMapper +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertTrue + +class NadelResultTrackerTest { + @Test + fun canSearch2dArrays() = runTest { + // Given + val result = resultOf( + """ + { + "data": [ + [ + {"Hello": null}, + {"World": null} + ], + [ + {"Greetings": null}, + {"Friend": null} + ], + [ + {"Bye": null}, + {"For now": null} + ] + ] + } + """.trimIndent() + ) + val data = result.getData>>() + + val subject = NadelResultTracker() + subject.complete(result) + + // Then + for (i in 0..2) { + for (j in 0..1) { + assertTrue( + subject.getResultPath( + NadelQueryPath.root, + JsonNode(data[i][j]), + ) == NadelResultPathBuilder() + .add(i) + .add(j) + .build(), + ) + } + } + } + + @Test + fun canSearch3dArrays() = runTest { + // Given + val result = resultOf( + """ + { + "data": { + "matrix": [ + [ + [ + {"Hello": null}, + {"World": null} + ], + [ + {"Greetings": null}, + {"Friend": null} + ], + [ + {"Bye": null}, + {"For now": null} + ] + ], + [ + [ + {"Hello": null}, + {"World": null} + ], + [ + {"Greetings": null}, + {"Friend": null} + ], + [ + {"Bye": null}, + {"For now": null} + ] + ], + [ + [ + {"Hello": null}, + {"World": null} + ], + [ + {"Greetings": null}, + {"Friend": null} + ], + [ + {"Bye": null}, + {"For now": null} + ] + ] + ] + } + } + """.trimIndent() + ) + + @Suppress("UNCHECKED_CAST") + val data = result.getData()["matrix"] as List>> + + val subject = NadelResultTracker() + subject.complete(result) + + // Then + for (i in 0..2) { + for (j in 0..2) { + for (k in 0..1) { + assertTrue( + subject.getResultPath( + NadelQueryPath(listOf("matrix")), + JsonNode(data[i][j][k]), + ) == NadelResultPathBuilder() + .add("matrix") + .add(i) + .add(j) + .add(k) + .build(), + ) + } + } + } + } + + /** + * Shouldn't really happen in a real scenario, but we can support it. + * + * i.e. this tests that matrix is not guaranteed to be a 2d or 3d array + */ + @Test + fun canNavigateVariableNesting() = runTest { + // Given + val result = resultOf( + """ + { + "data": { + "matrix": [ + [ + [ + [] + ], + [ + [], + {"World": 10}, + [] + ] + ], + [ + { + "id": 10 + } + ], + { + "name": 10 + } + ] + } + } + """.trimIndent() + ) + + val data = result.getData() + + val subject = NadelResultTracker() + subject.complete(result) + + // Then + JsonNode(data).dfs { path, value -> + if (value is AnyList || value is AnyMap) { + val queryPath = NadelQueryPath( + path + .filterIsInstance() + .map(NadelResultPathSegment.Object::key), + ) + + assertTrue(subject.getResultPath(queryPath, JsonNode(value)) == path) + } + } + } + + /** + * Ensure that when we reach a dead end array, we can keep searching. + */ + @Test + fun searchesEmptyArrays() = runTest { + // Given + val result = resultOf( + """ + { + "data": { + "users": [ + { + "friends": [] + }, + { + "friends": [ + { + "name": "Who was it again?" + } + ] + }, + { + "friends": [] + } + ] + } + } + """.trimIndent() + ) + + val data = result.getData() + + val toFindPath = NadelResultPathBuilder() + .add("users") + .add(1) + .add("friends") + .add(0) + .build() + val toFind = JsonNode(data).getAt(toFindPath)!! + + val subject = NadelResultTracker() + subject.complete(result) + + // Then + val queryPath = NadelQueryPath(listOf("users", "friends")) + + assertTrue(subject.getResultPath(queryPath, JsonNode(toFind)) == toFindPath) + } + + /** + * Ensure that when we reach a dead end array, we can keep searching. + */ + @Test + fun canHandleObjectDeadEnds() = runTest { + // Given + val result = resultOf( + """ + { + "data": { + "users": [ + {}, + { + "friends": [] + }, + { + "friends": [null] + }, + { + "friends": [ + null, + { + "user": { + "name": "Lando Won" + } + }, + {}, + null + ] + }, + { + "friends": [ + null, + {}, + null, + null + ] + }, + { + "friends": null + }, + {} + ] + } + } + """.trimIndent() + ) + + val data = result.getData() + + val toFindPath = NadelResultPathBuilder() + .add("users") + .add(3) + .add("friends") + .add(1) + .add("user") + .build() + val toFind = JsonNode(data).getAt(toFindPath)!! + assertTrue(toFind is AnyMap && toFind["name"] == "Lando Won") + + val subject = NadelResultTracker() + subject.complete(result) + + // Then + val queryPath = NadelQueryPath(listOf("users", "friends", "user")) + + assertTrue(subject.getResultPath(queryPath, JsonNode(toFind)) == toFindPath) + } + + /** + * Not really a real GraphQL scenario, but ensures we support navigating it. + * + * i.e. Here matrix.x and matrix.y are sometimes arrays, and sometimes objects. + */ + @Test + fun arraysMixedWithObjects() = runTest { + // Given + val result = resultOf( + """ + { + "data": { + "matrix": [ + { + "x": [ + {"value": 10} + ] + }, + { + "y": { + "value": 10 + } + }, + { + "y": [ + {"value": 10} + ] + }, + { + "x": { + "value": 10 + } + }, + { + "x": { + "value": 10 + } + }, + { + "z": { + "val": 10 + } + }, + { + "y": { + "value": 10 + } + } + ] + } + } + """.trimIndent() + ) + + val data = result.getData() + + val subject = NadelResultTracker() + subject.complete(result) + + // Then + JsonNode(data).dfs { path, value -> + if (value is AnyList || value is AnyMap) { + val queryPath = NadelQueryPath( + path + .filterIsInstance() + .map(NadelResultPathSegment.Object::key), + ) + + assertTrue(subject.getResultPath(queryPath, JsonNode(value)) == path) + } + } + } + + /** + * Just tests our [JsonNode.dfs] test implementation. + * + * If that didn't work then all our tests may silently pass. + */ + @Test + fun dfs() = runTest { + val result = resultOf( + """ + { + "data": { + "matrix": [ + { + "x": [ + {"value": 10} + ] + }, + { + "y": { + "value": 10 + } + }, + { + "x": { + "value": 10 + } + }, + { + "x": { + "value": 10 + } + }, + { + "z": { + "val": 10 + } + }, + { + "y": { + "value": 10 + } + } + ] + } + } + """.trimIndent() + ) + + val data = result.getData() + + // When + val visited = mutableListOf, Any?>>() + JsonNode(data).dfs { path, value -> + visited.add(path to value) + } + + // Then + assertTrue(visited.mapTo(HashSet()) { (path) -> path }.size == visited.size) + + // Code to generate expected + // visited.forEach { (path, value) -> + // val code = path + // .joinToString(prefix = "NadelResultPathBuilder()", postfix = ".build()", separator = "") { + // when (it) { + // is NadelResultPathSegment.Array -> ".add(${it.index})" + // is NadelResultPathSegment.Object -> ".add(${jsonObjectMapper.writeValueAsString(it.key)})" + // } + // } + // // Write twice to escape string + // val serializedValue = jsonObjectMapper.writeValueAsString( + // jsonObjectMapper.writeValueAsString(value), + // ) + // println("$code to \njsonObjectMapper.readValue(${serializedValue}),") + // } + + val expected = listOf( + NadelResultPathBuilder().build() to + jsonObjectMapper.readValue("""{"matrix":[{"x":[{"value":10}]},{"y":{"value":10}},{"x":{"value":10}},{"x":{"value":10}},{"z":{"val":10}},{"y":{"value":10}}]}"""), + NadelResultPathBuilder().add("matrix").build() to + jsonObjectMapper.readValue("""[{"x":[{"value":10}]},{"y":{"value":10}},{"x":{"value":10}},{"x":{"value":10}},{"z":{"val":10}},{"y":{"value":10}}]"""), + NadelResultPathBuilder().add("matrix").add(0).build() to + jsonObjectMapper.readValue("""{"x":[{"value":10}]}"""), + NadelResultPathBuilder().add("matrix").add(0).add("x").build() to + jsonObjectMapper.readValue("""[{"value":10}]"""), + NadelResultPathBuilder().add("matrix").add(0).add("x").add(0).build() to + jsonObjectMapper.readValue("""{"value":10}"""), + NadelResultPathBuilder().add("matrix").add(0).add("x").add(0).add("value").build() to + jsonObjectMapper.readValue("10"), + NadelResultPathBuilder().add("matrix").add(1).build() to + jsonObjectMapper.readValue("""{"y":{"value":10}}"""), + NadelResultPathBuilder().add("matrix").add(1).add("y").build() to + jsonObjectMapper.readValue("""{"value":10}"""), + NadelResultPathBuilder().add("matrix").add(1).add("y").add("value").build() to + jsonObjectMapper.readValue("10"), + NadelResultPathBuilder().add("matrix").add(2).build() to + jsonObjectMapper.readValue("""{"x":{"value":10}}"""), + NadelResultPathBuilder().add("matrix").add(2).add("x").build() to + jsonObjectMapper.readValue("""{"value":10}"""), + NadelResultPathBuilder().add("matrix").add(2).add("x").add("value").build() to + jsonObjectMapper.readValue("10"), + NadelResultPathBuilder().add("matrix").add(3).build() to + jsonObjectMapper.readValue("""{"x":{"value":10}}"""), + NadelResultPathBuilder().add("matrix").add(3).add("x").build() to + jsonObjectMapper.readValue("""{"value":10}"""), + NadelResultPathBuilder().add("matrix").add(3).add("x").add("value").build() to + jsonObjectMapper.readValue("10"), + NadelResultPathBuilder().add("matrix").add(4).build() to + jsonObjectMapper.readValue("""{"z":{"val":10}}"""), + NadelResultPathBuilder().add("matrix").add(4).add("z").build() to + jsonObjectMapper.readValue("""{"val":10}"""), + NadelResultPathBuilder().add("matrix").add(4).add("z").add("val").build() to + jsonObjectMapper.readValue("10"), + NadelResultPathBuilder().add("matrix").add(5).build() to + jsonObjectMapper.readValue("""{"y":{"value":10}}"""), + NadelResultPathBuilder().add("matrix").add(5).add("y").build() to + jsonObjectMapper.readValue("""{"value":10}"""), + NadelResultPathBuilder().add("matrix").add(5).add("y").add("value").build() to + jsonObjectMapper.readValue("10"), + ) + + assertTrue(visited == expected) + } + + private fun resultOf(json: String): ExecutionResult { + val result = jsonObjectMapper.readValue(json) + @Suppress("UNCHECKED_CAST") + return ExecutionResult.newExecutionResult() + .data(result["data"]) + .errors((result["errors"] as List?)?.map(::toGraphQLError)) + .extensions(result["extensions"] as Map?) + .build() + } + + /** + * But why are you testing NadelResultTracker by effectively duplicating the functionality?? + * + * Well the point of NadelResultTracker is that you're finding one node. + * The tricky part is creating one result node path and reusing it as we're traversing. + * It's much easier to reason the logic in this recursive function. + */ + private suspend fun JsonNode.dfs( + path: List = emptyList(), + onConsume: suspend (List, Any?) -> Unit, + ) { + onConsume(path, value) + + when (val value = value) { + is AnyMap -> value.forEach { (key, element) -> + assertTrue(key is String) + JsonNode(element).dfs(path.toBuilder().add(key).build(), onConsume) + } + is AnyList -> value.forEachIndexed { index, element -> + JsonNode(element).dfs(path.toBuilder().add(index).build(), onConsume) + } + } + } + + private fun JsonNode.getAt( + path: List, + ): Any? { + return path.foldWhileNotNull(value) { prev, pathSegment -> + when (pathSegment) { + is NadelResultPathSegment.Array -> (prev as AnyList)[pathSegment.index] + is NadelResultPathSegment.Object -> (prev as AnyMap)[pathSegment.key] + } + } + } +} From 7c18dabd0c87b5ba56a7635fbeefcf685c74d620 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Mon, 6 May 2024 13:32:43 +1200 Subject: [PATCH 05/22] Add some slight doco --- .../graphql/nadel/result/NadelResultTracker.kt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt b/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt index 8aaee080b..905a8dbbb 100644 --- a/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt +++ b/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt @@ -20,6 +20,21 @@ internal class NadelResultTracker { DeadEnd, } + /** + * So… in Nadel the result can change a lot. + * + * This function lets you track where a result node went to in the overall response sent to the user. + * + * I haven't benchmarked this yet, but in theory it should be more performant than say using + * [graphql.nadel.engine.transform.result.json.JsonNodes]. + * + * In the past we used to track the [NadelResultPathSegment]s for each [JsonNode] but that was horrible + * performance wise because we created one List for each result node + * i.e. as the result grew, both in depth and result node count, you'd allocate tons of (big) lists. + * + * This implementation keeps track of the _current_ [NadelResultPathSegment]s and returns that if it + * finds the [node] in question. + */ suspend fun getResultPath( queryPath: NadelQueryPath, node: JsonNode, From f28f4c4405b66cd77c7a7948499ff482dd5b57fb Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Wed, 15 May 2024 11:13:27 +1000 Subject: [PATCH 06/22] Add test for forwarding errors --- lib/build.gradle.kts | 4 +- ...Combiner.kt => CombineExecutionResults.kt} | 11 +- .../defer/HydrationDeferForwardsErrors.kt | 182 ++++++++++++++++++ ...ydrationDeferForwardsErrorsTestSnapshot.kt | 130 +++++++++++++ .../hydration/defer/HyrationDeferErrors.kt | 4 - 5 files changed, 321 insertions(+), 10 deletions(-) rename test/src/test/kotlin/graphql/nadel/tests/next/{IncrementalResultCombiner.kt => CombineExecutionResults.kt} (87%) create mode 100644 test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrors.kt create mode 100644 test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsTestSnapshot.kt delete mode 100644 test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HyrationDeferErrors.kt diff --git a/lib/build.gradle.kts b/lib/build.gradle.kts index 581dd633d..0cca3b393 100644 --- a/lib/build.gradle.kts +++ b/lib/build.gradle.kts @@ -29,6 +29,8 @@ dependencies { testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.10.1") testImplementation("org.slf4j:slf4j-simple:$slf4jVersion") + testImplementation("com.fasterxml.jackson.core:jackson-databind:2.17.0") + testImplementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.17.0") testImplementation("org.openjdk.jmh:jmh-core:1.37") testImplementation("org.openjdk.jmh:jmh-generator-annprocess:1.37") @@ -42,8 +44,6 @@ dependencies { testImplementation("io.mockk:mockk:1.13.8") testImplementation("com.tngtech.archunit:archunit:1.2.1") - - testImplementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.17.0") } // compileJava.source file("build/generated-src"), sourceSets.main.java diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/IncrementalResultCombiner.kt b/test/src/test/kotlin/graphql/nadel/tests/next/CombineExecutionResults.kt similarity index 87% rename from test/src/test/kotlin/graphql/nadel/tests/next/IncrementalResultCombiner.kt rename to test/src/test/kotlin/graphql/nadel/tests/next/CombineExecutionResults.kt index 1bbb0c565..89b106757 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/IncrementalResultCombiner.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/CombineExecutionResults.kt @@ -11,8 +11,8 @@ fun combineExecutionResults(result: JsonMap, incrementalResults: List): @Suppress("UNCHECKED_CAST") val resultData = deepClone["data"] as JsonMap? - @Suppress("UNCHECKED_CAST") // Ok I wanted MutableList<*> but seems like that doesn't work for some reason… - val resultErrors: MutableList = deepClone["errors"] as MutableList? ?: mutableListOf() + @Suppress("UNCHECKED_CAST") + val resultErrors = deepClone["errors"] as MutableList? ?: mutableListOf() if (resultData != null) { incrementalResults @@ -29,12 +29,15 @@ fun combineExecutionResults(result: JsonMap, incrementalResults: List): when { "data" in payload -> { - val data = payload["data"]!! - setDeferred(resultData, path, data) + val data = payload["data"] + if (data != null) { + setDeferred(resultData, path, data) + } } "items" in payload -> { throw UnsupportedOperationException("Merging @stream results is not supported yet") } + else -> throw UnsupportedOperationException() } val elements: List = (payload["errors"] as List<*>?) ?: emptyList() diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrors.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrors.kt new file mode 100644 index 000000000..a1e3d7b53 --- /dev/null +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrors.kt @@ -0,0 +1,182 @@ +package graphql.nadel.tests.next.fixtures.hydration.defer + +import graphql.ExecutionResult +import graphql.incremental.DelayedIncrementalPartialResult +import graphql.nadel.NadelExecutionHints +import graphql.nadel.engine.util.strictAssociateBy +import graphql.nadel.error.NadelGraphQLErrorException +import graphql.nadel.tests.next.NadelIntegrationTest +import org.intellij.lang.annotations.Language +import kotlin.test.assertTrue + +class HydrationDeferForwardsErrorsTest : HydrationDeferForwardsErrors( + query = """ + query { + issueByKey(key: "GQLGW-2") { + ... @defer { + assignee { + name + } + } + } + } + """.trimIndent(), +) { + override fun assert(result: ExecutionResult, incrementalResults: List?) { + assertTrue(incrementalResults?.isNotEmpty() == true) + assertTrue(incrementalResults?.single()?.incremental?.single()?.errors?.isNotEmpty() == true) + } +} + +abstract class HydrationDeferForwardsErrors( + @Language("GraphQL") + query: String, +) : NadelIntegrationTest( + query = query, + services = listOf( + Service( + name = "issues", + overallSchema = """ + directive @defer(if: Boolean, label: String) on FRAGMENT_SPREAD | INLINE_FRAGMENT + type Query { + issues: [Issue!] + issueGroups: [[Issue]] + issueByKey(key: String!): Issue + } + type Issue { + key: String! + assigneeId: ID! + self: Issue + @hydrated( + service: "issues" + field: "issueByKey" + arguments: [{name: "key", value: "$source.key"}] + ) + assignee: User + @hydrated( + service: "users" + field: "userById" + arguments: [{name: "id", value: "$source.assigneeId"}] + ) + related: [Issue!] + parent: Issue + } + """.trimIndent(), + runtimeWiring = { wiring -> + data class Issue( + val key: String, + val assigneeId: String, + val parentKey: String? = null, + val relatedKeys: List = emptyList(), + ) + + val issues = listOf( + Issue( + key = "GQLGW-1", + assigneeId = "ari:cloud:identity::user/1", + ), + Issue( + key = "GQLGW-2", + assigneeId = "ari:cloud:identity::user/0", + parentKey = "GQLGW-1", + relatedKeys = listOf("GQLGW-1"), + ), + Issue( + key = "GQLGW-3", + assigneeId = "ari:cloud:identity::user/1", + parentKey = "GQLGW-1", + relatedKeys = listOf("GQLGW-1", "GQLGW-2"), + ), + Issue( + key = "GQLGW-4", + assigneeId = "ari:cloud:identity::user/3", + parentKey = "GQLGW-1", + relatedKeys = listOf("GQLGW-1", "GQLGW-2", "GQLGW-3"), + ), + ) + val issuesByKey = issues.strictAssociateBy { it.key } + + wiring + .type("Query") { type -> + type + .dataFetcher("issueByKey") { env -> + issuesByKey[env.getArgument("key")] + } + .dataFetcher("issues") { env -> + issues + } + .dataFetcher("issueGroups") { + issues + .groupBy { + it.key.substringAfter("-").toInt() % 2 == 0 + } + .values + } + } + .type("Issue") { type -> + type + .dataFetcher("related") { env -> + env.getSource() + .relatedKeys + .map { + issuesByKey[it]!! + } + } + .dataFetcher("parent") { env -> + issuesByKey[env.getSource().parentKey] + } + } + }, + ), + Service( + name = "users", + overallSchema = """ + type Query { + userById(id: ID!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + runtimeWiring = { wiring -> + data class User( + val id: String, + val name: String, + ) + + val users = listOf( + User( + id = "ari:cloud:identity::user/1", + name = "Frank", + ), + User( + id = "ari:cloud:identity::user/2", + name = "Tom", + ), + User( + id = "ari:cloud:identity::user/3", + name = "Lin", + ), + ) + val usersById = users.strictAssociateBy { it.id } + + class UserNotFoundError(id: String) : NadelGraphQLErrorException(message = "No user: $id") + + wiring + .type("Query") { type -> + type + .dataFetcher("userById") { env -> + val id = env.getArgument("id") + usersById[id] ?: throw UserNotFoundError(id) + } + } + }, + ), + ), +) { + override fun makeExecutionHints(): NadelExecutionHints.Builder { + return super.makeExecutionHints() + .deferSupport { true } + } +} diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsTestSnapshot.kt new file mode 100644 index 000000000..8b087c980 --- /dev/null +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsTestSnapshot.kt @@ -0,0 +1,130 @@ +// @formatter:off +package graphql.nadel.tests.next.fixtures.hydration.defer + +import graphql.nadel.tests.next.ExpectedNadelResult +import graphql.nadel.tests.next.ExpectedServiceCall +import graphql.nadel.tests.next.TestSnapshot +import graphql.nadel.tests.next.listOfJsonStrings +import kotlin.Suppress +import kotlin.collections.List +import kotlin.collections.listOf + +/** + * This class is generated. Do NOT modify. + * + * Refer to [graphql.nadel.tests.next.UpdateTestSnapshots + */ +@Suppress("unused") +public class HydrationDeferForwardsErrorsTestSnapshot : TestSnapshot() { + override val calls: List = listOf( + ExpectedServiceCall( + service = "issues", + query = """ + | { + | issueByKey(key: "GQLGW-2") { + | hydration__assignee__assigneeId: assigneeId + | __typename__hydration__assignee: __typename + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "issueByKey": { + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/0", + | "__typename__hydration__assignee": "Issue" + | } + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/0") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "errors": [ + | { + | "message": "No user: ari:cloud:identity::user/0", + | "extensions": { + | "classification": "UserNotFoundError" + | } + | } + | ], + | "data": { + | "userById": null + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ) + + /** + * ```json + * { + * "data": { + * "issueByKey": { + * "assignee": null + * } + * }, + * "errors": [ + * { + * "message": "No user: ari:cloud:identity::user/0", + * "locations": [], + * "extensions": { + * "classification": "UserNotFoundError" + * } + * } + * ] + * } + * ``` + */ + override val result: ExpectedNadelResult = ExpectedNadelResult( + result = """ + | { + | "data": { + | "issueByKey": {} + | }, + | "hasNext": true + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + """ + | { + | "hasNext": false, + | "incremental": [ + | { + | "path": [ + | "issueByKey" + | ], + | "errors": [ + | { + | "message": "No user: ari:cloud:identity::user/0", + | "locations": [], + | "extensions": { + | "classification": "UserNotFoundError" + | } + | } + | ], + | "data": { + | "assignee": null + | } + | } + | ] + | } + """.trimMargin(), + ), + ) +} diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HyrationDeferErrors.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HyrationDeferErrors.kt deleted file mode 100644 index b245c291f..000000000 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HyrationDeferErrors.kt +++ /dev/null @@ -1,4 +0,0 @@ -package graphql.nadel.tests.next.fixtures.hydration.defer - -class HyrationDeferErrors { -} From 84409f0a43e86eb583fb54a9a8ef17adb241c7f9 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Mon, 6 May 2024 21:01:11 +1200 Subject: [PATCH 07/22] Set path for deferred hydration errors --- .../hydration/NadelHydrationTransform.kt | 21 +- .../graphql/nadel/engine/util/GraphQLUtil.kt | 16 +- .../kotlin/graphql/nadel/tests/EngineTests.kt | 4 +- .../graphql/nadel/tests/JsonAssertions.kt | 13 +- .../defer/HydrationDeferForwardsErrors.kt | 32 +- ...ardsErrorsFromEachHydrationTestSnapshot.kt | 274 ++++++++++++++++++ ...ydrationDeferForwardsErrorsTestSnapshot.kt | 8 + .../graphql/nadel/tests/util/StriktUtil.kt | 12 +- 8 files changed, 353 insertions(+), 27 deletions(-) create mode 100644 test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsFromEachHydrationTestSnapshot.kt 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 ad8bd08be..8834ed9ba 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 @@ -250,6 +250,12 @@ internal class NadelHydrationTransform( .map { hydration -> // Hydration of one parent node val data = hydration.newValue + val parentPath = executionContext.resultTracker.getResultPath( + overallField.queryPath.dropLast(1), + hydration.parentNode, + )!! + val path = parentPath + overallField.resultKey + DeferPayload.newDeferredItem() .label(label) .data( @@ -257,13 +263,16 @@ internal class NadelHydrationTransform( overallField.resultKey to data?.value, ), ) - .path( - executionContext.resultTracker.getResultPath( - overallField.queryPath.dropLast(1), - hydration.parentNode, - )!! + .path(parentPath) + .errors( + hydration.errors + .map { + toGraphQLError( + raw = it, + path = path, + ) + }, ) - .errors(hydration.errors.map(::toGraphQLError)) .build() } ) 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 341436cb5..5fa6f5742 100644 --- a/lib/src/main/java/graphql/nadel/engine/util/GraphQLUtil.kt +++ b/lib/src/main/java/graphql/nadel/engine/util/GraphQLUtil.kt @@ -89,16 +89,22 @@ fun newGraphQLError( .build() } +/** + * Maps a [raw] [JsonMap] to a [GraphQLError] with optional overrides in the parameters. + */ fun toGraphQLError( raw: JsonMap, + message: String? = raw["message"] as String?, + extensions: JsonMap? = raw["extensions"] as JsonMap?, + path: AnyList? = raw["path"] as AnyList?, ): GraphQLError { val errorBuilder = newError() - .message((raw["message"] as String?) ?: "An error has occurred") - raw["extensions"]?.let { extensions -> - errorBuilder.extensions(extensions as JsonMap) + .message(message ?: "An error has occurred") + if (extensions != null) { + errorBuilder.extensions(extensions) } - raw["path"]?.let { path -> - errorBuilder.path(path as AnyList) + if (path != null) { + errorBuilder.path(path) } return errorBuilder.build() } diff --git a/test/src/test/kotlin/graphql/nadel/tests/EngineTests.kt b/test/src/test/kotlin/graphql/nadel/tests/EngineTests.kt index 058e4b3e0..5043c9b47 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/EngineTests.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/EngineTests.kt @@ -349,8 +349,8 @@ private suspend fun execute( val expectedResponse = fixture.response if (expectedResponse != null) { // TODO: check extensions one day - right now they don't match up as dumped tests weren't fully E2E but tests are - assertJsonObject( - subject = response.toSpecification().let { + assertJsonEquals( + actual = response.toSpecification().let { mapOf( "data" to it["data"], "errors" to (it["errors"] ?: emptyList()), diff --git a/test/src/test/kotlin/graphql/nadel/tests/JsonAssertions.kt b/test/src/test/kotlin/graphql/nadel/tests/JsonAssertions.kt index 71b62f319..d46311bc2 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/JsonAssertions.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/JsonAssertions.kt @@ -5,15 +5,9 @@ import graphql.nadel.engine.util.AnyMap import graphql.nadel.engine.util.JsonMap import graphql.nadel.tests.util.keysEqual import strikt.api.Assertion -import strikt.api.expectThat import strikt.assertions.isA -fun assertJsonObject(subject: JsonMap, expected: JsonMap) { - return expectThat(subject) { - assertJsonObject(expectedMap = expected) - } -} - +@Deprecated("Do not use") fun Assertion.Builder.assertJsonKeys(): Assertion.Builder { assert("keys are all strings") { subject -> @Suppress("UNCHECKED_CAST") // We're checking if the erased type holds up @@ -30,6 +24,7 @@ fun Assertion.Builder.assertJsonKeys(): Assertion.Builder { return this as Assertion.Builder } +@Deprecated("Do not use") private fun Assertion.Builder.assertJsonObject(expectedMap: JsonMap) { keysEqual(expectedMap.keys) @@ -44,11 +39,13 @@ private fun Assertion.Builder.assertJsonObject(expectedMap: JsonMap) { } } +@Deprecated("Do not use") private fun Assertion.Builder.assertJsonEntry(key: String, subjectValue: Any?, expectedValue: Any?) { get("""entry "$key"""") { subjectValue } .assertJsonValue(subjectValue, expectedValue) } +@Deprecated("Do not use") private fun jsonTypeOf(element: Any?): String { return when (element) { is AnyList -> "JSON array" @@ -64,6 +61,7 @@ private fun jsonTypeOf(element: Any?): String { } } +@Deprecated("Do not use") private fun Assertion.Builder.assertJsonValue(subjectValue: Any?, expectedValue: Any?) { when (subjectValue) { is AnyMap -> { @@ -100,6 +98,7 @@ private fun Assertion.Builder.assertJsonValue(subjectValue: Any?, expecte } } +@Deprecated("Do not use") private fun Assertion.Builder>.assertJsonArray(expectedValue: List) { compose("all elements match expected:") { subject -> assert("size matches expected") { diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrors.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrors.kt index a1e3d7b53..011d51e5d 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrors.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrors.kt @@ -2,6 +2,7 @@ package graphql.nadel.tests.next.fixtures.hydration.defer import graphql.ExecutionResult import graphql.incremental.DelayedIncrementalPartialResult +import graphql.incremental.IncrementalExecutionResult import graphql.nadel.NadelExecutionHints import graphql.nadel.engine.util.strictAssociateBy import graphql.nadel.error.NadelGraphQLErrorException @@ -23,11 +24,32 @@ class HydrationDeferForwardsErrorsTest : HydrationDeferForwardsErrors( """.trimIndent(), ) { override fun assert(result: ExecutionResult, incrementalResults: List?) { + assertTrue(result is IncrementalExecutionResult) assertTrue(incrementalResults?.isNotEmpty() == true) assertTrue(incrementalResults?.single()?.incremental?.single()?.errors?.isNotEmpty() == true) } } +class HydrationDeferForwardsErrorsFromEachHydrationTest : HydrationDeferForwardsErrors( + query = """ + query { + issuesByKeys(keys: ["GQLGW-2", "GQLGW-3", "GQLGW-4"]) { + key + ... @defer { + assignee { + name + } + } + } + } + """.trimIndent(), +) { + override fun assert(result: ExecutionResult, incrementalResults: List?) { + assertTrue(result is IncrementalExecutionResult) + assertTrue(incrementalResults?.isNotEmpty() == true) + } +} + abstract class HydrationDeferForwardsErrors( @Language("GraphQL") query: String, @@ -40,6 +62,7 @@ abstract class HydrationDeferForwardsErrors( directive @defer(if: Boolean, label: String) on FRAGMENT_SPREAD | INLINE_FRAGMENT type Query { issues: [Issue!] + issuesByKeys(keys: [String!]!): [Issue!] issueGroups: [[Issue]] issueByKey(key: String!): Issue } @@ -89,7 +112,7 @@ abstract class HydrationDeferForwardsErrors( ), Issue( key = "GQLGW-4", - assigneeId = "ari:cloud:identity::user/3", + assigneeId = "ari:cloud:identity::user/10", parentKey = "GQLGW-1", relatedKeys = listOf("GQLGW-1", "GQLGW-2", "GQLGW-3"), ), @@ -102,6 +125,13 @@ abstract class HydrationDeferForwardsErrors( .dataFetcher("issueByKey") { env -> issuesByKey[env.getArgument("key")] } + .dataFetcher("issuesByKeys") { env -> + val keys = env.getArgument>("keys").toSet() + issues + .filter { + it.key in keys + } + } .dataFetcher("issues") { env -> issues } diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsFromEachHydrationTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsFromEachHydrationTestSnapshot.kt new file mode 100644 index 000000000..749e459e2 --- /dev/null +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsFromEachHydrationTestSnapshot.kt @@ -0,0 +1,274 @@ +// @formatter:off +package graphql.nadel.tests.next.fixtures.hydration.defer + +import graphql.nadel.tests.next.ExpectedNadelResult +import graphql.nadel.tests.next.ExpectedServiceCall +import graphql.nadel.tests.next.TestSnapshot +import graphql.nadel.tests.next.listOfJsonStrings +import kotlin.Suppress +import kotlin.collections.List +import kotlin.collections.listOf + +/** + * This class is generated. Do NOT modify. + * + * Refer to [graphql.nadel.tests.next.UpdateTestSnapshots + */ +@Suppress("unused") +public class HydrationDeferForwardsErrorsFromEachHydrationTestSnapshot : TestSnapshot() { + override val calls: List = listOf( + ExpectedServiceCall( + service = "issues", + query = """ + | { + | issuesByKeys(keys: ["GQLGW-2", "GQLGW-3", "GQLGW-4"]) { + | key + | hydration__assignee__assigneeId: assigneeId + | __typename__hydration__assignee: __typename + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "issuesByKeys": [ + | { + | "key": "GQLGW-2", + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/0", + | "__typename__hydration__assignee": "Issue" + | }, + | { + | "key": "GQLGW-3", + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/1", + | "__typename__hydration__assignee": "Issue" + | }, + | { + | "key": "GQLGW-4", + | "hydration__assignee__assigneeId": "ari:cloud:identity::user/10", + | "__typename__hydration__assignee": "Issue" + | } + | ] + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/0") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "errors": [ + | { + | "message": "No user: ari:cloud:identity::user/0", + | "extensions": { + | "classification": "UserNotFoundError" + | } + | } + | ], + | "data": { + | "userById": null + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/1") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "data": { + | "userById": { + | "name": "Frank" + | } + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ExpectedServiceCall( + service = "users", + query = """ + | { + | userById(id: "ari:cloud:identity::user/10") { + | name + | } + | } + """.trimMargin(), + variables = "{}", + result = """ + | { + | "errors": [ + | { + | "message": "No user: ari:cloud:identity::user/10", + | "extensions": { + | "classification": "UserNotFoundError" + | } + | } + | ], + | "data": { + | "userById": null + | } + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + ), + ), + ) + + /** + * ```json + * { + * "data": { + * "issuesByKeys": [ + * { + * "key": "GQLGW-2", + * "assignee": null + * }, + * { + * "key": "GQLGW-3", + * "assignee": { + * "name": "Frank" + * } + * }, + * { + * "key": "GQLGW-4", + * "assignee": null + * } + * ] + * }, + * "errors": [ + * { + * "message": "No user: ari:cloud:identity::user/0", + * "locations": [], + * "path": [ + * "issuesByKeys", + * 0, + * "assignee" + * ], + * "extensions": { + * "classification": "UserNotFoundError" + * } + * }, + * { + * "message": "No user: ari:cloud:identity::user/10", + * "locations": [], + * "path": [ + * "issuesByKeys", + * 2, + * "assignee" + * ], + * "extensions": { + * "classification": "UserNotFoundError" + * } + * } + * ] + * } + * ``` + */ + override val result: ExpectedNadelResult = ExpectedNadelResult( + result = """ + | { + | "data": { + | "issuesByKeys": [ + | { + | "key": "GQLGW-2" + | }, + | { + | "key": "GQLGW-3" + | }, + | { + | "key": "GQLGW-4" + | } + | ] + | }, + | "hasNext": true + | } + """.trimMargin(), + delayedResults = listOfJsonStrings( + """ + | { + | "hasNext": false, + | "incremental": [ + | { + | "path": [ + | "issuesByKeys", + | 0 + | ], + | "errors": [ + | { + | "message": "No user: ari:cloud:identity::user/0", + | "locations": [], + | "path": [ + | "issuesByKeys", + | 0, + | "assignee" + | ], + | "extensions": { + | "classification": "UserNotFoundError" + | } + | } + | ], + | "data": { + | "assignee": null + | } + | }, + | { + | "path": [ + | "issuesByKeys", + | 1 + | ], + | "data": { + | "assignee": { + | "name": "Frank" + | } + | } + | }, + | { + | "path": [ + | "issuesByKeys", + | 2 + | ], + | "errors": [ + | { + | "message": "No user: ari:cloud:identity::user/10", + | "locations": [], + | "path": [ + | "issuesByKeys", + | 2, + | "assignee" + | ], + | "extensions": { + | "classification": "UserNotFoundError" + | } + | } + | ], + | "data": { + | "assignee": null + | } + | } + | ] + | } + """.trimMargin(), + ), + ) +} diff --git a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsTestSnapshot.kt b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsTestSnapshot.kt index 8b087c980..c13da780b 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsTestSnapshot.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/next/fixtures/hydration/defer/HydrationDeferForwardsErrorsTestSnapshot.kt @@ -83,6 +83,10 @@ public class HydrationDeferForwardsErrorsTestSnapshot : TestSnapshot() { * { * "message": "No user: ari:cloud:identity::user/0", * "locations": [], + * "path": [ + * "issueByKey", + * "assignee" + * ], * "extensions": { * "classification": "UserNotFoundError" * } @@ -113,6 +117,10 @@ public class HydrationDeferForwardsErrorsTestSnapshot : TestSnapshot() { | { | "message": "No user: ari:cloud:identity::user/0", | "locations": [], + | "path": [ + | "issueByKey", + | "assignee" + | ], | "extensions": { | "classification": "UserNotFoundError" | } diff --git a/test/src/test/kotlin/graphql/nadel/tests/util/StriktUtil.kt b/test/src/test/kotlin/graphql/nadel/tests/util/StriktUtil.kt index 563e4c3f8..b694aa7f9 100644 --- a/test/src/test/kotlin/graphql/nadel/tests/util/StriktUtil.kt +++ b/test/src/test/kotlin/graphql/nadel/tests/util/StriktUtil.kt @@ -7,6 +7,7 @@ import graphql.nadel.engine.util.JsonMap import graphql.nadel.tests.assertJsonKeys import strikt.api.Assertion +@Deprecated("Do not use") fun , K, V> Assertion.Builder.keysEqual(expectedKeys: Collection): Assertion.Builder { compose("keys match expected") { actual -> val actualKeys = actual.keys @@ -33,21 +34,25 @@ fun , K, V> Assertion.Builder.keysEqual(expectedKeys: Collectio return this } +@Deprecated("Do not use") val Assertion.Builder.extensions: Assertion.Builder get() { return get { extensions as AnyMap }.assertJsonKeys() } +@Deprecated("Do not use") val Assertion.Builder.errors: Assertion.Builder> get() { return get { errors } } +@Deprecated("Do not use") val Assertion.Builder.message: Assertion.Builder get() { return get { message } } +@Deprecated("Do not use") val Assertion.Builder.data: Assertion.Builder get() { return get { @@ -55,12 +60,7 @@ val Assertion.Builder.data: Assertion.Builder } } -fun Assertion.Builder.getHashCode(): Assertion.Builder { - return get { - hashCode() - } -} - +@Deprecated("Do not use") fun Assertion.Builder.getToString(): Assertion.Builder { return get { toString() From 994970121f833e4983dbe8f8cc11371e21619119 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Tue, 7 May 2024 14:19:58 +1200 Subject: [PATCH 08/22] Delete comment --- .../graphql/nadel/engine/transform/result/json/JsonNodes.kt | 3 --- 1 file changed, 3 deletions(-) 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 272fd2403..fd7d85c14 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 @@ -22,9 +22,6 @@ class JsonNodes( return getNodesAt(rootNode, queryPath, flatten) } - /** - * todo: can this be a sequence? - */ /** * Extracts the nodes at the given query selection path. */ From 5ed2b95147969413a3f8262b872f7eb82075e2d3 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Tue, 7 May 2024 14:38:30 +1200 Subject: [PATCH 09/22] Clean up code --- .../nadel/result/NadelResultTracker.kt | 43 ++----------------- 1 file changed, 3 insertions(+), 40 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt b/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt index 905a8dbbb..c60d5343b 100644 --- a/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt +++ b/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt @@ -48,40 +48,9 @@ internal class NadelResultTracker { val queue = mutableListOf(data) - fun printState(): String { - return """ - Current query path: ${ - currentQueryPathSegments.joinToString( - separator = ",", - prefix = "[", - postfix = "]" - ) - } - Current result path: ${ - currentResultPathSegments.joinToString( - separator = ",", - prefix = "[", - postfix = "]" - ) - } - """.replaceIndent(' '.toString().repeat(n = 4)) - } - - println( - "Trying to find $node from ${ - queryPath.segments.joinToString( - separator = ",", - prefix = "[", - postfix = "]" - ) - }" - ) - while (queue.isNotEmpty()) { val element = queue.removeLast() - println("Visiting $element") if (element === node.value) { - println("Got em $currentResultPathSegments") return currentResultPathSegments } @@ -101,7 +70,7 @@ internal class NadelResultTracker { val nextElement = element[nextQueryPathSegment] if (nextElement == null) { - NavigationOutcome.DeadEnd // todo: tests + NavigationOutcome.DeadEnd } else { queue.add(nextElement) currentResultPathSegments.add(NadelResultPathSegment.Object(nextQueryPathSegment)) @@ -109,10 +78,10 @@ internal class NadelResultTracker { NavigationOutcome.QueuedChild } } else { - NavigationOutcome.DeadEnd // todo: tests + NavigationOutcome.DeadEnd } } - else -> NavigationOutcome.DeadEnd // todo: tests + else -> NavigationOutcome.DeadEnd } when (outcome) { @@ -129,10 +98,8 @@ internal class NadelResultTracker { is NadelResultPathSegment.Array -> { if (last.index == 0) { // Nothing more to visit in the array, remember that we traverse end -> front - println("Popping dead end array") currentResultPathSegments.removeLast() } else { - println("Moving to next array element ${last.index - 1}") // We're moving to the next element currentResultPathSegments[currentResultPathSegments.lastIndex] = NadelResultPathSegment.Array(last.index - 1) @@ -141,7 +108,6 @@ internal class NadelResultTracker { } } is NadelResultPathSegment.Object -> { - println("Popping up dead end object") currentResultPathSegments.removeLast() currentQueryPathSegments.removeLast() } @@ -150,11 +116,8 @@ internal class NadelResultTracker { } } } - - println("Loop end\n${printState()}") } - println("Didn't get em") return null } From 18c83101e3f43d8a12999af62d8ee019f845a121 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Thu, 9 May 2024 14:10:09 +1200 Subject: [PATCH 10/22] Improvements + refactoring after benchmarking --- .../result/NadelResultTransformer.kt | 21 +- .../engine/transform/result/json/JsonNode.kt | 1 + .../transform/result/json/JsonNodeIterator.kt | 191 ++++++ .../engine/transform/result/json/JsonNodes.kt | 47 +- .../nadel/result/NadelResultPathSegment.kt | 10 +- .../nadel/result/NadelResultTracker.kt | 95 +-- .../result/json/JsonNodeIteratorTest.kt | 600 ++++++++++++++++++ 7 files changed, 853 insertions(+), 112 deletions(-) create mode 100644 lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodeIterator.kt create mode 100644 lib/src/test/kotlin/graphql/nadel/engine/transform/result/json/JsonNodeIteratorTest.kt 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 e13f25e91..ef8a67459 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 @@ -56,7 +56,7 @@ internal class NadelResultTransformer(private val executionBlueprint: NadelOvera deferredInstructions.add( async { - getRemoveArtificialFieldInstructions(artificialFields, nodes) + getRemoveArtificialFieldInstructions(executionContext, artificialFields, nodes) }, ) } @@ -108,21 +108,24 @@ internal class NadelResultTransformer(private val executionBlueprint: NadelOvera } private fun getRemoveArtificialFieldInstructions( + executionContext: NadelExecutionContext, artificialFields: List, nodes: JsonNodes, ): List { return artificialFields .asSequence() .flatMap { field -> - nodes.getNodesAt( - queryPath = field.queryPath.dropLast(1), - flatten = true, - ).map { parentNode -> - NadelResultInstruction.Remove( - subject = parentNode, - key = NadelResultKey(field.resultKey), + nodes + .getNodesAt( + queryPath = field.queryPath.dropLast(1), + flatten = true, ) - } + .map { parentNode -> + NadelResultInstruction.Remove( + subject = parentNode, + key = NadelResultKey(field.resultKey), + ) + } } .toList() } 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 84a355728..100f86fc2 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 @@ -2,6 +2,7 @@ package graphql.nadel.engine.transform.result.json import graphql.nadel.engine.util.AnyList import graphql.nadel.engine.util.AnyMap +import graphql.nadel.result.NadelResultPathSegment /** * todo: one day split this into a sealed class that acts like a union type for each possible type diff --git a/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodeIterator.kt b/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodeIterator.kt new file mode 100644 index 000000000..149e5385c --- /dev/null +++ b/lib/src/main/java/graphql/nadel/engine/transform/result/json/JsonNodeIterator.kt @@ -0,0 +1,191 @@ +package graphql.nadel.engine.transform.result.json + +import graphql.nadel.engine.transform.query.NadelQueryPath +import graphql.nadel.engine.transform.result.json.EphemeralJsonNode.Companion.component1 +import graphql.nadel.engine.transform.result.json.EphemeralJsonNode.Companion.component2 +import graphql.nadel.engine.transform.result.json.EphemeralJsonNode.Companion.component3 +import graphql.nadel.engine.util.AnyList +import graphql.nadel.engine.util.AnyMap +import graphql.nadel.result.NadelResultPathSegment + +/** + * In theory what should be the [JsonNodeExtractor] replacement. + * + * Though, replacement is a tall order they have different iteration patterns. + * + * Because of that, things like GraphQL queries to underlying services are different + * and the tests need to be updated. + * + * So for now, I'll leave this in here in case a new feature uses it in the future. + */ +internal class IteratingJsonNodes( + private val data: Any?, +) : JsonNodes { + override fun getNodesAt(queryPath: NadelQueryPath, flatten: Boolean): List { + val iterator = JsonNodeIterator( + root = data, + queryPath = queryPath, + flatten = flatten, + ) + + // So, I actually tested and using a Sequence here is somehow significantly slower + // So let's stick with the good ol for loop + val results = mutableListOf() + iterator.forEach { (q, r, e) -> + if (q.size == queryPath.segments.size) { + results.add(JsonNode(e)) + } + } + + return results + } +} + +/** + * A JSON node [value] with [queryPath] and [resultPath] values. + * + * You should not store the [EphemeralJsonNode] as there is only one instance. + * Its values will change, but the enclosing [EphemeralJsonNode] instance iis the same. + * + * The values should not be stored either. + * + * This exists because of performance reasons, we avoid object allocations and reuse + * the same [List] values to avoid creating multiple arrays. + * + * The majority of the time the [resultPath] values are discarded anyway. + */ +internal abstract class EphemeralJsonNode { + abstract val queryPath: List + abstract val resultPath: List + abstract val value: Any? + + companion object { + operator fun EphemeralJsonNode.component1(): List = queryPath + operator fun EphemeralJsonNode.component2(): List = resultPath + operator fun EphemeralJsonNode.component3(): Any? = value + } +} + +/** + * Does a DFS search through the response to the given `queryPath`. + */ +internal class JsonNodeIterator( + root: Any?, + queryPath: NadelQueryPath, + private val flatten: Boolean, +) : Iterator { + private var hasNext = true + + override fun hasNext(): Boolean { + return hasNext || calculateNext() + } + + override fun next(): EphemeralJsonNode { + if (!hasNext && !calculateNext()) { + throw NoSuchElementException() + } + + hasNext = false + ephemeralJsonNode.value = parents.last() + + return ephemeralJsonNode + } + + private val queryPathSegments = queryPath.segments + private val currentQueryPathSegments: MutableList = ArrayList(queryPathSegments.size) + private val currentResultPathSegments: MutableList = ArrayList(queryPathSegments.size + 6) + + companion object { + private val NONE = Any() + } + + private val ephemeralJsonNode = object : EphemeralJsonNode() { + override val queryPath get() = currentQueryPathSegments + override val resultPath get() = currentResultPathSegments + override var value: Any? = NONE + } + + /** + * These are the parents of the current element, and includes the current element + * at the end of a traversal iteration. + */ + private val parents: MutableList = ArrayList(queryPathSegments.size + 6).also { + it.add(root) + } + + private val objectPathSegmentCache = queryPathSegments.associateWith(NadelResultPathSegment::Object) + + private fun calculateNext(): Boolean { + val advanced: Boolean = when (val current = parents.last()) { + is AnyList -> { + if (currentQueryPathSegments.size == queryPathSegments.size && !flatten) { + // Shortcut to avoid traversing children at all if not asked for + false + } else { + if (current.isEmpty()) { + false + } else { + if (currentResultPathSegments.lastIndex < parents.lastIndex) { + // Traverse children + currentResultPathSegments.add(NadelResultPathSegment.Array(current.lastIndex)) + } + + val arraySegment = currentResultPathSegments[parents.lastIndex] as NadelResultPathSegment.Array + parents.add(current[arraySegment.index]) + true + } + } + } + is AnyMap -> { + if (currentQueryPathSegments.size < queryPathSegments.size) { + val nextQueryPathSegment = queryPathSegments[currentQueryPathSegments.lastIndex + 1] + val nextElement = current.getOrDefault(nextQueryPathSegment, NONE) + if (nextElement === NONE) { + false + } else { + currentQueryPathSegments.add(nextQueryPathSegment) + currentResultPathSegments.add(objectPathSegmentCache[nextQueryPathSegment]!!) + parents.add(nextElement) + true + } + } else { + false + } + } + else -> { + false + } + } + + if (!advanced) { + while (currentResultPathSegments.isNotEmpty()) { + val last = currentResultPathSegments.lastOrNull() ?: break + + when (last) { + is NadelResultPathSegment.Array -> { + if (last.index == 0) { + // Nothing more to visit in the array, remember that we traverse end -> front + currentResultPathSegments.removeLast() + parents.removeLast() + } else { + // We're moving to the next element + currentResultPathSegments[currentResultPathSegments.lastIndex] = + NadelResultPathSegment.Array(last.index - 1) + parents.removeLast() + // Iterate to next element + return calculateNext() + } + } + is NadelResultPathSegment.Object -> { + currentResultPathSegments.removeLast() + currentQueryPathSegments.removeLast() + parents.removeLast() + } + } + } + } + + hasNext = currentResultPathSegments.isNotEmpty() + return hasNext + } +} 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 fd7d85c14..d2269e090 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 @@ -6,18 +6,37 @@ import graphql.nadel.engine.util.AnyMap import graphql.nadel.engine.util.JsonMap import java.util.concurrent.ConcurrentHashMap +/** + * Use [CachingJsonNodes] for the most part because that is faster. + * + * In an ideal world we switch to + */ +interface JsonNodes { + /** + * Extracts the nodes at the given query selection path. + */ + fun getNodesAt(queryPath: NadelQueryPath, flatten: Boolean = false): List + + companion object { + internal var nodesFactory: (JsonMap) -> JsonNodes = { + CachingJsonNodes(it) + } + + operator fun invoke(data: JsonMap): JsonNodes { + return nodesFactory(data) + } + } +} + /** * Utility class to extract data out of the given [data]. */ -class JsonNodes( +class CachingJsonNodes( private val data: JsonMap, -) { +) : JsonNodes { private val nodes = ConcurrentHashMap>() - /** - * Extracts the nodes at the given query selection path. - */ - fun getNodesAt(queryPath: NadelQueryPath, flatten: Boolean = false): List { + override fun getNodesAt(queryPath: NadelQueryPath, flatten: Boolean): List { val rootNode = JsonNode(data) return getNodesAt(rootNode, queryPath, flatten) } @@ -66,15 +85,15 @@ class JsonNodes( flattenLists: Boolean, ): Sequence { val value = map[segment] - // We flatten lists as these nodes contribute to the BFS queue if (value is AnyList && flattenLists) { return getFlatNodes(value) } - - return sequenceOf( - JsonNode(value), + val node = JsonNode( + value = value, ) + + return sequenceOf(node) } /** @@ -89,15 +108,15 @@ class JsonNodes( * * etc. */ - private fun getFlatNodes(values: AnyList): Sequence { + private fun getFlatNodes( + values: AnyList, + ): Sequence { return values .asSequence() .flatMap { value -> when (value) { is AnyList -> getFlatNodes(value) - else -> sequenceOf( - JsonNode(value), - ) + else -> sequenceOf(JsonNode(value = value)) } } } diff --git a/lib/src/main/java/graphql/nadel/result/NadelResultPathSegment.kt b/lib/src/main/java/graphql/nadel/result/NadelResultPathSegment.kt index 16b396717..aca970371 100644 --- a/lib/src/main/java/graphql/nadel/result/NadelResultPathSegment.kt +++ b/lib/src/main/java/graphql/nadel/result/NadelResultPathSegment.kt @@ -5,5 +5,13 @@ internal sealed interface NadelResultPathSegment { value class Object(val key: String) : NadelResultPathSegment @JvmInline - value class Array(val index: Int) : NadelResultPathSegment + value class Array private constructor(val index: Int) : NadelResultPathSegment { + companion object { + private val cache = List(2000, ::Array) + + operator fun invoke(index: Int): Array { + return cache.getOrNull(index) ?: Array(index) + } + } + } } diff --git a/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt b/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt index c60d5343b..ffff2fa30 100644 --- a/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt +++ b/lib/src/main/java/graphql/nadel/result/NadelResultTracker.kt @@ -1,11 +1,9 @@ package graphql.nadel.result import graphql.ExecutionResult -import graphql.incremental.DelayedIncrementalPartialResult import graphql.nadel.engine.transform.query.NadelQueryPath import graphql.nadel.engine.transform.result.json.JsonNode -import graphql.nadel.engine.util.AnyList -import graphql.nadel.engine.util.AnyMap +import graphql.nadel.engine.transform.result.json.JsonNodeIterator import kotlinx.coroutines.CompletableDeferred /** @@ -14,12 +12,6 @@ import kotlinx.coroutines.CompletableDeferred internal class NadelResultTracker { private val result = CompletableDeferred() - private enum class NavigationOutcome { - QueuedChild, - ExpandedArray, - DeadEnd, - } - /** * So… in Nadel the result can change a lot. * @@ -40,81 +32,13 @@ internal class NadelResultTracker { node: JsonNode, ): List? { val result = result.await() - val data = result.getData() - - val queryPathSegments = queryPath.segments - val currentResultPathSegments = mutableListOf() - val currentQueryPathSegments = mutableListOf() - - val queue = mutableListOf(data) - - while (queue.isNotEmpty()) { - val element = queue.removeLast() - if (element === node.value) { - return currentResultPathSegments - } + val data = result.toSpecification()["data"] - val outcome: NavigationOutcome = when (element) { - is AnyList -> { - if (element.isNotEmpty()) { - queue.addAll(element) - currentResultPathSegments.add(NadelResultPathSegment.Array(element.lastIndex)) - NavigationOutcome.ExpandedArray - } else { - NavigationOutcome.DeadEnd - } - } - is AnyMap -> { - if (currentQueryPathSegments.size < queryPathSegments.size) { - val nextQueryPathSegment = queryPathSegments[currentQueryPathSegments.size] - val nextElement = element[nextQueryPathSegment] - - if (nextElement == null) { - NavigationOutcome.DeadEnd - } else { - queue.add(nextElement) - currentResultPathSegments.add(NadelResultPathSegment.Object(nextQueryPathSegment)) - currentQueryPathSegments.add(nextQueryPathSegment) - NavigationOutcome.QueuedChild - } - } else { - NavigationOutcome.DeadEnd - } - } - else -> NavigationOutcome.DeadEnd - } - - when (outcome) { - NavigationOutcome.QueuedChild -> { - } - NavigationOutcome.ExpandedArray -> { - } - NavigationOutcome.DeadEnd -> { - if (queue.isNotEmpty()) { // i.e. we have other array elements to visit - while (currentResultPathSegments.isNotEmpty()) { - val last = currentResultPathSegments.lastOrNull() ?: break - - when (last) { - is NadelResultPathSegment.Array -> { - if (last.index == 0) { - // Nothing more to visit in the array, remember that we traverse end -> front - currentResultPathSegments.removeLast() - } else { - // We're moving to the next element - currentResultPathSegments[currentResultPathSegments.lastIndex] = - NadelResultPathSegment.Array(last.index - 1) - // Nothing further to fix, stop - break - } - } - is NadelResultPathSegment.Object -> { - currentResultPathSegments.removeLast() - currentQueryPathSegments.removeLast() - } - } - } - } - } + val jsonNodeIterator = JsonNodeIterator(root = data, queryPath = queryPath, flatten = true) + for (ephemeralNode in jsonNodeIterator) { + if (ephemeralNode.queryPath.size == queryPath.segments.size && ephemeralNode.value === node.value) { + // Clone because underlying values are ephemeral too + return ephemeralNode.resultPath.toList() } } @@ -124,9 +48,4 @@ internal class NadelResultTracker { fun complete(value: ExecutionResult) { result.complete(value) } - - fun complete(value: DelayedIncrementalPartialResult) { - // result.complete(value) - // todo: track here - } } diff --git a/lib/src/test/kotlin/graphql/nadel/engine/transform/result/json/JsonNodeIteratorTest.kt b/lib/src/test/kotlin/graphql/nadel/engine/transform/result/json/JsonNodeIteratorTest.kt new file mode 100644 index 000000000..3dddc8cef --- /dev/null +++ b/lib/src/test/kotlin/graphql/nadel/engine/transform/result/json/JsonNodeIteratorTest.kt @@ -0,0 +1,600 @@ +package graphql.nadel.engine.transform.result.json + +import com.fasterxml.jackson.module.kotlin.readValue +import graphql.nadel.engine.transform.query.NadelQueryPath +import graphql.nadel.engine.util.JsonMap +import graphql.nadel.jsonObjectMapper +import graphql.nadel.result.NadelResultPathBuilder +import graphql.nadel.result.NadelResultPathSegment +import org.junit.jupiter.api.Test +import kotlin.test.assertTrue + +class JsonNodeIteratorTest { + private data class TraversedJsonNode( + override val queryPath: List, + override val resultPath: List, + override val value: Any?, + ) : EphemeralJsonNode() { + constructor(other: EphemeralJsonNode) : this( + queryPath = other.queryPath.toList(), + resultPath = other.resultPath.toList(), + value = other.value, + ) + } + + @Test + fun traverseObjects() { + val root = jsonObjectMapper.readValue( + // language=JSON + """ + { + "users": { + "id": "100", + "friend": { + "id": "100", + "phoneNumber": { + "value": "+61" + } + }, + "email": "@.com" + } + } + """.trimIndent(), + ) + + val expectedTraversals = listOf( + TraversedJsonNode( + queryPath = emptyList(), + resultPath = emptyList(), + value = root, + ), + TraversedJsonNode( + queryPath = listOf("users"), + resultPath = NadelResultPathBuilder() + .add("users") + .build(), + value = jsonObjectMapper.readValue( + """{"id": "100", "friend": {"id": "100", "phoneNumber": {"value": "+61"}}, "email": "@.com"}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("users", "friend"), + resultPath = NadelResultPathBuilder() + .add("users") + .add("friend") + .build(), + value = jsonObjectMapper.readValue( + """{"id": "100", "phoneNumber": {"value": "+61"}}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("users", "friend", "phoneNumber"), + resultPath = NadelResultPathBuilder() + .add("users") + .add("friend") + .add("phoneNumber") + .build(), + value = jsonObjectMapper.readValue( + """{"value": "+61"}""", + ), + ), + ) + + val iterator = JsonNodeIterator( + root = root, + queryPath = NadelQueryPath(listOf("users", "friend", "phoneNumber")), + flatten = true, + ) + + // When + val traversed = iterator + .asSequence() + .map(::TraversedJsonNode) + .toList() + + // Then + val uniqueQueryPaths = traversed.mapTo(LinkedHashSet()) { it.queryPath } + val uniqueResultPaths = traversed.mapTo(LinkedHashSet()) { it.resultPath } + assertTrue(uniqueResultPaths.size == traversed.size) + assertTrue(uniqueQueryPaths.size == traversed.size) + + assertTrue(traversed.size == expectedTraversals.size) + traversed.zip(expectedTraversals) + .forEach { (actual, expected) -> + assertTrue(actual == expected) + } + } + + @Test + fun traverseNull() { + val root = jsonObjectMapper.readValue( + // language=JSON + """ + { + "users": { + "id": "100", + "friend": null, + "email": "@.com" + } + } + """.trimIndent(), + ) + + val expectedTraversals = listOf( + TraversedJsonNode( + queryPath = emptyList(), + resultPath = emptyList(), + value = root, + ), + TraversedJsonNode( + queryPath = listOf("users"), + resultPath = NadelResultPathBuilder() + .add("users") + .build(), + value = jsonObjectMapper.readValue( + """{"id": "100", "friend": null, "email": "@.com"}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("users", "friend"), + resultPath = NadelResultPathBuilder() + .add("users") + .add("friend") + .build(), + value = null, + ), + ) + + val iterator = JsonNodeIterator( + root = root, + queryPath = NadelQueryPath(listOf("users", "friend", "phoneNumber")), + flatten = true, + ) + + // When + val traversed = iterator + .asSequence() + .map(::TraversedJsonNode) + .toList() + + // Then + val uniqueQueryPaths = traversed.mapTo(LinkedHashSet()) { it.queryPath } + val uniqueResultPaths = traversed.mapTo(LinkedHashSet()) { it.resultPath } + assertTrue(uniqueResultPaths.size == traversed.size) + assertTrue(uniqueQueryPaths.size == traversed.size) + + assertTrue(traversed.size == expectedTraversals.size) + traversed.zip(expectedTraversals) + .forEach { (actual, expected) -> + assertTrue(actual == expected) + } + } + + @Test + fun traverseArrays() { + val root = jsonObjectMapper.readValue( + // language=JSON + """ + { + "activities": { + "workedOn": [ + { + "data": { + "id": 1 + } + }, + { + }, + { + "value": { + "friend": null + } + }, + { + "data": { + "friend": { + "id": 10 + } + } + } + ] + } + } + """.trimIndent(), + ) + + val expectedTraversals = listOf( + TraversedJsonNode( + queryPath = emptyList(), + resultPath = NadelResultPathBuilder().build(), + value = root + ), + TraversedJsonNode( + queryPath = listOf("activities"), + resultPath = NadelResultPathBuilder() + .add("activities") + .build(), + value = jsonObjectMapper.readValue( + """{"workedOn": [{"data": {"id": 1}}, {}, {"value": {"friend": null}}, {"data": {"friend": {"id": 10}}}]}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .build(), + value = jsonObjectMapper.readValue( + """[{"data": {"id": 1}}, {}, {"value": {"friend": null}}, {"data": {"friend": {"id": 10}}}]""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(3) + .build(), + value = jsonObjectMapper.readValue( + """{"data": {"friend": {"id": 10}}}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn", "data"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(3) + .add("data") + .build(), + value = jsonObjectMapper.readValue( + """{"friend": {"id": 10}}""" + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn", "data", "friend"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(3) + .add("data") + .add("friend") + .build(), + value = jsonObjectMapper.readValue( + """{"id": 10}""" + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(2) + .build(), + value = jsonObjectMapper.readValue( + """{"value": {"friend": null}}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(1) + .build(), + value = jsonObjectMapper.readValue( + "{}", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(0) + .build(), + value = jsonObjectMapper.readValue( + """{"data": {"id": 1}}""", + ) + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn", "data"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(0) + .add("data") + .build(), + value = jsonObjectMapper.readValue( + """{"id": 1}""", + ), + ), + ) + + val iterator = JsonNodeIterator( + root = root, + queryPath = NadelQueryPath(listOf("activities", "workedOn", "data", "friend")), + flatten = true, + ) + + // When + val traversed = iterator + .asSequence() + .map(::TraversedJsonNode) + .toList() + + // Then + val uniqueResultPaths = traversed.mapTo(LinkedHashSet()) { it.resultPath } + assertTrue(uniqueResultPaths.size == traversed.size) + + assertTrue(traversed.size == expectedTraversals.size) + traversed.zip(expectedTraversals) + .forEach { (actual, expected) -> + assertTrue(actual == expected) + } + } + + @Test + fun traverseNestedArrays() { + val root = jsonObjectMapper.readValue( + // language=JSON + """ + { + "activities": { + "workedOn": [ + [ + { + "data": { + "id": 1 + } + } + ], + [], + [ + { + }, + { + "value": { + "friend": null + } + }, + [ + { + "data": null + } + ] + ], + [ + { + "data": { + "friend": { + "id": 10 + } + } + } + ] + ] + } + } + """.trimIndent(), + ) + + val expectedTraversals = listOf( + TraversedJsonNode( + queryPath = emptyList(), + resultPath = NadelResultPathBuilder().build(), + value = root + ), + TraversedJsonNode( + queryPath = listOf("activities"), + resultPath = NadelResultPathBuilder() + .add("activities") + .build(), + value = jsonObjectMapper.readValue( + """{"workedOn":[[{"data":{"id":1}}],[],[{},{"value":{"friend":null}},[{"data":null}]],[{"data":{"friend":{"id":10}}}]]}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .build(), + value = jsonObjectMapper.readValue( + """[[{"data":{"id":1}}],[],[{},{"value":{"friend":null}},[{"data":null}]],[{"data":{"friend":{"id":10}}}]]""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(3) + .build(), + value = jsonObjectMapper.readValue( + """[{"data":{"friend":{"id":10}}}]""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(3) + .add(0) + .build(), + value = jsonObjectMapper.readValue( + """{"data":{"friend":{"id":10}}}""" + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn", "data"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(3) + .add(0) + .add("data") + .build(), + value = jsonObjectMapper.readValue( + """{"friend": {"id": 10}}""" + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn", "data", "friend"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(3) + .add(0) + .add("data") + .add("friend") + .build(), + value = jsonObjectMapper.readValue( + """{"id": 10}""" + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(2) + .build(), + value = jsonObjectMapper.readValue( + """[{},{"value":{"friend":null}},[{"data":null}]]""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(2) + .add(2) + .build(), + value = jsonObjectMapper.readValue( + """[{"data":null}]""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(2) + .add(2) + .add(0) + .build(), + value = jsonObjectMapper.readValue( + """{"data":null}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn", "data"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(2) + .add(2) + .add(0) + .add("data") + .build(), + value = null, + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(2) + .add(1) + .build(), + value = jsonObjectMapper.readValue( + """{"value":{"friend":null}}""" + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(2) + .add(0) + .build(), + value = jsonObjectMapper.readValue( + "{}", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(1) + .build(), + value = jsonObjectMapper.readValue( + "[]", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(0) + .build(), + value = jsonObjectMapper.readValue( + """[{"data":{"id":1}}]""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(0) + .add(0) + .build(), + value = jsonObjectMapper.readValue( + """{"data":{"id":1}}""", + ), + ), + TraversedJsonNode( + queryPath = listOf("activities", "workedOn", "data"), + resultPath = NadelResultPathBuilder() + .add("activities") + .add("workedOn") + .add(0) + .add(0) + .add("data") + .build(), + value = jsonObjectMapper.readValue( + """{"id":1}""", + ), + ), + ) + + val iterator = JsonNodeIterator( + root = root, + queryPath = NadelQueryPath(listOf("activities", "workedOn", "data", "friend")), + flatten = true, + ) + + // When + val traversed = iterator + .asSequence() + .map(::TraversedJsonNode) + .toList() + + // Then + val uniqueResultPaths = traversed.mapTo(LinkedHashSet()) { it.resultPath } + assertTrue(uniqueResultPaths.size == traversed.size) + + assertTrue(traversed.size == expectedTraversals.size) + traversed.zip(expectedTraversals) + .forEach { (actual, expected) -> + assertTrue(actual == expected) + } + } +} From a62f5da150c6510f814851fe20816f29ae36c8cd Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Wed, 15 May 2024 11:17:18 +1000 Subject: [PATCH 11/22] Update run config --- ...{CaptureTestData.run.xml => Update Test Snapshots.run.xml} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename .run/{CaptureTestData.run.xml => Update Test Snapshots.run.xml} (70%) diff --git a/.run/CaptureTestData.run.xml b/.run/Update Test Snapshots.run.xml similarity index 70% rename from .run/CaptureTestData.run.xml rename to .run/Update Test Snapshots.run.xml index fbf78b46b..dbff67a7e 100644 --- a/.run/CaptureTestData.run.xml +++ b/.run/Update Test Snapshots.run.xml @@ -1,6 +1,6 @@ - -