Rely on GraphQL-Schema instead of Schema-Definitions #62
Conversation
| </executions> | ||
| <configuration> | ||
| <includes> | ||
| <!-- TODO what to do with this part? --> |
There was a problem hiding this comment.
good question, need to check. I guess we just copied that from somewhere for maven central release.
| const val MUTATION = "Mutation" | ||
| } | ||
|
|
||
| abstract fun augmentType(type: GraphQLFieldsContainer, buildingEnv: BuildingEnv) |
There was a problem hiding this comment.
does this have to be abstract?
There was a problem hiding this comment.
since this class is the blueprint of a Handler: yes
There was a problem hiding this comment.
if this method was not abstract the handler could be handled by a closure
|
|
||
| abstract fun createDataFetcher(rootType: GraphQLObjectType, fieldDefinition: GraphQLFieldDefinition): DataFetcher<Cypher>? | ||
|
|
||
| fun input(name: String, type: GraphQLType): GraphQLArgument { |
There was a problem hiding this comment.
is this the right place for this function? we have similar things in other places.
# Conflicts: # src/main/kotlin/org/neo4j/graphql/Cypher.kt # src/main/kotlin/org/neo4j/graphql/SchemaBuilder.kt # src/test/kotlin/DataFetcherInterceptorDemo.kt
| var type = field.type as GraphQLType | ||
| type = getInputType(type) | ||
| type = if (forceOptionalProvider.invoke(field)) { | ||
| (type as? GraphQLNonNull)?.wrappedType ?: type |
There was a problem hiding this comment.
shouldn't there be an extension function for this?
There was a problem hiding this comment.
since it is operation on a list, I don't see the right cutting point here to extract the function as extension function.
| return relevantFields.map { field -> | ||
| var type = field.type as GraphQLType | ||
| type = getInputType(type) | ||
| type = if (forceOptionalProvider.invoke(field)) { |
There was a problem hiding this comment.
you can just use the call syntax forceOptionalProvider(field)
| val rootType = types[rootTypeName] | ||
| types[rootTypeName] = if (rootType == null) { | ||
| val builder = GraphQLObjectType.newObject() | ||
| builder.name(rootTypeName) |
There was a problem hiding this comment.
should it also be added to types ?
if not multiple calls to this method would create the root type several times
There was a problem hiding this comment.
the assignment is in line 50
| .build() | ||
| } else { | ||
| val existingRootType = (rootType as? GraphQLObjectType | ||
| ?: throw IllegalStateException("root type $rootTypeName is not an object type")) |
There was a problem hiding this comment.
should report what it actually is
| val existingRootType = (rootType as? GraphQLObjectType | ||
| ?: throw IllegalStateException("root type $rootTypeName is not an object type")) | ||
| if (existingRootType.getFieldDefinition(fieldDefinition.name) != null) { | ||
| return // definition already exists |
| if (existingRootType.getFieldDefinition(fieldDefinition.name) != null) { | ||
| return // definition already exists | ||
| } | ||
| existingRootType |
There was a problem hiding this comment.
will this create a new type or do an in place update?
in the former case we need to replace it in types map
There was a problem hiding this comment.
the assignment is in line 50
|
|
||
| fun addFilterType(type: GraphQLFieldsContainer, handled: MutableSet<String> = mutableSetOf()): String { | ||
| val filterName = "_${type.name}Filter" | ||
| if (handled.contains(filterName)) { |
| ?.let { types[it.inputDefinition] } as? GraphQLInputType | ||
| ?: throw IllegalArgumentException("Cannot find input type for ${inner.name}") | ||
| } | ||
| return type as? GraphQLInputType |
There was a problem hiding this comment.
wouldn't have been returned then already in the first if?
| return | ||
| } | ||
| val typeName = type.name() | ||
| val idField = type.fieldDefinitions.find { it.isID() } ?: return |
| .type(TypeName(typeName)) | ||
| val fieldDefinition = buildingEnv | ||
| .buildFieldDefinition("delete", type, listOf(idField), nullableResult = true) | ||
| .description("Deletes ${type.name} and returns its ID on successful deletion") |
There was a problem hiding this comment.
doesn't return the id but the type itself.
| if (!canHandle(type)) { | ||
| return null | ||
| } | ||
| val idField = type.fieldDefinitions.find { it.isID() } ?: return null |
| if (!schemaConfig.mutation.enabled || schemaConfig.mutation.exclude.contains(typeName)) { | ||
| return false | ||
| } | ||
| if (type.fieldDefinitions.find { it.isID() } == null) { |
2 participants
I questioned the whole
FieldDefinitionvsGraphQLFieldDefinitionand came to the conclusion thatGraphQLFieldDefinitionshould be the entity on which all enhancements rely on.I have refactored everything to that effect again.