Skip to content

Commit

Permalink
[3.x.x][generator] Only add directives with valid locations (#924)
Browse files Browse the repository at this point in the history
* Only add directives with valid locations

When generating a schema you can have an annotation on any part of the schema, assuming you haven't set the Target, so if you are restricting the directive locations to only specific places they may still be added to the schema and then fail generation as they are added to invalid locations. While this could be viewed as a customer error, this is more of an issue with graphql-kotlin because we can have input and output objects with the same directives and things like the KeyDirective from federation are only valid on output. So in that case we shouldn't fail if someone uses the type as input elsewhere, it will still not be marked as a federation type for the input

* Rename function name

* Add unit tests for coverage

Co-authored-by: Shane Myrick <[email protected]>
  • Loading branch information
smyrick and Shane Myrick authored Oct 29, 2020
1 parent 7f37f24 commit f553dd2
Show file tree
Hide file tree
Showing 20 changed files with 188 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.expediagroup.graphql.generator.extensions.isInterface
import com.expediagroup.graphql.generator.extensions.isListType
import com.expediagroup.graphql.generator.extensions.isUnion
import com.expediagroup.graphql.generator.extensions.safeCast
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLArgument
import kotlin.reflect.KClass
import kotlin.reflect.KParameter
Expand All @@ -49,7 +50,7 @@ internal fun generateArgument(generator: SchemaGenerator, parameter: KParameter)
.description(parameter.getGraphQLDescription())
.type(graphQLType.safeCast())

generateDirectives(generator, parameter).forEach {
generateDirectives(generator, parameter, DirectiveLocation.ARGUMENT_DEFINITION).forEach {
builder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.expediagroup.graphql.generator.extensions.getPropertyAnnotations
import com.expediagroup.graphql.generator.extensions.getSimpleName
import com.expediagroup.graphql.generator.extensions.getValidProperties
import com.expediagroup.graphql.generator.extensions.safeCast
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLArgument
import graphql.schema.GraphQLDirective
import java.lang.reflect.Field
Expand All @@ -29,20 +30,27 @@ import kotlin.reflect.KClass
import kotlin.reflect.KProperty
import com.expediagroup.graphql.annotations.GraphQLDirective as GraphQLDirectiveAnnotation

internal fun generateDirectives(generator: SchemaGenerator, element: KAnnotatedElement, parentClass: KClass<*>? = null): List<GraphQLDirective> {
internal fun generateDirectives(
generator: SchemaGenerator,
element: KAnnotatedElement,
location: DirectiveLocation,
parentClass: KClass<*>? = null
): List<GraphQLDirective> {
val annotations = when {
element is KProperty<*> && parentClass != null -> element.getPropertyAnnotations(parentClass)
else -> element.annotations
}

return annotations
.mapNotNull { it.getDirectiveInfo() }
.filter { it.directiveAnnotation.locations.contains(location) }
.map { getDirective(generator, it) }
}

internal fun generateFieldDirectives(generator: SchemaGenerator, field: Field): List<GraphQLDirective> =
internal fun generateEnumValueDirectives(generator: SchemaGenerator, field: Field): List<GraphQLDirective> =
field.annotations
.mapNotNull { it.getDirectiveInfo() }
.filter { it.directiveAnnotation.locations.contains(DirectiveLocation.ENUM_VALUE) }
.map { getDirective(generator, it) }

private fun getDirective(generator: SchemaGenerator, directiveInfo: DirectiveInfo): GraphQLDirective {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.expediagroup.graphql.generator.extensions.getGraphQLDescription
import com.expediagroup.graphql.generator.extensions.getGraphQLName
import com.expediagroup.graphql.generator.extensions.getSimpleName
import com.expediagroup.graphql.generator.extensions.safeCast
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLEnumType
import graphql.schema.GraphQLEnumValueDefinition
import kotlin.reflect.KClass
Expand All @@ -33,7 +34,7 @@ internal fun generateEnum(generator: SchemaGenerator, kClass: KClass<out Enum<*>
enumBuilder.name(kClass.getSimpleName())
enumBuilder.description(kClass.getGraphQLDescription())

generateDirectives(generator, kClass).forEach {
generateDirectives(generator, kClass, DirectiveLocation.ENUM).forEach {
enumBuilder.withDirective(it)
}

Expand All @@ -51,7 +52,7 @@ private fun getEnumValueDefinition(generator: SchemaGenerator, enum: Enum<*>, kC
valueBuilder.name(name)
valueBuilder.value(name)

generateFieldDirectives(generator, valueField).forEach {
generateEnumValueDirectives(generator, valueField).forEach {
valueBuilder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.expediagroup.graphql.generator.extensions.getGraphQLDescription
import com.expediagroup.graphql.generator.extensions.getValidArguments
import com.expediagroup.graphql.generator.extensions.safeCast
import com.expediagroup.graphql.generator.types.utils.getWrappedReturnType
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.FieldCoordinates
import graphql.schema.GraphQLFieldDefinition
import graphql.schema.GraphQLOutputType
Expand All @@ -40,7 +41,7 @@ internal fun generateFunction(generator: SchemaGenerator, fn: KFunction<*>, pare
builder.withDirective(deprecatedDirectiveWithReason(it))
}

generateDirectives(generator, fn).forEach {
generateDirectives(generator, fn, DirectiveLocation.FIELD_DEFINITION).forEach {
builder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.expediagroup.graphql.generator.extensions.getGraphQLDescription
import com.expediagroup.graphql.generator.extensions.getSimpleName
import com.expediagroup.graphql.generator.extensions.getValidProperties
import com.expediagroup.graphql.generator.extensions.safeCast
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLInputObjectType
import kotlin.reflect.KClass

Expand All @@ -30,7 +31,7 @@ internal fun generateInputObject(generator: SchemaGenerator, kClass: KClass<*>):
builder.name(kClass.getSimpleName(isInputClass = true))
builder.description(kClass.getGraphQLDescription())

generateDirectives(generator, kClass).forEach {
generateDirectives(generator, kClass, DirectiveLocation.INPUT_OBJECT).forEach {
builder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.expediagroup.graphql.generator.SchemaGenerator
import com.expediagroup.graphql.generator.extensions.getPropertyDescription
import com.expediagroup.graphql.generator.extensions.getPropertyName
import com.expediagroup.graphql.generator.extensions.safeCast
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLInputObjectField
import graphql.schema.GraphQLInputType
import kotlin.reflect.KClass
Expand All @@ -35,7 +36,7 @@ internal fun generateInputProperty(generator: SchemaGenerator, prop: KProperty<*
builder.name(prop.getPropertyName(parentClass))
builder.type(graphQLInputType)

generateDirectives(generator, prop, parentClass).forEach {
generateDirectives(generator, prop, DirectiveLocation.INPUT_FIELD_DEFINITION, parentClass).forEach {
builder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.expediagroup.graphql.generator.extensions.getValidSuperclasses
import com.expediagroup.graphql.generator.extensions.safeCast
import com.expediagroup.graphql.generator.state.AdditionalType
import graphql.TypeResolutionEnvironment
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLInterfaceType
import graphql.schema.GraphQLTypeReference
import kotlin.reflect.KClass
Expand All @@ -37,7 +38,7 @@ internal fun generateInterface(generator: SchemaGenerator, kClass: KClass<*>): G
builder.name(kClass.getSimpleName())
builder.description(kClass.getGraphQLDescription())

generateDirectives(generator, kClass).forEach {
generateDirectives(generator, kClass, DirectiveLocation.INTERFACE).forEach {
builder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.expediagroup.graphql.exceptions.InvalidMutationTypeException
import com.expediagroup.graphql.generator.SchemaGenerator
import com.expediagroup.graphql.generator.extensions.getValidFunctions
import com.expediagroup.graphql.generator.extensions.isNotPublic
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLObjectType

internal fun generateMutations(generator: SchemaGenerator, mutations: List<TopLevelObject>): GraphQLObjectType? {
Expand All @@ -37,7 +38,7 @@ internal fun generateMutations(generator: SchemaGenerator, mutations: List<TopLe
throw InvalidMutationTypeException(mutation.kClass)
}

generateDirectives(generator, mutation.kClass).forEach {
generateDirectives(generator, mutation.kClass, DirectiveLocation.OBJECT).forEach {
mutationBuilder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.expediagroup.graphql.generator.extensions.getValidFunctions
import com.expediagroup.graphql.generator.extensions.getValidProperties
import com.expediagroup.graphql.generator.extensions.getValidSuperclasses
import com.expediagroup.graphql.generator.extensions.safeCast
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLInterfaceType
import graphql.schema.GraphQLObjectType
import graphql.schema.GraphQLTypeReference
Expand All @@ -37,7 +38,7 @@ internal fun generateObject(generator: SchemaGenerator, kClass: KClass<*>): Grap
builder.name(name)
builder.description(kClass.getGraphQLDescription())

generateDirectives(generator, kClass).forEach {
generateDirectives(generator, kClass, DirectiveLocation.OBJECT).forEach {
builder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.expediagroup.graphql.generator.extensions.getPropertyDescription
import com.expediagroup.graphql.generator.extensions.getPropertyName
import com.expediagroup.graphql.generator.extensions.getSimpleName
import com.expediagroup.graphql.generator.extensions.safeCast
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.FieldCoordinates
import graphql.schema.GraphQLFieldDefinition
import graphql.schema.GraphQLOutputType
Expand All @@ -43,7 +44,7 @@ internal fun generateProperty(generator: SchemaGenerator, prop: KProperty<*>, pa
fieldBuilder.withDirective(deprecatedDirectiveWithReason(it))
}

generateDirectives(generator, prop, parentClass).forEach {
generateDirectives(generator, prop, DirectiveLocation.FIELD_DEFINITION, parentClass).forEach {
fieldBuilder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.expediagroup.graphql.exceptions.InvalidQueryTypeException
import com.expediagroup.graphql.generator.SchemaGenerator
import com.expediagroup.graphql.generator.extensions.getValidFunctions
import com.expediagroup.graphql.generator.extensions.isNotPublic
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLObjectType

internal fun generateQueries(generator: SchemaGenerator, queries: List<TopLevelObject>): GraphQLObjectType {
Expand All @@ -32,7 +33,7 @@ internal fun generateQueries(generator: SchemaGenerator, queries: List<TopLevelO
throw InvalidQueryTypeException(query.kClass)
}

generateDirectives(generator, query.kClass).forEach {
generateDirectives(generator, query.kClass, DirectiveLocation.OBJECT).forEach {
queryBuilder.withDirective(it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.expediagroup.graphql.exceptions.InvalidSubscriptionTypeException
import com.expediagroup.graphql.generator.SchemaGenerator
import com.expediagroup.graphql.generator.extensions.getValidFunctions
import com.expediagroup.graphql.generator.extensions.isNotPublic
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLObjectType

internal fun generateSubscriptions(generator: SchemaGenerator, subscriptions: List<TopLevelObject>): GraphQLObjectType? {
Expand All @@ -38,6 +39,10 @@ internal fun generateSubscriptions(generator: SchemaGenerator, subscriptions: Li
throw InvalidSubscriptionTypeException(kClass)
}

generateDirectives(generator, subscription.kClass, DirectiveLocation.OBJECT).forEach {
subscriptionBuilder.withDirective(it)
}

kClass.getValidFunctions(generator.config.hooks)
.forEach {
if (generator.config.hooks.isValidSubscriptionReturnType(kClass, it).not()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.expediagroup.graphql.generator.extensions.getGraphQLDescription
import com.expediagroup.graphql.generator.extensions.getSimpleName
import com.expediagroup.graphql.generator.extensions.safeCast
import graphql.TypeResolutionEnvironment
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLObjectType
import graphql.schema.GraphQLTypeReference
import graphql.schema.GraphQLUnionType
Expand All @@ -33,7 +34,7 @@ internal fun generateUnion(generator: SchemaGenerator, kClass: KClass<*>): Graph
builder.name(kClass.getSimpleName())
builder.description(kClass.getGraphQLDescription())

generateDirectives(generator, kClass).forEach {
generateDirectives(generator, kClass, DirectiveLocation.UNION).forEach {
builder.withDirective(it)
}

Expand Down
Loading

0 comments on commit f553dd2

Please sign in to comment.