From 34d812454bfc6ffe17c0bc0bcc70dcdaee360b98 Mon Sep 17 00:00:00 2001 From: Ricardo Veguilla Date: Mon, 8 Jul 2024 20:29:49 -0700 Subject: [PATCH 1/4] Support @deprecated annotation without a reason attribute --- .../generators/shared/DirectivesUtils.kt | 4 +- .../graphql/dgs/codegen/CodeGenTest.kt | 37 +++++++++++------- .../graphql/dgs/codegen/KotlinCodeGenTest.kt | 38 +++++++++++++------ 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt index 0b06f1e39..950665166 100644 --- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt +++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt @@ -46,7 +46,7 @@ fun applyDirectivesKotlin(directives: List, config: CodeGenConfig): M if (argumentMap.containsKey(ParserConstants.REASON)) { annotations.add(deprecatedAnnotation((argumentMap[ParserConstants.REASON] as StringValue).value)) } else { - throw IllegalArgumentException("Deprecated requires an argument `${ParserConstants.REASON}`") + annotations.add(deprecatedAnnotation("Deprecated.")) } } @@ -90,7 +90,7 @@ fun applyDirectivesJava(directives: List, config: CodeGenConfig): Pai commentFormat = "@deprecated ${reason.substringBefore(ParserConstants.REPLACE_WITH_STR)}. Replaced by $replace" } } else { - throw IllegalArgumentException("Deprecated requires an argument `${ParserConstants.REASON}`") + commentFormat = "" } } annotations diff --git a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt index 41baf7523..2517132f1 100644 --- a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt +++ b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt @@ -4208,25 +4208,36 @@ It takes a title and such. } @Test - fun deprecateAnnotationWithNoMesssage() { + fun deprecateAnnotationWithNoReason() { val schema = """ input Person @deprecated { name: String } """.trimIndent() - assertThrows { - CodeGen( - CodeGenConfig( - schemas = setOf(schema), - packageName = basePackageName, - includeImports = mapOf(Pair("validator", "com.test.validator")), - includeEnumImports = mapOf("ValidPerson" to mapOf("types" to "com.enums")), - generateCustomAnnotations = true, - addDeprecatedAnnotation = true - ) - ).generate() - } + val (dataTypes) = CodeGen( + CodeGenConfig( + schemas = setOf(schema), + packageName = basePackageName, + includeImports = mapOf(Pair("validator", "com.test.validator")), + includeEnumImports = mapOf("ValidPerson" to mapOf("types" to "com.enums")), + generateCustomAnnotations = true, + addDeprecatedAnnotation = true + ) + + ).generate() + + + assertThat(dataTypes.size).isEqualTo(1) + val person = dataTypes.single().typeSpec + assertThat(person.name).isEqualTo("Person") + assertThat(person.annotations).hasSize(1) + assertThat(((person.annotations[0] as AnnotationSpec).type as ClassName).simpleName()).isEqualTo("Deprecated") + assertThat(((person.annotations[0] as AnnotationSpec).type as ClassName).canonicalName()).isEqualTo("java.lang.Deprecated") + assertThat(person.javadoc.toString()).isEmpty() + val fields = person.fieldSpecs + assertThat(fields).hasSize(1) + assertThat(fields[0].annotations).hasSize(0) } @Test diff --git a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt index 19ed70139..61487b028 100644 --- a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt +++ b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt @@ -2780,23 +2780,37 @@ class KotlinCodeGenTest { } @Test - fun deprecateAnnotationWithNoMesssage() { + fun deprecateAnnotationWithNoReason() { val schema = """ - input Person @deprecated(message: "This is going bye bye") { + input Person @deprecated(reason: "This is going bye bye") { name: String @deprecated } """.trimIndent() - assertThrows { - CodeGen( - CodeGenConfig( - schemas = setOf(schema), - packageName = basePackageName, - language = Language.KOTLIN, - addDeprecatedAnnotation = true - ) - ).generate() - } + val dataTypes = CodeGen( + CodeGenConfig( + schemas = setOf(schema), + packageName = basePackageName, + language = Language.KOTLIN, + addDeprecatedAnnotation = true + ) + ).generate().kotlinDataTypes + + assertThat(dataTypes).hasSize(1) + assertThat(dataTypes[0].name).isEqualTo("Person") + + val annotationSpec = (dataTypes[0].members[0] as TypeSpec).annotations[0] + assertThat((annotationSpec.typeName as ClassName).canonicalName).isEqualTo("kotlin.Deprecated") + assertThat(annotationSpec.members).hasSize(1) + assertThat(annotationSpec.members[0]).extracting("formatParts", "args").asList().contains(listOf("message = ", "%S"), listOf("This is going bye bye")) + + val parameterSpec = (((dataTypes[0].members)[0] as TypeSpec).primaryConstructor as FunSpec).parameters[0] + assertThat(parameterSpec.name).isEqualTo("name") + assertThat(parameterSpec.annotations).hasSize(2) + assertThat((parameterSpec.annotations[0].typeName as ClassName).canonicalName).isEqualTo("com.fasterxml.jackson.annotation.JsonProperty") + assertThat((parameterSpec.annotations[1].typeName as ClassName).canonicalName).isEqualTo("kotlin.Deprecated") + assertThat(parameterSpec.annotations[1].members).hasSize(1) + assertThat(parameterSpec.annotations[1].members[0]).extracting("formatParts", "args").asList().contains(listOf("message = ", "%S"), listOf("Deprecated.")) } @Test From 833ac2fcd377ac0e1bb3f838b575df82959751b5 Mon Sep 17 00:00:00 2001 From: Ricardo Veguilla Date: Tue, 9 Jul 2024 08:54:15 -0700 Subject: [PATCH 2/4] Fix kotling linting error --- .../test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt index 2517132f1..a0b44a1a8 100644 --- a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt +++ b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt @@ -4227,7 +4227,6 @@ It takes a title and such. ).generate() - assertThat(dataTypes.size).isEqualTo(1) val person = dataTypes.single().typeSpec assertThat(person.name).isEqualTo("Person") From f65a281db3b779700709e261ef1fe6221a245c29 Mon Sep 17 00:00:00 2001 From: Ricardo Veguilla Date: Tue, 9 Jul 2024 09:08:39 -0700 Subject: [PATCH 3/4] Address PR feedback --- .../kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt | 7 +++++-- .../com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt index a0b44a1a8..65c4a1ed4 100644 --- a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt +++ b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt @@ -4211,7 +4211,7 @@ It takes a title and such. fun deprecateAnnotationWithNoReason() { val schema = """ input Person @deprecated { - name: String + name: String @deprecated } """.trimIndent() @@ -4236,7 +4236,10 @@ It takes a title and such. assertThat(person.javadoc.toString()).isEmpty() val fields = person.fieldSpecs assertThat(fields).hasSize(1) - assertThat(fields[0].annotations).hasSize(0) + assertThat(fields[0].annotations).hasSize(1) + assertThat(((fields[0].annotations[0] as AnnotationSpec).type as ClassName).simpleName()).isEqualTo("Deprecated") + assertThat(((fields[0].annotations[0] as AnnotationSpec).type as ClassName).canonicalName()).isEqualTo("java.lang.Deprecated") + assertThat(fields[0].javadoc.toString()).isEmpty() } @Test diff --git a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt index 61487b028..6f24d7a41 100644 --- a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt +++ b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt @@ -2782,7 +2782,7 @@ class KotlinCodeGenTest { @Test fun deprecateAnnotationWithNoReason() { val schema = """ - input Person @deprecated(reason: "This is going bye bye") { + input Person @deprecated { name: String @deprecated } """.trimIndent() @@ -2802,7 +2802,7 @@ class KotlinCodeGenTest { val annotationSpec = (dataTypes[0].members[0] as TypeSpec).annotations[0] assertThat((annotationSpec.typeName as ClassName).canonicalName).isEqualTo("kotlin.Deprecated") assertThat(annotationSpec.members).hasSize(1) - assertThat(annotationSpec.members[0]).extracting("formatParts", "args").asList().contains(listOf("message = ", "%S"), listOf("This is going bye bye")) + assertThat(annotationSpec.members[0]).extracting("formatParts", "args").asList().contains(listOf("message = ", "%S"), listOf("Deprecated.")) val parameterSpec = (((dataTypes[0].members)[0] as TypeSpec).primaryConstructor as FunSpec).parameters[0] assertThat(parameterSpec.name).isEqualTo("name") From f253285585f7ca0385ac0de1498abcd8c117b8bf Mon Sep 17 00:00:00 2001 From: Ricardo Veguilla Date: Tue, 9 Jul 2024 10:39:09 -0700 Subject: [PATCH 4/4] Use a more informative text as fallback deprecation reason. --- .../graphql/dgs/codegen/generators/shared/DirectivesUtils.kt | 4 ++-- .../kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt | 4 ++-- .../com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt index 950665166..2ce5aedc4 100644 --- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt +++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt @@ -46,7 +46,7 @@ fun applyDirectivesKotlin(directives: List, config: CodeGenConfig): M if (argumentMap.containsKey(ParserConstants.REASON)) { annotations.add(deprecatedAnnotation((argumentMap[ParserConstants.REASON] as StringValue).value)) } else { - annotations.add(deprecatedAnnotation("Deprecated.")) + annotations.add(deprecatedAnnotation("Deprecated in the GraphQL schema.")) } } @@ -90,7 +90,7 @@ fun applyDirectivesJava(directives: List, config: CodeGenConfig): Pai commentFormat = "@deprecated ${reason.substringBefore(ParserConstants.REPLACE_WITH_STR)}. Replaced by $replace" } } else { - commentFormat = "" + commentFormat = "Deprecated in the GraphQL schema." } } annotations diff --git a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt index 65c4a1ed4..4617f394e 100644 --- a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt +++ b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt @@ -4233,13 +4233,13 @@ It takes a title and such. assertThat(person.annotations).hasSize(1) assertThat(((person.annotations[0] as AnnotationSpec).type as ClassName).simpleName()).isEqualTo("Deprecated") assertThat(((person.annotations[0] as AnnotationSpec).type as ClassName).canonicalName()).isEqualTo("java.lang.Deprecated") - assertThat(person.javadoc.toString()).isEmpty() + assertThat(person.javadoc.toString()).isEqualTo("Deprecated in the GraphQL schema.") val fields = person.fieldSpecs assertThat(fields).hasSize(1) assertThat(fields[0].annotations).hasSize(1) assertThat(((fields[0].annotations[0] as AnnotationSpec).type as ClassName).simpleName()).isEqualTo("Deprecated") assertThat(((fields[0].annotations[0] as AnnotationSpec).type as ClassName).canonicalName()).isEqualTo("java.lang.Deprecated") - assertThat(fields[0].javadoc.toString()).isEmpty() + assertThat(fields[0].javadoc.toString()).isEqualTo("Deprecated in the GraphQL schema.") } @Test diff --git a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt index 6f24d7a41..c0335b88c 100644 --- a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt +++ b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/KotlinCodeGenTest.kt @@ -2802,7 +2802,7 @@ class KotlinCodeGenTest { val annotationSpec = (dataTypes[0].members[0] as TypeSpec).annotations[0] assertThat((annotationSpec.typeName as ClassName).canonicalName).isEqualTo("kotlin.Deprecated") assertThat(annotationSpec.members).hasSize(1) - assertThat(annotationSpec.members[0]).extracting("formatParts", "args").asList().contains(listOf("message = ", "%S"), listOf("Deprecated.")) + assertThat(annotationSpec.members[0]).extracting("formatParts", "args").asList().contains(listOf("message = ", "%S"), listOf("Deprecated in the GraphQL schema.")) val parameterSpec = (((dataTypes[0].members)[0] as TypeSpec).primaryConstructor as FunSpec).parameters[0] assertThat(parameterSpec.name).isEqualTo("name") @@ -2810,7 +2810,7 @@ class KotlinCodeGenTest { assertThat((parameterSpec.annotations[0].typeName as ClassName).canonicalName).isEqualTo("com.fasterxml.jackson.annotation.JsonProperty") assertThat((parameterSpec.annotations[1].typeName as ClassName).canonicalName).isEqualTo("kotlin.Deprecated") assertThat(parameterSpec.annotations[1].members).hasSize(1) - assertThat(parameterSpec.annotations[1].members[0]).extracting("formatParts", "args").asList().contains(listOf("message = ", "%S"), listOf("Deprecated.")) + assertThat(parameterSpec.annotations[1].members[0]).extracting("formatParts", "args").asList().contains(listOf("message = ", "%S"), listOf("Deprecated in the GraphQL schema.")) } @Test