Skip to content

Commit ab2acff

Browse files
smyrickdariuszkuc
authored andcommitted
Throw exception on multi-level interface (#420)
* Throw exception on multi-level interface Resolves #419 According to the spec, an interface can not implement another interface. This is not caught by our schema generator but instead just returns the first level interface as the type in the schema. We are now throwing an exception to help translate this understanding of GraphQL through the Kotlin code * Return superclasses from multiple levels deep
1 parent f0b3590 commit ab2acff

File tree

6 files changed

+146
-1
lines changed

6 files changed

+146
-1
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright 2019 Expedia, Inc
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.expediagroup.graphql.exceptions
18+
19+
/**
20+
* Thrown when a interface implements another interface or abstract class that is not exluded from the schema.
21+
*
22+
* This is an invalid schema until the GraphQL spec is updated
23+
* https://github.com/ExpediaGroup/graphql-kotlin/issues/419
24+
*/
25+
class InvalidInterfaceException : GraphQLKotlinException("Interfaces can not have any superclasses")

graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/extensions/kClassExtensions.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ internal fun KClass<*>.getValidSuperclasses(hooks: SchemaGeneratorHooks): List<K
5151
this.superclasses
5252
.filter { hooks.isValidSuperclass(it) }
5353
.filter { kClass -> superclassFilters.all { it.invoke(kClass) } }
54+
.ifEmpty {
55+
this.superclasses.flatMap { it.getValidSuperclasses(hooks) }
56+
}
5457

5558
internal fun KClass<*>.findConstructorParamter(name: String): KParameter? =
5659
this.primaryConstructor?.findParameterByName(name)

graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/filters/superclassFilters.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.expediagroup.graphql.generator.filters
1818

19+
import com.expediagroup.graphql.generator.extensions.isGraphQLIgnored
1920
import com.expediagroup.graphql.generator.extensions.isInterface
2021
import com.expediagroup.graphql.generator.extensions.isPublic
2122
import com.expediagroup.graphql.generator.extensions.isUnion
@@ -26,5 +27,6 @@ private typealias SuperclassFilter = (KClass<*>) -> Boolean
2627
private val isPublic: SuperclassFilter = { it.isPublic() }
2728
private val isInterface: SuperclassFilter = { it.isInterface() }
2829
private val isNotUnion: SuperclassFilter = { it.isUnion().not() }
30+
private val isNotIgnored: SuperclassFilter = { it.isGraphQLIgnored().not() }
2931

30-
internal val superclassFilters: List<SuperclassFilter> = listOf(isPublic, isInterface, isNotUnion)
32+
internal val superclassFilters: List<SuperclassFilter> = listOf(isPublic, isInterface, isNotUnion, isNotIgnored)

graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/types/InterfaceBuilder.kt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616

1717
package com.expediagroup.graphql.generator.types
1818

19+
import com.expediagroup.graphql.exceptions.InvalidInterfaceException
1920
import com.expediagroup.graphql.generator.SchemaGenerator
2021
import com.expediagroup.graphql.generator.TypeBuilder
2122
import com.expediagroup.graphql.generator.extensions.getGraphQLDescription
2223
import com.expediagroup.graphql.generator.extensions.getSimpleName
2324
import com.expediagroup.graphql.generator.extensions.getValidFunctions
2425
import com.expediagroup.graphql.generator.extensions.getValidProperties
26+
import com.expediagroup.graphql.generator.extensions.getValidSuperclasses
2527
import com.expediagroup.graphql.generator.extensions.safeCast
2628
import graphql.TypeResolutionEnvironment
2729
import graphql.schema.GraphQLInterfaceType
@@ -34,6 +36,12 @@ import kotlin.reflect.full.createType
3436
internal class InterfaceBuilder(generator: SchemaGenerator) : TypeBuilder(generator) {
3537
internal fun interfaceType(kClass: KClass<*>): GraphQLType {
3638
return state.cache.buildIfNotUnderConstruction(kClass) {
39+
40+
// Interfaces can not implement another interface in GraphQL
41+
if (kClass.getValidSuperclasses(config.hooks).isNotEmpty()) {
42+
throw InvalidInterfaceException()
43+
}
44+
3745
val builder = GraphQLInterfaceType.newInterface()
3846

3947
builder.name(kClass.getSimpleName())

graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/extensions/KClassExtensionsTest.kt

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.expediagroup.graphql.generator.extensions
1818

19+
import com.expediagroup.graphql.annotations.GraphQLIgnore
1920
import com.expediagroup.graphql.annotations.GraphQLName
2021
import com.expediagroup.graphql.exceptions.CouldNotGetNameOfKClassException
2122
import com.expediagroup.graphql.hooks.NoopSchemaGeneratorHooks
@@ -83,6 +84,38 @@ open class KClassExtensionsTest {
8384

8485
internal class UnionSuperclass : TestInterface
8586

87+
@GraphQLIgnore
88+
internal interface IgnoredInterface {
89+
val id: String
90+
}
91+
92+
@GraphQLIgnore
93+
interface IgnoredSecondLevelInterface : SomeInterface
94+
95+
internal class ClassWithSecondLevelInterface : IgnoredSecondLevelInterface {
96+
override val someField: String = "hello"
97+
98+
override fun someFunction(): String? = null
99+
}
100+
101+
@GraphQLIgnore
102+
abstract class IgnoredAbstractClass : SomeInterface
103+
104+
@GraphQLIgnore
105+
abstract class IgnoredSecondAbstractClass : IgnoredAbstractClass()
106+
107+
internal class ClassWithNoValidSuperclass(override val id: String) : IgnoredInterface
108+
109+
internal class ClassWithSecondLevelAbstractClass : IgnoredAbstractClass() {
110+
override val someField: String = "foo"
111+
override fun someFunction(): String? = "bar"
112+
}
113+
114+
internal class ClassWithThirdLevelAbstractClass : IgnoredSecondAbstractClass() {
115+
override val someField: String = "foo"
116+
override fun someFunction(): String? = "bar"
117+
}
118+
86119
internal class InterfaceSuperclass : InvalidFunctionUnionInterface {
87120
override fun getTest() = 2
88121
}
@@ -175,6 +208,30 @@ open class KClassExtensionsTest {
175208
assertTrue(superclasses.isEmpty())
176209
}
177210

211+
@Test
212+
fun `Superclasses are not included when marked as ignored`() {
213+
val superclasses = ClassWithNoValidSuperclass::class.getValidSuperclasses(noopHooks)
214+
assertTrue(superclasses.isEmpty())
215+
}
216+
217+
@Test
218+
fun `Return superclasses from abstract class two levels deep`() {
219+
val superclasses = ClassWithSecondLevelAbstractClass::class.getValidSuperclasses(noopHooks)
220+
assertEquals(expected = 1, actual = superclasses.size)
221+
}
222+
223+
@Test
224+
fun `Return superclasses from abstract class three levels deep`() {
225+
val superclasses = ClassWithThirdLevelAbstractClass::class.getValidSuperclasses(noopHooks)
226+
assertEquals(expected = 1, actual = superclasses.size)
227+
}
228+
229+
@Test
230+
fun `Return superclasses from interface two levels deep`() {
231+
val superclasses = ClassWithSecondLevelInterface::class.getValidSuperclasses(noopHooks)
232+
assertEquals(expected = 1, actual = superclasses.size)
233+
}
234+
178235
@Test
179236
fun `test findConstructorParamter`() {
180237
assertNotNull(MyTestClass::class.findConstructorParamter("publicProperty"))
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 2019 Expedia, Inc
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.expediagroup.graphql.test.integration
18+
19+
import com.expediagroup.graphql.TopLevelObject
20+
import com.expediagroup.graphql.exceptions.InvalidInterfaceException
21+
import com.expediagroup.graphql.testSchemaConfig
22+
import com.expediagroup.graphql.toSchema
23+
import org.junit.jupiter.api.Test
24+
import kotlin.test.assertFailsWith
25+
26+
class InterfaceOfInterfaceTest {
27+
28+
@Test
29+
fun `interface of interface`() {
30+
val queries = listOf(TopLevelObject(InterfaceOfInterfaceQuery()))
31+
32+
assertFailsWith(InvalidInterfaceException::class) {
33+
toSchema(queries = queries, config = testSchemaConfig)
34+
}
35+
}
36+
37+
interface FirstLevel {
38+
val id: String
39+
}
40+
41+
interface SecondLevel : FirstLevel {
42+
val name: String
43+
}
44+
45+
class MyClass(override val id: String, override val name: String) : SecondLevel
46+
47+
class InterfaceOfInterfaceQuery {
48+
fun getClass() = MyClass(id = "1", name = "fooBar")
49+
}
50+
}

0 commit comments

Comments
 (0)