fix: description of parameters (#118)

smyrick · brennantaylor · commit 4571cfdd6df7 · 2018-12-27T14:04:00.000-08:00
diff --git a/src/main/kotlin/com/expedia/graphql/generator/SchemaGenerator.kt b/src/main/kotlin/com/expedia/graphql/generator/SchemaGenerator.kt
@@ -14,6 +14,7 @@ import com.expedia.graphql.generator.types.ObjectTypeBuilder
 import com.expedia.graphql.generator.types.PropertyTypeBuilder
 import com.expedia.graphql.generator.types.ScalarTypeBuilder
 import com.expedia.graphql.generator.types.UnionTypeBuilder
+import graphql.schema.GraphQLDirective
 import graphql.schema.GraphQLInterfaceType
 import graphql.schema.GraphQLObjectType
 import graphql.schema.GraphQLSchema
@@ -105,7 +106,7 @@ internal class SchemaGenerator(
         listTypeBuilder.listType(type, inputType)
 
     internal fun arrayType(type: KType, inputType: Boolean) =
-            listTypeBuilder.arrayType(type, inputType)
+        listTypeBuilder.arrayType(type, inputType)
 
     internal fun objectType(kClass: KClass<*>, interfaceType: GraphQLInterfaceType? = null) =
         objectTypeBuilder.objectType(kClass, interfaceType)
@@ -125,6 +126,9 @@ internal class SchemaGenerator(
     internal fun scalarType(type: KType, annotatedAsID: Boolean = false) =
         scalarTypeBuilder.scalarType(type, annotatedAsID)
 
-    internal fun directives(element: KAnnotatedElement) =
-        directiveTypeBuilder.directives(element)
+    internal fun directives(element: KAnnotatedElement): List<GraphQLDirective> {
+        val directives = directiveTypeBuilder.directives(element)
+        state.directives.addAll(directives)
+        return directives
+    }
 }
diff --git a/src/main/kotlin/com/expedia/graphql/generator/extensions/kClassExtensions.kt b/src/main/kotlin/com/expedia/graphql/generator/extensions/kClassExtensions.kt
@@ -10,6 +10,7 @@ import kotlin.reflect.KParameter
 import kotlin.reflect.KProperty
 import kotlin.reflect.full.declaredMemberFunctions
 import kotlin.reflect.full.declaredMemberProperties
+import kotlin.reflect.full.findParameterByName
 import kotlin.reflect.full.isSubclassOf
 import kotlin.reflect.full.primaryConstructor
 
@@ -24,9 +25,7 @@ internal fun KClass<*>.getValidFunctions(hooks: SchemaGeneratorHooks): List<KFun
         .filter { func -> functionFilters.all { it.invoke(func) } }
 
 internal fun KClass<*>.findConstructorParamter(name: String): KParameter? =
-    this.primaryConstructor
-        ?.parameters
-        ?.find { it.name == name }
+    this.primaryConstructor?.findParameterByName(name)
 
 internal fun KClass<*>.isInterface(): Boolean = this.java.isInterface
 
diff --git a/src/main/kotlin/com/expedia/graphql/generator/extensions/kParameterExtensions.kt b/src/main/kotlin/com/expedia/graphql/generator/extensions/kParameterExtensions.kt
@@ -2,15 +2,11 @@ package com.expedia.graphql.generator.extensions
 
 import com.expedia.graphql.annotations.GraphQLContext
 import com.expedia.graphql.exceptions.CouldNotGetNameOfKParameterException
-import com.expedia.graphql.exceptions.InvalidInputFieldTypeException
 import kotlin.reflect.KParameter
 import kotlin.reflect.full.findAnnotation
 import kotlin.reflect.jvm.jvmErasure
 
-@Throws(InvalidInputFieldTypeException::class)
-internal fun KParameter.throwIfUnathorizedInterface() {
-    if (this.type.jvmErasure.isInterface()) throw InvalidInputFieldTypeException()
-}
+internal fun KParameter.isInterface() = this.type.jvmErasure.isInterface()
 
 internal fun KParameter.isGraphQLContext() = this.findAnnotation<GraphQLContext>() != null
 
diff --git a/src/main/kotlin/com/expedia/graphql/generator/types/FunctionTypeBuilder.kt b/src/main/kotlin/com/expedia/graphql/generator/types/FunctionTypeBuilder.kt
@@ -2,13 +2,14 @@ package com.expedia.graphql.generator.types
 
 import com.expedia.graphql.KotlinDataFetcher
 import com.expedia.graphql.Parameter
+import com.expedia.graphql.exceptions.InvalidInputFieldTypeException
 import com.expedia.graphql.generator.SchemaGenerator
 import com.expedia.graphql.generator.TypeBuilder
 import com.expedia.graphql.generator.extensions.getDeprecationReason
 import com.expedia.graphql.generator.extensions.getGraphQLDescription
 import com.expedia.graphql.generator.extensions.getName
 import com.expedia.graphql.generator.extensions.isGraphQLContext
-import com.expedia.graphql.generator.extensions.throwIfUnathorizedInterface
+import com.expedia.graphql.generator.extensions.isInterface
 import graphql.schema.DataFetcher
 import graphql.schema.GraphQLArgument
 import graphql.schema.GraphQLFieldDefinition
@@ -33,7 +34,6 @@ internal class FunctionTypeBuilder(generator: SchemaGenerator) : TypeBuilder(gen
 
         generator.directives(fn).forEach {
             builder.withDirective(it)
-            state.directives.add(it)
         }
 
         val args = mutableMapOf<String, Parameter>()
@@ -60,16 +60,20 @@ internal class FunctionTypeBuilder(generator: SchemaGenerator) : TypeBuilder(gen
         return config.hooks.onRewireGraphQLType(monadType, graphQLType) as GraphQLFieldDefinition
     }
 
+    @Throws(InvalidInputFieldTypeException::class)
     private fun argument(parameter: KParameter): GraphQLArgument {
-        parameter.throwIfUnathorizedInterface()
+
+        if (parameter.isInterface()) {
+            throw InvalidInputFieldTypeException()
+        }
+
         val builder = GraphQLArgument.newArgument()
-            .name(parameter.name)
-            .description(parameter.getGraphQLDescription() ?: parameter.type.getGraphQLDescription())
+            .name(parameter.getName())
+            .description(parameter.getGraphQLDescription())
             .type(graphQLTypeOf(parameter.type, true) as GraphQLInputType)
 
         generator.directives(parameter).forEach {
             builder.withDirective(it)
-            state.directives.add(it)
         }
 
         return config.hooks.onRewireGraphQLType(parameter.type, builder.build()) as GraphQLArgument
diff --git a/src/main/kotlin/com/expedia/graphql/generator/types/ObjectTypeBuilder.kt b/src/main/kotlin/com/expedia/graphql/generator/types/ObjectTypeBuilder.kt
@@ -26,7 +26,6 @@ internal class ObjectTypeBuilder(generator: SchemaGenerator) : TypeBuilder(gener
 
             generator.directives(kClass).forEach {
                 builder.withDirective(it)
-                state.directives.add(it)
             }
 
             if (interfaceType != null) {
diff --git a/src/main/kotlin/com/expedia/graphql/generator/types/PropertyTypeBuilder.kt b/src/main/kotlin/com/expedia/graphql/generator/types/PropertyTypeBuilder.kt
@@ -25,7 +25,6 @@ internal class PropertyTypeBuilder(generator: SchemaGenerator) : TypeBuilder(gen
 
         generator.directives(prop).forEach {
             fieldBuilder.withDirective(it)
-            state.directives.add(it)
         }
 
         val field = if (config.dataFetcherFactory != null && prop.isLateinit) {
diff --git a/src/test/kotlin/com/expedia/graphql/generator/extensions/KParameterExtensionsKtTest.kt b/src/test/kotlin/com/expedia/graphql/generator/extensions/KParameterExtensionsKtTest.kt
@@ -1,22 +1,31 @@
 package com.expedia.graphql.generator.extensions
 
+import com.expedia.graphql.annotations.GraphQLDescription
 import com.expedia.graphql.exceptions.CouldNotGetNameOfKParameterException
 import io.mockk.every
 import io.mockk.mockk
 import org.junit.jupiter.api.Test
 import kotlin.reflect.KParameter
-import kotlin.reflect.full.primaryConstructor
+import kotlin.reflect.full.findParameterByName
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
+import kotlin.test.assertNull
 
 internal class KParameterExtensionsKtTest {
 
+    @GraphQLDescription("class description")
     internal data class MyClass(val foo: String)
 
+    internal class Container {
+        internal fun noDescription(myClass: MyClass) = myClass
+
+        internal fun paramDescription(@GraphQLDescription("param description") myClass: MyClass) = myClass
+    }
+
     @Test
     fun getName() {
-        val param = MyClass::class.primaryConstructor?.parameters?.first()
-        assertEquals(expected = "foo", actual = param?.getName())
+        val param = Container::noDescription.findParameterByName("myClass")
+        assertEquals(expected = "myClass", actual = param?.getName())
     }
 
     @Test
@@ -27,4 +36,16 @@ internal class KParameterExtensionsKtTest {
             mockParam.getName()
         }
     }
+
+    @Test
+    fun `parameter description`() {
+        val param = Container::paramDescription.findParameterByName("myClass")
+        assertEquals(expected = "param description", actual = param?.getGraphQLDescription())
+    }
+
+    @Test
+    fun `no description`() {
+        val param = Container::noDescription.findParameterByName("myClass")
+        assertNull(param?.getGraphQLDescription())
+    }
 }
diff --git a/src/test/kotlin/com/expedia/graphql/generator/extensions/KTypeExtensionsKtTest.kt b/src/test/kotlin/com/expedia/graphql/generator/extensions/KTypeExtensionsKtTest.kt
@@ -18,13 +18,21 @@ internal class KTypeExtensionsKtTest {
     internal class MyClass {
         fun listFun(list: List<String>) = list.joinToString(separator = ",") { it }
 
+        fun arrayFun(array: Array<String>) = array.joinToString(separator = ",") { it }
+
+        fun primitiveArrayFun(intArray: IntArray) = intArray.joinToString(separator = ",") { it.toString() }
+
         fun stringFun(string: String) = "hello $string"
     }
 
     @Test
     fun getTypeOfFirstArgument() {
         assertEquals(String::class.starProjectedType, MyClass::listFun.findParameterByName("list")?.type?.getTypeOfFirstArgument())
 
+        assertEquals(String::class.starProjectedType, MyClass::arrayFun.findParameterByName("array")?.type?.getTypeOfFirstArgument())
+
+        assertEquals(Int::class.starProjectedType, MyClass::primitiveArrayFun.findParameterByName("intArray")?.type?.getArrayType())
+
         assertFailsWith(InvalidListTypeException::class) {
             MyClass::stringFun.findParameterByName("string")?.type?.getTypeOfFirstArgument()
         }
diff --git a/src/test/kotlin/com/expedia/graphql/generator/types/TypeTestHelper.kt b/src/test/kotlin/com/expedia/graphql/generator/types/TypeTestHelper.kt
@@ -51,7 +51,9 @@ internal open class TypeTestHelper {
 
         directiveTypeBuilder = spyk(DirectiveTypeBuilder(generator))
         every { generator.directives(any()) } answers {
-            directiveTypeBuilder!!.directives(it.invocation.args[0] as KAnnotatedElement)
+            val directives = directiveTypeBuilder!!.directives(it.invocation.args[0] as KAnnotatedElement)
+            state.directives.addAll(directives)
+            directives
         }
 
         beforeTest()

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,6 @@ internal class ObjectTypeBuilder(generator: SchemaGenerator) : TypeBuilder(gener`
`26`	`26`
`27`	`27`	`generator.directives(kClass).forEach {`
`28`	`28`	`builder.withDirective(it)`
`29`		`- state.directives.add(it)`
`30`	`29`	`}`
`31`	`30`
`32`	`31`	`if (interfaceType != null) {`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,6 @@ internal class PropertyTypeBuilder(generator: SchemaGenerator) : TypeBuilder(gen`
`25`	`25`
`26`	`26`	`generator.directives(prop).forEach {`
`27`	`27`	`fieldBuilder.withDirective(it)`
`28`		`- state.directives.add(it)`
`29`	`28`	`}`
`30`	`29`
`31`	`30`	`val field = if (config.dataFetcherFactory != null && prop.isLateinit) {`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,9 @@ internal open class TypeTestHelper {`
`51`	`51`
`52`	`52`	`directiveTypeBuilder = spyk(DirectiveTypeBuilder(generator))`
`53`	`53`	`every { generator.directives(any()) } answers {`
`54`		`- directiveTypeBuilder!!.directives(it.invocation.args[0] as KAnnotatedElement)`
	`54`	`+ val directives = directiveTypeBuilder!!.directives(it.invocation.args[0] as KAnnotatedElement)`
	`55`	`+ state.directives.addAll(directives)`
	`56`	`+ directives`
`55`	`57`	`}`
`56`	`58`
`57`	`59`	`beforeTest()`