fix: fix exception for Iterables without type variable (#545)

Mostly reverts d8ce008 to rely on
`Collection::class` instead of `Iterable::class` again, as the latter
broke Kotlin `IntRange` without adding clear benefits (it would IMO be
debatable if a range should even be represented as list).

Resolves #176

Author: stuebingerb

kgraphql/src/jvm/kotlin/com/apurebase/kgraphql/BenchmarkSchema.kt

@@ -5,29 +5,21 @@ import com.apurebase.kgraphql.schema.dsl.SchemaBuilder
 
 data class ModelOne(val name: String, val quantity: Int = 1, val active: Boolean = true)
 
-data class ModelTwo(val one: ModelOne, val range: FakeIntRange)
+data class ModelTwo(val one: ModelOne, val range: IntRange)
 
 data class ModelThree(val id: String, val twos: List<ModelTwo>)
 
-// wrapping needed, because https://github.com/stuebingerb/KGraphQL/commit/d8ce0085130b9f0f30c0c2f31ed52f44d6456981
-// destroyed compatibility with ranges in the schema.
-// please see: https://github.com/stuebingerb/KGraphQL/issues/176
-class FakeIntRange(range: IntRange) {
-    val start = range.first
-    val endInclusive = range.last
-}
-
 object BenchmarkSchema {
     val ones = listOf(ModelOne("DUDE"), ModelOne("GUY"), ModelOne("PAL"), ModelOne("FELLA"))
 
     val oneResolver: suspend () -> List<ModelOne> = { ones }
 
     val twoResolver: suspend (name: String) -> ModelTwo? = { name ->
-        ones.find { it.name == name }?.let { ModelTwo(it, FakeIntRange(it.quantity..12)) }
+        ones.find { it.name == name }?.let { ModelTwo(it, it.quantity..12) }
     }
 
     val threeResolver: suspend () -> ModelThree =
-        { ModelThree("", ones.map { ModelTwo(it, FakeIntRange(it.quantity..10)) }) }
+        { ModelThree("", ones.map { ModelTwo(it, it.quantity..10) }) }
 
     object HasOneResolver {
         fun oneResolver(): List<ModelOne> {

kgraphql/src/main/kotlin/com/apurebase/kgraphql/Extensions.kt

@@ -22,9 +22,9 @@ internal fun String.dropQuotes(): String = if (isLiteral()) {
 
 internal fun String.isLiteral(): Boolean = startsWith('\"') && endsWith('\"')
 
-internal fun KClass<*>.isIterable() = isSubclassOf(Iterable::class) || this in typeByPrimitiveArrayClass.keys
+internal fun KClass<*>.isCollectionType() = isSubclassOf(Collection::class) || this in typeByPrimitiveArrayClass.keys
 
-internal fun KType.isIterable() = jvmErasure.isIterable() || toString().startsWith("kotlin.Array")
+internal fun KType.isCollectionType() = jvmErasure.isCollectionType() || toString().startsWith("kotlin.Array")
 
 internal val typeByPrimitiveArrayClass = mapOf(
     IntArray::class to Int::class.createType(),
@@ -36,9 +36,11 @@ internal val typeByPrimitiveArrayClass = mapOf(
     BooleanArray::class to Boolean::class.createType()
 )
 
-internal fun KType.getIterableElementType(): KType {
-    require(isIterable()) { "KType $this is not collection type" }
-    return typeByPrimitiveArrayClass[jvmErasure] ?: arguments.firstOrNull()?.type ?: throw NoSuchElementException("KType $this has no type arguments")
+internal fun KType.getCollectionElementType(): KType {
+    require(isCollectionType()) { "KType $this is not collection type" }
+    return typeByPrimitiveArrayClass[jvmErasure]
+        ?: arguments.firstOrNull()?.type
+        ?: throw NoSuchElementException("KType $this has no type arguments")
 }
 
 internal suspend fun <T, R> Iterable<T>.mapIndexedParallel(

kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt

@@ -5,8 +5,8 @@ package com.apurebase.kgraphql.schema.structure
 import com.apurebase.kgraphql.Context
 import com.apurebase.kgraphql.configuration.SchemaConfiguration
 import com.apurebase.kgraphql.defaultKQLTypeName
-import com.apurebase.kgraphql.getIterableElementType
-import com.apurebase.kgraphql.isIterable
+import com.apurebase.kgraphql.getCollectionElementType
+import com.apurebase.kgraphql.isCollectionType
 import com.apurebase.kgraphql.request.isIntrospectionType
 import com.apurebase.kgraphql.schema.DefaultSchema
 import com.apurebase.kgraphql.schema.SchemaException
@@ -232,7 +232,7 @@ open class SchemaCompilation(
     }
 
     private suspend fun handlePossiblyWrappedType(kType: KType, typeCategory: TypeCategory): Type = when {
-        kType.isIterable() -> handleCollectionType(kType, typeCategory)
+        kType.isCollectionType() -> handleCollectionType(kType, typeCategory)
         kType.jvmErasure == Context::class && typeCategory == TypeCategory.INPUT -> contextType
         kType.jvmErasure == Execution.Node::class && typeCategory == TypeCategory.INPUT -> executionType
         kType.jvmErasure == Context::class && typeCategory == TypeCategory.QUERY -> throw SchemaException("Context type cannot be part of schema")
@@ -267,7 +267,7 @@ open class SchemaCompilation(
     }
 
     private suspend fun handleCollectionType(kType: KType, typeCategory: TypeCategory): Type {
-        val type = kType.getIterableElementType()
+        val type = kType.getCollectionElementType()
         val nullableListType = Type.AList(handlePossiblyWrappedType(type, typeCategory), kType.jvmErasure)
         return applyNullability(kType.isMarkedNullable, nullableListType)
     }

kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ListsSpecificationTest.kt

@@ -18,7 +18,7 @@ class ListsSpecificationTest {
     fun `list arguments are valid`() {
         val schema = KGraphQL.schema {
             query("list") {
-                resolver { list: Iterable<String> -> list }
+                resolver { list: List<String> -> list }
             }
         }
 
@@ -37,7 +37,7 @@ class ListsSpecificationTest {
     fun `lists with nullable entries are valid`() {
         val schema = KGraphQL.schema {
             query("list") {
-                resolver { list: Iterable<String?> -> list }
+                resolver { list: List<String?> -> list }
             }
         }
 
@@ -54,7 +54,7 @@ class ListsSpecificationTest {
     fun `lists with non-nullable entries should not accept list with null element`() {
         val schema = KGraphQL.schema {
             query("list") {
-                resolver { list: Iterable<String> -> list }
+                resolver { list: List<String> -> list }
             }
         }
 
@@ -71,7 +71,7 @@ class ListsSpecificationTest {
     fun `by default coerce single element input as collection`() {
         val schema = KGraphQL.schema {
             query("list") {
-                resolver { list: Iterable<String> -> list }
+                resolver { list: List<String> -> list }
             }
         }
 
@@ -88,7 +88,7 @@ class ListsSpecificationTest {
     fun `null value is not coerced as single element collection`() {
         val schema = KGraphQL.schema {
             query("list") {
-                resolver { list: Iterable<String>? -> list }
+                resolver { list: List<String>? -> list }
             }
         }
 
@@ -105,7 +105,7 @@ class ListsSpecificationTest {
     fun `list argument can be declared non-nullable`() {
         val schema = KGraphQL.schema {
             query("list") {
-                resolver { list: Iterable<String> -> list }
+                resolver { list: List<String> -> list }
             }
         }
 
@@ -119,9 +119,9 @@ class ListsSpecificationTest {
     }
 
     @Test
-    fun `Iterable implementations are treated as list`() {
+    fun `Collection implementations are treated as list`() {
 
-        fun getResult(): Iterable<String> = listOf("POTATO", "BATATO", "ROTATO")
+        fun getResult(): Collection<String> = listOf("POTATO", "BATATO", "ROTATO")
 
         val schema = KGraphQL.schema {
             query("list") {
@@ -130,7 +130,22 @@ class ListsSpecificationTest {
         }
 
         val response = deserialize(schema.executeBlocking("{ list }"))
-        response.extract<Iterable<String>>("data/list") shouldBe getResult()
+        response.extract<Collection<String>>("data/list") shouldBe getResult()
+    }
+
+    @Test
+    fun `Set implementations are treated as list`() {
+
+        fun getResult(): Set<String> = setOf("POTATO", "BATATO", "ROTATO")
+
+        val schema = KGraphQL.schema {
+            query("list") {
+                resolver { -> getResult() }
+            }
+        }
+
+        val response = deserialize(schema.executeBlocking("{ list }"))
+        response.extract<Set<String>>("data/list") shouldBe getResult()
     }
 
     @Test

kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ObjectsSpecificationTest.kt

@@ -486,4 +486,40 @@ class ObjectsSpecificationTest {
             indexOf("short") shouldBeGreaterThan indexOf("long")
         }
     }
+
+    // https://github.com/stuebingerb/KGraphQL/issues/176
+    @Test
+    fun `IntRange can be used`() {
+        data class Model(val intRange: IntRange)
+
+        val schema = schema {
+            query("model") {
+                resolver { -> Model(1..9) }
+            }
+        }
+
+        schema.printSchema() shouldBe """
+            type IntRange {
+              endExclusive: Int!
+              endInclusive: Int!
+              first: Int!
+              last: Int!
+              start: Int!
+              step: Int!
+            }
+
+            type Model {
+              intRange: IntRange!
+            }
+
+            type Query {
+              model: Model!
+            }
+
+        """.trimIndent()
+
+        schema.executeBlocking("{ model { intRange { start endInclusive endExclusive first last step } } }") shouldBe """
+           {"data":{"model":{"intRange":{"start":1,"endInclusive":9,"endExclusive":10,"first":1,"last":9,"step":1}}}}
+        """.trimIndent()
+    }
 }