From d97189dee3b84099e30ddcb0b1229987c3471466 Mon Sep 17 00:00:00 2001 From: "Michael D. Norman" Date: Wed, 16 Jan 2019 21:36:49 -0600 Subject: [PATCH 1/3] [issue-50] added failing query test along with a passing parser test --- .../kgraphql/integration/QueryTest.kt | 29 +++++++++++++++ .../kgraphql/request/DocumentParserTest.kt | 35 +++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/src/test/kotlin/com/github/pgutkowski/kgraphql/integration/QueryTest.kt b/src/test/kotlin/com/github/pgutkowski/kgraphql/integration/QueryTest.kt index 19589ffa..7b49d530 100644 --- a/src/test/kotlin/com/github/pgutkowski/kgraphql/integration/QueryTest.kt +++ b/src/test/kotlin/com/github/pgutkowski/kgraphql/integration/QueryTest.kt @@ -171,6 +171,35 @@ class QueryTest : BaseSchemaTest() { assertThat(map.extract("data/film/director/age"), equalTo(prestige.director.age)) } + @Test + fun `query with nested external fragment`(){ + val map = execute(""" + { + film { + title + ...dir + } + } + + fragment dir on Film { + director { + name + } + ...dirAge + } + + fragment dirAge on Film { + director { + age + } + } + """.trimIndent()) + assertNoErrors(map) + assertThat(map.extract("data/film/title"), equalTo(prestige.title)) + assertThat(map.extract("data/film/director/name"), equalTo(prestige.director.name)) + assertThat(map.extract("data/film/director/age"), equalTo(prestige.director.age)) + } + @Test fun `query with missing selection set`(){ expect("Missing selection set on property film of type Film"){ diff --git a/src/test/kotlin/com/github/pgutkowski/kgraphql/request/DocumentParserTest.kt b/src/test/kotlin/com/github/pgutkowski/kgraphql/request/DocumentParserTest.kt index 212924c0..9a39ca2a 100644 --- a/src/test/kotlin/com/github/pgutkowski/kgraphql/request/DocumentParserTest.kt +++ b/src/test/kotlin/com/github/pgutkowski/kgraphql/request/DocumentParserTest.kt @@ -226,4 +226,39 @@ class DocumentParserTest { ) assertThat(map.first().selectionTree, equalTo(expected)) } + + @Test + fun `nested fragment parsing`() { + val map = graphParser.parseDocument(""" + { + hero { + id + ...heroName + } + } + + fragment heroName on Hero { + name { + real + } + ...heroNameDetails + } + + fragment heroNameDetails on Hero { + name { + asHero + } + } + """.trimIndent()) + val expected = SelectionTree( + branch("hero", + leaf("id"), + extFragment("...heroName", "Hero", + branch("name", *leafs("real")), + extFragment("...heroNameDetails", "Hero", branch("name", *leafs("asHero"))) + ) + ) + ) + assertThat(map.first().selectionTree, equalTo(expected)) + } } \ No newline at end of file From 23de4b5d7a86e8a5c88f60d57a4c24ecdbe85691 Mon Sep 17 00:00:00 2001 From: "Michael D. Norman" Date: Thu, 17 Jan 2019 07:35:32 -0600 Subject: [PATCH 2/3] [issue-50] support nesting fragments --- .../kgraphql/schema/execution/Execution.kt | 2 +- .../kgraphql/schema/execution/Merge.kt | 36 +++++++++++++++++++ .../execution/ParallelRequestExecutor.kt | 7 +++- .../schema/structure2/RequestInterpreter.kt | 29 ++++++++------- .../kgraphql/integration/QueryTest.kt | 6 +++- 5 files changed, 62 insertions(+), 18 deletions(-) create mode 100644 src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/Merge.kt diff --git a/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/Execution.kt b/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/Execution.kt index 4d6ef600..f8706592 100644 --- a/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/Execution.kt +++ b/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/Execution.kt @@ -24,7 +24,7 @@ sealed class Execution { class Fragment( val condition: TypeCondition, - val elements : List, + val elements : List, val directives: Map? ) : Execution() diff --git a/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/Merge.kt b/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/Merge.kt new file mode 100644 index 00000000..a2416b46 --- /dev/null +++ b/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/Merge.kt @@ -0,0 +1,36 @@ +package com.github.pgutkowski.kgraphql.schema.execution + +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.node.ObjectNode + +fun MutableMap.merge(key: String, node: JsonNode?): MutableMap { + merge(key, node, this::get, this::set) + return this +} + +fun ObjectNode.merge(other: ObjectNode) { + other.fields().forEach { + merge(it.key, it.value) + } +} + +fun ObjectNode.merge(key: String, node: JsonNode?) { + merge(key, node, this::get, this::set) +} + +fun merge(key: String, node: JsonNode?, get: (String) -> JsonNode?, set: (String, JsonNode?) -> Any?) { + val existingNode = get(key) + if (existingNode != null) { + when { + node == null -> throw IllegalStateException("trying to merge null with non-null for $key") + node is ObjectNode -> { + check(existingNode is ObjectNode) { "trying to merge object with simple node for $key" } + existingNode.merge(node) + } + existingNode is ObjectNode -> throw IllegalStateException("trying to merge simple node with object node for $key") + node != existingNode -> throw IllegalStateException("trying to merge different simple nodes for $key") + } + } else { + set(key, node) + } +} diff --git a/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/ParallelRequestExecutor.kt b/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/ParallelRequestExecutor.kt index 1b557e65..24bf78c9 100644 --- a/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/ParallelRequestExecutor.kt +++ b/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/execution/ParallelRequestExecutor.kt @@ -222,7 +222,12 @@ class ParallelRequestExecutor(val schema: DefaultSchema) : RequestExecutor, Coro if (include) { if (expectedType.kind == TypeKind.OBJECT || expectedType.kind == TypeKind.INTERFACE) { if (expectedType.isInstance(value)) { - return container.elements.map { handleProperty(ctx, value, it, expectedType) }.toMap() + return container.elements.flatMap { child -> + when (child) { + is Execution.Fragment -> handleFragment(ctx, value, child).toList() + else -> listOf(handleProperty(ctx, value, child, expectedType)) + } + }.fold(mutableMapOf()) { map, entry -> map.merge(entry.first, entry.second) } } } else { throw IllegalStateException("fragments can be specified on object types, interfaces, and unions") diff --git a/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/structure2/RequestInterpreter.kt b/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/structure2/RequestInterpreter.kt index 1d3a8902..b1d37e49 100644 --- a/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/structure2/RequestInterpreter.kt +++ b/src/main/kotlin/com/github/pgutkowski/kgraphql/schema/structure2/RequestInterpreter.kt @@ -59,21 +59,8 @@ class RequestInterpreter(val schemaModel: SchemaModel) { return children } - private fun handleReturnTypeChildOrFragment(node: SelectionNode, returnType: Type): Execution { - val unwrappedType = returnType.unwrapped() - - return when(node){ - is Fragment -> { - val conditionType = findFragmentType(node, unwrappedType) - val condition = TypeCondition(conditionType) - val elements = node.fragmentGraph.map { conditionType.handleSelection(it) } - Execution.Fragment(condition, elements, node.directives?.lookup()) - } - else -> { - unwrappedType.handleSelection(node) - } - } - } + private fun handleReturnTypeChildOrFragment(node: SelectionNode, returnType: Type) = + returnType.unwrapped().handleSelectionFieldOrFragment(node) private fun findFragmentType(fragment: Fragment, enclosingType: Type) : Type { when(fragment){ @@ -91,6 +78,18 @@ class RequestInterpreter(val schemaModel: SchemaModel) { } } + private fun Type.handleSelectionFieldOrFragment(node: SelectionNode): Execution = when(node) { + is Fragment -> { + val conditionType = findFragmentType(node, this) + val condition = TypeCondition(conditionType) + val elements = node.fragmentGraph.map { conditionType.handleSelectionFieldOrFragment(it) } + Execution.Fragment(condition, elements, node.directives?.lookup()) + } + else -> { + this.handleSelection(node) + } + } + private fun Type.handleSelection(selectionNode: SelectionNode, variables: List? = null): Execution.Node { val field = this[selectionNode.key] diff --git a/src/test/kotlin/com/github/pgutkowski/kgraphql/integration/QueryTest.kt b/src/test/kotlin/com/github/pgutkowski/kgraphql/integration/QueryTest.kt index 7b49d530..598806b3 100644 --- a/src/test/kotlin/com/github/pgutkowski/kgraphql/integration/QueryTest.kt +++ b/src/test/kotlin/com/github/pgutkowski/kgraphql/integration/QueryTest.kt @@ -172,7 +172,7 @@ class QueryTest : BaseSchemaTest() { } @Test - fun `query with nested external fragment`(){ + fun `query with nested external fragment`() { val map = execute(""" { film { @@ -188,6 +188,10 @@ class QueryTest : BaseSchemaTest() { ...dirAge } + fragment dirIntermediate on Film { + ...dirAge + } + fragment dirAge on Film { director { age From 9e3d582b0ed36699c062fd46a51f830965c8bc7d Mon Sep 17 00:00:00 2001 From: "Michael D. Norman" Date: Sat, 2 Feb 2019 16:28:29 -0600 Subject: [PATCH 3/3] [issue-50] added merge tests --- .../pgutkowski/kgraphql/merge/MapMergeTest.kt | 69 ++++++++++++++++++ .../kgraphql/merge/ObjectNodeMergeTest.kt | 70 +++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 src/test/kotlin/com/github/pgutkowski/kgraphql/merge/MapMergeTest.kt create mode 100644 src/test/kotlin/com/github/pgutkowski/kgraphql/merge/ObjectNodeMergeTest.kt diff --git a/src/test/kotlin/com/github/pgutkowski/kgraphql/merge/MapMergeTest.kt b/src/test/kotlin/com/github/pgutkowski/kgraphql/merge/MapMergeTest.kt new file mode 100644 index 00000000..e4225cae --- /dev/null +++ b/src/test/kotlin/com/github/pgutkowski/kgraphql/merge/MapMergeTest.kt @@ -0,0 +1,69 @@ +package com.github.pgutkowski.kgraphql.merge + +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.node.JsonNodeFactory +import com.github.pgutkowski.kgraphql.expect +import com.github.pgutkowski.kgraphql.schema.execution.merge +import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Test + +class MapMergeTest { + private val jsonNodeFactory = JsonNodeFactory.instance + + @Test + fun `merge should add property`() { + val existing = createMap("param1" to jsonNodeFactory.textNode("value1")) + val update: JsonNode? = jsonNodeFactory.textNode("value2") + + existing.merge("param2", update) + + assertThat(existing.get("param2"), equalTo(update)) + } + + @Test + fun `merge should add nested property`() { + val existing = createMap("param1" to jsonNodeFactory.textNode("value1")) + val update: JsonNode? = jsonNodeFactory.objectNode().put("param2", "value2") + + existing.merge("sub", update) + + assertThat(existing.get("sub"), equalTo(update)) + } + + @Test + fun `merge should not change simple node`() { + val existingValue: JsonNode? = jsonNodeFactory.textNode("value1") + val existing = createMap("param" to existingValue) + val update = jsonNodeFactory.textNode("value2") + + expect("different simple nodes") { existing.merge("param", update) } + + assertThat(existing.get("param"), equalTo(existingValue)) + } + + @Test + fun `merge should not merge simple node with object node`() { + val existingValue: JsonNode? = jsonNodeFactory.textNode("value1") + val existing = createMap("param" to existingValue) + val update = jsonNodeFactory.objectNode() + + expect("merge object with simple node") { existing.merge("param", update) } + + val expected: JsonNode? = jsonNodeFactory.textNode("value1") + assertThat(existing.get("param"), equalTo(expected)) + } + + @Test + fun `merge should not merge object node with simple node`() { + val existingObj: JsonNode? = jsonNodeFactory.objectNode().put("other", "value1") + val existing = createMap("param" to existingObj) + val update = jsonNodeFactory.textNode("value2") + + expect("merge simple node with object node") { existing.merge("param", update) } + + assertThat(existing.get("param"), equalTo(existingObj)) + } + + private fun createMap(vararg pairs: Pair) = mutableMapOf(*pairs) +} diff --git a/src/test/kotlin/com/github/pgutkowski/kgraphql/merge/ObjectNodeMergeTest.kt b/src/test/kotlin/com/github/pgutkowski/kgraphql/merge/ObjectNodeMergeTest.kt new file mode 100644 index 00000000..0454c56b --- /dev/null +++ b/src/test/kotlin/com/github/pgutkowski/kgraphql/merge/ObjectNodeMergeTest.kt @@ -0,0 +1,70 @@ +package com.github.pgutkowski.kgraphql.merge + +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.node.JsonNodeFactory +import com.github.pgutkowski.kgraphql.expect +import com.github.pgutkowski.kgraphql.schema.execution.merge +import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Test + +class ObjectNodeMergeTest { + private val jsonNodeFactory = JsonNodeFactory.instance + + @Test + fun `merge should add property`() { + val existing = jsonNodeFactory.objectNode().put("param1", "value1") + val update = jsonNodeFactory.objectNode().put("param2", "value2") + + existing.merge(update) + + val expected: JsonNode? = jsonNodeFactory.textNode("value2") + assertThat(existing.get("param2"), equalTo(expected)) + } + + @Test + fun `merge should add nested property`() { + val existing = jsonNodeFactory.objectNode().put("param1", "value1") + val update = jsonNodeFactory.objectNode() + update.putObject("sub").put("param2", "value2") + + existing.merge(update) + + val expected: JsonNode? = jsonNodeFactory.objectNode().put("param2", "value2") + assertThat(existing.get("sub"), equalTo(expected)) + } + + @Test + fun `merge should not change simple node`() { + val existing = jsonNodeFactory.objectNode().put("param", "value1") + val update = jsonNodeFactory.objectNode().put("param", "value2") + + expect("different simple nodes") { existing.merge(update) } + + val expected: JsonNode? = jsonNodeFactory.textNode("value1") + assertThat(existing.get("param"), equalTo(expected)) + } + + @Test + fun `merge should not merge simple node with object node`() { + val existing = jsonNodeFactory.objectNode().put("param", "value1") + val update = jsonNodeFactory.objectNode() + update.putObject("param") + + expect("merge object with simple node") { existing.merge(update) } + + val expected: JsonNode? = jsonNodeFactory.textNode("value1") + assertThat(existing.get("param"), equalTo(expected)) + } + + @Test + fun `merge should not merge object node with simple node`() { + val existing = jsonNodeFactory.objectNode() + val existingObj: JsonNode? = existing.putObject("param").put("other", "value1") + val update = jsonNodeFactory.objectNode().put("param", "value2") + + expect("merge simple node with object node") { existing.merge(update) } + + assertThat(existing.get("param"), equalTo(existingObj)) + } +} \ No newline at end of file