Fixed direction handling for nested references and relationship types

jexp · jexp · commit 2632507f01b2 · 2019-03-30T15:46:58.000+01:00
Added a number of tests to the tck
diff --git a/pom.xml b/pom.xml
@@ -22,7 +22,7 @@
     <properties>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <java.version>1.8</java.version>
-        <kotlin.version>1.3.11</kotlin.version>
+        <kotlin.version>1.3.21</kotlin.version>
         <kotlin.compiler.jvmTarget>${java.version}</kotlin.compiler.jvmTarget>
         <neo4j.version>3.4.1</neo4j.version>
         <driver.version>1.7.2</driver.version>
diff --git a/readme.adoc b/readme.adoc
@@ -207,6 +207,7 @@ curl -XPOST http://localhost:4567/graphql -d'{"query":"{person {name}}"}'
 
 === Next
 
+* skip limit params
 * sorting (nested)
 * interfaces
 * input types
diff --git a/src/main/kotlin/org/neo4j/graphql/Predicates.kt b/src/main/kotlin/org/neo4j/graphql/Predicates.kt
@@ -55,26 +55,28 @@ fun GraphQLObjectType.relationshipFor(name:String, schema: GraphQLSchema) : Rela
     // label
     // out
 
-    val (relDirective, relFromType) = fieldObjectType.definition.getDirective("relation")?.let { it to true }
+    // TODO direction is depending on source/target type
+
+    val (relDirective, isRelFromType) = fieldObjectType.definition.getDirective("relation")?.let { it to true }
             ?: field.definition.getDirective("relation")?.let { it to false }
             ?: throw IllegalStateException("Field $field needs an @relation directive")
 
-    val (relType, outgoing, endField) = relDetails(relDirective, schema)
 
-    return RelationshipInfo(fieldObjectType, relDirective, relType,  outgoing,endField, relFromType)
+    val relInfo = relDetails(fieldObjectType, relDirective, schema)
+
+    val inverse = isRelFromType && fieldObjectType.getFieldDefinition(relInfo.startField).name != this.name
+    return if (inverse) relInfo.copy(out = relInfo.out?.let { !it }, startField = relInfo.endField, endField = relInfo.startField) else relInfo
 }
 
-fun relDetails(relDirective: Directive, schema: GraphQLSchema): Triple<String,Boolean?,String> {
+fun relDetails(source: GraphQLObjectType, relDirective: Directive, schema: GraphQLSchema): RelationshipInfo {
     val relType = relDirective.argumentString("name", schema)
     val outgoing =  when (relDirective.argumentString("direction", schema)) {
         "IN" -> false
         "BOTH" -> null
         "OUT" -> true
         else -> throw IllegalStateException("Unknown direction ${relDirective.argumentString("direction",schema)}")
     }
-    val endField = if (outgoing == true) relDirective.argumentString("to", schema)
-    else relDirective.argumentString("from", schema)
-    return Triple(relType, outgoing, endField)
+    return RelationshipInfo(source, relDirective, relType, outgoing, relDirective.argumentString("from", schema), relDirective.argumentString("to", schema))
 }
 
 fun arrows(outgoing: Boolean?): Pair<String, String> {
@@ -86,7 +88,7 @@ fun arrows(outgoing: Boolean?): Pair<String, String> {
 }
 
 
-data class RelationshipInfo(val objectType:GraphQLObjectType, val directive:Directive, val type:String, val out:Boolean?, val endField: String? = null, val relFromType: Boolean = false) {
+data class RelationshipInfo(val objectType:GraphQLObjectType, val directive:Directive, val type:String, val out:Boolean?, val startField: String? = null,val endField: String? = null, val isRelFromType: Boolean = false) {
     val arrows = arrows(out)
     val label = objectType.name
 }
diff --git a/src/main/kotlin/org/neo4j/graphql/Translator.kt b/src/main/kotlin/org/neo4j/graphql/Translator.kt
@@ -78,6 +78,7 @@ class Translator(val schema: GraphQLSchema) {
                         (params + mapProjection.params))
             }
 
+
     private fun propertyArguments(queryField: Field) =
             queryField.arguments.filterNot { listOf("first", "offset", "orderBy").contains(it.name) }
 
@@ -90,7 +91,7 @@ class Translator(val schema: GraphQLSchema) {
     }
 
     private fun where(variable: String, field: GraphQLFieldDefinition, type: GraphQLType, arguments: List<Argument>) : Cypher {
-        val all = preparePredicateArguments(field, arguments)
+        val all = preparePredicateArguments(field, arguments).filterNot { listOf("first", "offset", "orderBy").contains(it.name) }
         if (all.isEmpty()) return Cypher("")
         val (filterExpressions, filterParams) =
             filterExpressions(all.find{ it.name == "filter" }?.value, type as GraphQLObjectType)
@@ -216,7 +217,7 @@ class Translator(val schema: GraphQLSchema) {
         val parentIsRelationship = parent.definition.directivesByName.containsKey("relation")
         return when (parentIsRelationship) {
             true -> projectRelationshipParent(variable, field, fieldDefinition, parent, ctx)
-            else -> projectRichAndRegularRelationship(variable, field, fieldDefinition, ctx)
+            else -> projectRichAndRegularRelationship(variable, field, fieldDefinition, parent, ctx)
         }
     }
 
@@ -234,28 +235,31 @@ class Translator(val schema: GraphQLSchema) {
         return Cypher(comprehension + slice, (expression.params + fieldProjection.params)) // + where.params
 
     }
-    private fun projectRichAndRegularRelationship(variable: String, field: Field, fieldDefinition: GraphQLFieldDefinition, ctx: Context): Cypher {
+    private fun projectRichAndRegularRelationship(variable: String, field: Field, fieldDefinition: GraphQLFieldDefinition, parent: GraphQLObjectType, ctx: Context): Cypher {
         val fieldType = fieldDefinition.type
         val fieldObjectType = fieldType.inner() as GraphQLObjectType
         // todo combine both nestings if rel-entity
-        val (relDirective, relFromType) = fieldObjectType.definition.getDirective("relation")?.let { it to true }
+        val (relDirective, isRelFromType) = fieldObjectType.definition.getDirective("relation")?.let { it to true }
                 ?: fieldDefinition.definition.getDirective("relation")?.let { it to false }
                 ?: throw IllegalStateException("Field $field needs an @relation directive")
 
-        val (relType, outgoing, endField) = relDetails(relDirective)
-        val (inArrow, outArrow) = arrows(outgoing)
+        var relInfo = relDetails(fieldObjectType, relDirective)
+        val inverse = isRelFromType && fieldObjectType.getFieldDefinition(relInfo.startField).type.inner().name != parent.name
+        if (inverse) relInfo = relInfo.copy(out = relInfo.out?.let { !it }, startField = relInfo.endField, endField = relInfo.startField)
+
+        val (inArrow, outArrow) = relInfo.arrows
 
         val childVariable = variable + field.name.capitalize()
 
         val endNodePattern = when {
-            relFromType -> {
-                val label = fieldObjectType.getFieldDefinition(endField).type.inner().name
-                "$childVariable${endField.capitalize()}:$label"
+            isRelFromType -> {
+                val label = fieldObjectType.getFieldDefinition(relInfo.endField!!).type.inner().name
+                "$childVariable${relInfo.endField!!.capitalize()}:$label"
             }
             else -> "$childVariable:${fieldObjectType.name}"
         }
 
-        val relPattern = if (relFromType) "$childVariable:${relType}" else ":${relType}"
+        val relPattern = if (isRelFromType) "$childVariable:${relInfo.type}" else ":${relInfo.type}"
 
         val where = where(childVariable, fieldDefinition, fieldObjectType, propertyArguments(field))
         val fieldProjection = projectFields(childVariable, field, fieldObjectType, ctx)
@@ -270,14 +274,17 @@ class Translator(val schema: GraphQLSchema) {
         val fieldType = fieldDefinition.type
         val fieldObjectType = fieldType.inner() as GraphQLObjectType
         val relDirective = parent.definition.directivesByName.getValue("relation")
-        val (_, _, endField) = relDetails(relDirective)
+        var relInfo = relDetails(fieldObjectType, relDirective)
+
+        val inverse = parent.getFieldDefinition(relInfo.endField!!).type.inner().name != fieldObjectType.name
+        if (inverse) relInfo = relInfo.copy(out = relInfo.out?.let { !it }, startField = relInfo.endField, endField = relInfo.startField)
 
-        val fieldProjection = projectFields(variable + endField.capitalize(), field, fieldObjectType, ctx)
+        val fieldProjection = projectFields(variable + relInfo.endField!!.capitalize(), field, fieldObjectType, ctx)
 
         return Cypher(fieldProjection.query)
     }
 
-    private fun relDetails(relDirective: Directive) = relDetails(relDirective, schema)
+    private fun relDetails(target:GraphQLObjectType, relDirective: Directive) = relDetails(target, relDirective, schema)
 
     private fun slice(skipLimit: Pair<Int, Int>, list: Boolean = false) =
             if (list) {
diff --git a/src/test/kotlin/org/neo4j/graphql/MovieSchemaTest.kt b/src/test/kotlin/org/neo4j/graphql/MovieSchemaTest.kt
@@ -29,33 +29,10 @@ class MovieSchemaTest {
     }
 
     @Test fun testTck() {
-        TckTest(schema).testTck("movie-test.md",0, false)
+        TckTest(schema).testTck("movie-test.md",0, true)
     }
 
 /*
-fun `testCypher projection skip limit`() {
-  val graphQLQuery = """{
-    Movie(title: "River Runs Through It, A") {
-      title
-      actors {
-        name
-      }
-      similar(first: 3) {
-        title
-      }
-    }
-  }"""
-val expectedCypherQuery =
-      """MATCH (movie:Movie {title:${"$"}title}) RETURN movie { .title ,actors: [(movie)<-[:ACTED_IN]-(movie_actors:Actor) | movie_actors { .name }] ,similar: [ movie_similar IN apoc.cypher.runFirstColumn("WITH {this} AS this MATCH (this)--(:Genre)--(o:Movie) RETURN o", mapOf(this: movie, first: 3, offset: 0}, true) | movie_similar { .title }][..3] } AS movie SKIP ${"$"}offset""";
-
-    testTranslation(graphQLQuery, expectedCypherQuery, mapOf(
-      "title" to  "River Runs Through It, A",
-      """1_first": 3,
-      "first" to -1,
-      "offset" to 0
-    ))
-}
-
 fun `testHandle Query with name not aligning to type`() {
   val graphQLQuery = """{
   MoviesByYear(year: 2010) {
@@ -73,81 +50,6 @@ val expectedCypherQuery =
     ))
 }
 
-fun `testQuery without arguments, non-null type`() {
-  val graphQLQuery = """query {
-  Movie {
-    movieId
-  }
-}"""
-val expectedCypherQuery =
-      """MATCH (movie:Movie {}) RETURN movie { .movieId } AS movie SKIP ${"$"}offset""";
-
-    testTranslation(graphQLQuery, expectedCypherQuery, mapOf(
-      "first" to -1,
-      "offset" to 0
-    ))
-}
-
-fun `testQuery single object`() {
-  val graphQLQuery = """
-  {
-    MovieById(movieId: "18") {
-      title
-    }
-  }"""
-val expectedCypherQuery =
-      """MATCH (movie:Movie {movieId:${"$"}movieId}) RETURN movie { .title } AS movie SKIP ${"$"}offset""";
-
-    testTranslation(graphQLQuery, expectedCypherQuery, mapOf(
-      movieId: "18",
-      "first" to -1,
-      "offset" to 0
-    ))
-}
-
-fun `testQuery single object relation`() {
-  val graphQLQuery = """
-    {
-      MovieById(movieId: "3100") {
-        title
-        filmedIn {
-          name
-        }
-      }
-    }
-  """
-val expectedCypherQuery =
-      """MATCH (movie:Movie {movieId:${"$"}movieId}) RETURN movie { .title ,filmedIn: head([(movie)-[:FILMED_IN]->(movie_filmedIn:State) | movie_filmedIn { .name }]) } AS movie SKIP ${"$"}offset""";
-
-    testTranslation(graphQLQuery, expectedCypherQuery, mapOf(
-      movieId: "3100",
-      "first" to -1,
-      "offset" to 0
-    ))
-}
-
-fun `testQuery single object and array of objects relations`() {
-  val graphQLQuery = """
-    {
-      MovieById(movieId: "3100") {
-        title
-        actors {
-          name
-        }
-        filmedIn{
-          name
-        }
-      }
-    }"""
-val expectedCypherQuery =
-      """MATCH (movie:Movie {movieId:${"$"}movieId}) RETURN movie { .title ,actors: [(movie)<-[:ACTED_IN]-(movie_actors:Actor) | movie_actors { .name }] ,filmedIn: head([(movie)-[:FILMED_IN]->(movie_filmedIn:State) | movie_filmedIn { .name }]) } AS movie SKIP ${"$"}offset""";
-
-    testTranslation(graphQLQuery, expectedCypherQuery, mapOf(
-      movieId: "3100",
-      "first" to -1,
-      "offset" to 0
-    ))
-}
 
 fun `testDeeply nested object query`() {
   val graphQLQuery = """
@@ -449,34 +351,6 @@ fun `testHandle @cypher directive on Query Type`() {
   }
 }
   """
-val expectedCypherQuery = """WITH apoc.cypher.runFirstColumn("MATCH (g:Genre) WHERE toLower(g.name) CONTAINS toLower(${"$"}substring) RETURN g", mapOf(substring:${"$"}substring}, True) AS x UNWIND x AS genre
-    RETURN genre { .name ,movies: [(genre)<-[:IN_GENRE]-(genre_movies:Movie{}) | genre_movies { .title }][..3] } AS genre SKIP ${"$"}offset""";
-
-    testTranslation(graphQLQuery, expectedCypherQuery, mapOf(
-      substring: "Action",
-      "first" to -1,
-      offset: 0,
-      """1_first": 3
-    ))
-}
-
-test.cb("Handle @cypher directive on Mutation type`() {
-  val graphQLQuery = """mutation someMutation {
-  CreateGenre(name: "Wildlife Documentary") {
-    name
-  }
-}"""
-val expectedCypherQuery = """CALL apoc.cypher.doIt("CREATE (g:Genre) SET g.name = ${"$"}name RETURN g", mapOf(name:${"$"}name}) YIELD value
-    WITH apoc.map.values(value, [keys(value)[0]])[0] AS genre
-    RETURN genre { .name } AS genre SKIP ${"$"}offset""";
-
-  t.plan(2);
-  testTranslation(graphQLQuery, expectedCypherQuery, mapOf(
-    name: "Wildlife Documentary",
-    "first" to -1,
-    "offset" to 0
-  }
-}
 
 test.cb("Create node mutation`() {
   val graphQLQuery = """	mutation someMutation {
@@ -1387,35 +1261,6 @@ val expectedCypherQuery,
   );
 }
 
-fun `testquery using inline fragment`() {
-  val graphQLQuery = """
-  {
-    Movie(title: "River Runs Through It, A") {
-      title
-      ratings {
-        rating
-        User {
-          ... on User {
-            name
-            userId
-          }
-        }
-      }
-    }
-  }
-  """
-val expectedCypherQuery = """MATCH (movie:Movie {title:${"$"}title}) RETURN movie { .title ,ratings: [(movie)<-[movie_ratings_relation:RATED]-(:User) | movie_ratings_relation { .rating ,User: head([(:Movie)<-[movie_ratings_relation]-(movie_ratings_User:User) | movie_ratings_User { .name , .userId }]) }] } AS movie SKIP ${"$"}offset""";
-
-  t.plan(1);
-
-  return augmentedSchemaCypherTestRunner(
-    t,
-    graphQLQuery,
-    {}
-val expectedCypherQuery,
-    {}
-  );
-}
 
  */
 
diff --git a/src/test/kotlin/org/neo4j/graphql/TckTest.kt b/src/test/kotlin/org/neo4j/graphql/TckTest.kt
@@ -1,14 +1,16 @@
 package demo.org.neo4j.graphql
 
+import org.antlr.v4.runtime.RecognitionException
+import org.antlr.v4.runtime.misc.ParseCancellationException
 import org.codehaus.jackson.map.ObjectMapper
 import org.junit.Assert
 import org.neo4j.graphql.SchemaBuilder
 import org.neo4j.graphql.Translator
 import java.io.File
+import java.lang.RuntimeException
 
 class TckTest(val schema:String) {
 
-
     fun loadQueryPairsFrom(fileName: String): MutableList<Triple<String, String, Map<String, Any?>>> {
         val lines = File(javaClass.getResource("/$fileName").toURI())
                 .readLines()
@@ -25,8 +27,8 @@ class TckTest(val schema:String) {
                 "```params" -> params = ""
                 "```" ->
                     if (graphql != null && cypher != null) {
-                        testData.add(Triple(graphql.trim(),cypher.trim(),params?.let { ObjectMapper().readValue(params,Map::class.java) as Map<String,Any?> } ?: emptyMap()))
-                        params = null
+                        testData.add(Triple(graphql.trim(),cypher.trim(),params?.let { MAPPER.readValue(params,Map::class.java) as Map<String,Any?> } ?: emptyMap()))
+                        graphql = null
                         cypher = null
                         params = null
                     }
@@ -46,7 +48,12 @@ class TckTest(val schema:String) {
             try {
                 assertQuery(schema, it.first, it.second, it.third); null
             } catch (ae: Throwable) {
-                if (fail) throw ae else ae.message
+                if (fail) when (ae) {
+                    is ParseCancellationException -> throw RuntimeException((ae.cause!! as RecognitionException).let { "expected: ${it.expectedTokens} offending ${it.offendingToken}" } )
+                    else -> throw ae
+                }
+
+                else ae.message ?: ae.toString()
             }
         }
                 .filterNotNull()
@@ -56,10 +63,15 @@ class TckTest(val schema:String) {
     }
 
     companion object {
+        val MAPPER = ObjectMapper()
+
         fun assertQuery(schema:String, query: String, expected: String, params : Map<String,Any?> = emptyMap()) {
             val result = Translator(SchemaBuilder.buildSchema(schema)).translate(query).first()
+            println(result.query)
             Assert.assertEquals(expected.replace(Regex("\\s+")," "), result.query)
-            Assert.assertTrue("${params} IN ${result.params}", result.params.entries.containsAll(params.entries))
+            Assert.assertTrue("${params} IN ${result.params}", fixNumbers(result.params).entries.containsAll(fixNumbers(params).entries))
         }
+
+        private fun fixNumbers(params: Map<String, Any?>) = params.mapValues{ (_,v) -> when(v) { is Float -> v.toDouble(); is Int -> v.toLong(); else -> v } }
     }
 }
diff --git a/src/test/resources/movie-test.md b/src/test/resources/movie-test.md
diff --git a/src/test/resources/movies-test-schema.graphql b/src/test/resources/movies-test-schema.graphql
