From 80d59aca93c053f8273d5defaea0e36a4dfb98cd Mon Sep 17 00:00:00 2001 From: Franklin Wang <9077461+gnawf@users.noreply.github.com> Date: Tue, 10 Dec 2024 12:54:32 +1100 Subject: [PATCH] Treat some GraphQL Java types as pseudo sealed types (#636) * Treat some GraphQL Java types as pseudo sealed types * Kotlin spice --- .../NadelSchemaMemberCoordinates.kt | 27 +++--- .../blueprint/NadelFastSchemaTraverser.kt | 9 +- .../blueprint/NadelSchemaTraverserElement.kt | 69 ++++++------- .../engine/util/NadelPseudoSealedType.kt | 78 +++++++++++++++ .../archunit/ArchUnitEqualsClassesExactly.kt | 63 ++++++++++++ .../ForbiddenGraphQLJavaUsageTest.kt | 2 +- .../nadel/{ => archunit}/NadelPrefixTest.kt | 2 +- .../archunit/NadelPseudoSealedTypeKtTest.kt | 97 +++++++++++++++++++ 8 files changed, 287 insertions(+), 60 deletions(-) create mode 100644 lib/src/main/java/graphql/nadel/engine/util/NadelPseudoSealedType.kt create mode 100644 lib/src/test/kotlin/graphql/nadel/archunit/ArchUnitEqualsClassesExactly.kt rename lib/src/test/kotlin/graphql/nadel/{ => archunit}/ForbiddenGraphQLJavaUsageTest.kt (97%) rename lib/src/test/kotlin/graphql/nadel/{ => archunit}/NadelPrefixTest.kt (99%) create mode 100644 lib/src/test/kotlin/graphql/nadel/archunit/NadelPseudoSealedTypeKtTest.kt diff --git a/lib/src/main/java/graphql/nadel/definition/NadelSchemaMemberCoordinates.kt b/lib/src/main/java/graphql/nadel/definition/NadelSchemaMemberCoordinates.kt index e884ccf61..2d984f141 100644 --- a/lib/src/main/java/graphql/nadel/definition/NadelSchemaMemberCoordinates.kt +++ b/lib/src/main/java/graphql/nadel/definition/NadelSchemaMemberCoordinates.kt @@ -1,5 +1,6 @@ package graphql.nadel.definition +import graphql.nadel.engine.util.whenType import graphql.schema.GraphQLDirective import graphql.schema.GraphQLEnumType import graphql.schema.GraphQLFieldsContainer @@ -150,21 +151,19 @@ fun GraphQLDirective.coordinates(): NadelSchemaMemberCoordinates.Directive { } fun GraphQLFieldsContainer.coordinates(): NadelSchemaMemberCoordinates.FieldContainer { - return when (this) { - is GraphQLObjectType -> coordinates() - is GraphQLInterfaceType -> coordinates() - else -> throw IllegalArgumentException(javaClass.name) - } + return whenType( + interfaceType = GraphQLInterfaceType::coordinates, + objectType = GraphQLObjectType::coordinates, + ) } fun GraphQLNamedType.coordinates(): NadelSchemaMemberCoordinates.Type { - return when (this) { - is GraphQLUnionType -> coordinates() - is GraphQLInterfaceType -> coordinates() - is GraphQLEnumType -> coordinates() - is GraphQLInputObjectType -> coordinates() - is GraphQLObjectType -> coordinates() - is GraphQLScalarType -> coordinates() - else -> throw IllegalArgumentException(javaClass.name) - } + return whenType( + enumType = GraphQLEnumType::coordinates, + inputObjectType = GraphQLInputObjectType::coordinates, + interfaceType = GraphQLInterfaceType::coordinates, + objectType = GraphQLObjectType::coordinates, + scalarType = GraphQLScalarType::coordinates, + unionType = GraphQLUnionType::coordinates, + ) } diff --git a/lib/src/main/java/graphql/nadel/engine/blueprint/NadelFastSchemaTraverser.kt b/lib/src/main/java/graphql/nadel/engine/blueprint/NadelFastSchemaTraverser.kt index 6f8dd8c9b..105677442 100644 --- a/lib/src/main/java/graphql/nadel/engine/blueprint/NadelFastSchemaTraverser.kt +++ b/lib/src/main/java/graphql/nadel/engine/blueprint/NadelFastSchemaTraverser.kt @@ -17,10 +17,15 @@ internal class NadelSchemaTraverser { ) { val queue: MutableList = roots .mapNotNullTo(mutableListOf()) { typeName -> - val type = schema.typeMap[typeName] ?: schema.getDirective(typeName) + val type = schema.typeMap[typeName] // Types can be deleted by transformer, so they may not exist in end schema if (type == null) { - null + val directive = schema.getDirective(typeName) + if (directive == null) { + null + } else { + NadelSchemaTraverserElement.from(directive) + } } else { NadelSchemaTraverserElement.from(type) } diff --git a/lib/src/main/java/graphql/nadel/engine/blueprint/NadelSchemaTraverserElement.kt b/lib/src/main/java/graphql/nadel/engine/blueprint/NadelSchemaTraverserElement.kt index e7dc7777a..eb8d5424f 100644 --- a/lib/src/main/java/graphql/nadel/engine/blueprint/NadelSchemaTraverserElement.kt +++ b/lib/src/main/java/graphql/nadel/engine/blueprint/NadelSchemaTraverserElement.kt @@ -1,6 +1,7 @@ package graphql.nadel.engine.blueprint -import graphql.nadel.engine.util.unwrapAll +import graphql.nadel.engine.util.whenType +import graphql.nadel.engine.util.whenUnmodifiedType import graphql.schema.GraphQLAppliedDirective import graphql.schema.GraphQLAppliedDirectiveArgument import graphql.schema.GraphQLArgument @@ -42,14 +43,13 @@ internal sealed interface NadelSchemaTraverserElement { sealed interface OutputType : NadelSchemaTraverserElement, Type { companion object { fun from(type: GraphQLOutputType): OutputType { - return when (val node = type.unwrapAll()) { - is GraphQLEnumType -> EnumType(node) - is GraphQLInterfaceType -> InterfaceType(node) - is GraphQLObjectType -> ObjectType(node) - is GraphQLScalarType -> ScalarType(node) - is GraphQLUnionType -> UnionType(node) - else -> throw UnsupportedOperationException(type.javaClass.name) - } + return type.whenUnmodifiedType( + enumType = ::EnumType, + interfaceType = ::InterfaceType, + objectType = ::ObjectType, + scalarType = ::ScalarType, + unionType = ::UnionType, + ) } } } @@ -57,12 +57,11 @@ internal sealed interface NadelSchemaTraverserElement { sealed interface InputType : NadelSchemaTraverserElement, Type { companion object { fun from(type: GraphQLInputType): InputType { - return when (val node = type.unwrapAll()) { - is GraphQLEnumType -> EnumType(node) - is GraphQLInputObjectType -> InputObjectType(node) - is GraphQLScalarType -> ScalarType(node) - else -> throw UnsupportedOperationException(type.javaClass.name) - } + return type.whenUnmodifiedType( + enumType = ::EnumType, + inputObjectType = ::InputObjectType, + scalarType = ::ScalarType, + ) } } } @@ -215,33 +214,19 @@ internal sealed interface NadelSchemaTraverserElement { } companion object { - fun from(type: GraphQLNamedSchemaElement): NadelSchemaTraverserElement { - return when (type) { - is GraphQLEnumType -> { - EnumType(type) - } - is GraphQLInputObjectType -> { - InputObjectType(type) - } - is GraphQLInterfaceType -> { - InterfaceType(type) - } - is GraphQLObjectType -> { - ObjectType(type) - } - is GraphQLScalarType -> { - ScalarType(type) - } - is GraphQLUnionType -> { - UnionType(type) - } - is GraphQLDirective -> { - Directive(type) - } - else -> { - throw UnsupportedOperationException(type.javaClass.name) - } - } + fun from(type: GraphQLNamedType): NadelSchemaTraverserElement { + return type.whenType( + enumType = ::EnumType, + inputObjectType = ::InputObjectType, + interfaceType = ::InterfaceType, + objectType = ::ObjectType, + scalarType = ::ScalarType, + unionType = ::UnionType, + ) + } + + fun from(type: GraphQLDirective): NadelSchemaTraverserElement { + return Directive(type) } } } diff --git a/lib/src/main/java/graphql/nadel/engine/util/NadelPseudoSealedType.kt b/lib/src/main/java/graphql/nadel/engine/util/NadelPseudoSealedType.kt new file mode 100644 index 000000000..38ee69db7 --- /dev/null +++ b/lib/src/main/java/graphql/nadel/engine/util/NadelPseudoSealedType.kt @@ -0,0 +1,78 @@ +/** + * Util functions to treat some GraphQL Java classes _like_ sealed types. + * + * There are tests in `NadelPseudoSealedTypeKtTest` to ensure these assumptions are correct. + */ +package graphql.nadel.engine.util + +import graphql.schema.GraphQLEnumType +import graphql.schema.GraphQLFieldsContainer +import graphql.schema.GraphQLInputObjectType +import graphql.schema.GraphQLInputType +import graphql.schema.GraphQLInterfaceType +import graphql.schema.GraphQLNamedType +import graphql.schema.GraphQLObjectType +import graphql.schema.GraphQLOutputType +import graphql.schema.GraphQLScalarType +import graphql.schema.GraphQLUnionType + +internal inline fun GraphQLFieldsContainer.whenType( + interfaceType: (GraphQLInterfaceType) -> T, + objectType: (GraphQLObjectType) -> T, +): T { + return when (this) { + is GraphQLInterfaceType -> interfaceType(this) + is GraphQLObjectType -> objectType(this) + else -> throw IllegalStateException("Should never happen") + } +} + +internal inline fun GraphQLNamedType.whenType( + enumType: (GraphQLEnumType) -> T, + inputObjectType: (GraphQLInputObjectType) -> T, + interfaceType: (GraphQLInterfaceType) -> T, + objectType: (GraphQLObjectType) -> T, + scalarType: (GraphQLScalarType) -> T, + unionType: (GraphQLUnionType) -> T, +): T { + return when (this) { + is GraphQLEnumType -> enumType(this) + is GraphQLInputObjectType -> inputObjectType(this) + is GraphQLInterfaceType -> interfaceType(this) + is GraphQLObjectType -> objectType(this) + is GraphQLScalarType -> scalarType(this) + is GraphQLUnionType -> unionType(this) + else -> throw IllegalStateException("Should never happen") + } +} + +internal inline fun GraphQLOutputType.whenUnmodifiedType( + enumType: (GraphQLEnumType) -> T, + interfaceType: (GraphQLInterfaceType) -> T, + objectType: (GraphQLObjectType) -> T, + scalarType: (GraphQLScalarType) -> T, + unionType: (GraphQLUnionType) -> T, +): T { + return when (val unmodifiedType = this.unwrapAll()) { + is GraphQLEnumType -> enumType(unmodifiedType) + is GraphQLInterfaceType -> interfaceType(unmodifiedType) + is GraphQLObjectType -> objectType(unmodifiedType) + is GraphQLScalarType -> scalarType(unmodifiedType) + is GraphQLUnionType -> unionType(unmodifiedType) + else -> throw IllegalStateException("Should never happen") + } +} + +internal inline fun GraphQLInputType.whenUnmodifiedType( + enumType: (GraphQLEnumType) -> T, + inputObjectType: (GraphQLInputObjectType) -> T, + scalarType: (GraphQLScalarType) -> T, +): T { + return when (val unmodifiedType = this.unwrapAll()) { + is GraphQLEnumType -> enumType(unmodifiedType) + is GraphQLInputObjectType -> inputObjectType(unmodifiedType) + is GraphQLScalarType -> scalarType(unmodifiedType) + else -> throw IllegalStateException("Should never happen") + } +} + diff --git a/lib/src/test/kotlin/graphql/nadel/archunit/ArchUnitEqualsClassesExactly.kt b/lib/src/test/kotlin/graphql/nadel/archunit/ArchUnitEqualsClassesExactly.kt new file mode 100644 index 000000000..8dc293e87 --- /dev/null +++ b/lib/src/test/kotlin/graphql/nadel/archunit/ArchUnitEqualsClassesExactly.kt @@ -0,0 +1,63 @@ +package graphql.nadel.archunit + +import com.tngtech.archunit.core.domain.JavaClass +import com.tngtech.archunit.lang.ArchCondition +import com.tngtech.archunit.lang.ConditionEvents +import com.tngtech.archunit.lang.SimpleConditionEvent +import com.tngtech.archunit.lang.syntax.elements.ClassesShouldConjunction +import com.tngtech.archunit.lang.syntax.elements.GivenClassesConjunction +import kotlin.reflect.KClass + +fun GivenClassesConjunction.equalsExactly( + vararg requiredClasses: KClass, +): ClassesShouldConjunction { + return should(ArchUnitEqualsClassesExactly(requiredClasses.map { it.java })) +} + +class ArchUnitEqualsClassesExactly( + private val requiredClasses: List>, +) : ArchCondition("equal exactly the given classes") { + private val actualClasses: MutableList = mutableListOf() + + override fun check(item: JavaClass, events: ConditionEvents) { + actualClasses.add(item) + } + + override fun finish(events: ConditionEvents) { + val requiredClassNames = requiredClasses.mapTo(LinkedHashSet()) { it.name } + val actualClassNames = actualClasses.mapTo(LinkedHashSet()) { it.fullName } + + val isValid = requiredClassNames == actualClassNames + if (isValid) { + events.add( + SimpleConditionEvent( + requiredClassNames, + true, + "Classes match $requiredClassNames", + ), + ) + } else { + val extraClassNames = actualClassNames - requiredClassNames + val missingClassNames = requiredClassNames - actualClassNames + + if (extraClassNames.isNotEmpty()) { + events.add( + SimpleConditionEvent( + requiredClassNames, + false, + "Found extra classes $extraClassNames", + ), + ) + } + if (missingClassNames.isNotEmpty()) { + events.add( + SimpleConditionEvent( + requiredClassNames, + false, + "Missing classes $missingClassNames" + ), + ) + } + } + } +} diff --git a/lib/src/test/kotlin/graphql/nadel/ForbiddenGraphQLJavaUsageTest.kt b/lib/src/test/kotlin/graphql/nadel/archunit/ForbiddenGraphQLJavaUsageTest.kt similarity index 97% rename from lib/src/test/kotlin/graphql/nadel/ForbiddenGraphQLJavaUsageTest.kt rename to lib/src/test/kotlin/graphql/nadel/archunit/ForbiddenGraphQLJavaUsageTest.kt index e1c182490..8f0843301 100644 --- a/lib/src/test/kotlin/graphql/nadel/ForbiddenGraphQLJavaUsageTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/archunit/ForbiddenGraphQLJavaUsageTest.kt @@ -1,4 +1,4 @@ -package graphql.nadel +package graphql.nadel.archunit import com.tngtech.archunit.base.DescribedPredicate import com.tngtech.archunit.core.domain.JavaAccess diff --git a/lib/src/test/kotlin/graphql/nadel/NadelPrefixTest.kt b/lib/src/test/kotlin/graphql/nadel/archunit/NadelPrefixTest.kt similarity index 99% rename from lib/src/test/kotlin/graphql/nadel/NadelPrefixTest.kt rename to lib/src/test/kotlin/graphql/nadel/archunit/NadelPrefixTest.kt index 1f1c9a7b9..8e7cde90d 100644 --- a/lib/src/test/kotlin/graphql/nadel/NadelPrefixTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/archunit/NadelPrefixTest.kt @@ -1,4 +1,4 @@ -package graphql.nadel +package graphql.nadel.archunit import com.tngtech.archunit.base.DescribedPredicate import com.tngtech.archunit.core.domain.JavaClass diff --git a/lib/src/test/kotlin/graphql/nadel/archunit/NadelPseudoSealedTypeKtTest.kt b/lib/src/test/kotlin/graphql/nadel/archunit/NadelPseudoSealedTypeKtTest.kt new file mode 100644 index 000000000..e266cd22c --- /dev/null +++ b/lib/src/test/kotlin/graphql/nadel/archunit/NadelPseudoSealedTypeKtTest.kt @@ -0,0 +1,97 @@ +package graphql.nadel.archunit + +import com.tngtech.archunit.core.importer.ClassFileImporter +import com.tngtech.archunit.core.importer.ImportOption +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes +import graphql.schema.GraphQLEnumType +import graphql.schema.GraphQLFieldsContainer +import graphql.schema.GraphQLInputObjectType +import graphql.schema.GraphQLInputType +import graphql.schema.GraphQLInterfaceType +import graphql.schema.GraphQLNamedType +import graphql.schema.GraphQLObjectType +import graphql.schema.GraphQLOutputType +import graphql.schema.GraphQLScalarType +import graphql.schema.GraphQLTypeReference +import graphql.schema.GraphQLUnionType +import graphql.schema.GraphQLUnmodifiedType +import kotlin.test.Test + +/** + * Tests that expected implementations of GraphQL Java classes have not changed. + */ +class NadelPseudoSealedTypeKtTest { + private val schemaClasses = ClassFileImporter() + .withImportOption(ImportOption.DoNotIncludeTests()) + .importPackages("graphql.schema") + + @Test + fun `whenType(GraphQLFieldsContainer)`() { + classes() + .that() + .areAssignableTo(GraphQLFieldsContainer::class.java) + .and() + .areNotInterfaces() + .equalsExactly( + GraphQLInterfaceType::class, + GraphQLObjectType::class, + ) + .check(schemaClasses) + } + + @Test + fun `whenType(GraphQLNamedType)`() { + classes() + .that() + .areAssignableTo(GraphQLNamedType::class.java) + .and() + .areNotInterfaces() + .equalsExactly( + GraphQLEnumType::class, + GraphQLInputObjectType::class, + GraphQLInterfaceType::class, + GraphQLObjectType::class, + GraphQLScalarType::class, + GraphQLUnionType::class, + // Should almost never be used though + GraphQLTypeReference::class, + ) + .check(schemaClasses) + } + + @Test + fun `whenUnmodifiedType(GraphQLOutputType)`() { + classes() + .that() + .areAssignableTo(GraphQLOutputType::class.java) + .and() + .areAssignableTo(GraphQLUnmodifiedType::class.java) + .and() + .areNotInterfaces() + .equalsExactly( + GraphQLEnumType::class, + GraphQLInterfaceType::class, + GraphQLObjectType::class, + GraphQLScalarType::class, + GraphQLUnionType::class, + ) + .check(schemaClasses) + } + + @Test + fun `whenUnmodifiedType(GraphQLInputType)`() { + classes() + .that() + .areAssignableTo(GraphQLInputType::class.java) + .and() + .areAssignableTo(GraphQLUnmodifiedType::class.java) + .and() + .areNotInterfaces() + .equalsExactly( + GraphQLEnumType::class, + GraphQLInputObjectType::class, + GraphQLScalarType::class, + ) + .check(schemaClasses) + } +}