update graphql-java to version 15 (#120)

* update graphql-java to version 15

This is an old version of a core dependency used in most graphql related projects in java. If this dependency isn't updated then you have conflicts with incompatible transitive versions of graphql-java.
Unfortunately, it looks like a committer to graphql-java likes to make backwards a lot of uneccessarily incompatible changes to the library. So I had to make a lot of changes for this to compile and for the tests to pass. I had to do some archeology in the history of graphql-java to understand some of the changes.

* GraphQLType.name no longer exists. simplePrint(GraphQLType) is used instead
* GraphQLFieldDefinition is no longer a GraphQLType, but it looks like GraphQLFieldDefinition.originalType will get the type we are looking for.
* ScalarInfo.STANDARD_SCALAR_DEFINITIONS was replaced with ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS
* The value generic for the return type of GraphQLSchema.typeMap is now more specific so BuildingEnv's constructor had to change.
* ShemaPrinter lost some include methods. One was previously mispelled.
* DefaultSchemaPrinterComparatorRegistry became DefaultGraphqlTypeComparatorRegistry.

I had to make some changes to the tests to make them pass, which makes me quite uncomfortable with this. Please check these over to see if I should have made more changes. I'm going ahead with this because the overall functionality still seems legit. I have no idea why these were needed.
* In translator-tests1 'render values' I had to move the clauses of the where clause around. It mateches the order of the cypher params now, so it kind of makes sense.
* The one line GraphQL query no longer works. It had to be broken up to multilines in several tests.
* relationship-tests is a weird one. I changed the generated Cypher, but it makes more sense now. The relationship is player through memberships to team. So `playerMembershipsTeam: Team` makes a lot more sense than the original `playerMembershipsPlayer: Player`. It actually looks like it was incorrect originally.

* adjustments for graphql-java update

Co-authored-by: Andreas Berger <andreas@berger-ecommerce.com>
Co-authored-by: Michael Hunger <github@jexp.de>

Author: 3 people

pom.xml

@@ -88,7 +88,7 @@
         <dependency>
             <groupId>com.graphql-java</groupId>
             <artifactId>graphql-java</artifactId>
-            <version>12.0</version>
+            <version>15.0</version>
         </dependency>
         <dependency>
             <groupId>org.junit.jupiter</groupId>

src/main/kotlin/org/neo4j/graphql/BuildingEnv.kt

@@ -3,7 +3,9 @@ package org.neo4j.graphql
 import graphql.Scalars
 import graphql.schema.*
 
-class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
+import graphql.schema.GraphQLTypeUtil.simplePrint
+
+class BuildingEnv(val types: MutableMap<String, GraphQLNamedType>) {
 
     private val typesForRelation = types.values
         .filterIsInstance<GraphQLObjectType>()
@@ -23,7 +25,7 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
             type = GraphQLNonNull(type)
         }
         return GraphQLFieldDefinition.newFieldDefinition()
-            .name("$prefix${resultType.name}")
+            .name("$prefix${simplePrint(resultType)}")
             .arguments(getInputValueDefinitions(scalarFields, forceOptionalProvider))
             .type(type.ref() as GraphQLOutputType)
     }
@@ -71,7 +73,7 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
         }
         val existingFilterType = types[filterName]
         if (existingFilterType != null) {
-            return (existingFilterType as? GraphQLInputType)?.name
+            return simplePrint(existingFilterType as? GraphQLInputType)
                     ?: throw IllegalStateException("Filter type $filterName is already defined but not an input type")
         }
         createdTypes.add(filterName)
@@ -87,7 +89,7 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
             .forEach { field ->
                 val typeDefinition = field.type.inner()
                 val filterType = when {
-                    typeDefinition.isNeo4jType() -> getInputType(typeDefinition).name
+                    typeDefinition.isNeo4jType() -> getInputType(typeDefinition).requiredName()
                     typeDefinition.isScalar() -> typeDefinition.innerName()
                     typeDefinition is GraphQLEnumType -> typeDefinition.innerName()
                     else -> addFilterType(getInnerFieldsContainer(typeDefinition), createdTypes)
@@ -122,7 +124,7 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
         val orderingName = "_${type.name}Ordering"
         var existingOrderingType = types[orderingName]
         if (existingOrderingType != null) {
-            return (existingOrderingType as? GraphQLInputType)?.name
+            return (existingOrderingType as? GraphQLInputType)?.requiredName()
                     ?: throw IllegalStateException("Ordering type $type.name is already defined but not an input type")
         }
         val sortingFields = type.fieldDefinitions
@@ -190,7 +192,7 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
                     ?: throw IllegalArgumentException("${innerType.name} is unknown")
         }
         return innerType as? GraphQLFieldsContainer
-                ?: throw IllegalArgumentException("${innerType.name} is neither an object nor an interface")
+                ?: throw IllegalArgumentException("${innerType.name()} is neither an object nor an interface")
     }
 
     private fun getInputType(type: GraphQLType): GraphQLInputType {
@@ -200,12 +202,12 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
         }
         if (inner.isNeo4jType()) {
             return neo4jTypeDefinitions
-                .find { it.typeDefinition == inner.name }
+                .find { it.typeDefinition == inner.name() }
                 ?.let { types[it.inputDefinition] } as? GraphQLInputType
-                    ?: throw IllegalArgumentException("Cannot find input type for ${inner.name}")
+                    ?: throw IllegalArgumentException("Cannot find input type for ${inner.name()}")
         }
         return type as? GraphQLInputType
-                ?: throw IllegalArgumentException("${type.name} is not allowed for input")
+                ?: throw IllegalArgumentException("${type.name()} is not allowed for input")
     }
 
-}
+}

src/main/kotlin/org/neo4j/graphql/GraphQLExtensions.kt

@@ -33,8 +33,11 @@ fun GraphQLType.inner(): GraphQLType = when (this) {
     else -> this
 }
 
+fun GraphQLType.name(): String? = (this as? GraphQLNamedType)?.name
+fun GraphQLType.requiredName(): String = requireNotNull(name()) { -> "name is required but cannot be determined for " + this.javaClass }
+
 fun GraphQLType.isList() = this is GraphQLList || (this is GraphQLNonNull && this.wrappedType is GraphQLList)
-fun GraphQLType.isScalar() = this.inner().let { it is GraphQLScalarType || it.name.startsWith("_Neo4j") }
+fun GraphQLType.isScalar() = this.inner().let { it is GraphQLScalarType || it.innerName().startsWith("_Neo4j") }
 fun GraphQLType.isNeo4jType() = this.innerName().startsWith("_Neo4j")
 fun GraphQLType.isNeo4jSpatialType() = this.innerName().startsWith("_Neo4jPoint")
 fun GraphQLFieldDefinition.isNeo4jType(): Boolean = this.type.isNeo4jType()
@@ -119,7 +122,7 @@ fun GraphQLType.ref(): GraphQLType = when (this) {
     is GraphQLScalarType -> this
     is GraphQLEnumType -> this
     is GraphQLTypeReference -> this
-    else -> GraphQLTypeReference(name)
+    else -> GraphQLTypeReference(name())
 }
 
 fun relDetails(type: GraphQLFieldsContainer, relDirective: GraphQLDirective): RelationshipInfo {
@@ -183,7 +186,8 @@ data class RelationshipInfo(
 }
 
 fun Field.aliasOrName() = (this.alias ?: this.name).quote()
-fun GraphQLType.innerName(): String = inner().name
+fun GraphQLType.innerName(): String = inner().name()
+        ?: throw IllegalStateException("inner name cannot be retrieved for " + this.javaClass)
 
 fun GraphQLFieldDefinition.propertyName() = getDirectiveArgument(PROPERTY, PROPERTY_NAME, this.name)!!
 

src/main/kotlin/org/neo4j/graphql/Neo4jTypes.kt

@@ -58,7 +58,7 @@ data class Neo4jQueryConversion(val name: String, val propertyName: String, val
             if (!isNeo4jType) {
                 Neo4jQueryConversion(name, name)
             }
-            val converter = getNeo4jTypeConverter(fieldDefinition.type.inner().name)
+            val converter = getNeo4jTypeConverter(fieldDefinition.type.innerName())
             val objectValue = (value as? ObjectValue)
                 ?.objectFields
                 ?.map { it.name to it.value }
@@ -80,4 +80,4 @@ val neo4jTypeDefinitions = listOf(
         TypeDefinition("Time", "_Neo4jLocalTime"),
         TypeDefinition("LocalDateTime", "_Neo4jLocalDateTime"),
         TypeDefinition("Point", "_Neo4jPoint")
-)
+)

src/main/kotlin/org/neo4j/graphql/Predicates.kt

@@ -112,7 +112,7 @@ data class ExpressionPredicate(
 ) : Predicate {
     val not = if (op.not) "NOT " else ""
     override fun toExpression(variable: String): Cypher {
-        val paramName: String = ProjectionBase.FILTER + paramName(variable, name, value).capitalize() + "_" + op.name + nestedField.replace('.','_')
+        val paramName: String = ProjectionBase.FILTER + paramName(variable, name, value).capitalize() + "_" + op.name + nestedField.replace('.', '_')
         val query = if (fieldDefinition.isNativeId()) {
             if (op.list) {
                 "${not}ID($variable) ${op.op} [id IN \$$paramName | toInteger(id)]"
@@ -151,7 +151,7 @@ data class RelationPredicate(val fieldName: String, val op: RelationOperator, va
             RelationOperator.NOT -> "ALL" // bc of not
             else -> op.op
         }
-        if (type.getFieldDefinition(fieldName).isList()) {
+        if (type.getFieldDefinition(fieldName).type.isList()) {
             if (op == RelationOperator.EQ_OR_NOT_EXISTS) {
                 LOGGER.info("$fieldName on type ${type.name} was used for filtering, consider using ${fieldName}${RelationOperator.EVERY.suffix} instead")
             }
@@ -254,7 +254,7 @@ enum class FieldOperator(
         fun resolve(queriedField: String, field: GraphQLFieldDefinition, value: Any?): FieldOperator? {
             val fieldName = field.name
             if (value == null) {
-                return listOf(IS_NULL, IS_NOT_NULL).find { queriedField == fieldName + it.suffix } ?: return null
+                return listOf(IS_NULL, IS_NOT_NULL).find { queriedField == fieldName + it.suffix }
             }
             val ops = enumValues<FieldOperator>().filterNot { it == IS_NULL || it == IS_NOT_NULL }
             return ops.find { queriedField == fieldName + it.suffix }
@@ -263,7 +263,6 @@ enum class FieldOperator(
                     } else {
                         null
                     }
-                    ?: return null
         }
 
         fun forType(type: GraphQLType): List<FieldOperator> =
@@ -277,7 +276,7 @@ enum class FieldOperator(
                     // todo list types
                     !type.isScalar() -> listOf(EQ, NEQ, IN, NIN)
                     else -> listOf(EQ, NEQ, IN, NIN, LT, LTE, GT, GTE) +
-                            if (type.name == "String" || type.name == "ID") listOf(C, NC, SW, NSW, EW, NEW) else emptyList()
+                            if (type.name() == "String" || type.name() == "ID") listOf(C, NC, SW, NSW, EW, NEW) else emptyList()
                 }
     }
 

src/main/kotlin/org/neo4j/graphql/SchemaBuilder.kt

@@ -4,7 +4,7 @@ import graphql.Scalars
 import graphql.language.*
 import graphql.schema.*
 import graphql.schema.idl.RuntimeWiring
-import graphql.schema.idl.ScalarInfo.STANDARD_SCALAR_DEFINITIONS
+import graphql.schema.idl.ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS
 import graphql.schema.idl.SchemaGenerator
 import graphql.schema.idl.SchemaParser
 import graphql.schema.idl.TypeDefinitionRegistry
@@ -43,7 +43,7 @@ object SchemaBuilder {
 
         val builder = RuntimeWiring.newRuntimeWiring()
         typeDefinitionRegistry.scalars()
-            .filterNot { entry -> STANDARD_SCALAR_DEFINITIONS.containsKey(entry.key) }
+            .filterNot { entry -> GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(entry.key) }
             .forEach { (name, definition) ->
                 val scalar = when (name) {
                     "DynamicProperties" -> DynamicProperties.INSTANCE

src/test/kotlin/org/neo4j/graphql/TranslatorExceptionTests.kt

@@ -16,16 +16,26 @@ class TranslatorExceptionTests : AsciiDocTestSuite("translator-tests1.adoc") {
     }
 
     override fun schemaTestFactory(schema: String): List<DynamicNode> {
-        val translator = Translator(SchemaBuilder.buildSchema(schema));
+        val translator = Translator(SchemaBuilder.buildSchema(schema))
         return listOf(
                 DynamicTest.dynamicTest("unknownType") {
                     Assertions.assertThrows(IllegalArgumentException::class.java) {
-                        translator.translate(" { company { name } } ")
+                        translator.translate("""
+                        {
+                          company {
+                            name
+                          }
+                        }
+                        """)
                     }
                 },
                 DynamicTest.dynamicTest("mutation") {
                     Assertions.assertThrows(InvalidSyntaxException::class.java) {
-                        translator.translate(" { createPerson() } ")
+                        translator.translate("""
+                        {
+                          createPerson()
+                        }
+                        """.trimIndent())
                     }
                 }
 

src/test/kotlin/org/neo4j/graphql/utils/GraphQLSchemaTestSuite.kt

@@ -1,21 +1,23 @@
 package org.neo4j.graphql.utils
 
 import graphql.language.InterfaceTypeDefinition
-import graphql.schema.GraphQLFieldDefinition
-import graphql.schema.GraphQLObjectType
 import graphql.schema.GraphQLSchema
 import graphql.schema.GraphQLType
 import graphql.schema.diff.DiffSet
 import graphql.schema.diff.SchemaDiff
 import graphql.schema.diff.reporting.CapturingReporter
-import graphql.schema.idl.*
+import graphql.schema.idl.RuntimeWiring
+import graphql.schema.idl.SchemaGenerator
+import graphql.schema.idl.SchemaParser
+import graphql.schema.idl.SchemaPrinter
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Assumptions
 import org.junit.jupiter.api.DynamicNode
 import org.junit.jupiter.api.DynamicTest
 import org.neo4j.graphql.DynamicProperties
 import org.neo4j.graphql.SchemaBuilder
 import org.neo4j.graphql.SchemaConfig
+import org.neo4j.graphql.requiredName
 import org.opentest4j.AssertionFailedError
 import java.util.*
 import java.util.regex.Pattern
@@ -76,40 +78,16 @@ class GraphQLSchemaTestSuite(fileName: String) : AsciiDocTestSuite(fileName) {
         private val SCHEMA_PRINTER = SchemaPrinter(SchemaPrinter.Options.defaultOptions()
             .includeDirectives(true)
             .includeScalarTypes(true)
-            .includeExtendedScalarTypes(true)
-            .includeSchemaDefintion(true)
+            .includeSchemaDefinition(true)
             .includeIntrospectionTypes(false)
-            .setComparators(DefaultSchemaPrinterComparatorRegistry.newComparators()
-                .addComparator({ env ->
-                    env.parentType(GraphQLObjectType::class.java)
-                    env.elementType(GraphQLFieldDefinition::class.java)
-                }, GraphQLFieldDefinition::class.java, fun(o1: GraphQLFieldDefinition, o2: GraphQLFieldDefinition): Int {
-                    val (op1, name1) = o1.splitName()
-                    val (op2, name2) = o2.splitName()
-                    if (op1 == null && op2 == null) {
-                        return name1.compareTo(name2)
-                    }
-                    if (op1 == null) {
-                        return -1
-                    }
-                    if (op2 == null) {
-                        return 1
-                    }
-                    val prio1 = name1.compareTo(name2)
-                    if (prio1 == 0) {
-                        return op1.compareTo(op2)
-                    }
-                    return prio1
-                })
-                .build())
         )
 
         fun GraphQLType.splitName(): Pair<String?, String> {
-            val m = METHOD_PATTERN.matcher(this.name)
+            val m = METHOD_PATTERN.matcher(this.requiredName())
             return if (m.find()) {
                 m.group(1) to m.group(2).toLowerCase()
             } else {
-                null to this.name.toLowerCase()
+                null to this.requiredName().toLowerCase()
             }
         }
 

src/test/resources/translator-tests1.adoc

@@ -499,12 +499,12 @@ query($_param:String) { p:values(_param:$_param) { age } }
 ----
 MATCH (p:Person)
 WHERE p._param = $_param
+  AND p._string = $p_string
+  AND p._int = $p_int
+  AND p._float = $p_float
   AND p._array = $p_array
-  AND p._boolean = $p_boolean
   AND p._enum = $p_enum
-  AND p._float = $p_float
-  AND p._int = $p_int
-  AND p._string = $p_string
+  AND p._boolean = $p_boolean
 RETURN p { .age } AS p
 ----