Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor instruction definitions #644

Draft
wants to merge 8 commits into
base: improve-factory
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/src/main/java/graphql/nadel/NadelSchemas.kt
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,11 @@ data class NadelSchemas(
val underlyingSchemaGenerator = UnderlyingSchemaGenerator()

return builder.overallSchemaReaders.map { (serviceName, reader) ->
val nadelDefinitions = SchemaUtil.parseSchemaDefinitions(
val schemaDefinitions = SchemaUtil.parseSchemaDefinitions(
reader,
captureSourceLocation = captureSourceLocation,
)
val nadelDefinitionRegistry = NadelDefinitionRegistry.from(nadelDefinitions)
val typeDefinitionRegistry = NadelTypeDefinitionRegistry.from(schemaDefinitions)

// Builder should enforce non-null entry
val underlyingSchema = underlyingSchemaGenerator.buildUnderlyingSchema(
Expand All @@ -217,7 +217,7 @@ data class NadelSchemas(

val serviceExecution = serviceExecutionFactory.getServiceExecution(serviceName)

Service(serviceName, underlyingSchema, serviceExecution, nadelDefinitionRegistry)
Service(serviceName, underlyingSchema, serviceExecution, typeDefinitionRegistry)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import java.util.Collections
* Alternative to [graphql.schema.idl.TypeDefinitionRegistry] but is more generic
* and tailored to Nadel specific operations to build the overall schema.
*/
class NadelDefinitionRegistry {
class NadelTypeDefinitionRegistry {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed for clarity as I have also created a NadelInstructionDefinitionRegistry

private val _definitions: MutableList<AnySDLDefinition> = ArrayList()
private val definitionsByClass = LinkedHashMap<Class<out AnySDLDefinition>, MutableList<AnySDLDefinition>>()
private val definitionsByName = LinkedHashMap<String, MutableList<AnySDLDefinition>>()
Expand Down Expand Up @@ -101,8 +101,8 @@ class NadelDefinitionRegistry {

companion object {
@JvmStatic
fun from(definitions: List<AnySDLDefinition>): NadelDefinitionRegistry {
val registry = NadelDefinitionRegistry()
fun from(definitions: List<AnySDLDefinition>): NadelTypeDefinitionRegistry {
val registry = NadelTypeDefinitionRegistry()
definitions.forEach(registry::add)
return registry
}
Expand Down
2 changes: 1 addition & 1 deletion lib/src/main/java/graphql/nadel/Service.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ open class Service(
/**
* These are the GraphQL definitions that a service contributes to the OVERALL schema.
*/
val definitionRegistry: NadelDefinitionRegistry,
val definitionRegistry: NadelTypeDefinitionRegistry,
) {
override fun toString(): String {
return "Service{name='$name'}"
Expand Down
3 changes: 0 additions & 3 deletions lib/src/main/java/graphql/nadel/definition/NadelDefinition.kt

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package graphql.nadel.definition

interface NadelInstructionDefinition
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed from NadelDefinition

File too small for Git to pick up on the rename.

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql.nadel.definition

import graphql.nadel.engine.blueprint.NadelSchemaTraverserElement
import graphql.schema.GraphQLDirective
import graphql.schema.GraphQLEnumType
import graphql.schema.GraphQLFieldsContainer
Expand Down Expand Up @@ -168,3 +169,14 @@ fun GraphQLNamedType.coordinates(): NadelSchemaMemberCoordinates.Type {
else -> throw IllegalArgumentException(javaClass.name)
}
}

internal fun NadelSchemaTraverserElement.Type.coordinates(): NadelSchemaMemberCoordinates.Type {
return when (this) {
is NadelSchemaTraverserElement.EnumType -> NadelSchemaMemberCoordinates.Enum(node.name)
is NadelSchemaTraverserElement.InputObjectType -> NadelSchemaMemberCoordinates.InputObject(node.name)
is NadelSchemaTraverserElement.ScalarType -> NadelSchemaMemberCoordinates.Scalar(node.name)
is NadelSchemaTraverserElement.InterfaceType -> NadelSchemaMemberCoordinates.Interface(node.name)
is NadelSchemaTraverserElement.ObjectType -> NadelSchemaMemberCoordinates.Object(node.name)
is NadelSchemaTraverserElement.UnionType -> NadelSchemaMemberCoordinates.Union(node.name)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,27 @@ import graphql.language.ArrayValue
import graphql.language.DirectiveDefinition
import graphql.language.FieldDefinition
import graphql.language.ObjectValue
import graphql.nadel.definition.NadelDefinition
import graphql.nadel.definition.NadelInstructionDefinition
import graphql.nadel.definition.hydration.NadelHydrationDefinition.Keyword
import graphql.nadel.engine.util.JsonMap
import graphql.nadel.engine.util.parseDefinition
import graphql.nadel.validation.NadelValidationContext
import graphql.schema.GraphQLAppliedDirective
import graphql.schema.GraphQLFieldDefinition

fun FieldDefinition.hasHydratedDefinition(): Boolean {
return hasDirective(Keyword.hydrated)
}

context(NadelValidationContext)
@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR)
fun GraphQLFieldDefinition.hasHydratedDefinition(): Nothing {
throw UnsupportedOperationException()
}

fun GraphQLFieldDefinition.hasHydratedDefinition(): Boolean {
return hasAppliedDirective(Keyword.hydrated)
}

context(NadelValidationContext)
@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR)
fun GraphQLFieldDefinition.parseHydrationDefinitions(): Nothing {
throw UnsupportedOperationException()
}

fun GraphQLFieldDefinition.parseHydrationDefinitions(): List<NadelHydrationDefinition> {
return getAppliedDirectives(Keyword.hydrated)
.map(::NadelHydrationDefinitionImpl)
}

interface NadelHydrationDefinition : NadelDefinition {
interface NadelHydrationDefinition : NadelInstructionDefinition {
companion object {
val directiveDefinition = parseDefinition<DirectiveDefinition>(
// language=GraphQL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ package graphql.nadel.definition.partition

import graphql.language.DirectiveDefinition
import graphql.language.FieldDefinition
import graphql.nadel.definition.NadelDefinition
import graphql.nadel.definition.NadelInstructionDefinition
import graphql.nadel.definition.partition.NadelPartitionDefinition.Keyword
import graphql.nadel.engine.util.parseDefinition
import graphql.schema.GraphQLAppliedDirective
import graphql.schema.GraphQLFieldDefinition

class NadelPartitionDefinition(
private val appliedDirective: GraphQLAppliedDirective,
) : NadelDefinition {
) : NadelInstructionDefinition {
companion object {
val directiveDefinition = parseDefinition<DirectiveDefinition>(
// language=GraphQL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package graphql.nadel.definition.renamed

import graphql.language.DirectiveDefinition
import graphql.language.DirectivesContainer
import graphql.nadel.definition.NadelDefinition
import graphql.nadel.definition.NadelInstructionDefinition
import graphql.nadel.definition.renamed.NadelRenamedDefinition.Keyword
import graphql.nadel.engine.util.parseDefinition
import graphql.nadel.validation.NadelValidationContext
Expand All @@ -11,7 +11,7 @@ import graphql.schema.GraphQLDirectiveContainer
import graphql.schema.GraphQLFieldDefinition
import graphql.schema.GraphQLNamedType

sealed class NadelRenamedDefinition : NadelDefinition {
sealed class NadelRenamedDefinition : NadelInstructionDefinition {
companion object {
val directiveDefinition = parseDefinition<DirectiveDefinition>(
// language=GraphQL
Expand Down Expand Up @@ -48,12 +48,6 @@ sealed class NadelRenamedDefinition : NadelDefinition {
}
}

context(NadelValidationContext)
@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR)
fun GraphQLDirectiveContainer.hasRenameDefinition(): Nothing {
throw UnsupportedOperationException()
}

fun GraphQLDirectiveContainer.hasRenameDefinition(): Boolean {
return hasAppliedDirective(Keyword.renamed)
}
Expand All @@ -62,25 +56,13 @@ fun DirectivesContainer<*>.hasRenameDefinition(): Boolean {
return hasDirective(Keyword.renamed)
}

context(NadelValidationContext)
@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR)
fun GraphQLFieldDefinition.parseRenamedOrNull(): Nothing {
throw UnsupportedOperationException()
}

fun GraphQLFieldDefinition.parseRenamedOrNull(): NadelRenamedDefinition.Field? {
val directive = getAppliedDirective(Keyword.renamed)
?: return null

return NadelRenamedDefinition.Field(directive)
}

context(NadelValidationContext)
@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR)
fun GraphQLNamedType.parseRenamedOrNull(): Nothing {
throw UnsupportedOperationException()
}

fun GraphQLNamedType.parseRenamedOrNull(): NadelRenamedDefinition.Type? {
val directive = (this as GraphQLDirectiveContainer).getAppliedDirective(Keyword.renamed)
?: return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package graphql.nadel.definition.virtualType

import graphql.language.DirectiveDefinition
import graphql.language.DirectivesContainer
import graphql.nadel.definition.NadelDefinition
import graphql.nadel.definition.NadelInstructionDefinition
import graphql.nadel.engine.util.parseDefinition
import graphql.schema.GraphQLDirectiveContainer
import graphql.schema.GraphQLSchemaElement
Expand All @@ -16,7 +16,7 @@ internal fun DirectivesContainer<*>.hasVirtualTypeDefinition(): Boolean {
return hasDirective(NadelVirtualTypeDefinition.directiveDefinition.name)
}

internal class NadelVirtualTypeDefinition : NadelDefinition {
internal class NadelVirtualTypeDefinition : NadelInstructionDefinition {
companion object {
val directiveDefinition = parseDefinition<DirectiveDefinition>(
// language=GraphQL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import graphql.ExecutionInput
import graphql.GraphQL
import graphql.GraphqlErrorHelper.toSpecification
import graphql.language.AstPrinter
import graphql.nadel.NadelDefinitionRegistry
import graphql.nadel.NadelTypeDefinitionRegistry
import graphql.nadel.Service
import graphql.nadel.ServiceExecution
import graphql.nadel.ServiceExecutionParameters
Expand All @@ -24,7 +24,7 @@ import java.util.concurrent.CompletableFuture
internal class IntrospectionService(
schema: GraphQLSchema,
introspectionRunnerFactory: NadelIntrospectionRunnerFactory,
) : Service(name, schema, introspectionRunnerFactory.make(schema), NadelDefinitionRegistry()) {
) : Service(name, schema, introspectionRunnerFactory.make(schema), NadelTypeDefinitionRegistry()) {
companion object {
const val name = "__introspection"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import graphql.language.ObjectTypeDefinition.newObjectTypeDefinition
import graphql.language.ScalarTypeDefinition.newScalarTypeDefinition
import graphql.language.SchemaDefinition
import graphql.language.SourceLocation
import graphql.nadel.NadelDefinitionRegistry
import graphql.nadel.NadelTypeDefinitionRegistry
import graphql.nadel.NadelOperationKind
import graphql.nadel.util.AnyNamedNode
import graphql.nadel.util.AnySDLDefinition
Expand All @@ -22,7 +22,7 @@ import graphql.schema.idl.WiringFactory

internal class OverallSchemaGenerator {
fun buildOverallSchema(
serviceRegistries: List<NadelDefinitionRegistry>,
serviceRegistries: List<NadelTypeDefinitionRegistry>,
wiringFactory: WiringFactory,
): GraphQLSchema {
val schemaGenerator = SchemaGenerator()
Expand All @@ -33,7 +33,7 @@ internal class OverallSchemaGenerator {
return schemaGenerator.makeExecutableSchema(createTypeRegistry(serviceRegistries), runtimeWiring)
}

private fun createTypeRegistry(serviceRegistries: List<NadelDefinitionRegistry>): TypeDefinitionRegistry {
private fun createTypeRegistry(serviceRegistries: List<NadelTypeDefinitionRegistry>): TypeDefinitionRegistry {
val topLevelFields = NadelOperationKind.entries
.associateWith {
mutableListOf<FieldDefinition>()
Expand Down Expand Up @@ -128,7 +128,7 @@ internal class OverallSchemaGenerator {
private fun collectTypes(
topLevelFields: Map<NadelOperationKind, MutableList<FieldDefinition>>,
allDefinitions: MutableList<AnySDLDefinition>,
definitionRegistry: NadelDefinitionRegistry,
definitionRegistry: NadelTypeDefinitionRegistry,
) {
val opDefinitions = definitionRegistry.operationMap
val opTypeNames: MutableSet<String> = HashSet(3)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package graphql.nadel.validation

import graphql.nadel.engine.util.unwrapAll
import graphql.schema.GraphQLType

class NadelAssignableTypeValidation internal constructor(
private val typeWrappingValidation: NadelTypeWrappingValidation,
) {
context(NadelValidationContext)
fun isOutputTypeAssignable(
overallType: GraphQLType,
underlyingType: GraphQLType,
): Boolean {
// Note: the (supplied) value comes from the underlying service, and that value needs to fit the (required) overall field's type
return isTypeAssignable(
suppliedType = underlyingType,
requiredType = overallType,
// Compare underlying type names
suppliedTypeName = underlyingType.unwrapAll().name,
requiredTypeName = instructionDefinitions.getUnderlyingTypeName(overallType.unwrapAll()),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the util into NadelInstructionDefinitionRegistry

)
}

context(NadelValidationContext)
fun isInputTypeAssignable(
overallType: GraphQLType,
underlyingType: GraphQLType,
): Boolean {
// Note: the (supplied) value comes from the user (overall schema) and needs to be assigned to the (required) underlying type
return isTypeAssignable(
suppliedType = overallType,
requiredType = underlyingType,
// Compare underlying type names
suppliedTypeName = instructionDefinitions.getUnderlyingTypeName(overallType.unwrapAll()),
requiredTypeName = underlyingType.unwrapAll().name,
)
}

context(NadelValidationContext)
fun isTypeAssignable(
suppliedType: GraphQLType,
requiredType: GraphQLType,
suppliedTypeName: String,
requiredTypeName: String,
): Boolean {
return suppliedTypeName == requiredTypeName && isTypeWrappingValid(suppliedType, requiredType)
}

private fun isTypeWrappingValid(
suppliedType: GraphQLType,
requiredType: GraphQLType,
): Boolean {
return typeWrappingValidation.isTypeWrappingValid(
lhs = suppliedType,
rhs = requiredType,
rule = NadelTypeWrappingValidation.Rule.LHS_MUST_BE_STRICTER_OR_SAME,
)
}
}
Loading