refactor: more clean up (#66)

ExpediaGroup · Nov 13, 2018 · d28248f · d28248f
1 parent 46c00cf
commit d28248f
Show file tree

Hide file tree

Showing 13 changed files with 83 additions and 53 deletions.
diff --git a/detekt_baseline.xml b/detekt_baseline.xml
@@ -5,9 +5,6 @@
     <ID>LargeClass:SchemaGenerator.kt$SchemaGenerator</ID>
     <ID>MethodOverloading:SchemaGeneratorTest.kt$SchemaGeneratorTest</ID>
     <ID>TooManyFunctions:SchemaGenerator.kt$SchemaGenerator</ID>
-    <ID>UnsafeCast:SchemaGeneratorAsyncTests.kt$SchemaGeneratorAsyncTests$schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDo").type as GraphQLNonNull</ID>
-    <ID>UnsafeCast:SchemaGeneratorAsyncTests.kt$SchemaGeneratorAsyncTests$schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDoSingle").type as GraphQLNonNull</ID>
-    <ID>UnsafeCast:SchemaGeneratorAsyncTests.kt$SchemaGeneratorAsyncTests$schema.getObjectType("TopLevelQuery").getFieldDefinition("maybe").type as GraphQLNonNull</ID>
     <ID>ComplexInterface:SchemaGeneratorHooks.kt$SchemaGeneratorHooks</ID>
   </Whitelist>
 </SmellBaseline>
diff --git a/src/main/kotlin/com/expedia/graphql/schema/extensions/annotationExtensions.kt b/src/main/kotlin/com/expedia/graphql/schema/extensions/annotationExtensions.kt
@@ -15,7 +15,7 @@ import kotlin.reflect.full.findAnnotation
 import com.expedia.graphql.annotations.GraphQLDirective as DirectiveAnnotation
 
 internal fun KAnnotatedElement.graphQLDescription(): String? {
-    val directiveNames = listOfDirectives().map { it.normalizeDirectiveName() }
+    val directiveNames = listOfDirectives().map { normalizeDirectiveName(it) }
 
     val description = this.findAnnotation<GraphQLDescription>()?.value
 
@@ -86,7 +86,7 @@ private fun DirectiveAnnotation.getGraphQLDirective(): GraphQLDirective {
     }
 
     @Suppress("Detekt.SpreadOperator")
-    builder.name(name.normalizeDirectiveName())
+    builder.name(normalizeDirectiveName(name))
         .validLocations(*this.locations)
         .description(this.description)
 
@@ -95,10 +95,15 @@ private fun DirectiveAnnotation.getGraphQLDirective(): GraphQLDirective {
         val value = kFunction.call(kClass)
         @Suppress("Detekt.UnsafeCast")
         val type = defaultGraphQLScalars(kFunction.returnType) as GraphQLInputType
-        builder.argument(GraphQLArgument.newArgument().name(propertyName).value(value).type(type).build())
+        val argument = GraphQLArgument.newArgument()
+            .name(propertyName)
+            .value(value)
+            .type(type)
+            .build()
+        builder.argument(argument)
     }
 
     return builder.build()
 }
 
-private fun String.normalizeDirectiveName() = CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, this)
+private fun normalizeDirectiveName(string: String) = CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, string)
diff --git a/src/main/kotlin/com/expedia/graphql/schema/extensions/kClassExtensions.kt b/src/main/kotlin/com/expedia/graphql/schema/extensions/kClassExtensions.kt
@@ -2,17 +2,20 @@ package com.expedia.graphql.schema.extensions
 
 import com.expedia.graphql.schema.generator.functionFilters
 import com.expedia.graphql.schema.generator.propertyFilters
+import com.expedia.graphql.schema.hooks.NoopSchemaGeneratorHooks
 import com.expedia.graphql.schema.hooks.SchemaGeneratorHooks
 import kotlin.reflect.KClass
 import kotlin.reflect.full.declaredMemberFunctions
 import kotlin.reflect.full.declaredMemberProperties
 
-internal fun KClass<*>.getValidProperties(hooks: SchemaGeneratorHooks? = null) = this.declaredMemberProperties
-    .filter { hooks?.isValidProperty(it) ?: true }
+private val noopHooks = NoopSchemaGeneratorHooks()
+
+internal fun KClass<*>.getValidProperties(hooks: SchemaGeneratorHooks = noopHooks) = this.declaredMemberProperties
+    .filter { hooks.isValidProperty(it) }
     .filter { prop -> propertyFilters.all { it.invoke(prop) } }
 
-internal fun KClass<*>.getValidFunctions(hooks: SchemaGeneratorHooks? = null) = this.declaredMemberFunctions
-    .filter { hooks?.isValidFunction(it) ?: true }
+internal fun KClass<*>.getValidFunctions(hooks: SchemaGeneratorHooks = noopHooks) = this.declaredMemberFunctions
+    .filter { hooks.isValidFunction(it) }
     .filter { func -> functionFilters.all { it.invoke(func) } }
 
 internal fun KClass<*>.canBeGraphQLInterface(): Boolean = this.java.isInterface

diff --git a/src/main/kotlin/com/expedia/graphql/schema/generator/SchemaGenerator.kt b/src/main/kotlin/com/expedia/graphql/schema/generator/SchemaGenerator.kt
@@ -16,11 +16,11 @@ import com.expedia.graphql.schema.extensions.isGraphQLContext
 import com.expedia.graphql.schema.extensions.isGraphQLID
 import com.expedia.graphql.schema.extensions.throwIfUnathorizedInterface
 import com.expedia.graphql.schema.extensions.wrapInNonNull
+import com.expedia.graphql.schema.generator.models.KGraphQLType
 import com.expedia.graphql.schema.generator.state.SchemaGeneratorState
 import com.expedia.graphql.schema.generator.types.defaultGraphQLScalars
 import com.expedia.graphql.schema.generator.types.enumType
 import com.expedia.graphql.schema.generator.types.getInputClassName
-import com.expedia.graphql.schema.models.KGraphQLType
 import graphql.TypeResolutionEnvironment
 import graphql.schema.DataFetcher
 import graphql.schema.GraphQLArgument
@@ -271,7 +271,7 @@ internal class SchemaGenerator(
     }
 
     private fun interfaceType(kClass: KClass<*>): GraphQLType {
-        return state.cache.buildIfNotUnderConstruction(kClass) {
+        return state.cache.buildIfNotUnderConstruction(kClass) { _ ->
             val builder = GraphQLInterfaceType.newInterface()
 
             builder.name(kClass.simpleName)
@@ -287,47 +287,43 @@ internal class SchemaGenerator(
             val interfaceType = builder.build()
 
             val implementations = subTypeMapper.getSubTypesOf(kClass)
-            implementations
-                    .filterNot { it.kotlin.isAbstract }
-                    .forEach {
-                        val objectType = objectType(it.kotlin, interfaceType)
+            implementations.forEach {
+                val objectType = objectType(it.kotlin, interfaceType)
 
-                        if (objectType !is GraphQLTypeReference) {
-                            state.additionalTypes.add(objectType)
-                        }
-                        state.cache.removeTypeUnderConstruction(it.kotlin)
-                    }
+                if (objectType !is GraphQLTypeReference) {
+                    state.additionalTypes.add(objectType)
+                }
+                state.cache.removeTypeUnderConstruction(it.kotlin)
+            }
 
             interfaceType
         }
     }
 
     private fun unionType(kClass: KClass<*>): GraphQLType {
-        return state.cache.buildIfNotUnderConstruction(kClass) {
+        return state.cache.buildIfNotUnderConstruction(kClass) { _ ->
             val builder = GraphQLUnionType.newUnionType()
 
             builder.name(kClass.simpleName)
             builder.description(kClass.graphQLDescription())
             builder.typeResolver { env: TypeResolutionEnvironment -> env.schema.getObjectType(env.getObject<Any>().javaClass.simpleName) }
 
             val implementations = subTypeMapper.getSubTypesOf(kClass)
-            implementations
-                    .filterNot { it.kotlin.isAbstract }
-                    .forEach {
-                        val objectType = state.cache.get(TypesCacheKey(it.kotlin.createType(), false)) ?: objectType(it.kotlin)
+            implementations.forEach {
+                val objectType = state.cache.get(TypesCacheKey(it.kotlin.createType(), false)) ?: objectType(it.kotlin)
 
-                        val key = TypesCacheKey(it.kotlin.createType(), false)
+                val key = TypesCacheKey(it.kotlin.createType(), false)
 
-                        if (objectType is GraphQLTypeReference) {
-                            builder.possibleType(objectType)
-                        } else {
-                            builder.possibleType(objectType as GraphQLObjectType)
-                        }
+                if (objectType is GraphQLTypeReference) {
+                    builder.possibleType(objectType)
+                } else {
+                    builder.possibleType(objectType as GraphQLObjectType)
+                }
 
-                        if (state.cache.doesNotContain(it.kotlin)) {
-                            state.cache.put(key, KGraphQLType(it.kotlin, objectType))
-                        }
-                    }
+                if (state.cache.doesNotContain(it.kotlin)) {
+                    state.cache.put(key, KGraphQLType(it.kotlin, objectType))
+                }
+            }
 
             builder.build()
         }

diff --git a/src/main/kotlin/com/expedia/graphql/schema/generator/SubTypeMapper.kt b/src/main/kotlin/com/expedia/graphql/schema/generator/SubTypeMapper.kt
@@ -7,6 +7,7 @@ internal class SubTypeMapper(supportedPackages: List<String>) {
 
     private val reflections = Reflections(supportedPackages)
 
-    fun getSubTypesOf(kclass: KClass<*>): MutableSet<out Class<out Any>> =
+    fun getSubTypesOf(kclass: KClass<*>): List<Class<*>> =
         reflections.getSubTypesOf(Class.forName(kclass.javaObjectType.name))
+            .filterNot { it.kotlin.isAbstract }
 }
diff --git a/src/main/kotlin/com/expedia/graphql/schema/generator/TypesCache.kt b/src/main/kotlin/com/expedia/graphql/schema/generator/TypesCache.kt
@@ -5,7 +5,7 @@ import com.expedia.graphql.schema.exceptions.ConflictingTypesException
 import com.expedia.graphql.schema.exceptions.CouldNotGetJvmNameOfKTypeException
 import com.expedia.graphql.schema.exceptions.CouldNotGetNameOfEnumException
 import com.expedia.graphql.schema.exceptions.TypeNotSupportedException
-import com.expedia.graphql.schema.models.KGraphQLType
+import com.expedia.graphql.schema.generator.models.KGraphQLType
 import graphql.schema.GraphQLType
 import graphql.schema.GraphQLTypeReference
 import kotlin.reflect.KClass

diff --git a/...dia/graphql/schema/models/KGraphQLType.kt → ...l/schema/generator/models/KGraphQLType.kt b/...dia/graphql/schema/models/KGraphQLType.kt → ...l/schema/generator/models/KGraphQLType.kt
@@ -1,4 +1,4 @@
-package com.expedia.graphql.schema.models
+package com.expedia.graphql.schema.generator.models
 
 import graphql.schema.GraphQLType
 import kotlin.reflect.KClass

diff --git a/src/main/kotlin/com/expedia/graphql/toSchema.kt b/src/main/kotlin/com/expedia/graphql/toSchema.kt
@@ -13,7 +13,7 @@ import graphql.schema.GraphQLSchema
  * @param config Schema generation configuration
  */
 fun toSchema(
-    queries: List<TopLevelObjectDef> = emptyList(),
+    queries: List<TopLevelObjectDef>,
     mutations: List<TopLevelObjectDef> = emptyList(),
     config: SchemaGeneratorConfig
 ): GraphQLSchema {

diff --git a/src/test/kotlin/com/expedia/graphql/schema/generator/SchemaGeneratorAsyncTests.kt b/src/test/kotlin/com/expedia/graphql/schema/generator/SchemaGeneratorAsyncTests.kt
@@ -29,31 +29,31 @@ class SchemaGeneratorAsyncTests {
     fun `SchemaGenerator strips type argument from CompletableFuture to support async servlet`() {
         val schema = toSchema(listOf(TopLevelObjectDef(AsyncQuery())), config = testSchemaConfig)
         val returnTypeName =
-            (schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDo").type as GraphQLNonNull).wrappedType.name
+            (schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDo").type as? GraphQLNonNull)?.wrappedType?.name
         assertEquals("Int", returnTypeName)
     }
 
     @Test
     fun `SchemaGenerator strips type argument from RxJava2 Observable`() {
         val schema = toSchema(listOf(TopLevelObjectDef(RxJava2Query())), config = configWithRxJavaMonads)
         val returnTypeName =
-            (schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDo").type as GraphQLNonNull).wrappedType.name
+            (schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDo").type as? GraphQLNonNull)?.wrappedType?.name
         assertEquals("Int", returnTypeName)
     }
 
     @Test
     fun `SchemaGenerator strips type argument from RxJava2 Single`() {
         val schema = toSchema(listOf(TopLevelObjectDef(RxJava2Query())), config = configWithRxJavaMonads)
         val returnTypeName =
-            (schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDoSingle").type as GraphQLNonNull).wrappedType.name
+            (schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDoSingle").type as? GraphQLNonNull)?.wrappedType?.name
         assertEquals("Int", returnTypeName)
     }
 
     @Test
     fun `SchemaGenerator strips type argument from RxJava2 Maybe`() {
         val schema = toSchema(listOf(TopLevelObjectDef(RxJava2Query())), config = configWithRxJavaMonads)
         val returnTypeName =
-            (schema.getObjectType("TopLevelQuery").getFieldDefinition("maybe").type as GraphQLNonNull).wrappedType.name
+            (schema.getObjectType("TopLevelQuery").getFieldDefinition("maybe").type as? GraphQLNonNull)?.wrappedType?.name
         assertEquals("Int", returnTypeName)
     }
 

diff --git a/src/test/kotlin/com/expedia/graphql/schema/generator/SchemaGeneratorTest.kt b/src/test/kotlin/com/expedia/graphql/schema/generator/SchemaGeneratorTest.kt
@@ -23,7 +23,10 @@ import kotlin.test.assertFailsWith
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
 
-@Suppress("Detekt.UnusedPrivateMember", "Detekt.FunctionOnlyReturningConstant", "Detekt.LargeClass")
+@Suppress("Detekt.UnusedPrivateMember",
+    "Detekt.FunctionOnlyReturningConstant",
+    "Detekt.LargeClass",
+    "Detekt.MethodOverloading")
 class SchemaGeneratorTest {
     @Test
     fun `SchemaGenerator generates a simple GraphQL schema`() {
@@ -266,7 +269,7 @@ class SchemaGeneratorTest {
 
     @Test
     fun `SchemaGenerator supports Scalar GraphQLID for input types`() {
-        val schema = toSchema(mutations = listOf(TopLevelObjectDef(MutationWithId())), config = testSchemaConfig)
+        val schema = toSchema(queries = emptyList(), mutations = listOf(TopLevelObjectDef(MutationWithId())), config = testSchemaConfig)
 
         val furnitureType = schema.getObjectType("Furniture")
         val serialField = furnitureType.getFieldDefinition("serial").type as? GraphQLNonNull

diff --git a/src/test/kotlin/com/expedia/graphql/schema/generator/SubTypeMapperTest.kt b/src/test/kotlin/com/expedia/graphql/schema/generator/SubTypeMapperTest.kt
@@ -3,6 +3,7 @@ package com.expedia.graphql.schema.generator
 import org.junit.jupiter.api.Test
 import kotlin.test.assertEquals
 
+@Suppress("Detekt.MethodOverloading")
 internal class SubTypeMapperTest {
 
     private interface MyInterface {
@@ -17,21 +18,45 @@ internal class SubTypeMapperTest {
         override fun getValue() = 2
     }
 
+    @Suppress("Detekt.UnnecessaryAbstractClass")
+    private abstract class MyAbstractClass {
+        abstract fun getValue(): Int
+    }
+
+    private class ThirdClass : MyAbstractClass() {
+        override fun getValue() = 3
+    }
+
+    private abstract class FourthClass : MyAbstractClass() {
+        override fun getValue() = 3
+
+        abstract fun getSecondAbsctractValue(): Int
+    }
+
     @Test
     fun `valid subtypes`() {
 
         val mapper = SubTypeMapper(listOf("com.expedia.graphql"))
-        val set = mapper.getSubTypesOf(MyInterface::class)
+        val list = mapper.getSubTypesOf(MyInterface::class)
+
+        assertEquals(expected = 2, actual = list.size)
+    }
+
+    @Test
+    fun `abstract subtypes`() {
+
+        val mapper = SubTypeMapper(listOf("com.expedia.graphql"))
+        val list = mapper.getSubTypesOf(MyAbstractClass::class)
 
-        assertEquals(expected = 2, actual = set.size)
+        assertEquals(expected = 1, actual = list.size)
     }
 
     @Test
     fun `subtypes of non-supported packages`() {
 
         val mapper = SubTypeMapper(listOf("com.example"))
-        val set = mapper.getSubTypesOf(MyInterface::class)
+        val list = mapper.getSubTypesOf(MyInterface::class)
 
-        assertEquals(expected = 0, actual = set.size)
+        assertEquals(expected = 0, actual = list.size)
     }
 }
diff --git a/src/test/kotlin/com/expedia/graphql/schema/generator/TypesCacheTest.kt b/src/test/kotlin/com/expedia/graphql/schema/generator/TypesCacheTest.kt
@@ -1,6 +1,6 @@
 package com.expedia.graphql.schema.generator
 
-import com.expedia.graphql.schema.models.KGraphQLType
+import com.expedia.graphql.schema.generator.models.KGraphQLType
 import graphql.schema.GraphQLType
 import org.junit.jupiter.api.Test
 import kotlin.reflect.full.starProjectedType

diff --git a/...graphql/schema/models/KGraphQLTypeTest.kt → ...hema/generator/models/KGraphQLTypeTest.kt b/...graphql/schema/models/KGraphQLTypeTest.kt → ...hema/generator/models/KGraphQLTypeTest.kt
@@ -1,4 +1,4 @@
-package com.expedia.graphql.schema.models
+package com.expedia.graphql.schema.generator.models
 
 import graphql.schema.GraphQLType
 import org.junit.jupiter.api.Test