refactor(bulk-model-sync-gradle): cleanup

mhuster23 · mhuster23 · commit e472371cbe56 · 2023-09-13T16:35:05.000+02:00
diff --git a/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/ModelSyncGradlePlugin.kt b/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/ModelSyncGradlePlugin.kt
@@ -33,8 +33,6 @@ import org.modelix.model.sync.bulk.gradle.tasks.ImportIntoModelServer
 import org.modelix.model.sync.bulk.gradle.tasks.ImportIntoMps
 import org.modelix.model.sync.bulk.gradle.tasks.ValidateSyncSettings
 import java.io.File
-import java.net.URL
-import java.util.Enumeration
 import java.util.Properties
 
 class ModelSyncGradlePlugin : Plugin<Project> {
@@ -83,7 +81,7 @@ class ModelSyncGradlePlugin : Plugin<Project> {
         val sourceTask = when (syncDirection.source) {
             is LocalSource -> registerTasksForLocalSource(syncDirection, project, previousTask, jsonDir)
             is ServerSource -> registerTasksForServerSource(syncDirection, project, previousTask, jsonDir)
-            else -> previousTask
+            else -> error("Unknown sync direction source")
         }
 
         when (syncDirection.target) {
@@ -226,8 +224,8 @@ class ModelSyncGradlePlugin : Plugin<Project> {
     }
 
     private fun readModelixCoreVersion(): String? {
-        val resources: Enumeration<URL>? = javaClass.classLoader.getResources("modelix.core.version.properties")
-        while (resources != null && resources.hasMoreElements()) {
+        val resources = javaClass.classLoader.getResources("modelix.core.version.properties") ?: return null
+        if (resources.hasMoreElements()) {
             val properties = resources.nextElement().openStream().use { Properties().apply { load(it) } }
             return properties.getProperty("modelix.core.version")
         }
diff --git a/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/config/ModelSyncGradleSettings.kt b/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/config/ModelSyncGradleSettings.kt
@@ -77,23 +77,23 @@ data class SyncDirection(
 }
 
 interface SyncEndPoint {
-    fun getValidationErrors(): String
+    fun getValidationErrors(): List<String>
 }
 
 sealed interface LocalEndpoint : SyncEndPoint {
     var mpsHome: File?
     var mpsHeapSize: String
     var repositoryDir: File?
 
-    override fun getValidationErrors(): String {
-        return buildString {
-            if (mpsHome == null) {
-                appendUndefinedLocalFieldError("mpsHome")
-            }
-            if (repositoryDir == null) {
-                appendUndefinedLocalFieldError("repositoryDir")
-            }
+    override fun getValidationErrors(): List<String> {
+        val errors = mutableListOf<String>()
+        if (mpsHome == null) {
+            errors.addUndefinedLocalFieldError("mpsHome")
         }
+        if (repositoryDir == null) {
+            errors.addUndefinedLocalFieldError("repositoryDir")
+        }
+        return errors
     }
 }
 
@@ -114,12 +114,12 @@ sealed interface ServerEndpoint : SyncEndPoint {
     var repositoryId: String?
     var branchName: String?
 
-    override fun getValidationErrors(): String {
-        return buildString {
-            if (url == null) {
-                appendUndefinedServerFieldError("url")
-            }
+    override fun getValidationErrors(): List<String> {
+        val errors = mutableListOf<String>()
+        if (url == null) {
+            errors.addUndefinedServerFieldError("url")
         }
+        return errors
     }
 }
 
@@ -129,19 +129,28 @@ data class ServerSource(
     override var branchName: String? = null,
     var revision: String? = null,
 ) : ServerEndpoint {
-    override fun getValidationErrors(): String {
-        return buildString {
-            append(super.getValidationErrors())
-            if (revision == null) {
-                if (repositoryId == null && branchName == null) {
-                    appendLine("Invalid server source. Please either specify a revision or repositoryId and branchName.")
-                } else if (repositoryId == null) {
-                    appendUndefinedServerFieldError("repositoryId")
-                } else if (branchName == null) {
-                    appendUndefinedServerFieldError("branchName")
-                }
-            }
+    override fun getValidationErrors(): List<String> {
+        val errors = mutableListOf<String>()
+        errors.addAll(super.getValidationErrors())
+        if (revision != null) {
+            // If a revision is specified, repo and branch are not required
+            return errors
+        }
+
+        if (repositoryId == null && branchName == null) {
+            // Give hint is configuration is completely off
+            errors.add("Invalid server source. Please either specify a revision or repositoryId and branchName.")
+            return errors
         }
+
+        // Configuration is incomplete
+        if (repositoryId == null) {
+            errors.addUndefinedServerFieldError("repositoryId")
+        } else if (branchName == null) {
+            errors.addUndefinedServerFieldError("branchName")
+        }
+
+        return errors
     }
 }
 
@@ -150,29 +159,29 @@ data class ServerTarget(
     override var repositoryId: String? = null,
     override var branchName: String? = null,
 ) : ServerEndpoint {
-    override fun getValidationErrors(): String {
-        return buildString {
-            append(super.getValidationErrors())
+    override fun getValidationErrors(): List<String> {
+        val errors = mutableListOf<String>()
+        errors.addAll(super.getValidationErrors())
 
-            if (repositoryId == null) {
-                appendUndefinedServerFieldError("repositoryId")
-            }
+        if (repositoryId == null) {
+            errors.addUndefinedServerFieldError("repositoryId")
+        }
 
-            if (branchName == null) {
-                appendUndefinedServerFieldError("branchName")
-            }
+        if (branchName == null) {
+            errors.addUndefinedServerFieldError("branchName")
         }
+        return errors
     }
 }
 
-private fun StringBuilder.appendUndefinedLocalFieldError(fieldName: String) {
-    appendUndefinedFieldError(fieldName, "LocalEndpoint")
+private fun MutableList<String>.addUndefinedLocalFieldError(fieldName: String) {
+    addUndefinedFieldError(fieldName, "LocalEndpoint")
 }
 
-private fun StringBuilder.appendUndefinedServerFieldError(fieldName: String) {
-    appendUndefinedFieldError(fieldName, "ServerEndpoint")
+private fun MutableList<String>.addUndefinedServerFieldError(fieldName: String) {
+    addUndefinedFieldError(fieldName, "ServerEndpoint")
 }
 
-private fun StringBuilder.appendUndefinedFieldError(fieldName: String, block: String) {
-    appendLine("Undefined '$fieldName' in '$block'.")
+private fun MutableList<String>.addUndefinedFieldError(fieldName: String, block: String) {
+    add("Undefined '$fieldName' in '$block'.")
 }
diff --git a/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/tasks/ExportFromModelServer.kt b/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/tasks/ExportFromModelServer.kt
@@ -70,10 +70,11 @@ abstract class ExportFromModelServer @Inject constructor(of: ObjectFactory) : De
 
         branch.runRead {
             val root = branch.getRootNode()
-            println("Got root node: $root")
+            logger.info("Got root node: {}", root)
             val outputDir = outputDir.get().asFile
             root.allChildren.forEach {
-                val outputFile = outputDir.resolve("${it.getPropertyValue(IProperty.fromName(RepositoryLanguage.NamePropertyUID))}.json")
+                val nameRole = IProperty.fromName(RepositoryLanguage.NamePropertyUID)
+                val outputFile = outputDir.resolve("${it.getPropertyValue(nameRole)}.json")
                 ModelExporter(it).export(outputFile)
             }
         }
diff --git a/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/tasks/ImportIntoModelServer.kt b/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/tasks/ImportIntoModelServer.kt
@@ -72,14 +72,15 @@ abstract class ImportIntoModelServer @Inject constructor(of: ObjectFactory) : De
             client.getReplicatedModel(branchRef).start()
         }
 
-        val files = inputDir.listFiles()?.filter { it.extension == "json" } ?: error("no json files found")
+        val files = inputDir.listFiles()?.filter { it.extension == "json" }
+        if (files.isNullOrEmpty()) error("no json files found")
 
         branch.runWrite {
             val rootNode = branch.getRootNode()
-            println("Got root node: $rootNode")
-            println("Importing...")
+            logger.info("Got root node: {}", rootNode)
+            logger.info("Importing...")
             ModelImporter(branch.getRootNode()).importFilesAsRootChildren(*files.toTypedArray())
-            println("Import finished")
+            logger.info("Import finished")
         }
     }
 }
diff --git a/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/tasks/ValidateSyncSettings.kt b/bulk-model-sync-gradle/src/main/kotlin/org/modelix/model/sync/bulk/gradle/tasks/ValidateSyncSettings.kt
@@ -30,6 +30,11 @@ import org.modelix.model.sync.bulk.gradle.config.ServerTarget
 import org.modelix.model.sync.bulk.gradle.config.SyncDirection
 import javax.inject.Inject
 
+/**
+ * Instead of throwing exceptions for single configuration errors,
+ * this task collects all configuration errors and puts them into a single exception,
+ * so the user can see all steps that must be taken at a glance.
+ */
 @CacheableTask
 abstract class ValidateSyncSettings @Inject constructor(of: ObjectFactory) : DefaultTask() {
 
@@ -67,14 +72,8 @@ abstract class ValidateSyncSettings @Inject constructor(of: ObjectFactory) : Def
                 appendLine("Undefined name.")
             }
 
-            if (direction.source == null) {
-                appendLine("Undefined source.")
-            }
-            if (direction.target == null) {
-                appendLine("Undefined target.")
-            }
-
             if (direction.source == null || direction.target == null) {
+                appendLine("Both source and target have to be defined.")
                 return@buildString
             }
 
@@ -88,15 +87,15 @@ abstract class ValidateSyncSettings @Inject constructor(of: ObjectFactory) : Def
                 return@buildString
             }
 
-            val sourceErrors = direction.source?.getValidationErrors() ?: ""
+            val sourceErrors = direction.source?.getValidationErrors() ?: emptyList()
             if (sourceErrors.isNotEmpty()) {
                 appendLine()
-                appendLine(sourceErrors)
+                appendLine(sourceErrors.joinToString(separator = "\n") { it })
             }
 
-            val targetErrors = direction.target?.getValidationErrors() ?: ""
+            val targetErrors = direction.target?.getValidationErrors() ?: emptyList()
             if (targetErrors.isNotEmpty()) {
-                appendLine(targetErrors)
+                appendLine(targetErrors.joinToString(separator = "\n") { it })
             }
         }
 

Original file line number	Diff line number	Diff line change
`@@ -70,10 +70,11 @@ abstract class ExportFromModelServer @Inject constructor(of: ObjectFactory) : De`
`70`	`70`
`71`	`71`	`branch.runRead {`
`72`	`72`	`val root = branch.getRootNode()`
`73`		`- println("Got root node: $root")`
	`73`	`+ logger.info("Got root node: {}", root)`
`74`	`74`	`val outputDir = outputDir.get().asFile`
`75`	`75`	`root.allChildren.forEach {`
`76`		`- val outputFile = outputDir.resolve("${it.getPropertyValue(IProperty.fromName(RepositoryLanguage.NamePropertyUID))}.json")`
	`76`	`+ val nameRole = IProperty.fromName(RepositoryLanguage.NamePropertyUID)`
	`77`	`+ val outputFile = outputDir.resolve("${it.getPropertyValue(nameRole)}.json")`
`77`	`78`	`ModelExporter(it).export(outputFile)`
`78`	`79`	`}`
`79`	`80`	`}`
Original file line number	Diff line number	Diff line change
`@@ -72,14 +72,15 @@ abstract class ImportIntoModelServer @Inject constructor(of: ObjectFactory) : De`
`72`	`72`	`client.getReplicatedModel(branchRef).start()`
`73`	`73`	`}`
`74`	`74`
`75`		`- val files = inputDir.listFiles()?.filter { it.extension == "json" } ?: error("no json files found")`
	`75`	`+ val files = inputDir.listFiles()?.filter { it.extension == "json" }`
	`76`	`+ if (files.isNullOrEmpty()) error("no json files found")`
`76`	`77`
`77`	`78`	`branch.runWrite {`
`78`	`79`	`val rootNode = branch.getRootNode()`
`79`		`- println("Got root node: $rootNode")`
`80`		`- println("Importing...")`
	`80`	`+ logger.info("Got root node: {}", rootNode)`
	`81`	`+ logger.info("Importing...")`
`81`	`82`	`ModelImporter(branch.getRootNode()).importFilesAsRootChildren(*files.toTypedArray())`
`82`		`- println("Import finished")`
	`83`	`+ logger.info("Import finished")`
`83`	`84`	`}`
`84`	`85`	`}`
`85`	`86`	`}`