cleanup after review (#67)

Author: Andy2003

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

@@ -54,26 +54,26 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
                 .build()
         } else {
             val existingRootType = (rootType as? GraphQLObjectType
-                    ?: throw IllegalStateException("root type $rootTypeName is not an object type"))
+                    ?: throw IllegalStateException("root type $rootTypeName is not an object type but ${rootType.javaClass}"))
             if (existingRootType.getFieldDefinition(fieldDefinition.name) != null) {
-                return // definition already exists
+                return // definition already exists, we don't override it
             }
             existingRootType
                 .transform { builder -> builder.field(fieldDefinition) }
         }
     }
 
-    fun addFilterType(type: GraphQLFieldsContainer, handled: MutableSet<String> = mutableSetOf()): String {
+    fun addFilterType(type: GraphQLFieldsContainer, createdTypes: MutableSet<String> = mutableSetOf()): String {
         val filterName = "_${type.name}Filter"
-        if (handled.contains(filterName)) {
+        if (createdTypes.contains(filterName)) {
             return filterName
         }
         val existingFilterType = types[filterName]
         if (existingFilterType != null) {
             return (existingFilterType as? GraphQLInputType)?.name
                     ?: throw IllegalStateException("Filter type $filterName is already defined but not an input type")
         }
-        handled.add(filterName)
+        createdTypes.add(filterName)
         val builder = GraphQLInputObjectType.newInputObject()
             .name(filterName)
         listOf("AND", "OR", "NOT").forEach {
@@ -89,7 +89,7 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
                     typeDefinition.isNeo4jType() -> getInputType(typeDefinition).name
                     typeDefinition.isScalar() -> typeDefinition.innerName()
                     typeDefinition is GraphQLEnumType -> typeDefinition.innerName()
-                    else -> addFilterType(getInnerFieldsContainer(typeDefinition), handled)
+                    else -> addFilterType(getInnerFieldsContainer(typeDefinition), createdTypes)
                 }
 
                 Operators.forType(types[filterType] ?: typeDefinition).forEach { op ->
@@ -160,6 +160,7 @@ class BuildingEnv(val types: MutableMap<String, GraphQLType>) {
 
     private fun getInputValueDefinitions(relevantFields: List<GraphQLFieldDefinition>): List<GraphQLInputObjectField> {
         return relevantFields.map {
+            // just make evrything optional
             val type = (it.type as? GraphQLNonNull)?.wrappedType ?: it.type
             GraphQLInputObjectField
                 .newInputObjectField()

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

@@ -233,3 +233,4 @@ fun paramName(variable: String, argName: String, value: Any?): String = when (va
 
 fun GraphQLFieldDefinition.isID() = this.type.inner() == Scalars.GraphQLID
 fun GraphQLFieldDefinition.isNativeId() = this.name == ProjectionBase.NATIVE_ID
+fun GraphQLFieldsContainer.getIdField() = this.fieldDefinitions.find { it.isID() }

src/main/kotlin/org/neo4j/graphql/handler/DeleteHandler.kt

@@ -16,11 +16,11 @@ class DeleteHandler private constructor(
             if (!canHandle(type)) {
                 return
             }
-            val idField = type.fieldDefinitions.find { it.isID() } ?: return
+            val idField = type.getIdField() ?: return
 
             val fieldDefinition = buildingEnv
                 .buildFieldDefinition("delete", type, listOf(idField), nullableResult = true)
-                .description("Deletes ${type.name} and returns its ID on successful deletion")
+                .description("Deletes ${type.name} and returns the type itself")
                 .type(type.ref() as GraphQLOutputType)
                 .build()
             buildingEnv.addOperation(MUTATION, fieldDefinition)
@@ -38,7 +38,7 @@ class DeleteHandler private constructor(
             if (!canHandle(type)) {
                 return null
             }
-            val idField = type.fieldDefinitions.find { it.isID() } ?: return null
+            val idField = type.getIdField() ?: return null
             return when {
                 fieldDefinition.name == "delete${type.name}" -> DeleteHandler(type, idField, fieldDefinition)
                 else -> null
@@ -50,10 +50,7 @@ class DeleteHandler private constructor(
             if (!schemaConfig.mutation.enabled || schemaConfig.mutation.exclude.contains(typeName)) {
                 return false
             }
-            if (type.fieldDefinitions.find { it.isID() } == null) {
-                return false
-            }
-            return true
+            return type.getIdField() != null
         }
     }
 

src/main/kotlin/org/neo4j/graphql/handler/MergeOrUpdateHandler.kt

@@ -42,7 +42,7 @@ class MergeOrUpdateHandler private constructor(
             if (!canHandle(type)) {
                 return null
             }
-            val idField = type.fieldDefinitions.find { it.isID() } ?: return null
+            val idField = type.getIdField() ?: return null
             return when {
                 fieldDefinition.name == "merge${type.name}" -> MergeOrUpdateHandler(type, true, idField, fieldDefinition)
                 fieldDefinition.name == "update${type.name}" -> MergeOrUpdateHandler(type, false, idField, fieldDefinition)
@@ -55,7 +55,7 @@ class MergeOrUpdateHandler private constructor(
             if (!schemaConfig.mutation.enabled || schemaConfig.mutation.exclude.contains(typeName)) {
                 return false
             }
-            if (type.fieldDefinitions.find { it.isID() } == null) {
+            if (type.getIdField() == null) {
                 return false
             }
             if (type.relevantFields().none { !it.isID() }) {

src/main/kotlin/org/neo4j/graphql/handler/relation/BaseRelationHandler.kt

@@ -27,9 +27,9 @@ abstract class BaseRelationHandler(
                 nullableResult: Boolean
         ): GraphQLFieldDefinition.Builder? {
 
-            val targetType = targetField.type.getInnerFieldsContainer();
-            val sourceIdField = source.fieldDefinitions.find { it.isID() }
-            val targetIdField = targetType.fieldDefinitions.find { it.isID() }
+            val targetType = targetField.type.getInnerFieldsContainer()
+            val sourceIdField = source.getIdField()
+            val targetIdField = targetType.getIdField()
             if (sourceIdField == null || targetIdField == null) {
                 return null
             }
@@ -59,7 +59,7 @@ abstract class BaseRelationHandler(
                 // TODO we do not mutate the node but the relation, I think this check should be different
                 return false
             }
-            if (type.fieldDefinitions.find { it.isID() } == null) {
+            if (type.getIdField() == null) {
                 return false
             }
             return true
@@ -70,7 +70,7 @@ abstract class BaseRelationHandler(
             if (targetField.getDirective(DirectiveConstants.RELATION) == null) {
                 return false
             }
-            if (type.fieldDefinitions.find { it.isID() } == null) {
+            if (type.getIdField() == null) {
                 return false
             }
             return true
@@ -103,9 +103,9 @@ abstract class BaseRelationHandler(
             }
             val relation = sourceType.relationshipFor(targetField.name) ?: return null
 
-            val targetType = targetField.type.getInnerFieldsContainer();
-            val sourceIdField = sourceType.fieldDefinitions.find { it.isID() }
-            val targetIdField = targetType.fieldDefinitions.find { it.isID() }
+            val targetType = targetField.type.getInnerFieldsContainer()
+            val sourceIdField = sourceType.getIdField()
+            val targetIdField = targetType.getIdField()
             if (sourceIdField == null || targetIdField == null) {
                 return null
             }