From 4bf0fc6d7129264e08f8de2f780c2d0eb88d2e3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernd=20St=C3=BCbinger?= <41049452+stuebingerb@users.noreply.github.com> Date: Thu, 30 Jan 2025 15:27:01 +0100 Subject: [PATCH] fix: Sort arguments by name in SDL `SchemaPrinter` already sorts types and fields by name but similar sorting for arguments was missing. Arguments in GraphQL are unordered, so let's make it consistent and a bit more readable. Also fixes test setups by enforcing unique names for input objects (cf. #167) and skips checking input objects for null or empty input fields, as they must have at least one. --- .../kgraphql/schema/SchemaPrinter.kt | 11 +++---- .../kgraphql/schema/model/ast/TypeNode.kt | 1 - .../kgraphql/schema/SchemaPrinterTest.kt | 33 +++++++++++-------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/SchemaPrinter.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/SchemaPrinter.kt index 826a1766..a20c6c07 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/SchemaPrinter.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/SchemaPrinter.kt @@ -159,8 +159,7 @@ class SchemaPrinter(private val config: SchemaPrinterConfig = SchemaPrinterConfi // Input Types // https://spec.graphql.org/draft/#sec-Input-Objects val inputTypes = buildString { - schemaTypes[TypeKind.INPUT_OBJECT]?.filter { !it.inputFields.isNullOrEmpty() } - ?.forEachIndexed { index, type -> + schemaTypes[TypeKind.INPUT_OBJECT]?.forEachIndexed { index, type -> if (index > 0) { appendLine() } @@ -184,7 +183,7 @@ class SchemaPrinter(private val config: SchemaPrinterConfig = SchemaPrinterConfi if (index > 0) { appendLine() } - val args = directive.args.takeIf { it.isNotEmpty() } + val args = directive.args.sortedByName().takeIf { it.isNotEmpty() } ?.joinToString(", ", prefix = "(", postfix = ")") { arg -> arg.name + ": " + arg.type.typeReference() + arg.defaultInfo() } ?: "" @@ -263,15 +262,15 @@ class SchemaPrinter(private val config: SchemaPrinterConfig = SchemaPrinterConfi // otherwise print them on a single line if (field.args.any { it.isDeprecated || (config.includeDescriptions && !it.description.isNullOrBlank()) }) { appendLine("${indentation}${field.name}(") - field.args.forEach { arg -> + field.args.sortedByName().forEach { arg -> val argsIndentation = "$indentation$indentation" appendDescription(arg, argsIndentation) appendLine("$argsIndentation${arg.name}: ${arg.type.typeReference()}${arg.defaultInfo()}${arg.deprecationInfo()}") } appendLine("${indentation}): ${field.type.typeReference()}${field.deprecationInfo()}") } else { - val args = - field.args.takeIf { it.isNotEmpty() }?.joinToString(", ", prefix = "(", postfix = ")") { arg -> + val args = field.args.sortedByName().takeIf { it.isNotEmpty() } + ?.joinToString(", ", prefix = "(", postfix = ")") { arg -> arg.name + ": " + arg.type.typeReference() + arg.defaultInfo() } ?: "" appendLine("${indentation}${field.name}$args: ${field.type.typeReference()}${field.deprecationInfo()}") diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/model/ast/TypeNode.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/model/ast/TypeNode.kt index b9e089d6..d80e1320 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/model/ast/TypeNode.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/model/ast/TypeNode.kt @@ -9,7 +9,6 @@ sealed class TypeNode(override val loc: Location?) : ASTNode() { class NonNullTypeNode(loc: Location?, val type: TypeNode) : TypeNode(loc) - val isNullable get() = this !is NonNullTypeNode val isList get() = this is ListTypeNode || (this is NonNullTypeNode && type is ListTypeNode) diff --git a/kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaPrinterTest.kt b/kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaPrinterTest.kt index ba138d90..b37bb33a 100644 --- a/kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaPrinterTest.kt +++ b/kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaPrinterTest.kt @@ -130,7 +130,7 @@ class SchemaPrinterTest { returnType = linked nullable = true description = "link to pdf representation of scenario" - resolver { scenario: Scenario -> null } + resolver { null } } } } @@ -163,20 +163,20 @@ class SchemaPrinterTest { } sealed class Child - data class Child1(val one: String): Child() - data class Child2(val two: String?): Child() + data class Child1(val one: String) : Child() + data class Child2(val two: String?) : Child() @Test fun `schema with union types out of sealed classes should be printed as expected`() { val schema = KGraphQL.schema { query("child") { - resolver { -> Child1("one") } + resolver { Child1("one") } } query("childs") { - resolver> { -> listOf(Child2("one")) } + resolver> { listOf(Child2("one")) } } query("nullchilds") { - resolver?> { -> null } + resolver?> { null } } } @@ -367,7 +367,7 @@ class SchemaPrinterTest { getNullString: String getObject(nullObject: Boolean!): TestObject getString: String! - randomInt(min: Int!, max: Int): Int! + randomInt(max: Int, min: Int!): Int! randomString(possibleReturns: [String!]!): String! } @@ -460,7 +460,7 @@ class SchemaPrinterTest { type Query { getStringsForTypes(types: [TestEnum!] = [TYPE1, TYPE2]): [String!]! - getStringWithDefault(type: TestEnum! = TYPE1, string: String!): String! + getStringWithDefault(string: String!, type: TestEnum! = TYPE1): String! } enum TestEnum { @@ -493,6 +493,7 @@ class SchemaPrinterTest { } } query("data") { + @Suppress("UNUSED_ANONYMOUS_PARAMETER") resolver { oldOptional: String?, new: String -> "" }.withArgs { arg(String::class, typeOf()) { name = "oldOptional"; defaultValue = "\"\""; deprecate("deprecated arg") @@ -514,8 +515,8 @@ class SchemaPrinterTest { type Query { data( - oldOptional: String = "" @deprecated(reason: "deprecated arg") new: String! + oldOptional: String = "" @deprecated(reason: "deprecated arg") ): String! } @@ -543,6 +544,9 @@ class SchemaPrinterTest { description = "This is the name" } } + inputType { + name = "TestObjectInput" + } enum { value(TestEnum.TYPE1) { description = "Enum value description" @@ -594,7 +598,7 @@ class SchemaPrinterTest { "Add a test object" "With some multi-line description" "(& special characters like " and \n)" - addObject(toAdd: TestObject!): TestObject! + addObject(toAdd: TestObjectInput!): TestObject! } "Query object" @@ -623,7 +627,7 @@ class SchemaPrinterTest { TYPE2 } - input TestObject { + input TestObjectInput { name: String! } @@ -638,6 +642,9 @@ class SchemaPrinterTest { description = "This is the name" } } + inputType { + name = "TestObjectInput" + } enum { value(TestEnum.TYPE1) { description = "Enum value description" @@ -663,7 +670,7 @@ class SchemaPrinterTest { SchemaPrinter(SchemaPrinterConfig(includeDescriptions = false)).print(schema) shouldBeEqualTo """ type Mutation { - addObject(toAdd: TestObject!): TestObject! + addObject(toAdd: TestObjectInput!): TestObject! } type Query { @@ -683,7 +690,7 @@ class SchemaPrinterTest { TYPE2 } - input TestObject { + input TestObjectInput { name: String! }