From 00fac631c7375f8e287fac5df724bdc6eac8baaf Mon Sep 17 00:00:00 2001 From: Shane Myrick Date: Tue, 22 Oct 2019 13:58:16 -0700 Subject: [PATCH 1/2] Remove reflections lib in favor of ClassGraph Fixes https://github.com/ExpediaGroup/graphql-kotlin/issues/449 There is a securty issue with one of the dependencies of org.reflections:reflection:0.9.11. Instead of resolving the dep issue we should migrate away from this library since the last version was released in 2017. https://github.com/classgraph/classgraph is an active supported library with reported faster implementation of reflection --- graphql-kotlin-federation/pom.xml | 4 +++ .../federation/FederatedSchemaGenerator.kt | 11 ++++--- .../FederatedSchemaGeneratorTest.kt | 10 +++--- .../graphql/federation}/data/TestResolvers.kt | 6 ++-- .../graphql/federation}/data/TestSchema.kt | 4 +-- .../data/queries/federated/Product.kt | 2 +- .../data/queries/simple/NestedQuery.kt | 2 +- .../data/queries/simple/SimpleQuery.kt | 2 +- .../execution/EntityQueryResolverTest.kt | 8 ++--- .../execution/FederatedQueryResolverTest.kt | 6 ++-- .../execution/ServiceQueryResolverTest.kt | 8 ++--- graphql-kotlin-schema-generator/pom.xml | 10 ++++-- .../graphql/generator/SubTypeMapper.kt | 31 ++++++++++++++++--- .../graphql/generator/SubTypeMapperTest.kt | 20 ++++++++++++ pom.xml | 8 ++--- 15 files changed, 94 insertions(+), 38 deletions(-) rename graphql-kotlin-federation/src/test/kotlin/{test => com/expediagroup/graphql/federation}/data/TestResolvers.kt (89%) rename graphql-kotlin-federation/src/test/kotlin/{test => com/expediagroup/graphql/federation}/data/TestSchema.kt (89%) rename graphql-kotlin-federation/src/test/kotlin/{test => com/expediagroup/graphql/federation}/data/queries/federated/Product.kt (98%) rename graphql-kotlin-federation/src/test/kotlin/{test => com/expediagroup/graphql/federation}/data/queries/simple/NestedQuery.kt (93%) rename graphql-kotlin-federation/src/test/kotlin/{test => com/expediagroup/graphql/federation}/data/queries/simple/SimpleQuery.kt (91%) diff --git a/graphql-kotlin-federation/pom.xml b/graphql-kotlin-federation/pom.xml index 935e41ebb9..c71dd0cec2 100644 --- a/graphql-kotlin-federation/pom.xml +++ b/graphql-kotlin-federation/pom.xml @@ -33,6 +33,10 @@ graphql-kotlin-schema-generator ${project.parent.version} + + io.github.classgraph + classgraph + org.junit.jupiter junit-jupiter-params diff --git a/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/federation/FederatedSchemaGenerator.kt b/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/federation/FederatedSchemaGenerator.kt index 3a5d1ea737..c16f74dfc1 100644 --- a/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/federation/FederatedSchemaGenerator.kt +++ b/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/federation/FederatedSchemaGenerator.kt @@ -20,8 +20,9 @@ import com.expediagroup.graphql.TopLevelObject import com.expediagroup.graphql.federation.directives.ExtendsDirective import com.expediagroup.graphql.generator.SchemaGenerator import graphql.schema.GraphQLSchema -import org.reflections.Reflections +import io.github.classgraph.ClassGraph import kotlin.reflect.full.createType +import kotlin.reflect.jvm.jvmName /** * Generates federated GraphQL schemas based on the specified configuration. @@ -41,10 +42,12 @@ open class FederatedSchemaGenerator(generatorConfig: FederatedSchemaGeneratorCon /** * Scans specified packages for all the federated (extended) types and adds them to the target schema. */ + @Suppress("Detekt.SpreadOperator") fun GraphQLSchema.Builder.federation(supportedPackages: List): GraphQLSchema.Builder { - supportedPackages - .map { pkg -> Reflections(pkg).getTypesAnnotatedWith(ExtendsDirective::class.java).map { it.kotlin } } - .flatten() + val scanResult = ClassGraph().enableAllInfo().whitelistPackages(*supportedPackages.toTypedArray()).scan() + + scanResult.getClassesWithAnnotation(ExtendsDirective::class.jvmName) + .map { it.loadClass().kotlin } .map { val graphQLType = if (it.isAbstract) { interfaceType(it) diff --git a/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/FederatedSchemaGeneratorTest.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/FederatedSchemaGeneratorTest.kt index f256bcb7f4..cbbff12cff 100644 --- a/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/FederatedSchemaGeneratorTest.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/FederatedSchemaGeneratorTest.kt @@ -18,12 +18,12 @@ package com.expediagroup.graphql.federation import com.expediagroup.graphql.TopLevelObject import com.expediagroup.graphql.extensions.print +import com.expediagroup.graphql.federation.data.queries.simple.NestedQuery +import com.expediagroup.graphql.federation.data.queries.simple.SimpleQuery import com.expediagroup.graphql.federation.execution.FederatedTypeRegistry import graphql.schema.GraphQLUnionType import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test -import test.data.queries.simple.NestedQuery -import test.data.queries.simple.SimpleQuery import kotlin.test.assertNotNull import kotlin.test.assertTrue @@ -104,7 +104,7 @@ class FederatedSchemaGeneratorTest { @Test fun `verify can generate federated schema`() { val config = FederatedSchemaGeneratorConfig( - supportedPackages = listOf("test.data.queries.federated"), + supportedPackages = listOf("com.expediagroup.graphql.federation.data.queries.federated"), hooks = FederatedSchemaGeneratorHooks(FederatedTypeRegistry()) ) @@ -137,7 +137,7 @@ class FederatedSchemaGeneratorTest { """.trimIndent() val config = FederatedSchemaGeneratorConfig( - supportedPackages = listOf("test.data.queries.simple"), + supportedPackages = listOf("com.expediagroup.graphql.federation.data.queries.simple"), hooks = FederatedSchemaGeneratorHooks(FederatedTypeRegistry()) ) @@ -169,7 +169,7 @@ class FederatedSchemaGeneratorTest { """.trimIndent() val config = FederatedSchemaGeneratorConfig( - supportedPackages = listOf("test.data.queries.simple"), + supportedPackages = listOf("com.expediagroup.graphql.federation.data.queries.simple"), hooks = FederatedSchemaGeneratorHooks(FederatedTypeRegistry()) ) diff --git a/graphql-kotlin-federation/src/test/kotlin/test/data/TestResolvers.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/TestResolvers.kt similarity index 89% rename from graphql-kotlin-federation/src/test/kotlin/test/data/TestResolvers.kt rename to graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/TestResolvers.kt index b22eba5513..676b97d2bd 100644 --- a/graphql-kotlin-federation/src/test/kotlin/test/data/TestResolvers.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/TestResolvers.kt @@ -14,11 +14,11 @@ * limitations under the License. */ -package test.data +package com.expediagroup.graphql.federation.data +import com.expediagroup.graphql.federation.data.queries.federated.Book +import com.expediagroup.graphql.federation.data.queries.federated.User import com.expediagroup.graphql.federation.execution.FederatedTypeResolver -import test.data.queries.federated.Book -import test.data.queries.federated.User internal class BookResolver : FederatedTypeResolver { override suspend fun resolve(representations: List>): List { diff --git a/graphql-kotlin-federation/src/test/kotlin/test/data/TestSchema.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/TestSchema.kt similarity index 89% rename from graphql-kotlin-federation/src/test/kotlin/test/data/TestSchema.kt rename to graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/TestSchema.kt index 13d2be9b3e..e7c19159d8 100644 --- a/graphql-kotlin-federation/src/test/kotlin/test/data/TestSchema.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/TestSchema.kt @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.data +package com.expediagroup.graphql.federation.data import com.expediagroup.graphql.federation.FederatedSchemaGeneratorConfig import com.expediagroup.graphql.federation.FederatedSchemaGeneratorHooks @@ -25,7 +25,7 @@ import graphql.schema.GraphQLSchema internal fun federatedTestSchema(federatedTypeResolvers: Map> = emptyMap()): GraphQLSchema { val config = FederatedSchemaGeneratorConfig( - supportedPackages = listOf("test.data.queries.federated"), + supportedPackages = listOf("com.expediagroup.graphql.federation.data.queries.federated"), hooks = FederatedSchemaGeneratorHooks(FederatedTypeRegistry(federatedTypeResolvers)) ) diff --git a/graphql-kotlin-federation/src/test/kotlin/test/data/queries/federated/Product.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/federated/Product.kt similarity index 98% rename from graphql-kotlin-federation/src/test/kotlin/test/data/queries/federated/Product.kt rename to graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/federated/Product.kt index 7e10c9cbc8..fc2c98e3ec 100644 --- a/graphql-kotlin-federation/src/test/kotlin/test/data/queries/federated/Product.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/federated/Product.kt @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.data.queries.federated +package com.expediagroup.graphql.federation.data.queries.federated import com.expediagroup.graphql.annotations.GraphQLDirective import com.expediagroup.graphql.annotations.GraphQLIgnore diff --git a/graphql-kotlin-federation/src/test/kotlin/test/data/queries/simple/NestedQuery.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/simple/NestedQuery.kt similarity index 93% rename from graphql-kotlin-federation/src/test/kotlin/test/data/queries/simple/NestedQuery.kt rename to graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/simple/NestedQuery.kt index 43290217c0..dfd37ec3a9 100644 --- a/graphql-kotlin-federation/src/test/kotlin/test/data/queries/simple/NestedQuery.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/simple/NestedQuery.kt @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.data.queries.simple +package com.expediagroup.graphql.federation.data.queries.simple import kotlin.random.Random diff --git a/graphql-kotlin-federation/src/test/kotlin/test/data/queries/simple/SimpleQuery.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/simple/SimpleQuery.kt similarity index 91% rename from graphql-kotlin-federation/src/test/kotlin/test/data/queries/simple/SimpleQuery.kt rename to graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/simple/SimpleQuery.kt index 872363a26f..242ff7f3be 100644 --- a/graphql-kotlin-federation/src/test/kotlin/test/data/queries/simple/SimpleQuery.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/queries/simple/SimpleQuery.kt @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.data.queries.simple +package com.expediagroup.graphql.federation.data.queries.simple /* type Query { diff --git a/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/EntityQueryResolverTest.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/EntityQueryResolverTest.kt index 9381515647..bf551580ff 100644 --- a/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/EntityQueryResolverTest.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/EntityQueryResolverTest.kt @@ -24,10 +24,10 @@ import io.mockk.every import io.mockk.mockk import io.mockk.spyk import org.junit.jupiter.api.Test -import test.data.BookResolver -import test.data.UserResolver -import test.data.queries.federated.Book -import test.data.queries.federated.User +import com.expediagroup.graphql.federation.data.BookResolver +import com.expediagroup.graphql.federation.data.UserResolver +import com.expediagroup.graphql.federation.data.queries.federated.Book +import com.expediagroup.graphql.federation.data.queries.federated.User import kotlin.test.assertEquals class EntityQueryResolverTest { diff --git a/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/FederatedQueryResolverTest.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/FederatedQueryResolverTest.kt index b0639e1d62..095cc06206 100644 --- a/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/FederatedQueryResolverTest.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/FederatedQueryResolverTest.kt @@ -19,9 +19,9 @@ package com.expediagroup.graphql.federation.execution import graphql.ExecutionInput import graphql.GraphQL import org.junit.jupiter.api.Test -import test.data.BookResolver -import test.data.UserResolver -import test.data.federatedTestSchema +import com.expediagroup.graphql.federation.data.BookResolver +import com.expediagroup.graphql.federation.data.UserResolver +import com.expediagroup.graphql.federation.data.federatedTestSchema import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull diff --git a/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/ServiceQueryResolverTest.kt b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/ServiceQueryResolverTest.kt index 0e2356719d..7bc5a7f140 100644 --- a/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/ServiceQueryResolverTest.kt +++ b/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/execution/ServiceQueryResolverTest.kt @@ -23,8 +23,8 @@ import com.expediagroup.graphql.federation.toFederatedSchema import graphql.ExecutionInput import graphql.GraphQL import org.junit.jupiter.api.Test -import test.data.queries.simple.NestedQuery -import test.data.queries.simple.SimpleQuery +import com.expediagroup.graphql.federation.data.queries.simple.NestedQuery +import com.expediagroup.graphql.federation.data.queries.simple.SimpleQuery import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -72,7 +72,7 @@ class ServiceQueryResolverTest { @Test fun `verify can retrieve SDL using _service query`() { val config = FederatedSchemaGeneratorConfig( - supportedPackages = listOf("test.data.queries.federated"), + supportedPackages = listOf("com.expediagroup.graphql.federation.data.queries.federated"), hooks = FederatedSchemaGeneratorHooks(FederatedTypeRegistry()) ) @@ -101,7 +101,7 @@ class ServiceQueryResolverTest { @Test fun `verify can retrieve SDL using _service query for non-federated schemas`() { val config = FederatedSchemaGeneratorConfig( - supportedPackages = listOf("test.data.queries.simple"), + supportedPackages = listOf("com.expediagroup.graphql.federation.data.queries.simple"), hooks = FederatedSchemaGeneratorHooks(FederatedTypeRegistry()) ) diff --git a/graphql-kotlin-schema-generator/pom.xml b/graphql-kotlin-schema-generator/pom.xml index 2a13632459..8bbb07473d 100644 --- a/graphql-kotlin-schema-generator/pom.xml +++ b/graphql-kotlin-schema-generator/pom.xml @@ -16,6 +16,7 @@ ${project.basedir}/.. 2.2.12 + 28.0-jre @@ -32,8 +33,13 @@ kotlin-reflect - org.reflections - reflections + io.github.classgraph + classgraph + + + com.google.guava + guava + ${guava.version} com.fasterxml.jackson.module diff --git a/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt b/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt index 63309e5587..f4ae92bce0 100644 --- a/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt +++ b/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt @@ -16,12 +16,35 @@ package com.expediagroup.graphql.generator -import org.reflections.Reflections +import io.github.classgraph.ClassGraph +import io.github.classgraph.ClassInfo +import io.github.classgraph.ClassInfoList import kotlin.reflect.KClass +import kotlin.reflect.jvm.jvmName -internal class SubTypeMapper(supportedPackages: List) { +internal class SubTypeMapper(val supportedPackages: List) { - private val reflections = Reflections(supportedPackages) + @Suppress("Detekt.SpreadOperator") + private val scanResult = ClassGraph() + .enableAllInfo() + .whitelistPackages(*supportedPackages.toTypedArray()) + .scan() - fun getSubTypesOf(kclass: KClass<*>): List> = reflections.getSubTypesOf(kclass.java).filterNot { it.kotlin.isAbstract }.map { it.kotlin } + fun getSubTypesOf(kclass: KClass<*>): List> { + val classInfo = scanResult.getClassInfo(kclass.jvmName) ?: return emptyList() + + return getImplementingClasses(classInfo) + .union(classInfo.subclasses) + .map { it.loadClass().kotlin } + .filterNot { it.isAbstract } + } + + @Suppress("Detekt.SwallowedException") + private fun getImplementingClasses(classInfo: ClassInfo) = + try { + classInfo.classesImplementing + } catch (e: IllegalArgumentException) { + // Ignored, just return empty list + ClassInfoList.emptyList() + } } diff --git a/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/SubTypeMapperTest.kt b/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/SubTypeMapperTest.kt index 95f3768976..1678f49a4d 100644 --- a/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/SubTypeMapperTest.kt +++ b/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/SubTypeMapperTest.kt @@ -22,6 +22,10 @@ import kotlin.test.assertEquals @Suppress("Detekt.UnusedPrivateClass") internal class SubTypeMapperTest { + private interface NoSubTypesInterface + + private abstract class NoSubTypesClass + private interface MyInterface { fun getValue(): Int } @@ -75,4 +79,20 @@ internal class SubTypeMapperTest { assertEquals(expected = 0, actual = list.size) } + + @Test + fun `interface with no subtypes`() { + val mapper = SubTypeMapper(listOf("com.expediagroup.graphql")) + val list = mapper.getSubTypesOf(NoSubTypesInterface::class) + + assertEquals(expected = 0, actual = list.size) + } + + @Test + fun `abstract class with no subtypes`() { + val mapper = SubTypeMapper(listOf("com.expediagroup.graphql")) + val list = mapper.getSubTypesOf(NoSubTypesClass::class) + + assertEquals(expected = 0, actual = list.size) + } } diff --git a/pom.xml b/pom.xml index 97cf53ba37..7ddc119916 100644 --- a/pom.xml +++ b/pom.xml @@ -66,7 +66,7 @@ 2.10.0 1.3.50 1.3.2 - 0.9.11 + 4.8.52 2.2.0.RELEASE @@ -273,9 +273,9 @@ ${kotlin.version} - org.reflections - reflections - ${reflections.version} + io.github.classgraph + classgraph + ${classgraph.version} com.fasterxml.jackson.module From 087462bf72c96e6d7cfd172e161f270d4e3a91e3 Mon Sep 17 00:00:00 2001 From: Shane Myrick Date: Tue, 22 Oct 2019 14:18:51 -0700 Subject: [PATCH 2/2] Remove val property from SubTypeMapper --- .../kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt b/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt index f4ae92bce0..78828dc34d 100644 --- a/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt +++ b/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SubTypeMapper.kt @@ -22,7 +22,7 @@ import io.github.classgraph.ClassInfoList import kotlin.reflect.KClass import kotlin.reflect.jvm.jvmName -internal class SubTypeMapper(val supportedPackages: List) { +internal class SubTypeMapper(supportedPackages: List) { @Suppress("Detekt.SpreadOperator") private val scanResult = ClassGraph()