feat: move default implementations to interface (#68)

smyrick · dariuszkuc · commit 11f2e9a9586c · 2018-11-16T11:55:10.000-06:00
* feat: move default implementations to interface

* fix: update unit tests
diff --git a/example/src/main/kotlin/com.expedia.graphql.sample/extension/CustomSchemaGeneratorHooks.kt b/example/src/main/kotlin/com.expedia.graphql.sample/extension/CustomSchemaGeneratorHooks.kt
@@ -2,7 +2,7 @@ package com.expedia.graphql.sample.extension
 
 import com.expedia.graphql.sample.validation.DataFetcherExecutionValidator
 import com.expedia.graphql.schema.hooks.DataFetcherExecutionPredicate
-import com.expedia.graphql.schema.hooks.NoopSchemaGeneratorHooks
+import com.expedia.graphql.schema.hooks.SchemaGeneratorHooks
 import graphql.language.StringValue
 import graphql.schema.Coercing
 import graphql.schema.GraphQLScalarType
@@ -15,7 +15,7 @@ import kotlin.reflect.KType
 /**
  * Schema generator hook that adds additional scalar types.
  */
-class CustomSchemaGeneratorHooks(validator: Validator) : NoopSchemaGeneratorHooks() {
+class CustomSchemaGeneratorHooks(validator: Validator) : SchemaGeneratorHooks {
 
     /**
      * Register additional GraphQL scalar types.
diff --git a/src/main/kotlin/com/expedia/graphql/schema/extensions/annotationExtensions.kt b/src/main/kotlin/com/expedia/graphql/schema/extensions/annotationExtensions.kt
@@ -5,6 +5,7 @@ import com.expedia.graphql.annotations.GraphQLID
 import com.expedia.graphql.annotations.GraphQLIgnore
 import com.expedia.graphql.schema.exceptions.CouldNotGetNameOfAnnotationException
 import com.expedia.graphql.schema.generator.types.defaultGraphQLScalars
+import com.expedia.graphql.schema.hooks.SchemaGeneratorHooks
 import com.google.common.base.CaseFormat
 import graphql.schema.GraphQLArgument
 import graphql.schema.GraphQLDirective
@@ -36,8 +37,9 @@ private fun KAnnotatedElement.listOfDirectives(): List<String> {
 
     return this.annotations.asSequence()
         .mapNotNull { it.getDirectiveInfo() }
-        .map { when {
-                it.effectiveName != null -> "@${it.effectiveName}"
+        .map {
+            when {
+                it.effectiveName.isNullOrEmpty().not() -> "@${it.effectiveName}"
                 else -> null
             }
         }
@@ -69,14 +71,14 @@ private fun Annotation.getDirectiveInfo(): DirectiveInfo? {
     }
 }
 
-internal fun KAnnotatedElement.directives() =
+internal fun KAnnotatedElement.directives(hooks: SchemaGeneratorHooks) =
     this.annotations.asSequence()
         .mapNotNull { it.getDirectiveInfo() }
-        .map { it.getGraphQLDirective() }
+        .map { it.getGraphQLDirective(hooks) }
         .toList()
 
 @Throws(CouldNotGetNameOfAnnotationException::class)
-private fun DirectiveInfo.getGraphQLDirective(): GraphQLDirective {
+private fun DirectiveInfo.getGraphQLDirective(hooks: SchemaGeneratorHooks): GraphQLDirective {
     val kClass: KClass<out DirectiveAnnotation> = this.annotation.annotationClass
     val builder = GraphQLDirective.newDirective()
     val name: String = this.effectiveName ?: throw CouldNotGetNameOfAnnotationException(kClass)
@@ -87,7 +89,7 @@ private fun DirectiveInfo.getGraphQLDirective(): GraphQLDirective {
         .validLocations(*this.annotation.locations)
         .description(this.annotation.description)
 
-    kClass.getValidFunctions().forEach { kFunction ->
+    kClass.getValidFunctions(hooks).forEach { kFunction ->
         val propertyName = kFunction.name
         val value = kFunction.call(kClass)
         @Suppress("Detekt.UnsafeCast")
diff --git a/src/main/kotlin/com/expedia/graphql/schema/generator/SchemaGenerator.kt b/src/main/kotlin/com/expedia/graphql/schema/generator/SchemaGenerator.kt
@@ -120,7 +120,7 @@ internal class SchemaGenerator(
             builder.deprecate(it)
         }
 
-        fn.directives().forEach {
+        fn.directives(config.hooks).forEach {
             builder.withDirective(it)
             state.directives.add(it)
         }
@@ -221,7 +221,7 @@ internal class SchemaGenerator(
             builder.name(kClass.simpleName)
             builder.description(kClass.graphQLDescription())
 
-            kClass.directives().forEach {
+            kClass.directives(config.hooks).forEach {
                 builder.withDirective(it)
                 state.directives.add(it)
             }
diff --git a/src/main/kotlin/com/expedia/graphql/schema/hooks/NoopSchemaGeneratorHooks.kt b/src/main/kotlin/com/expedia/graphql/schema/hooks/NoopSchemaGeneratorHooks.kt
@@ -1,44 +1,7 @@
 package com.expedia.graphql.schema.hooks
 
-import graphql.schema.DataFetcher
-import graphql.schema.GraphQLFieldDefinition
-import graphql.schema.GraphQLSchema
-import graphql.schema.GraphQLType
-import java.util.concurrent.CompletableFuture
-import kotlin.reflect.KFunction
-import kotlin.reflect.KProperty
-import kotlin.reflect.KType
-
 /**
- * Default hooks that do not override or set anything.
- *
- * To set your own custom hooks, it is easier to extend this class instead of SchemaGeneratorHooks
- * and just override the methods you need.
+ * Default hooks that do not override or set anything. Only used internally.
+ * If you don't need hooks, the configuration will default to these.
  */
-open class NoopSchemaGeneratorHooks : SchemaGeneratorHooks {
-
-    override fun willBuildSchema(builder: GraphQLSchema.Builder): GraphQLSchema.Builder = builder
-
-    override fun willGenerateGraphQLType(type: KType): GraphQLType? = null
-
-    override fun willResolveMonad(type: KType): KType =
-        if (type.classifier == CompletableFuture::class) {
-            type.arguments.firstOrNull()?.type ?: type
-        } else {
-            type
-        }
-
-    override fun isValidProperty(property: KProperty<*>) = true
-
-    override fun isValidFunction(function: KFunction<*>) = true
-
-    override fun didGenerateGraphQLType(type: KType, generatedType: GraphQLType) = Unit
-
-    override fun didGenerateDataFetcher(function: KFunction<*>, dataFetcher: DataFetcher<*>) = dataFetcher
-
-    override fun didGenerateQueryType(function: KFunction<*>, fieldDefinition: GraphQLFieldDefinition) = fieldDefinition
-
-    override fun didGenerateMutationType(function: KFunction<*>, fieldDefinition: GraphQLFieldDefinition) = fieldDefinition
-
-    override val dataFetcherExecutionPredicate: DataFetcherExecutionPredicate? = null
-}
+internal class NoopSchemaGeneratorHooks : SchemaGeneratorHooks
diff --git a/src/main/kotlin/com/expedia/graphql/schema/hooks/SchemaGeneratorHooks.kt b/src/main/kotlin/com/expedia/graphql/schema/hooks/SchemaGeneratorHooks.kt
@@ -4,6 +4,7 @@ import graphql.schema.DataFetcher
 import graphql.schema.GraphQLFieldDefinition
 import graphql.schema.GraphQLSchema
 import graphql.schema.GraphQLType
+import java.util.concurrent.CompletableFuture
 import kotlin.reflect.KFunction
 import kotlin.reflect.KProperty
 import kotlin.reflect.KType
@@ -18,56 +19,65 @@ interface SchemaGeneratorHooks {
      * Called before the final GraphQL schema is built.
      * This doesn't prevent the called from rebuilding the final schema using java-graphql's functionality
      */
-    fun willBuildSchema(builder: GraphQLSchema.Builder): GraphQLSchema.Builder
+    fun willBuildSchema(builder: GraphQLSchema.Builder): GraphQLSchema.Builder = builder
 
     /**
      * Called before using reflection to generate the graphql object type for the given KType.
      * This allows supporting objects that the caller does not want to use reflection on for special handling
      */
-    fun willGenerateGraphQLType(type: KType): GraphQLType?
+    @Suppress("Detekt.FunctionOnlyReturningConstant")
+    fun willGenerateGraphQLType(type: KType): GraphQLType? = null
 
     /**
      * Called before resolving a Monad or Future type to its wrapped KType.
      * This allows for a custom resolver on how to extract the wrapped value.
      */
-    fun willResolveMonad(type: KType): KType
+    fun willResolveMonad(type: KType): KType =
+        if (type.classifier == CompletableFuture::class) {
+            type.arguments.firstOrNull()?.type ?: type
+        } else {
+            type
+        }
 
     /**
      * Called when looking at the KClass properties to determine if it valid for adding to the generated schema.
      * If any filter returns false, it is rejected.
      */
-    fun isValidProperty(property: KProperty<*>): Boolean
+    @Suppress("Detekt.FunctionOnlyReturningConstant")
+    fun isValidProperty(property: KProperty<*>): Boolean = true
 
     /**
      * Called when looking at the KClass functions to determine if it valid for adding to the generated schema.
      * If any filter returns false, it is rejected.
      */
-    fun isValidFunction(function: KFunction<*>): Boolean
+    @Suppress("Detekt.FunctionOnlyReturningConstant")
+    fun isValidFunction(function: KFunction<*>): Boolean = true
 
     /**
      * Called after wrapping the type based on nullity but before adding the generated type to the schema
      */
-    fun didGenerateGraphQLType(type: KType, generatedType: GraphQLType)
+    fun didGenerateGraphQLType(type: KType, generatedType: GraphQLType) = Unit
 
     /**
      * Called after converting the function to a data fetcher allowing wrapping the fetcher to modify data or instrument it.
      * This is more useful than the graphql.execution.instrumentation.Instrumentation as you have the function type here
      */
-    fun didGenerateDataFetcher(function: KFunction<*>, dataFetcher: DataFetcher<*>): DataFetcher<*>
+    fun didGenerateDataFetcher(function: KFunction<*>, dataFetcher: DataFetcher<*>): DataFetcher<*> = dataFetcher
 
     /**
      * Called after converting the function to a field definition but before adding to the schema to allow customization
      */
-    fun didGenerateQueryType(function: KFunction<*>, fieldDefinition: GraphQLFieldDefinition): GraphQLFieldDefinition
+    fun didGenerateQueryType(function: KFunction<*>, fieldDefinition: GraphQLFieldDefinition): GraphQLFieldDefinition = fieldDefinition
 
     /**
      * Called after converting the function to a field definition but before adding to the schema to allow customization
      */
-    fun didGenerateMutationType(function: KFunction<*>, fieldDefinition: GraphQLFieldDefinition): GraphQLFieldDefinition
+    fun didGenerateMutationType(function: KFunction<*>, fieldDefinition: GraphQLFieldDefinition): GraphQLFieldDefinition = fieldDefinition
 
     /**
      * Execute a predicate on each function parameters after their deserialization
      * If the execution is unsuccessful the `onFailure` method will be invoked
      */
     val dataFetcherExecutionPredicate: DataFetcherExecutionPredicate?
+        get() = null
 }
diff --git a/src/test/kotlin/com/expedia/graphql/schema/dataFetchers/DataFetchPredicateTests.kt b/src/test/kotlin/com/expedia/graphql/schema/dataFetchers/DataFetchPredicateTests.kt
@@ -4,7 +4,7 @@ import com.expedia.graphql.TopLevelObjectDef
 import com.expedia.graphql.schema.Parameter
 import com.expedia.graphql.schema.getTestSchemaConfigWithHooks
 import com.expedia.graphql.schema.hooks.DataFetcherExecutionPredicate
-import com.expedia.graphql.schema.hooks.NoopSchemaGeneratorHooks
+import com.expedia.graphql.schema.hooks.SchemaGeneratorHooks
 import com.expedia.graphql.toSchema
 import graphql.ExceptionWhileDataFetching
 import graphql.GraphQL
@@ -57,7 +57,7 @@ class QueryWithValidations {
 
 data class Person(val name: String, val age: Int)
 
-class PredicateHooks : NoopSchemaGeneratorHooks() {
+class PredicateHooks : SchemaGeneratorHooks {
     override val dataFetcherExecutionPredicate: DataFetcherExecutionPredicate? = TestDataFetcherPredicate()
 }
 
diff --git a/src/test/kotlin/com/expedia/graphql/schema/extensions/KClassExtensionsTest.kt b/src/test/kotlin/com/expedia/graphql/schema/extensions/KClassExtensionsTest.kt
@@ -1,6 +1,6 @@
 package com.expedia.graphql.schema.extensions
 
-import com.expedia.graphql.schema.hooks.NoopSchemaGeneratorHooks
+import com.expedia.graphql.schema.hooks.SchemaGeneratorHooks
 import org.junit.jupiter.api.Test
 import kotlin.reflect.KFunction
 import kotlin.reflect.KProperty
@@ -21,7 +21,7 @@ class KClassExtensionsTest {
         private fun privateTestFunction() = "private function"
     }
 
-    class FilterHooks : NoopSchemaGeneratorHooks() {
+    class FilterHooks : SchemaGeneratorHooks {
         override fun isValidProperty(property: KProperty<*>) =
             property.name.contains("filteredProperty").not()
 
diff --git a/src/test/kotlin/com/expedia/graphql/schema/generator/SchemaGeneratorAsyncTests.kt b/src/test/kotlin/com/expedia/graphql/schema/generator/SchemaGeneratorAsyncTests.kt
@@ -2,7 +2,7 @@ package com.expedia.graphql.schema.generator
 
 import com.expedia.graphql.TopLevelObjectDef
 import com.expedia.graphql.schema.getTestSchemaConfigWithHooks
-import com.expedia.graphql.schema.hooks.NoopSchemaGeneratorHooks
+import com.expedia.graphql.schema.hooks.SchemaGeneratorHooks
 import com.expedia.graphql.schema.testSchemaConfig
 import com.expedia.graphql.toSchema
 import graphql.schema.GraphQLNonNull
@@ -16,7 +16,7 @@ import kotlin.test.assertEquals
 
 class SchemaGeneratorAsyncTests {
 
-    private class MonadHooks : NoopSchemaGeneratorHooks() {
+    private class MonadHooks : SchemaGeneratorHooks {
         override fun willResolveMonad(type: KType): KType = when (type.classifier) {
             Observable::class, Single::class, Maybe::class -> type.arguments.firstOrNull()?.type
             else -> type
diff --git a/src/test/kotlin/com/expedia/graphql/schema/generator/SchemaGeneratorTest.kt b/src/test/kotlin/com/expedia/graphql/schema/generator/SchemaGeneratorTest.kt
@@ -13,14 +13,13 @@ import graphql.GraphQL
 import graphql.Scalars
 import graphql.schema.GraphQLNonNull
 import graphql.schema.GraphQLObjectType
-import org.junit.jupiter.api.Assertions.assertNull
-import org.junit.jupiter.api.Assertions.assertThrows
 import java.net.CookieManager
 import java.util.UUID
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 import kotlin.test.assertNotNull
+import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
 @Suppress("Detekt.UnusedPrivateMember",
@@ -209,21 +208,21 @@ class SchemaGeneratorTest {
 
     @Test
     fun `SchemaGenerator throws when encountering java stdlib`() {
-        assertThrows(RuntimeException::class.java) {
+        assertFailsWith(RuntimeException::class) {
             toSchema(listOf(TopLevelObjectDef(QueryWithJavaClass())), config = testSchemaConfig)
         }
     }
 
     @Test
     fun `SchemaGenerator throws when encountering conflicting types`() {
-        assertThrows(ConflictingTypesException::class.java) {
+        assertFailsWith(ConflictingTypesException::class) {
             toSchema(queries = listOf(TopLevelObjectDef(QueryWithConflictingTypes())), config = testSchemaConfig)
         }
     }
 
     @Test
     fun `SchemaGenerator should throw exception if no queries and no mutations are specified`() {
-        assertThrows(InvalidSchemaException::class.java) {
+        assertFailsWith(InvalidSchemaException::class) {
             toSchema(emptyList(), emptyList(), config = testSchemaConfig)
         }
     }
@@ -260,7 +259,7 @@ class SchemaGeneratorTest {
 
     @Test
     fun `SchemaGenerator throws an exception for invalid GraphQLID`() {
-        val exception = assertThrows(IllegalArgumentException::class.java) {
+        val exception = assertFailsWith(IllegalArgumentException::class) {
             toSchema(queries = listOf(TopLevelObjectDef(QueryWithInvalidId())), config = testSchemaConfig)
         }
 
diff --git a/src/test/kotlin/com/expedia/graphql/schema/hooks/SchemaGeneratorHooksTest.kt b/src/test/kotlin/com/expedia/graphql/schema/hooks/SchemaGeneratorHooksTest.kt
@@ -26,7 +26,7 @@ class SchemaGeneratorHooksTest {
 
     @Test
     fun `calls hook before schema is built`() {
-        class MockSchemaGeneratorHooks : NoopSchemaGeneratorHooks() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
             var willBuildSchemaCalled = false
             override fun willBuildSchema(builder: GraphQLSchema.Builder): GraphQLSchema.Builder {
                 willBuildSchemaCalled = true
@@ -46,7 +46,7 @@ class SchemaGeneratorHooksTest {
 
     @Test
     fun `calls hook before generating object type`() {
-        class MockSchemaGeneratorHooks : NoopSchemaGeneratorHooks() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
             var willGenerateGraphQLTypeCalled = false
             override fun willGenerateGraphQLType(type: KType): GraphQLType? {
                 willGenerateGraphQLTypeCalled = true
@@ -67,7 +67,7 @@ class SchemaGeneratorHooksTest {
 
     @Test
     fun `calls hook to filter property`() {
-        class MockSchemaGeneratorHooks : NoopSchemaGeneratorHooks() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
             var calledFilterFunction = false
 
             override fun isValidProperty(property: KProperty<*>): Boolean {
@@ -88,7 +88,7 @@ class SchemaGeneratorHooksTest {
 
     @Test
     fun `calls hook to filter functions`() {
-        class MockSchemaGeneratorHooks : NoopSchemaGeneratorHooks() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
             var calledFilterFunction = false
 
             override fun isValidFunction(function: KFunction<*>): Boolean {
@@ -108,7 +108,7 @@ class SchemaGeneratorHooksTest {
 
     @Test
     fun `calls hook after generating object type`() {
-        class MockSchemaGeneratorHooks : NoopSchemaGeneratorHooks() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
             var lastSeenType: KType? = null
             var lastSeenGeneratedType: GraphQLType? = null
             override fun didGenerateGraphQLType(type: KType, generatedType: GraphQLType) {
@@ -128,7 +128,7 @@ class SchemaGeneratorHooksTest {
 
     @Test
     fun `calls hook before adding data fetcher`() {
-        class MockSchemaGeneratorHooks : NoopSchemaGeneratorHooks() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
             var didGenerateDataFetcherCalled = false
             var lastSeenFunction: KFunction<*>? = null
             var lastReturnedDataFetcher: WrappingDataFetcher? = null
@@ -156,7 +156,7 @@ class SchemaGeneratorHooksTest {
 
     @Test
     fun `calls hook before adding query to schema`() {
-        class MockSchemaGeneratorHooks : NoopSchemaGeneratorHooks() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
             override fun didGenerateQueryType(
                 function: KFunction<*>,
                 fieldDefinition: GraphQLFieldDefinition
@@ -182,7 +182,7 @@ class SchemaGeneratorHooksTest {
 
     @Test
     fun `calls hook before adding mutation to schema`() {
-        class MockSchemaGeneratorHooks : NoopSchemaGeneratorHooks() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
             override fun didGenerateMutationType(
                 function: KFunction<*>,
                 fieldDefinition: GraphQLFieldDefinition

Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ internal class SchemaGenerator(`
`120`	`120`	`builder.deprecate(it)`
`121`	`121`	`}`
`122`	`122`
`123`		`- fn.directives().forEach {`
	`123`	`+ fn.directives(config.hooks).forEach {`
`124`	`124`	`builder.withDirective(it)`
`125`	`125`	`state.directives.add(it)`
`126`	`126`	`}`
`@@ -221,7 +221,7 @@ internal class SchemaGenerator(`
`221`	`221`	`builder.name(kClass.simpleName)`
`222`	`222`	`builder.description(kClass.graphQLDescription())`
`223`	`223`
`224`		`- kClass.directives().forEach {`
	`224`	`+ kClass.directives(config.hooks).forEach {`
`225`	`225`	`builder.withDirective(it)`
`226`	`226`	`state.directives.add(it)`
`227`	`227`	`}`