Merge pull request #87 from swisscom/feature/45607-change-error-dir-behaviour

Feature/45607 change error dir behaviour
* Don't create subdirectories in the error folder.
* Ensure that error-file names are not getting too long.
* Only move BAD_REQUEST responses to the error folder, other 4xx responses only get renamed and are retried directly or after the client is restarted.
* Only allow absolute paths to be used for source/target/archive/error folders.

Author: Lesrac

cdr-client-common/src/main/kotlin/com/swisscom/health/des/cdr/client/common/Constants.kt

@@ -9,6 +9,8 @@ object Constants {
     val SHUTDOWN_DELAY: Duration = Duration.ofMillis(SHUTDOWN_DELAY_MILLIS)
 
     const val EMPTY_STRING: String = ""
+    const val ERROR_DIR_NAME: String = "error"
+    const val RESTART_FILE_EXTENSION: String = "clientRestartRequired"
 
     const val CONFIG_CHANGE_EXIT_CODE = 29
     const val UNKNOWN_EXIT_CODE = 31

cdr-client-common/src/main/kotlin/com/swisscom/health/des/cdr/client/common/DTOs.kt

@@ -122,7 +122,10 @@ class DTOs {
         FILE_BUSY_TEST_TIMEOUT_TOO_LONG,
         NO_CONNECTOR_CONFIGURED,
         VALUE_IS_PLACEHOLDER,
-        CREDENTIAL_VALIDATION_FAILED
+        CREDENTIAL_VALIDATION_FAILED,
+        ERROR_AS_NON_ERROR_FOLDER_NAME_USED,
+        ERROR_DIR_OVERLAPS_NON_ERROR_DIR,
+        DIRECTORY_NEEDS_ABSOLUTE_PATH
     }
 
 

cdr-client-service/build.gradle.kts

@@ -252,6 +252,10 @@ dockerCompose {
     dockerComposeWorkingDirectory.set(rootProject.file("docker-compose"))
     dockerComposeStopTimeout.set(Duration.ofSeconds(5))  // time before docker-compose sends SIGTERM to the running containers after the composeDown task has been started
     ignorePullFailure.set(true)
+    val rancherDocker = file("${System.getProperty("user.home")}/.rd/bin/docker")
+    if (rancherDocker.exists()) {
+        dockerExecutable.set(rancherDocker.absolutePath)
+    }
     isRequiredBy(tasks.getByName("integrationTest"))
 }
 /***************************

cdr-client-service/src/main/kotlin/com/swisscom/health/des/cdr/client/config/CdrClientConfig.kt

@@ -3,6 +3,7 @@ package com.swisscom.health.des.cdr.client.config
 import com.fasterxml.jackson.annotation.JsonIgnore
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties
 import com.swisscom.health.des.cdr.client.common.Constants.EMPTY_STRING
+import com.swisscom.health.des.cdr.client.common.Constants.ERROR_DIR_NAME
 import com.swisscom.health.des.cdr.client.config.CdrClientConfig.Mode
 import com.swisscom.health.des.cdr.client.xml.DocumentType
 import io.github.oshai.kotlinlogging.KotlinLogging
@@ -17,7 +18,6 @@ import java.time.LocalDate
 import java.time.format.DateTimeFormatter
 import java.time.temporal.ChronoUnit
 import kotlin.io.path.createDirectories
-import kotlin.io.path.isDirectory
 
 
 private val logger = KotlinLogging.logger {}
@@ -152,13 +152,8 @@ internal data class Connector(
     val sourceArchiveEnabled: Boolean = false,
 
     /**
-     * Directory to archive uploaded files to. If you specify a relative path, it will be resolved relative to the source directory.
-     * If you specify an absolute path, the path will be used as is for all archive directories (see [docTypeFolders]).
-     *
-     * Beware: On Linux empty string, `.`, and `./` all resolve to the current working directory, while `./archive` (and just `archive`) resolve
-     * to `<source_dir>/archive`.
-     *
-     * Default is the system temp directory.
+     * Directory to archive uploaded files to.
+     * Needs to be an absolute path, the path will be used as is for all archive directories (see [docTypeFolders]).
      *
      * @see sourceArchiveEnabled
      * @see getEffectiveSourceArchiveFolder
@@ -167,13 +162,8 @@ internal data class Connector(
     val sourceArchiveFolder: Path? = null,
 
     /**
-     * Directory to move documents to for which the upload has failed. If you specify a relative path, it will be resolved relative
-     * to the source directory. If you specify an absolute path, the path will be used as is for all error directories, such as for all [docTypeFolders].
-     *
-     * Beware: On Linux empty string, `.`, and `./` all resolve to the current working directory, while `./archive` (and just `archive`) resolve
-     * to `<source_dir>/archive`.
-     *
-     * Default is the system temp directory.
+     * Directory to move documents to for which the upload has failed.
+     * Needs to be an absolute path, the path will be used as is for all error directories, such as for all [docTypeFolders].
      *
      * @see getEffectiveSourceErrorFolder
      * @see sourceFolder
@@ -199,46 +189,28 @@ internal data class Connector(
 
     companion object {
         const val PROPERTY_NAME = ""
-
-        @JvmStatic
-        val TEMP_DIR_PATH: Path = Path.of(System.getProperty("java.io.tmpdir"))
     }
 
     /**
-     * If [sourceArchiveEnabled] is set to `true` returns the archive directory resolved against the source directory with a subdirectory
-     * for the current date. The directories will be created if they do not exist. If [sourceArchiveEnabled] is `false` returns an
-     * empty path.
+     * If [sourceArchiveEnabled] is set to `true` returns the archive directory with a subdirectory for the current date.
+     * The directory will be created if it does not exist.
+     * If [sourceArchiveEnabled] is `false` returns an empty path.
      *
      * @see sourceArchiveEnabled
      * @see sourceArchiveFolder
-     * @see sourceFolder
      */
     @JsonIgnore
-    fun getEffectiveSourceArchiveFolder(path: Path): Path? =
+    fun getEffectiveSourceArchiveFolder(): Path? =
         if (sourceArchiveEnabled) {
-            if (path.isDirectory()) {
-                path
-            } else {
-                path.parent
-            }.resolve((sourceArchiveFolder ?: TEMP_DIR_PATH).resolve(getDateNow()))
-                .also { createDirectoryIfMissing(it) }
+            createDirectoryIfMissing(when (sourceArchiveFolder) {
+                null -> sourceFolder.resolve("archive")
+                sourceFolder -> sourceFolder.resolve("archive")
+                else -> sourceArchiveFolder
+            }.resolve(getDateNow()))
         } else {
             null
         }
 
-    /**
-     * Convenience property to get the connector archive directory that is used in all cases where no message type related directories are defined.
-     * @see getEffectiveSourceArchiveFolder
-     */
-    val effectiveConnectorSourceArchiveFolder: Path?
-        @JsonIgnore
-        get() =
-            if (sourceArchiveEnabled)
-                sourceFolder.resolve((sourceArchiveFolder ?: TEMP_DIR_PATH).resolve(getDateNow()))
-                    .also { createDirectoryIfMissing(it) }
-            else
-                null
-
     /**
      * Returns all source directories for all document types of this connector. If a [DocTypeFolders.sourceFolder] is not set, the entry is omitted.
      */
@@ -269,35 +241,20 @@ internal data class Connector(
 
     private fun getDateNow(): String = LocalDate.now().format(DateTimeFormatter.BASIC_ISO_DATE)
 
+
     /**
-     * Returns the error directory resolved against the source directory with a subdirectory for the current date. The directories will be
-     * created if they do not exist.
+     * Returns the effective source error folder path. The directory will be created if it does not exist.
      *
      * @see sourceErrorFolder
      * @see sourceFolder
      */
     @JsonIgnore
-    fun getEffectiveSourceErrorFolder(path: Path): Path =
-        (sourceErrorFolder ?: Path.of(EMPTY_STRING)).let { errorDir ->
-            if (path.isDirectory()) {
-                path
-            } else {
-                path.parent
-            }.resolve(errorDir.resolve(getDateNow()))
-                .also { createDirectoryIfMissing(it) }
-        }
-
-
-    /**
-     * Convenience property to get the connector error directory that is used in all cases where no message type related directories are defined.
-     * @see getEffectiveSourceErrorFolder
-     */
-    val effectiveConnectorSourceErrorFolder: Path
-        @JsonIgnore
-        get() = (sourceErrorFolder ?: Path.of(EMPTY_STRING)).let { errorDir ->
-            sourceFolder.resolve(errorDir.resolve(getDateNow()))
-                .also { createDirectoryIfMissing(it) }
-        }
+    fun getEffectiveSourceErrorFolder(): Path =
+        when (sourceErrorFolder) {
+            null -> sourceFolder.resolve(ERROR_DIR_NAME)
+            sourceFolder -> sourceFolder.resolve(ERROR_DIR_NAME)
+            else -> sourceErrorFolder
+        }.let { createDirectoryIfMissing(it) }
 
     override fun toString(): String {
         return "Connector(connectorId='$connectorId', targetFolder=$targetFolder, sourceFolder=$sourceFolder, " +
@@ -309,23 +266,8 @@ internal data class Connector(
                     EMPTY_STRING
                 } +
                 "contentType=$contentType, uploadArchiveEnabled=$sourceArchiveEnabled, sourceArchiveFolder=$sourceArchiveFolder, " +
-                "effectiveSourceArchiveFolder=${effectiveConnectorSourceArchiveFolder}, " +
-                if (sourceArchiveEnabled && effectiveDocTypeFolders.isNotEmpty())
-                    "additionalEffectiveSourceArchiveFolders=[${
-                        effectiveDocTypeFolders.entries.joinToString("; ") { "${it.key}=${getEffectiveSourceArchiveFolder(it.value.sourceFolder!!)}" }
-                    }], "
-                else {
-                    EMPTY_STRING
-                } +
-                "sourceErrorFolder=$sourceErrorFolder, effectiveSourceErrorFolder=${effectiveConnectorSourceErrorFolder} " +
-                if (effectiveDocTypeFolders.isNotEmpty())
-                    "additionalEffectiveSourceErrorFolders=[${
-                        effectiveDocTypeFolders.entries.filter { it.value.sourceFolder != null }
-                            .joinToString(", ") { "${it.key}=${getEffectiveSourceErrorFolder(it.value.sourceFolder!!)}" }
-                    }], "
-                else {
-                    EMPTY_STRING
-                } +
+                "effectiveSourceArchiveFolder=${getEffectiveSourceArchiveFolder()}, " +
+                "sourceErrorFolder=$sourceErrorFolder, effectiveSourceErrorFolder=${getEffectiveSourceErrorFolder()} " +
                 "mode=$mode)"
     }
 

cdr-client-service/src/main/kotlin/com/swisscom/health/des/cdr/client/config/ConfigConverter.kt

@@ -41,20 +41,20 @@ internal fun Connector.toDto(): DTOs.CdrClientConfig.Connector {
             Map<DTOs.CdrClientConfig.DocumentType, DTOs.CdrClientConfig.Connector.DocTypeFolders> {
         return map { (key, value) ->
             DTOs.CdrClientConfig.DocumentType.entries.first { it.name == key.name } to DTOs.CdrClientConfig.Connector.DocTypeFolders(
-                sourceFolder = value.sourceFolder?.absolutePathString(),
-                targetFolder = value.targetFolder?.absolutePathString(),
+                sourceFolder = value.sourceFolder?.toString(),
+                targetFolder = value.targetFolder?.toString(),
             )
         }.toMap()
     }
 
     return DTOs.CdrClientConfig.Connector(
         connectorId = connectorId.id,
-        targetFolder = targetFolder.absolutePathString(),
-        sourceFolder = sourceFolder.absolutePathString(),
+        targetFolder = targetFolder.toString(),
+        sourceFolder = sourceFolder.toString(),
         contentType = contentType,
         sourceArchiveEnabled = sourceArchiveEnabled,
-        sourceArchiveFolder = sourceArchiveFolder?.absolutePathString(),
-        sourceErrorFolder = sourceErrorFolder?.absolutePathString(),
+        sourceArchiveFolder = sourceArchiveFolder?.toString(),
+        sourceErrorFolder = sourceErrorFolder?.toString(),
         mode = DTOs.CdrClientConfig.Mode.entries.first { it.name == mode.name },
         docTypeFolders = effectiveDocTypeFolders.toDto(),
     )

cdr-client-service/src/main/kotlin/com/swisscom/health/des/cdr/client/handler/CdrApiClient.kt

@@ -172,24 +172,40 @@ internal class CdrApiClient(
                             logger.debug { "Upload '$file' done" }
                         }
 
-                        response.code in 400..499 -> UploadDocumentResult.UploadClientErrorResponse(
+                        response.code == HttpStatus.BAD_REQUEST.value() -> UploadDocumentResult.UploadClientErrorResponse(
                             code = response.code,
                             responseBody = response.body.string()
                         ).also { result ->
-                            logger.info { "Upload '$file' encountered client error: '$result'" }
+                            logger.debug { "Upload '$file' encountered client error: '$result'" }
+                        }
+
+                        response.code == HttpStatus.NOT_FOUND.value() || response.code == HttpStatus.TOO_MANY_REQUESTS.value() ->
+                            UploadDocumentResult.UploadClientRetryableErrorResponse(
+                                code = response.code,
+                                responseBody = response.body.string()
+                            ).also { result ->
+                                logger.debug { "Upload '$file' encountered client config error: '$result'" }
+                            }
+
+                        response.code in 400..499 -> UploadDocumentResult.UploadClientConfigNonRetryableErrorResponse(
+                            code = response.code,
+                            responseBody = response.body.string()
+                        ).also { result ->
+                            logger.debug { "Upload '$file' encountered client config non-retryable error: '$result'" }
                         }
 
                         else -> UploadDocumentResult.UploadServerErrorResponse(
                             code = response.code,
                             responseBody = response.body.string()
                         ).also { result ->
-                            logger.info { "Upload '$file' encountered server error: '$result'" }
+                            logger.debug { "Upload '$file' encountered server error: '$result'" }
                         }
                     }
                 }
         }.fold(
             onSuccess = { it },
             onFailure = { t ->
+                // 5xx responses from the server will also go through here, because of our okHttpClient Bean
                 logger.error { "Upload file '$file' failed: ${t.message}" }
                 UploadDocumentResult.UploadError(message = t.message ?: "Unknown error", t = t)
             }
@@ -409,6 +425,8 @@ internal class CdrApiClient(
     sealed interface UploadDocumentResult {
         object Success : UploadDocumentResult
         data class UploadClientErrorResponse(val code: Int, val responseBody: String) : UploadDocumentResult
+        data class UploadClientRetryableErrorResponse(val code: Int, val responseBody: String) : UploadDocumentResult
+        data class UploadClientConfigNonRetryableErrorResponse(val code: Int, val responseBody: String) : UploadDocumentResult
         data class UploadServerErrorResponse(val code: Int, val responseBody: String) : UploadDocumentResult
         data class UploadError(val message: String, val t: Throwable? = null) : UploadDocumentResult
     }