Repository: ImFlog/schema-registry-plugin
Pull request #160 (2024-01-04 19:13:09)

[GH-158] Add 'pretty' flag to support formatting of downloaded AVRO and JSON schemas
I haven't worked much with Kotlin so I tried to mostly keep the code style consistent with the rest. If there's any style choices you're not quite satisfied with, feel free to suggest changes and/or modify.

Protobuf schema downloads include newlines and hence are, in fact, already formatted. Regardless, so as to not be a 'soft' breaking change, I've made the `pretty` option `false` by default. Setting a default of `true` would be more consistent with the protobuf handling though. 

Re: tests: I added both a unit test and an integration test but I'm not quite happy with the integration test at the moment since I didn't want to spend too much time on it. Feedback are welcome.

One last thing: I didn't have a chance to test *registering* a formatted avro/json schema. I know that the Schema Registry API does _not_ support it and expects an escaped JSON string - I'm not sure if the programmatic client calls support it. Still need a test case for that...

filpano: I also just noticed that I forgot to update the `README` - I can do it tomorrow.
ImFlog: LGTM except for the failing the test.
I will try to take time to fix the test ASAP.
filpano: I've managed to narrow the issue down to commit 04e52e3c - though I'm not sure how since I didn't touch the failing test in that commit. I can take a quick look.
filpano: The above assumption was correct. I'm not 100% sure why the `client.reset()` didn't work if it was registered under the same name in that case, but I did verify that the `@Teardown` method is called after every iteration of the parametrized test. ¯\_(ツ)_/¯

Patch:
diff --git a/README.md b/README.md
index f02955f..c1b086c 100644
--- a/README.md
+++ b/README.md
@@ -50,6 +50,7 @@ schemaRegistry {
     url = 'http://registry-url:8081/'
     quiet = true
     outputDirectory = "/home/kafka/results"
+    pretty = true
 }
 ```
 
@@ -59,7 +60,8 @@ schemaRegistry {
   Could be removed if https://github.com/gradle/gradle/issues/1010 is fixed.
 * `outputDirectory` is the directory where action result will be stored as files (only register for now).
   This is an optional parameter.
-
+* `pretty` is whether the downloaded Avro or json schemas should be formatted ("pretty-printed") or minified. 
+  This is an optional parameter.
 ### Download schemas
 
 Like the name of the task imply, this task is responsible for retrieving schemas from a schema registry.
diff --git a/build.gradle.kts b/build.gradle.kts
index b4d8d6d..d0bbc6d 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -17,7 +17,7 @@ repositories {
 }
 
 // Dependencies versions
-val confluentVersion = "7.5.2"
+val confluentVersion = "7.5.3"
 val avroVersion = "1.11.2"
 dependencies {
     shadow(gradleApi())
diff --git a/src/integration/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskIT.kt b/src/integration/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskIT.kt
index 3dfaf52..ac1bb68 100644
--- a/src/integration/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskIT.kt
+++ b/src/integration/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskIT.kt
@@ -1,5 +1,9 @@
 package com.github.imflog.schema.registry.tasks.download
 
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.databind.PropertyNamingStrategies
+import com.fasterxml.jackson.databind.SerializationFeature
+import com.github.imflog.schema.registry.SchemaType
 import com.github.imflog.schema.registry.toSchemaType
 import com.github.imflog.schema.registry.utils.KafkaTestContainersUtils
 import io.confluent.kafka.schemaregistry.ParsedSchema
@@ -28,6 +32,9 @@ class DownloadTaskIT : KafkaTestContainersUtils() {
 
     private lateinit var folderRule: TemporaryFolder
     private lateinit var buildFile: File
+    private val objectMapper = ObjectMapper()
+        .configure(SerializationFeature.INDENT_OUTPUT, true)
+        .setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
 
     @BeforeEach
     fun init() {
@@ -95,6 +102,65 @@ class DownloadTaskIT : KafkaTestContainersUtils() {
         Assertions.assertThat(resultFile2.readText()).contains("description")
     }
 
+    @Test
+    fun `Should format supported schema types when pretty is specified`() {
+
+        val type = AvroSchema.TYPE
+        val schema =  AvroSchema(
+                """{
+                            "type": "record",
+                            "name": "User",
+                            "fields": [
+                                { "name": "name", "type": "string" }, 
+                                { "name": "description", "type": ["null", "string"], "default": null }
+                            ]
+                        }""".filter { !it.isWhitespace() }
+        )
+
+        // Given
+        val subjectName = "prettyprinted-$type"
+
+        client.register(subjectName, schema)
+
+        buildFile = folderRule.newFile("build.gradle")
+        buildFile.writeText(
+            """
+            plugins {
+                id 'java'
+                id 'com.github.imflog.kafka-schema-registry-gradle-plugin'
+            }
+
+            schemaRegistry {
+                url = '$schemaRegistryEndpoint'
+                pretty = true
+                download {
+                    subject('$subjectName', '${folderRule.root.absolutePath}/src/main/$type/test')
+                }
+            }
+        """
+        )
+
+        // When
+        val result: BuildResult? = GradleRunner.create()
+            .withGradleVersion("7.6")
+            .withProjectDir(folderRule.root)
+            .withArguments(DownloadTask.TASK_NAME)
+            .withPluginClasspath()
+            .withDebug(true)
+            .build()
+
+        // Then
+        val schemaType = schema.schemaType().toSchemaType()
+        val schemaFile = "$subjectName.${schemaType.extension}"
+
+        Assertions.assertThat(File(folderRule.root, "src/main/$type/test")).exists()
+        Assertions.assertThat(File(folderRule.root, "src/main/$type/test/$schemaFile")).exists()
+        Assertions.assertThat(result?.task(":downloadSchemasTask")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
+        Assertions.assertThat(File(folderRule.root, "src/main/$type/test/$schemaFile")).hasContent(
+            objectMapper.readTree(schema.toString()).toPrettyString()
+        )
+    }
+
     @ParameterizedTest(name = "{0}")
     @ArgumentsSource(SchemaArgumentProvider::class)
     fun `Should download schemas by subject name pattern`(
diff --git a/src/main/kotlin/com/github/imflog/schema/registry/SchemaRegistryExtension.kt b/src/main/kotlin/com/github/imflog/schema/registry/SchemaRegistryExtension.kt
index 86595a2..c7180ce 100644
--- a/src/main/kotlin/com/github/imflog/schema/registry/SchemaRegistryExtension.kt
+++ b/src/main/kotlin/com/github/imflog/schema/registry/SchemaRegistryExtension.kt
@@ -20,4 +20,8 @@ open class SchemaRegistryExtension(objects: ObjectFactory) {
     }
 
     val outputDirectory: Property<String> = objects.property(String::class.java)
+
+    val pretty: Property<Boolean> = objects.property(Boolean::class.java).apply {
+        convention(false)
+    }
 }
diff --git a/src/main/kotlin/com/github/imflog/schema/registry/SchemaRegistryPlugin.kt b/src/main/kotlin/com/github/imflog/schema/registry/SchemaRegistryPlugin.kt
index 3567bd4..82c5707 100644
--- a/src/main/kotlin/com/github/imflog/schema/registry/SchemaRegistryPlugin.kt
+++ b/src/main/kotlin/com/github/imflog/schema/registry/SchemaRegistryPlugin.kt
@@ -59,6 +59,7 @@ class SchemaRegistryPlugin : Plugin<Project> {
                     downloadTask.ssl.set(sslExtension.configs)
                     downloadTask.subjects.set(downloadExtension.subjects)
                     downloadTask.metadataConfig.set(downloadExtension.metadata)
+                    downloadTask.pretty.set(globalExtension.pretty)
                 }
 
             tasks.register(RegisterSchemasTask.TASK_NAME, RegisterSchemasTask::class.java)
diff --git a/src/main/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTask.kt b/src/main/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTask.kt
index 4b27e6c..4657933 100644
--- a/src/main/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTask.kt
+++ b/src/main/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTask.kt
@@ -38,13 +38,17 @@ open class DownloadTask @Inject constructor(objects: ObjectFactory) : DefaultTas
     @Input
     val ssl: MapProperty<String, String> = objects.mapProperty(String::class.java, String::class.java)
 
+    @Input
+    val pretty: Property<Boolean> = objects.property(Boolean::class.java)
+
     @TaskAction
     fun downloadSchemas() {
         val errorCount = DownloadTaskAction(
             RegistryClientWrapper.client(url.get(), basicAuth.get(), ssl.get()),
             project.rootDir,
             subjects.get(),
-            metadataConfig.get()
+            metadataConfig.get(),
+            pretty.get()
         ).run()
         if (errorCount > 0) {
             throw GradleScriptException("$errorCount schemas not downloaded, see logs for details", Throwable())
diff --git a/src/main/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskAction.kt b/src/main/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskAction.kt
index d94270d..81ac0cc 100644
--- a/src/main/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskAction.kt
+++ b/src/main/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskAction.kt
@@ -9,9 +9,11 @@ import com.github.imflog.schema.registry.SchemaType
 import com.github.imflog.schema.registry.toSchemaType
 import com.google.common.base.Suppliers
 import io.confluent.kafka.schemaregistry.ParsedSchema
+import io.confluent.kafka.schemaregistry.avro.AvroSchema
 import io.confluent.kafka.schemaregistry.client.SchemaMetadata
 import io.confluent.kafka.schemaregistry.client.SchemaRegistryClient
 import io.confluent.kafka.schemaregistry.client.rest.entities.SchemaReference
+import io.confluent.kafka.schemaregistry.json.JsonSchema
 import org.gradle.api.logging.Logging
 import java.io.File
 import java.util.regex.PatternSyntaxException
@@ -20,7 +22,8 @@ class DownloadTaskAction(
     private val client: SchemaRegistryClient,
     private val rootDir: File,
     private val subjects: List<DownloadSubject>,
-    private val metadataConfiguration: MetadataExtension
+    private val metadataConfiguration: MetadataExtension,
+    private val pretty: Boolean = false
 ) {
 
     private val logger = Logging.getLogger(DownloadTaskAction::class.java)
@@ -49,7 +52,7 @@ class DownloadTaskAction(
                 if (metadataConfiguration.enabled) {
                     writeSchemaMetadata(downloadSubject, metadata, metadataDirectory)
                 }
-                writeSchemaFile(downloadSubject, metadata, outputDir)
+                writeSchemaFile(downloadSubject, metadata, pretty, outputDir)
 
                 if (downloadSubject.downloadReferences) {
                     metadata.references.forEach {
@@ -64,7 +67,7 @@ class DownloadTaskAction(
                         if (metadataConfiguration.enabled) {
                             writeSchemaMetadata(referenceSubject, referenceMetadata, metadataDirectory)
                         }
-                        writeSchemaFile(referenceSubject, referenceMetadata, outputDir)
+                        writeSchemaFile(referenceSubject, referenceMetadata, pretty, outputDir)
                     }
                 }
             } catch (e: Exception) {
@@ -110,7 +113,7 @@ class DownloadTaskAction(
         if (subject.version == null) client.getLatestSchemaMetadata(subject.subject)
         else client.getSchemaMetadata(subject.subject, subject.version)
 
-    private fun writeSchemaFile(downloadSubject: DownloadSubject, schemaMetadata: SchemaMetadata, outputDir: File) {
+    private fun writeSchemaFile(downloadSubject: DownloadSubject, schemaMetadata: SchemaMetadata, pretty: Boolean, outputDir: File) {
         val parsedSchema = parseSchemaWithRemoteReferences(
             downloadSubject.subject,
             schemaMetadata.schemaType.toSchemaType(),
@@ -120,17 +123,29 @@ class DownloadTaskAction(
         val fileName = downloadSubject.outputFileName ?: downloadSubject.subject
         val outputFile = File(outputDir, "${fileName}.${parsedSchema.schemaType().toSchemaType().extension}")
         outputFile.createNewFile()
-        logger.infoIfNotQuiet("Writing file  $outputFile")
+        logger.infoIfNotQuiet("Writing file $outputFile")
         outputFile.printWriter().use { out ->
-            out.println(parsedSchema.toString())
+            out.println(getSchemaString(parsedSchema, pretty))
         }
     }
 
+    private fun getSchemaString(parsedSchema: ParsedSchema, pretty: Boolean): String {
+        return if (pretty && isSupportedPrettyType(parsedSchema)) objectMapper.readTree(parsedSchema.toString()).toPrettyString() else parsedSchema.toString()
+    }
+
+    /**
+     * Checks whether the current schema type should be pretty-printed.
+     * Avro and Json are considered eligible for pretty formatting, Protobuf is not.
+     */
+    private fun isSupportedPrettyType(parsedSchema: ParsedSchema): Boolean {
+        return parsedSchema.schemaType() == AvroSchema.TYPE || parsedSchema.schemaType() == JsonSchema.TYPE
+    }
+
     private fun writeSchemaMetadata(downloadSubject: DownloadSubject, schemaMetadata: SchemaMetadata, outputDir: File) {
         val fileName = downloadSubject.outputFileName ?: downloadSubject.subject
         val outputFile = File(outputDir, "${fileName}-metadata.json")
         outputFile.createNewFile()
-        logger.infoIfNotQuiet("Writing metadata file  $outputFile")
+        logger.infoIfNotQuiet("Writing metadata file $outputFile")
         outputFile.printWriter().use { out ->
             out.println(
                 objectMapper.writeValueAsString(schemaMetadata)


Test patch:
diff --git a/src/test/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskActionTest.kt b/src/test/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskActionTest.kt
index 7fbd913..236e9ee 100644
--- a/src/test/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskActionTest.kt
+++ b/src/test/kotlin/com/github/imflog/schema/registry/tasks/download/DownloadTaskActionTest.kt
@@ -526,4 +526,84 @@ class DownloadTaskActionTest {
         Assertions.assertThat(File(folderRoot, "src/main/avro/external/test_lib.avsc")).isNotNull
         // Would be cleaner to use a JSON assertion library but I am not sure this is really required for now
     }
+
+    @Test
+    fun `Should format supported schema types when pretty is specified`() {
+        // given
+        val testSubject = "test"
+        val fooSubject = "foo"
+        val outputDir = "src/main/avro/external"
+
+        val registryClient =
+            MockSchemaRegistryClient(listOf(AvroSchemaProvider(), JsonSchemaProvider(), ProtobufSchemaProvider()))
+
+        registryClient.register(
+            testSubject,
+            registryClient.parseSchema(
+                AvroSchema.TYPE,
+                """{
+                    "type": "record",
+                    "name": "test",
+                    "fields": [
+                        { "name": "name", "type": "string" }
+                    ]
+                }""",
+                listOf()
+            ).get()
+        )
+        registryClient.register(
+            fooSubject,
+            registryClient.parseSchema(
+                AvroSchema.TYPE,
+                """{
+                    "type": "record",
+                    "name": "foo",
+                    "fields": [{ "name": "name", "type": "string" }]
+                }""",
+                listOf()
+            ).get()
+        )
+
+        folderRule.resolve("src/main/avro/external").toFile().mkdir()
+        val folderRoot = folderRule.toFile()
+
+        // when
+        val errorCount = DownloadTaskAction(
+            registryClient,
+            folderRoot,
+            arrayListOf(
+                DownloadSubject(testSubject, outputDir),
+                DownloadSubject(fooSubject, outputDir)
+            ),
+            MetadataExtension(),
+            true
+        ).run()
+
+        // then
+        Assertions.assertThat(errorCount).isEqualTo(0)
+        Assertions.assertThat(File(folderRoot, "src/main/avro/external/test.avsc")).isNotNull
+        Assertions.assertThat(File(folderRoot, "src/main/avro/external/test.avsc").readText())
+            .isEqualTo("""
+                {
+                  "type" : "record",
+                  "name" : "test",
+                  "fields" : [ {
+                    "name" : "name",
+                    "type" : "string"
+                  } ]
+                }
+            """.trimIndent() + "\n") // trimIndent removes trailing newline
+        Assertions.assertThat(File(folderRoot, "src/main/avro/external/foo.avsc")).isNotNull
+        Assertions.assertThat(File(folderRoot, "src/main/avro/external/foo.avsc").readText())
+            .isEqualTo("""
+                {
+                  "type" : "record",
+                  "name" : "foo",
+                  "fields" : [ {
+                    "name" : "name",
+                    "type" : "string"
+                  } ]
+                }
+            """.trimIndent() + "\n") // trimIndent removes trailing newline
+    }
 }