Merge pull request #91 from swisscom/bugfix/46506-directory-check-on-windows

#46506 fix handling of directory check on Windows system

Author: Lesrac

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

@@ -111,6 +111,7 @@ class DTOs {
     enum class ValidationMessageKey {
         NOT_A_DIRECTORY,
         DIRECTORY_NOT_FOUND,
+        ILLEGAL_VALUE,
         NOT_READ_WRITABLE,
         LOCAL_DIR_OVERLAPS_WITH_SOURCE_DIRS,
         LOCAL_DIR_OVERLAPS_WITH_TARGET_DIRS,

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

@@ -45,7 +45,6 @@ import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.RestController
 import java.net.URI
-import java.nio.file.Path
 import java.time.Instant
 
 private val logger = KotlinLogging.logger {}
@@ -140,14 +139,16 @@ internal class WebOperations(
      * query parameter and their results are combined.
      *
      * @param config the CDR Client configuration to use for validating single use of directories
-     * @param directory the directory to validate
+     * @param directory the directory to validate (as a String to handle invalid paths gracefully)
      * @param validations the list of validation types to perform on the directory
      * @return a [DTOs.ValidationResult] indicating the result of the validation
      */
     @PutMapping("api/validate-directory")
     internal suspend fun validateDirectory(
         @RequestBody config: DTOs.CdrClientConfig,
-        @RequestParam(name = "dir") directory: Path,
+        // don't use java.nio.file.Path here to allow the endpoint to handle invalid paths (e.g., with trailing spaces on Windows) gracefully
+        // and return validation errors instead of throwing exceptions
+        @RequestParam(name = "dir") directory: String,
         @RequestParam(name = "validation") validations: List<DomainObjects.ValidationType>,
     ): ResponseEntity<ValidationResult> = runCatching {
         logger.debug { "validating dir: '$directory', validations: '$validations'" }

cdr-client-service/src/main/kotlin/com/swisscom/health/des/cdr/client/http/WebOperations.kt

@@ -1,3 +1,5 @@
+@file:Suppress("LargeClass")
+
 package com.swisscom.health.des.cdr.client.handler
 
 import com.swisscom.health.des.cdr.client.common.DTOs
@@ -574,6 +576,35 @@ internal class ConfigValidationServiceTest {
         }
     }
 
+    @OptIn(ExperimentalPathApi::class)
+    @Test
+    fun `validateDirectoriesAreAbsolute should fail when sourceArchiveEnabled is true but archive folder is null`() {
+        val connector = Connector(
+            connectorId = ConnectorId("test-connector-archive-null"),
+            sourceFolder = sourceFolder0,
+            targetFolder = targetFolder0,
+            contentType = "application/xml",
+            mode = CdrClientConfig.Mode.TEST,
+            docTypeFolders = emptyMap(),
+            sourceErrorFolder = null,
+            sourceArchiveFolder = null,
+            sourceArchiveEnabled = true
+        )
+
+        val config = createCdrClientConfig(listOf(connector))
+        val service = ConfigValidationService(config)
+        val result = service.validateDirectoriesAreAbsolute(config.toDto())
+
+        assertInstanceOf<DTOs.ValidationResult.Failure>(result)
+        assertEquals(1, result.validationDetails.size)
+
+        val validationDetail = result.validationDetails.first()
+        assertInstanceOf<DTOs.ValidationDetail.ConfigItemDetail>(validationDetail)
+        val configItemDetail = validationDetail
+        assertEquals(DomainObjects.ConfigurationItem.ARCHIVE_DIRECTORY, configItemDetail.configItem)
+        assertEquals(DTOs.ValidationMessageKey.NOT_A_DIRECTORY, configItemDetail.messageKey)
+    }
+
     private fun createCdrClientConfig(customers: List<Connector>, defaultLocalFolder: Path = localFolder0): CdrClientConfig =
         CdrClientConfig(
             fileSynchronizationEnabled = FileSynchronization.ENABLED,
@@ -730,4 +761,97 @@ internal class ConfigValidationServiceTest {
             Files.deleteIfExists(tempDir)
         }
     }
+
+    @Test
+    fun `test validation handles path with trailing space gracefully`() {
+        val tempSourceDir = Files.createTempDirectory("testSourceDir")
+        val tempTargetDir = Files.createTempDirectory("testTargetDir")
+        try {
+            // Create connector with valid paths
+            val connector = Connector(
+                connectorId = ConnectorId("test-connector"),
+                sourceFolder = tempSourceDir,
+                targetFolder = tempTargetDir,
+                contentType = "application/xml",
+                mode = CdrClientConfig.Mode.TEST,
+                docTypeFolders = emptyMap(),
+                sourceErrorFolder = null,
+                sourceArchiveFolder = null,
+                sourceArchiveEnabled = false
+            )
+
+            val config = createCdrClientConfig(listOf(connector))
+            val service = ConfigValidationService(config)
+
+            // Now create a DTO with paths that have trailing spaces
+            val configDto = config.toDto()
+            val connectorWithTrailingSpace = configDto.customer.first().copy(
+                sourceFolder = "$tempSourceDir ",
+                targetFolder = "$tempTargetDir "
+            )
+            val configDtoWithTrailingSpace = configDto.copy(
+                customer = listOf(connectorWithTrailingSpace)
+            )
+
+            val result = service.validateAllConfigurationItems(configDtoWithTrailingSpace)
+
+            // The paths with trailing spaces should FAIL validation
+            // On Windows: InvalidPathException -> null path -> directory not found
+            // On Unix: Path created with trailing space that doesn't exist -> directory not found
+            assertInstanceOf<DTOs.ValidationResult.Failure>(result)
+            val failure = result
+            assertTrue(failure.validationDetails.isNotEmpty())
+            // Should have at least one path detail with directory not found
+            assertTrue(failure.validationDetails.any {
+                it is DTOs.ValidationDetail.PathDetail &&
+                it.messageKey == DTOs.ValidationMessageKey.DIRECTORY_NOT_FOUND
+            })
+        } finally {
+            Files.deleteIfExists(tempSourceDir)
+            Files.deleteIfExists(tempTargetDir)
+        }
+    }
+
+    @Test
+    fun `test validation handles UNC paths without NullPointerException`() {
+        val tempSourceDir = Files.createTempDirectory("testSourceDir")
+        val tempTargetDir = Files.createTempDirectory("testTargetDir")
+        try {
+            // Create connector with a UNC path (this simulates Windows network share paths)
+            val connector = Connector(
+                connectorId = ConnectorId("test-connector"),
+                sourceFolder = tempSourceDir,
+                targetFolder = tempTargetDir,
+                contentType = "application/xml",
+                mode = CdrClientConfig.Mode.TEST,
+                docTypeFolders = emptyMap(),
+                sourceErrorFolder = null,
+                sourceArchiveFolder = null,
+                sourceArchiveEnabled = false
+            )
+
+            val config = createCdrClientConfig(listOf(connector))
+            val service = ConfigValidationService(config)
+
+            // Create a DTO with a UNC path (Windows network share)
+            val configDto = config.toDto()
+            val connectorWithUncPath = configDto.customer.first().copy(
+                sourceFolder = """\\cdrintstpublic.file.core.windows.net\test-file-share"""
+            )
+            val configDtoWithUncPath = configDto.copy(
+                customer = listOf(connectorWithUncPath)
+            )
+
+            // This should not throw NullPointerException when checking fileName
+            // It should return a validation error (directory not found)
+            val result = service.validateAllConfigurationItems(configDtoWithUncPath)
+
+            // The UNC path likely won't exist, so we expect a failure
+            // The important thing is that it doesn't crash with NullPointerException
+            assertInstanceOf<DTOs.ValidationResult.Failure>(result)
+        } finally {
+            Files.deleteIfExists(tempSourceDir)
+            Files.deleteIfExists(tempTargetDir)
+        }
+    }
 }

cdr-client-service/src/test/kotlin/com/swisscom/health/des/cdr/client/handler/ConfigValidationServiceTest.kt

@@ -18,6 +18,7 @@ kotlin {
 
     sourceSets {
         val desktopMain by getting
+        val desktopTest by getting
 
         commonMain.dependencies {
             implementation(compose.runtime)
@@ -51,6 +52,12 @@ kotlin {
             implementation(libs.conveyor.control)
             implementation(libs.compose.native.tray)
         }
+
+        desktopTest.dependencies {
+            implementation(kotlin("test"))
+            implementation(libs.mockk)
+            implementation(libs.kotlinx.coroutines.test)
+        }
     }
 }
 

cdr-client-ui/build.gradle.kts

@@ -102,4 +102,5 @@
     <string name="error_folder_needs_absolute_path_error">Directories need to be defined with an absolute path (e.g. C:\home\myuser\archive)</string>
     <string name="error_proxy_url_must_start_with_http_or_https">Proxy URL must start with http:// or https://</string>
     <string name="error_proxy_url_invalid_format">Invalid proxy URL format</string>
+    <string name="error_illegal_value">Value is not valid/can't be used here</string>
 </resources>

cdr-client-ui/src/commonMain/composeResources/values/strings.xml

@@ -12,6 +12,17 @@ import java.nio.file.Path
 
 private val logger = KotlinLogging.logger {}
 
+/**
+ * Safely compares two path strings, handling invalid paths (e.g., with trailing spaces on Windows).
+ * Returns true if the paths are equal, false otherwise or if either path is invalid.
+ */
+private fun pathsEqual(path1: String, path2: String): Boolean = runCatching {
+    Path.of(path1) == Path.of(path2)
+}.getOrElse {
+    // If either path is invalid (e.g., trailing spaces on Windows), fall back to string comparison
+    path1 == path2
+}
+
 private typealias ValidationErrorHandler = suspend (Map<String, Any>, DomainObjects.ConfigurationItem) -> DTOs.ValidationResult
 private typealias ValidationSuccessHandler<T> = suspend (T, DomainObjects.ConfigurationItem) -> DTOs.ValidationResult
 
@@ -99,7 +110,7 @@ internal class CdrConfigViewRemoteValidations(
                     // check if any path validation error is matching the path we are validating; if yes, return the failure
                     if (validationDetails
                             .any { validationDetail: DTOs.ValidationDetail ->
-                                validationDetail is DTOs.ValidationDetail.PathDetail && path != null && Path.of(validationDetail.path) == Path.of(path)
+                                validationDetail is DTOs.ValidationDetail.PathDetail && path != null && pathsEqual(validationDetail.path, path)
                             }
                     )
                         this

cdr-client-ui/src/commonMain/kotlin/com/swisscom/health/des/cdr/client/ui/CdrConfigViewRemoteValidations.kt

@@ -67,6 +67,7 @@ import com.swisscom.health.des.cdr.client.ui.cdr_client_ui.generated.resources.e
 import com.swisscom.health.des.cdr.client.ui.cdr_client_ui.generated.resources.error_proxy_url_must_start_with_http_or_https
 import com.swisscom.health.des.cdr.client.ui.cdr_client_ui.generated.resources.error_test_timeout_too_long
 import com.swisscom.health.des.cdr.client.ui.cdr_client_ui.generated.resources.error_value_is_mandatory
+import com.swisscom.health.des.cdr.client.ui.cdr_client_ui.generated.resources.error_illegal_value
 import io.github.oshai.kotlinlogging.KotlinLogging
 import org.jetbrains.compose.resources.StringResource
 import org.jetbrains.compose.resources.painterResource
@@ -115,6 +116,7 @@ internal val DTOs.ValidationMessageKey.stringResource: StringResource
             DTOs.ValidationMessageKey.DIRECTORY_NEEDS_ABSOLUTE_PATH -> Res.string.error_folder_needs_absolute_path_error
             DTOs.ValidationMessageKey.PROXY_URL_MUST_START_WITH_HTTP_OR_HTTPS -> Res.string.error_proxy_url_must_start_with_http_or_https
             DTOs.ValidationMessageKey.PROXY_URL_INVALID_FORMAT -> Res.string.error_proxy_url_invalid_format
+            DTOs.ValidationMessageKey.ILLEGAL_VALUE -> Res.string.error_illegal_value
         }
 
 @Composable