Skip to content

Commit

Permalink
fix!: Fix validation on root types
Browse files Browse the repository at this point in the history
According to the spec, root operation types must be of type `Object`,
and `Object` types must have at least one defined field. This was
already validated for regular types but not for `Query`.

Effectively, this also means that every valid schema needs to define
at least one query.

Resolves #156

BREAKING CHANGE: Schemas without Queries no longer work
  • Loading branch information
stuebingerb committed Jan 12, 2025
1 parent e44f523 commit fa0472e
Show file tree
Hide file tree
Showing 14 changed files with 215 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@ import org.junit.jupiter.api.Test
class KtorConfigurationTest : KtorTest() {

@Test
fun `default configuration should`() {
fun `default configuration should use Parallel executor`() {
var checked = false
testApplication {
application {
val config = install(GraphQL) {}
val config = install(GraphQL) {
schema {
query("dummy") {
resolver { -> "dummy" }
}
}
}
checked = true
config.schema.configuration.executor shouldBeEqualTo Executor.Parallel
}
Expand All @@ -28,6 +34,11 @@ class KtorConfigurationTest : KtorTest() {
application {
val config = install(GraphQL) {
executor = Executor.DataLoaderPrepared
schema {
query("dummy") {
resolver { -> "dummy" }
}
}
}
checked = true
config.schema.configuration.executor shouldBeEqualTo Executor.DataLoaderPrepared
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ class KtorFeatureTest : KtorTest() {
@Test
fun `Simple mutation test`() {
val server = withServer {
query("dummy") {
resolver { -> "dummy" }
}
mutation("hello") {
resolver { -> "World! mutation" }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class SchemaPrinter(private val config: SchemaPrinterConfig = SchemaPrinterConfi
// Objects (includes Query, Mutation, and Subscription) with non-empty fields
// https://spec.graphql.org/draft/#sec-Objects
val objects = buildString {
schemaTypes[TypeKind.OBJECT]?.filter { !it.fields.isNullOrEmpty() }?.forEachIndexed { index, type ->
schemaTypes[TypeKind.OBJECT]?.forEachIndexed { index, type ->
if (index > 0) {
appendLine()
}
Expand Down Expand Up @@ -215,9 +215,9 @@ class SchemaPrinter(private val config: SchemaPrinterConfig = SchemaPrinterConfi
(subscriptionType != null && subscriptionType?.name != "Subscription")

private fun __Schema.hasRegularTypeWithDefaultRootTypeName() = types.any {
(it != queryType && it.name == "Query") ||
(it != mutationType && it.name == "Mutation") ||
(it != subscriptionType && it.name == "Subscription")
(it.name != queryType.name && it.name == "Query") ||
(it.name != mutationType?.name && it.name == "Mutation") ||
(it.name != subscriptionType?.name && it.name == "Subscription")
}

private fun Depreciable.deprecationInfo(): String = if (isDeprecated) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,8 @@ class SchemaCompilation(

val model = SchemaModel(
query = queryType,
mutation = if (mutationType.fields.isNullOrEmpty()) {
null
} else {
mutationType
},
subscription = if (subscriptionType.fields.isNullOrEmpty()) {
null
} else {
subscriptionType
},
mutation = mutationType,
subscription = subscriptionType,
queryTypes = queryTypeProxies + enums + scalars,
inputTypes = inputTypeProxies + enums + scalars,
allTypes = queryTypeProxies.values
Expand Down Expand Up @@ -135,30 +127,44 @@ class SchemaCompilation(

private suspend fun handleQueries(): Type {
val __typenameField = typenameField(FunctionWrapper.on { -> "Query" })
val declaredFields = definition.queries.map { handleOperation(it) }
if (declaredFields.isEmpty()) {
throw SchemaException("Schema must define at least one query")
}
return Type.OperationObject(
name = "Query",
description = "Query object",
fields = definition.queries.map { handleOperation(it) } + introspectionSchemaQuery() + introspectionTypeQuery() + __typenameField
fields = declaredFields + __typenameField + introspectionSchemaQuery() + introspectionTypeQuery()
)
}

private suspend fun handleMutations(): Type {
private suspend fun handleMutations(): Type? {
val __typenameField = typenameField(FunctionWrapper.on { -> "Mutation" })
return Type.OperationObject(
"Mutation",
"Mutation object",
definition.mutations.map { handleOperation(it) } + __typenameField)
val declaredFields = definition.mutations.map { handleOperation(it) }
return if (declaredFields.isNotEmpty()) {
Type.OperationObject(
name = "Mutation",
description = "Mutation object",
fields = declaredFields + __typenameField
)
} else {
null
}
}

// https://spec.graphql.org/October2021/#sec-Type-Name-Introspection
// "__typename may not be included as a root field in a subscription operation."
private suspend fun handleSubscriptions(): Type {
// https://spec.graphql.org/October2021/#sec-Type-Name-Introspection
// "__typename may not be included as a root field in a subscription operation."
return Type.OperationObject(
"Subscription",
"Subscription object",
definition.subscriptions.map { handleOperation(it) })
private suspend fun handleSubscriptions(): Type? {
val declaredFields = definition.subscriptions.map { handleOperation(it) }
return if (declaredFields.isNotEmpty()) {
Type.OperationObject(
name = "Subscription",
description = "Subscription object",
// https://spec.graphql.org/October2021/#sec-Type-Name-Introspection
// "__typename may not be included as a root field in a subscription operation."
fields = definition.subscriptions.map { handleOperation(it) }
)
} else {
null
}
}

private suspend fun introspectionSchemaQuery() = handleOperation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,7 @@ import org.junit.jupiter.api.Test

class ParserTest {

private fun parse(source: String, options: Parser.Options? = null) = try {
Parser(source, options).parseDocument()
} catch (e: GraphQLError) {
println(e.prettyPrint())
throw e
}
private fun parse(source: String, options: Parser.Options? = null) = Parser(source, options).parseDocument()

private fun parse(source: Source) = Parser(source).parseDocument()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@ class SchemaBuilderTest {
@Test
fun `Schema should map input types`() {
val schema = defaultSchema {
query("createInput") {
resolver { input: InputTwo -> input.one }
}
inputType<InputTwo>()
}

Expand Down Expand Up @@ -723,6 +726,9 @@ class SchemaBuilderTest {
@Test
fun `Schema can have same type and input type with different names`() {
val schema = defaultSchema {
query("createType") {
resolver { input: InputOne -> input }
}
inputType<InputOne> {
name = "TypeAsInput"
}
Expand Down Expand Up @@ -822,6 +828,9 @@ class SchemaBuilderTest {
@Test
fun `specifying return type explicitly allows generic property creation`() {
val schema = defaultSchema {
query("scenario") {
resolver { -> "dummy" }
}
type<Scenario> {
createGenericProperty(InputOne("generic"))
}
Expand All @@ -841,12 +850,14 @@ class SchemaBuilderTest {

@Test
fun `creation of properties from a list`() {

val props = listOf(
Prop(typeOf<Int>()) { 0 },
Prop(typeOf<String>()) { "test" })

val schema = defaultSchema {
query("scenario") {
resolver { -> "dummy" }
}
type<Scenario> {
props.forEach { prop ->
createGenericPropertyExplicitly(prop.resultType, prop.resolver())
Expand All @@ -873,4 +884,27 @@ class SchemaBuilderTest {
message shouldBeEqualTo "Java class 'LatLng' as inputType is not supported"
}
}

// https://github.com/stuebingerb/KGraphQL/issues/156
@Test
fun `empty schema should be invalid`() {
invoking {
KGraphQL.schema {}
} shouldThrow SchemaException::class withMessage "Schema must define at least one query"
}

// https://github.com/stuebingerb/KGraphQL/issues/156
@Test
fun `schema without query should be invalid`() {
invoking {
KGraphQL.schema {
mutation("dummy") {
resolver { -> "dummy" }
}
subscription("dummy") {
resolver { -> "dummy" }
}
}
} shouldThrow SchemaException::class withMessage "Schema must define at least one query"
}
}
Loading

0 comments on commit fa0472e

Please sign in to comment.