From 807317a60562cea3b63108f8ee10ba1dee300738 Mon Sep 17 00:00:00 2001 From: Bastian Doetsch Date: Wed, 21 May 2025 10:01:19 +0200 Subject: [PATCH 1/2] fix: not initialized exception in error handling [IDE-1228] (#704) * fix(lsp): improve process handling in LanguageServerWrapper [IDE-1228] - Renamed lsIsAlive() to processIsAlive() for better clarity - Added proper process alive checks before destroying - Added tests to verify initialization and shutdown behavior - Ensures the process is only terminated when it exists, preventing exceptions * chore: add windsurfrules file [IDE-1228] Added windsurfrules file to define standards for AI assistant interactions with the codebase. This includes guidelines for testing, security scanning, issue fixing, and commit message formatting. * chore: remove unneeded imports * docs: updated CHANGELOG.md * test(lsp): improve LanguageServerWrapper shutdown test [IDE-1228] - Improved the shutdown test to properly verify workspace folder clearing and process termination - Removed ineffective test that didn't test actual implementation behavior - Added test to verify process lifecycle management during shutdown (cherry picked from commit ad2b147757d51bec8473b923b6f4de5428a03706) --- .windsurfrules | 27 ++++++++ CHANGELOG.md | 4 ++ .../snyk/common/lsp/LanguageServerWrapper.kt | 10 +-- .../common/lsp/LanguageServerWrapperTest.kt | 65 +++++++++++++++++-- 4 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 .windsurfrules diff --git a/.windsurfrules b/.windsurfrules new file mode 100644 index 000000000..03179808a --- /dev/null +++ b/.windsurfrules @@ -0,0 +1,27 @@ +** general ** +always be concise, direct and don't try to appease me. + +** how to implement ** +always write and update test cases. iterate until they pass. +use existing mocks, don't write new ones. +if you use mocks, use mockk to generate them. +always run the tests after editing. + +** security ** +determine the absolute path of the project directory. you can do that e.g. by executing pwd on the shell within the directory. +always use snyk for sca and code scanning. scan with both tools. you need to pass the absolute path of the directory we are scanning. +run snyk code tests after each edit. pass the absolute path of the project directory as a parameter +run snyk sca tests after updating the package manager / manifest file / build file. pass the absolute path of the project directory as a parameter. +run snyk sca and code test before committing. fix issues before committing. + +** fixing issues ** +fix issues if they are fixable. take the snyk scan results and the test results as input. + +** committing ** +when asked to commit, always use conventional commit messages (Conventional Commit Style (Subject + Body)). be descriptive in the body. if you find a JIRA issue (IDE-XXXX) in the branch name, use it as a postfix to the subject line in the format [IDE-XXXX] +when asked to push, always use 'git push --set-upstream origin $(git_current_branch)' with git_current_branch being the current branch we are on +never force push +never push without asking +never commit the hashicorp gomod +regularly fetch main branch and offer to merge it into git_current_branch +don't touch the copyright header diff --git a/CHANGELOG.md b/CHANGELOG.md index 80d39d3f6..65e8c82e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,8 @@ # Snyk Security Changelog +## [2.13.1] +### Fixed +- fixed not initialized exception in error handling during language server startup + ## [2.13.0] ### Changed diff --git a/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt b/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt index b84fbfd06..06b7d5f15 100644 --- a/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt +++ b/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt @@ -203,7 +203,7 @@ class LanguageServerWrapper( } } catch (e: Exception) { logger.warn(e) - process.destroy() + if (processIsAlive()) process.destroyForcibly() isInitialized = false } } @@ -217,7 +217,7 @@ class LanguageServerWrapper( lsp4jLogger.level = Level.OFF messageProducerLogger.level = Level.OFF try { - val shouldShutdown = lsIsAlive() + val shouldShutdown = processIsAlive() executorService.submit { if (shouldShutdown) { val project = ProjectUtil.getActiveProject() @@ -233,11 +233,11 @@ class LanguageServerWrapper( // we don't care } finally { try { - if (lsIsAlive()) languageServer.exit() + if (processIsAlive()) languageServer.exit() } catch (ignore: Exception) { // do nothing } finally { - if (lsIsAlive()) process.destroyForcibly() + if (processIsAlive()) process.destroyForcibly() } lsp4jLogger.level = previousLSP4jLogLevel messageProducerLogger.level = previousMessageProducerLevel @@ -245,7 +245,7 @@ class LanguageServerWrapper( } } - private fun lsIsAlive() = ::process.isInitialized && process.isAlive + private fun processIsAlive() = ::process.isInitialized && process.isAlive fun getWorkspaceFoldersFromRoots(project: Project): Set { if (disposed || project.isDisposed) return emptySet() diff --git a/src/test/kotlin/snyk/common/lsp/LanguageServerWrapperTest.kt b/src/test/kotlin/snyk/common/lsp/LanguageServerWrapperTest.kt index a71abd46d..3f227a393 100644 --- a/src/test/kotlin/snyk/common/lsp/LanguageServerWrapperTest.kt +++ b/src/test/kotlin/snyk/common/lsp/LanguageServerWrapperTest.kt @@ -39,7 +39,7 @@ import java.util.concurrent.CompletableFuture class LanguageServerWrapperTest { private val folderConfigSettingsMock: FolderConfigSettings = mockk(relaxed = true) - private val applicationMock: Application = mockk(relaxed = true) + private val applicationMock: Application = mockk() private val projectMock: Project = mockk() private val lsMock: LanguageServer = mockk() private val settings = SnykApplicationSettingsStateService() @@ -142,9 +142,9 @@ class LanguageServerWrapperTest { scanType = "testScan", uniqueIssueCount = ScanDoneEvent.UniqueIssueCount(0, 0, 0, 0), ), + ), ), - ), - ) + ) verify(exactly = 0) { lsMock.workspaceService.executeCommand(any()) } } @@ -322,12 +322,55 @@ class LanguageServerWrapperTest { } } - private fun simulateRunningLS() { - cut.languageClient = mockk(relaxed = true) + @Test + fun `ensureLanguageServerInitialized should not proceed when disposed`() { + every { applicationMock.isDisposed } returns true + + val wrapper = LanguageServerWrapper("dummy") + assertFalse(wrapper.ensureLanguageServerInitialized()) + } + + + @Test + fun `shutdown should handle process termination`() { + // Setup val processMock = mockk(relaxed = true) - cut.process = processMock - every { processMock.info().startInstant().isPresent } returns true every { processMock.isAlive } returns true + + // Create wrapper instance + val wrapper = LanguageServerWrapper("dummy") + wrapper.process = processMock + wrapper.languageServer = lsMock + wrapper.isInitialized = true + + // Add some test workspace folders + val workspaceFolder = WorkspaceFolder("test://uri", "Test Folder") + wrapper.configuredWorkspaceFolders.add(workspaceFolder) + + // Mock language server shutdown + val completableFuture = CompletableFuture.completedFuture(Any()) + every { lsMock.shutdown() } returns completableFuture + justRun { lsMock.exit() } + + // Mock loggers to avoid NPE + mockkStatic("java.util.logging.Logger") + val loggerMock = mockk(relaxed = true) + every { java.util.logging.Logger.getLogger(any()) } returns loggerMock + + // Mock executor service + val executorMock = mockk(relaxed = true) + val executorField = LanguageServerWrapper::class.java.getDeclaredField("executorService") + executorField.isAccessible = true + executorField.set(wrapper, executorMock) + + // Act + wrapper.shutdown() + + // Assert + // Check the workspace folders were cleared + assertTrue(wrapper.configuredWorkspaceFolders.isEmpty()) + // Check the process was destroyed if alive + verify { processMock.destroyForcibly() } } @Test @@ -350,4 +393,12 @@ class LanguageServerWrapperTest { assertEquals(settings.organization, actual.organization) assertEquals(settings.isDeltaFindingsEnabled().toString(), actual.enableDeltaFindings) } + + private fun simulateRunningLS() { + cut.languageClient = mockk(relaxed = true) + val processMock = mockk(relaxed = true) + cut.process = processMock + every { processMock.info().startInstant().isPresent } returns true + every { processMock.isAlive } returns true + } } From 4192e5c0d8850b684f4ade9e5cf9e1f87fb34a9c Mon Sep 17 00:00:00 2001 From: Bastian Doetsch Date: Fri, 23 May 2025 15:40:57 +0200 Subject: [PATCH 2/2] fix: URI handling for URIs with special/blank characters [IDE-1203][IDE-1235] (#706) * fix: improve path and URI conversion for cross-platform compatibility This commit enhances the path and URI conversion functionality to ensure reliable file path handling across different operating systems. Specifically improves Windows path handling with proper URI encoding/decoding and adds comprehensive tests to verify the bidirectional conversion between paths and URIs works correctly on both POSIX and Windows systems. * fix: more test cases and use ASCII strings * fix: toLanguageServerURL * fix: toLanguageServerURL tests * fix: improve language server initialization and URI handling [IDE-1203] - Increase language server initialization timeout from 20L to 2000L - Add proper error handling when adding content roots - Run content root addition asynchronously to prevent UI freezes - Improve error logging with more descriptive messages - Add user notification when language server fails to initialize - Replace size > 0 check with isNotEmpty() for better readability - Fix code formatting and parameter organization * fix: tests in ReferenceChooserDialogTest.kt * fix: error handling in fromUriToPath Co-authored-by: windsurf-bot[bot] <189301087+windsurf-bot[bot]@users.noreply.github.com> * fix: windsurf suggestion * fix: test setup for ReferenceChooserDialogTest.kt * chore: revert timeout * docs: CHANGELOG.md * fix: normalize path before using it to persist folderConfig * fix: Improve folder config path normalization and add tests [IDE-1203] Ensures that FolderConfigSettings consistently handles folder paths by normalizing and absolutizing them. The 'folderPath' property of FolderConfig objects managed by FolderConfigSettings will now always reflect this normalized, absolute path. This resolves an issue where the stored folderPath in FolderConfig objects did not always represent the fully normalized and absolute path used as the key in the settings map, leading to potential inconsistencies and failing tests for path normalization. Key changes include: - Added FolderConfigSettingsTest.kt with comprehensive unit tests for path normalization, covering various scenarios including paths with '.', '..', and equivalent path representations. - Converted tests to JUnit 4 syntax as per project standards. - Updated FolderConfigSettings: - 'addFolderConfig' now stores a copy of the FolderConfig with its 'folderPath' correctly normalized and absolutized. - 'createEmpty' now directly instantiates FolderConfig with the normalized path, improving clarity and efficiency. - Fixed a compile error in ReferenceChooserDialogTest.kt by refactoring a direct private field access to use a public method. * chore: update initialization messaging in summary [IDE-1235] * refactor: ReferenceChooserDialogTest.kt setup * refactor: add tests, normalize more paths * refactor: ensure file separator suffix in folder configs * fix: tests --------- Co-authored-by: Abdelrahman Shawki Hassan (cherry picked from commit 9ba9809cf8977e705ce980a5148d36f9030c5c4f) --- .windsurfrules | 1 + CHANGELOG.md | 3 +- src/main/kotlin/io/snyk/plugin/Utils.kt | 81 +---- .../plugin/services/SnykTaskQueueService.kt | 9 +- .../io/snyk/plugin/ui/SnykSettingsDialog.kt | 2 +- .../snyk/common/lsp/LanguageServerWrapper.kt | 45 ++- .../snyk/common/lsp/SnykLanguageClient.kt | 2 +- .../lsp/settings/FolderConfigSettings.kt | 37 ++- src/main/kotlin/snyk/trust/TrustedProjects.kt | 1 - src/main/resources/html/ScanSummaryInit.html | 5 +- src/test/kotlin/io/snyk/plugin/UtilsKtTest.kt | 108 +++--- .../plugin/ui/ReferenceChooserDialogTest.kt | 8 +- .../lsp/settings/FolderConfigSettingsTest.kt | 308 ++++++++++++++++++ 13 files changed, 462 insertions(+), 148 deletions(-) create mode 100644 src/test/kotlin/snyk/common/lsp/settings/FolderConfigSettingsTest.kt diff --git a/.windsurfrules b/.windsurfrules index 03179808a..70e26e667 100644 --- a/.windsurfrules +++ b/.windsurfrules @@ -6,6 +6,7 @@ always write and update test cases. iterate until they pass. use existing mocks, don't write new ones. if you use mocks, use mockk to generate them. always run the tests after editing. +use junit4 syntax ** security ** determine the absolute path of the project directory. you can do that e.g. by executing pwd on the shell within the directory. diff --git a/CHANGELOG.md b/CHANGELOG.md index 65e8c82e5..c64e7c9ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # Snyk Security Changelog ## [2.13.1] ### Fixed -- fixed not initialized exception in error handling during language server startup +- fixed not initialized exception in error handling during language server startup +- fixed handling of special characters in filepaths ## [2.13.0] diff --git a/src/main/kotlin/io/snyk/plugin/Utils.kt b/src/main/kotlin/io/snyk/plugin/Utils.kt index 99a964980..492e84c73 100644 --- a/src/main/kotlin/io/snyk/plugin/Utils.kt +++ b/src/main/kotlin/io/snyk/plugin/Utils.kt @@ -421,12 +421,17 @@ fun String.toVirtualFile(): VirtualFile { return if (!this.startsWith("file:")) { StandardFileSystems.local().refreshAndFindFileByPath(this) ?: throw FileNotFoundException(this) } else { - val filePath = Paths.get(this.toFilePathString()) + val filePath = fromUriToPath() VirtualFileManager.getInstance().refreshAndFindFileByNioPath(filePath) ?: throw FileNotFoundException(this) } } +fun String.fromUriToPath(): Path { + val filePath = Paths.get(URI.create(this)) + return filePath.normalize() +} + fun String.toVirtualFileOrNull(): VirtualFile? { return try { this.toVirtualFile() @@ -436,77 +441,11 @@ fun String.toVirtualFileOrNull(): VirtualFile? { } fun VirtualFile.toLanguageServerURI(): String { - return this.url.toFileURIString() -} - -/** - * Normalizes a string that represents a file path or URI. - * - * This should be called on a string that represents an absolute path to a local file, or a file uri for a local file. - * Relative paths and files on network shares are not currently supported. - * - * We deliberately avoid use of the Path, File and URI libraries as these will make decisions on how paths are handled - * based on the underlying operating system. This approach provides consistency. - * - * @param forceUseForwardSlashes Whether to force the use of forward slashses as path separators, even for files on - * Windows. Unix systems will always use forward slashes. - * @return The normalized string. - */ -private fun String.toNormalizedFilePath(forceUseForwardSlashes: Boolean): String { - val fileScheme = "file:" - val windowsSeparator = "\\" - val unixSeparator = "/" - - // Strip the scheme and standardise separators on Unix for now. - val normalizedPath = this.removePrefix(fileScheme).replace(windowsSeparator, unixSeparator) - var targetSeparator = unixSeparator - - // Split the path into parts, filtering out any blanks or references to the current directory. - val parts = normalizedPath.split(unixSeparator).filter { it.isNotBlank() && it != "." }.mapIndexed { idx, value -> - if (idx == 0) { - // Since we only support local files, we can use the first element of the path can tell us whether we - // are dealing with a Windows or unix file. - if (value.startsWithWindowsDriveLetter()) { - // Change to using Windows separators (if allowed) and capitalize the drive letter. - if (!forceUseForwardSlashes) targetSeparator = windowsSeparator - value.uppercase() - } else { - // On a Unix system, start with a slash representing root. - unixSeparator + value - } - } else value - } - - // Removing any references to the parent directory (we have already removed references to the current directory). - val stack = mutableListOf() - for (part in parts) { - if (part == "..") { - if (stack.isNotEmpty()) stack.removeAt(stack.size - 1) - } else stack.add(part) - } - return stack.joinToString(targetSeparator) -} - -/** - * Converts a string representing a file path to a normalised form. @see io.snyk.plugin.UtilsKt.toNormalizedFilePath - */ -fun String.toFilePathString(): String { - return this.toNormalizedFilePath(forceUseForwardSlashes = false) + return this.path.fromPathToUriString() } -/** - * Converts a string representing a file path to a normalised form. @see io.snyk.plugin.UtilsKt.toNormalizedFilePath - */ -fun String.toFileURIString(): String { - var pathString = this.toNormalizedFilePath(forceUseForwardSlashes = true) - - // If we are handling a Windows path it may not have a leading slash, so add one. - if (pathString.startsWithWindowsDriveLetter()) { - pathString = "/$pathString" - } - - // Add a file scheme. We use two slashes as standard. - return "file://$pathString" +fun String.fromPathToUriString(): String { + return Paths.get(this).normalize().toUri().toASCIIString() } private fun String.startsWithWindowsDriveLetter(): Boolean { @@ -517,7 +456,7 @@ fun VirtualFile.getDocument(): Document? = runReadAction { FileDocumentManager.g fun Project.getContentRootPaths(): SortedSet { return getContentRootVirtualFiles() - .mapNotNull { it.path.toNioPathOrNull() } + .mapNotNull { it.path.toNioPathOrNull()?.normalize() } .toSortedSet() } diff --git a/src/main/kotlin/io/snyk/plugin/services/SnykTaskQueueService.kt b/src/main/kotlin/io/snyk/plugin/services/SnykTaskQueueService.kt index 244871af6..e0118bcea 100644 --- a/src/main/kotlin/io/snyk/plugin/services/SnykTaskQueueService.kt +++ b/src/main/kotlin/io/snyk/plugin/services/SnykTaskQueueService.kt @@ -22,6 +22,7 @@ import io.snyk.plugin.pluginSettings import io.snyk.plugin.refreshAnnotationsForOpenFiles import io.snyk.plugin.ui.SnykBalloonNotificationHelper import org.jetbrains.annotations.TestOnly +import org.jetbrains.concurrency.runAsync import snyk.common.lsp.LanguageServerWrapper import snyk.common.lsp.ScanState import snyk.trust.confirmScanningAndSetWorkspaceTrustedStateIfNeeded @@ -56,7 +57,13 @@ class SnykTaskQueueService(val project: Project) { // wait for modules to be loaded and indexed so we can add all relevant content roots DumbService.getInstance(project).runWhenSmart { - languageServerWrapper.addContentRoots(project) + runAsync { + try { + languageServerWrapper.addContentRoots(project) + } catch (e: RuntimeException) { + logger.error("unable to add content roots for project $project", e) + } + } } } diff --git a/src/main/kotlin/io/snyk/plugin/ui/SnykSettingsDialog.kt b/src/main/kotlin/io/snyk/plugin/ui/SnykSettingsDialog.kt index cb352eb7f..8b17fd9a9 100644 --- a/src/main/kotlin/io/snyk/plugin/ui/SnykSettingsDialog.kt +++ b/src/main/kotlin/io/snyk/plugin/ui/SnykSettingsDialog.kt @@ -185,7 +185,7 @@ class SnykSettingsDialog( // TODO: check for concrete project roots, and if we received a message for them // this is an edge case, when a project is opened after ls initialization and // preferences dialog is opened before ls sends the additional parameters - additionalParametersTextField.isEnabled = LanguageServerWrapper.getInstance().folderConfigsRefreshed.isNotEmpty() + additionalParametersTextField.isEnabled = LanguageServerWrapper.getInstance().getFolderConfigsRefreshed().isNotEmpty() additionalParametersTextField.text = getAdditionalParams(project) scanOnSaveCheckbox.isSelected = applicationSettings.scanOnSave cliReleaseChannelDropDown.selectedItem = applicationSettings.cliReleaseChannel diff --git a/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt b/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt index 06b7d5f15..a6a369499 100644 --- a/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt +++ b/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt @@ -12,14 +12,15 @@ import com.intellij.openapi.project.ProjectManager import com.intellij.openapi.util.Disposer import com.intellij.openapi.util.io.toNioPathOrNull import com.intellij.openapi.vfs.VirtualFile +import io.snyk.plugin.fromUriToPath import io.snyk.plugin.getCliFile import io.snyk.plugin.getContentRootVirtualFiles import io.snyk.plugin.getSnykTaskQueueService import io.snyk.plugin.getWaitForResultsTimeout import io.snyk.plugin.pluginSettings import io.snyk.plugin.runInBackground -import io.snyk.plugin.toFilePathString import io.snyk.plugin.toLanguageServerURI +import io.snyk.plugin.ui.SnykBalloonNotificationHelper import io.snyk.plugin.ui.toolwindow.SnykPluginDisposable import org.eclipse.lsp4j.ClientCapabilities import org.eclipse.lsp4j.ClientInfo @@ -64,9 +65,9 @@ import snyk.common.lsp.commands.COMMAND_WORKSPACE_FOLDER_SCAN import snyk.common.lsp.commands.SNYK_GENERATE_ISSUE_DESCRIPTION import snyk.common.lsp.progress.ProgressManager import snyk.common.lsp.settings.FolderConfigSettings +import snyk.common.lsp.settings.IssueViewOptions import snyk.common.lsp.settings.LanguageServerSettings import snyk.common.lsp.settings.SeverityFilter -import snyk.common.lsp.settings.IssueViewOptions import snyk.common.removeTrailingSlashesIfPresent import snyk.pluginInfo import snyk.trust.WorkspaceTrustService @@ -97,7 +98,7 @@ class LanguageServerWrapper( // internal for test set up internal val configuredWorkspaceFolders: MutableSet = Collections.synchronizedSet(mutableSetOf()) - internal var folderConfigsRefreshed: MutableMap = ConcurrentHashMap() + private var folderConfigsRefreshed: MutableMap = ConcurrentHashMap() private var disposed = false get() { return ApplicationManager.getApplication().isDisposed || field @@ -199,10 +200,10 @@ class LanguageServerWrapper( LanguageServerRestartListener.getInstance() refreshFeatureFlags() } else { - logger.warn("Language Server initialization did not succeed") + logger.error("Snyk Language Server process launch failed.") } } catch (e: Exception) { - logger.warn(e) + logger.error("Initialization of Snyk Language Server failed", e) if (processIsAlive()) process.destroyForcibly() isInitialized = false } @@ -336,7 +337,7 @@ class LanguageServerWrapper( added.filter { !configuredWorkspaceFolders.contains(it) }, removed.filter { configuredWorkspaceFolders.contains(it) }, ) - if (params.event.added.size > 0 || params.event.removed.size > 0) { + if (params.event.added.isNotEmpty() || params.event.removed.isNotEmpty()) { languageServer.workspaceService.didChangeWorkspaceFolders(params) configuredWorkspaceFolders.removeAll(removed) configuredWorkspaceFolders.addAll(added) @@ -485,11 +486,12 @@ class LanguageServerWrapper( // the folderConfigs in language server val folderConfigs = configuredWorkspaceFolders .filter { - val folderPath = it.uri.toFilePathString() + val folderPath = it.uri.fromUriToPath().toString() folderConfigsRefreshed[folderPath] == true }.map { - val folderPath = it.uri.toFilePathString() - service().getFolderConfig(folderPath) } + val folderPath = it.uri.fromUriToPath().toString() + service().getFolderConfig(folderPath) + } .toList() return LanguageServerSettings( @@ -621,7 +623,13 @@ class LanguageServerWrapper( fun addContentRoots(project: Project) { if (disposed || project.isDisposed) return - ensureLanguageServerInitialized() + if (!ensureLanguageServerInitialized()) { + SnykBalloonNotificationHelper.showWarn( + "Unable to initialize the Snyk Language Server. The plugin will be non-functional.", + project + ) + return + } ensureLanguageServerProtocolVersion(project) updateConfiguration(false) val added = getWorkspaceFoldersFromRoots(project) @@ -665,7 +673,13 @@ class LanguageServerWrapper( executeCommand(param) } - fun sendSubmitIgnoreRequestCommand(workflow: String, issueId: String, ignoreType: String, ignoreReason: String, ignoreExpirationDate: String) { + fun sendSubmitIgnoreRequestCommand( + workflow: String, + issueId: String, + ignoreType: String, + ignoreReason: String, + ignoreExpirationDate: String + ) { if (!ensureLanguageServerInitialized()) throw RuntimeException("couldn't initialize language server") try { val param = ExecuteCommandParams() @@ -753,6 +767,15 @@ class LanguageServerWrapper( shutdown() } + fun getFolderConfigsRefreshed(): Map { + return Collections.unmodifiableMap(this.folderConfigsRefreshed) + } + + fun updateFolderConfigRefresh(folderPath: String, refreshed: Boolean) { + val path = Paths.get(folderPath).normalize().toAbsolutePath().toString() + this.folderConfigsRefreshed[path] = refreshed + } + companion object { private var instance: LanguageServerWrapper? = null diff --git a/src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt b/src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt index f7edec6ba..601a844f6 100644 --- a/src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt +++ b/src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt @@ -199,7 +199,7 @@ class SnykLanguageClient : val service = service() service.addAll(folderConfigs) folderConfigs.forEach { - LanguageServerWrapper.getInstance().folderConfigsRefreshed[it.folderPath] = true + LanguageServerWrapper.getInstance().updateFolderConfigRefresh(it.folderPath, true) } } } diff --git a/src/main/kotlin/snyk/common/lsp/settings/FolderConfigSettings.kt b/src/main/kotlin/snyk/common/lsp/settings/FolderConfigSettings.kt index d673df0ad..9d08dc892 100644 --- a/src/main/kotlin/snyk/common/lsp/settings/FolderConfigSettings.kt +++ b/src/main/kotlin/snyk/common/lsp/settings/FolderConfigSettings.kt @@ -2,11 +2,14 @@ package snyk.common.lsp.settings import com.intellij.openapi.components.Service import com.intellij.openapi.project.Project +import io.snyk.plugin.fromUriToPath import io.snyk.plugin.getContentRootPaths -import io.snyk.plugin.toFilePathString +import io.snyk.plugin.suffixIfNot import org.jetbrains.annotations.NotNull import snyk.common.lsp.FolderConfig import snyk.common.lsp.LanguageServerWrapper +import java.io.File +import java.nio.file.Paths import java.util.concurrent.ConcurrentHashMap import java.util.stream.Collectors @@ -17,19 +20,35 @@ class FolderConfigSettings { @Suppress("UselessCallOnNotNull", "USELESS_ELVIS", "UNNECESSARY_SAFE_CALL", "RedundantSuppression") fun addFolderConfig(@NotNull folderConfig: FolderConfig) { - if (folderConfig?.folderPath.isNullOrBlank() ?: true) return - configs[folderConfig.folderPath] = folderConfig + if (folderConfig.folderPath.isNullOrBlank()) return + val normalizedAbsolutePath = normalizePath(folderConfig.folderPath) + + val configToStore = folderConfig.copy(folderPath = normalizedAbsolutePath) + configs[normalizedAbsolutePath] = configToStore + } + + private fun normalizePath(folderPath: String): String { + val normalizedAbsolutePath = + Paths.get(folderPath) + .normalize() + .toAbsolutePath() + .toString() + .suffixIfNot(File.separator) + return normalizedAbsolutePath } internal fun getFolderConfig(folderPath: String): FolderConfig { - val folderConfig = configs[folderPath] ?: createEmpty(folderPath) + val normalizedPath = normalizePath(folderPath) + val folderConfig = configs[normalizedPath] ?: createEmpty(normalizedPath) return folderConfig } - private fun createEmpty(folderPath: String): FolderConfig { - val folderConfig = FolderConfig(folderPath = folderPath, baseBranch = "main") - addFolderConfig(folderConfig) - return folderConfig + private fun createEmpty(normalizedAbsolutePath: String): FolderConfig { + val newConfig = FolderConfig(folderPath = normalizedAbsolutePath, baseBranch = "main") + // Directly add to map, as addFolderConfig would re-normalize and copy, which is redundant here + // since normalizedAbsolutePath is already what we want for the key and the object's path. + configs[normalizedAbsolutePath] = newConfig + return newConfig } fun getAll(): Map { @@ -58,7 +77,7 @@ class FolderConfigSettings { val additionalParameters = LanguageServerWrapper.getInstance().getWorkspaceFoldersFromRoots(project) .asSequence() .filter { LanguageServerWrapper.getInstance().configuredWorkspaceFolders.contains(it) } - .map { getFolderConfig(it.uri.toFilePathString()) } + .map { getFolderConfig(it.uri.fromUriToPath().toString()) } .filter { it.additionalParameters?.isNotEmpty() ?: false } .map { it.additionalParameters?.joinToString(" ") } .joinToString(" ") diff --git a/src/main/kotlin/snyk/trust/TrustedProjects.kt b/src/main/kotlin/snyk/trust/TrustedProjects.kt index 1b66ee6b0..508816c03 100644 --- a/src/main/kotlin/snyk/trust/TrustedProjects.kt +++ b/src/main/kotlin/snyk/trust/TrustedProjects.kt @@ -4,7 +4,6 @@ package snyk.trust import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.invokeAndWaitIfNeeded -import com.intellij.openapi.application.runInEdt import com.intellij.openapi.components.service import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.project.Project diff --git a/src/main/resources/html/ScanSummaryInit.html b/src/main/resources/html/ScanSummaryInit.html index 7e6534be8..20d8584dd 100644 --- a/src/main/resources/html/ScanSummaryInit.html +++ b/src/main/resources/html/ScanSummaryInit.html @@ -39,13 +39,10 @@ ${ideStyle} -
-

Snyk Security is loading...

-
-

Waiting for the Snyk CLI to be downloaded and the Language Server to be initialized.

+

Snyk Security is loading...

diff --git a/src/test/kotlin/io/snyk/plugin/UtilsKtTest.kt b/src/test/kotlin/io/snyk/plugin/UtilsKtTest.kt index 5fcafb670..ce7ab495b 100644 --- a/src/test/kotlin/io/snyk/plugin/UtilsKtTest.kt +++ b/src/test/kotlin/io/snyk/plugin/UtilsKtTest.kt @@ -9,10 +9,10 @@ import junit.framework.TestCase.assertEquals import junit.framework.TestCase.assertFalse import junit.framework.TestCase.assertTrue import org.apache.commons.lang3.SystemProperties +import org.apache.commons.lang3.SystemUtils import org.junit.After import org.junit.Before import org.junit.Test -import java.io.File class UtilsKtTest { @@ -29,21 +29,28 @@ class UtilsKtTest { } @Test - fun toLanguageServerURL() { - val path = "C:/Users/username/file.txt" - var uri = "file://$path" - var virtualFile = mockk() - every { virtualFile.url } returns uri + fun `toLanguageServerURL (windows)`() { + unmockkAll() + if (!SystemUtils.IS_OS_WINDOWS) return + val path = "C:\\Users\\username\\file.txt" + val virtualFile = mockk() + every { virtualFile.path } returns path - assertEquals("file:///$path", virtualFile.toLanguageServerURI()) + assertEquals("file:///C:/Users/username/file.txt", virtualFile.toLanguageServerURI()) + } - uri = "file:///$path" - virtualFile = mockk() - every { virtualFile.url } returns uri + @Test + fun `toLanguageServerURL (posix)`() { + unmockkAll() + if (SystemUtils.IS_OS_WINDOWS) return + val path = "/Users/username/file.txt" + val virtualFile = mockk() + every { virtualFile.path } returns path - assertEquals("file:///$path", virtualFile.toLanguageServerURI()) + assertEquals("file:///Users/username/file.txt", virtualFile.toLanguageServerURI()) } + @Test fun isAdditionalParametersValid() { assertFalse(isAdditionalParametersValid("-d")) @@ -51,47 +58,60 @@ class UtilsKtTest { } @Test - fun toFilePathString() { - - // Windows files - var pathsToTest = arrayOf( - "C:\\Users\\username\\file.txt", // Valid path with Windows style separators - "c:\\Users\\username\\file.txt", // Valid path with Windows style separators and a lowercase drive letter - "C:/Users/username/file.txt", // Valid path with Unix style separators - "C:\\Users\\.\\username\\..\\username\\file.txt", // valid path with extra relative sub paths - "file:///C:/Users/username/file.txt", // Valid URI with blank host - "file:///c:/Users/username/file.txt", // Valid URI with blank host and a lowercase drive letter - "file:/C:/Users/username/file.txt", // Valid URI with no host - "file:///C:/Users/./username/../username/file.txt", // Valid URI and extra relative sub paths - "file://C:/Users/username/file.txt", // Invalid URI - "file://C:\\Users\\username\\file.txt", // Invalid URI + fun `conversion between path and uri - ensure we can convert a URI to a path and back (posix)`() { + unmockkAll() + if (SystemUtils.IS_OS_WINDOWS) return + val expectedPaths = arrayOf( + "/Users/username/file.txt", + "/Users/Username/file.txt", + "/Users/user name/file.txt", + "/Users/user name/hyphenated - folder/file.txt", + ) + val inputUris = arrayOf( + "file:///Users/username/file.txt", // URI + "file:///Users/Username/file.txt", // URI + "file:///Users/user%20name/file.txt", // URI with space + "file:///Users/user%20name/hyphenated%20-%20folder/file.txt", // URI with hyphen and space ) - var expectedPath = "C:\\Users\\username\\file.txt" - var expectedUri = "file:///C:/Users/username/file.txt" - for (path in pathsToTest) { - assertEquals("Testing path $path normalization", expectedPath, path.toFilePathString()) - assertEquals("Testing path $path URI conversion", expectedUri, path.toFileURIString()) + var i = 0 + for (uri in inputUris) { + val actualPath = uri.fromUriToPath().toString() + assertEquals("Testing $uri to path conversion", expectedPaths[i], actualPath) + assertEquals("Testing $actualPath to uri conversion", uri, actualPath.fromPathToUriString()) + i++ } + } - // Unix style files - pathsToTest = arrayOf( - "\\users\\username\\file.txt", // Valid path with Windows style separators - "/users/username/file.txt", // Valid path with Unix style separators - "/users/./username/../username/file.txt", // valid path with extra relative sub paths - "file:///users/username/file.txt", // Valid path with scheme - "file:/users/username/file.txt", // Valid path with scheme - "file:///users/./username/../username/file.txt", // Valid path with scheme and extra relative sub paths - "file://users/username/file.txt", // Invalid path with scheme. + @Test + fun `conversion between path and uri - ensure we can convert a URI to a path and back (windows)`() { + unmockkAll() + if (!SystemUtils.IS_OS_WINDOWS) return + val expectedPaths = arrayOf( + "C:\\Users\\username\\file.txt", // Valid path + "c:\\Users\\username\\file.txt", // Valid path + "C:\\Users\\username with spaces\\file.txt", // Valid path + "C:\\Users\\username with hyphenated - spaces\\file.txt", // Valid path + "C:\\Users\\username with \$peci@l characters\\file.txt", // Valid path + "\\\\myserver\\shared folder\\file name with spaces \$peci@l%.txt" ) - expectedPath = "/users/username/file.txt" - expectedUri = "file:///users/username/file.txt" + val inputUris = arrayOf( + "file:///C:/Users/username/file.txt", // Valid URI + "file:///c:/Users/username/file.txt", // Valid URI + "file:///C:/Users/username%20with%20spaces/file.txt", // Valid URI + "file:///C:/Users/username%20with%20hyphenated%20-%20spaces/file.txt", // Valid URI + "file:///C:/Users/username%20with%20\$peci@l%20characters/file.txt", // Valid URI + "file://myserver/shared%20folder/file%20name%20with%20spaces%20\$peci@l%25.txt" // UNC + ) - for (path in pathsToTest) { - assertEquals("Testing path $path normalization", expectedPath, path.toFilePathString()) - assertEquals("Testing path $path URI conversion", expectedUri, path.toFileURIString()) + var i = 0 + for (uri in inputUris) { + val actualPath = uri.fromUriToPath().toString() + assertEquals("Testing $uri to path conversion", expectedPaths[i], actualPath) + assertEquals("Testing $actualPath to uri conversion", uri, actualPath.fromPathToUriString()) + i++ } } } diff --git a/src/test/kotlin/io/snyk/plugin/ui/ReferenceChooserDialogTest.kt b/src/test/kotlin/io/snyk/plugin/ui/ReferenceChooserDialogTest.kt index 26dad2c50..78b9efeba 100644 --- a/src/test/kotlin/io/snyk/plugin/ui/ReferenceChooserDialogTest.kt +++ b/src/test/kotlin/io/snyk/plugin/ui/ReferenceChooserDialogTest.kt @@ -8,8 +8,8 @@ import io.mockk.CapturingSlot import io.mockk.mockk import io.mockk.unmockkAll import io.mockk.verify +import io.snyk.plugin.fromPathToUriString import io.snyk.plugin.getContentRootPaths -import io.snyk.plugin.toFilePathString import org.eclipse.lsp4j.DidChangeConfigurationParams import org.eclipse.lsp4j.WorkspaceFolder import org.eclipse.lsp4j.services.LanguageServer @@ -34,12 +34,12 @@ class ReferenceChooserDialogTest : LightPlatform4TestCase() { languageServerWrapper.languageServer = lsMock project.getContentRootPaths().forEach { - val absolutePathString = it.toString().toFilePathString() + val absolutePathString = it.toAbsolutePath().normalize().toString() service().addTrustedPath(absolutePathString) folderConfig = FolderConfig(absolutePathString, "testBranch") service().addFolderConfig(folderConfig) - languageServerWrapper.configuredWorkspaceFolders.add(WorkspaceFolder(absolutePathString, "test")) - languageServerWrapper.folderConfigsRefreshed[folderConfig.folderPath] = true + languageServerWrapper.configuredWorkspaceFolders.add(WorkspaceFolder(absolutePathString.fromPathToUriString(), "test")) + languageServerWrapper.updateFolderConfigRefresh(absolutePathString, true) } cut = ReferenceChooserDialog(project) } diff --git a/src/test/kotlin/snyk/common/lsp/settings/FolderConfigSettingsTest.kt b/src/test/kotlin/snyk/common/lsp/settings/FolderConfigSettingsTest.kt new file mode 100644 index 000000000..e06f63037 --- /dev/null +++ b/src/test/kotlin/snyk/common/lsp/settings/FolderConfigSettingsTest.kt @@ -0,0 +1,308 @@ +package snyk.common.lsp.settings + +import io.snyk.plugin.suffixIfNot +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import snyk.common.lsp.FolderConfig +import java.io.File +import java.nio.file.Paths + +class FolderConfigSettingsTest { + + private lateinit var settings: FolderConfigSettings + + @Before + fun setUp() { + settings = FolderConfigSettings() + settings.clear() + } + + @Test + fun `addFolderConfig stores and getFolderConfig retrieves with simple path`() { + val path = "/test/projectA" + val normalizedPath = Paths.get(path).normalize().toAbsolutePath().toString().suffixIfNot(File.separator) + val config = FolderConfig( + folderPath = path, + baseBranch = "main", + additionalParameters = listOf("--scan-all-unmanaged") + ) + + settings.addFolderConfig(config) + + val retrievedConfig = settings.getFolderConfig(path) + assertNotNull("Retrieved config should not be null", retrievedConfig) + assertEquals("Normalized path should match", normalizedPath, retrievedConfig.folderPath) + assertEquals("Base branch should match", "main", retrievedConfig.baseBranch) + assertEquals( + "Additional parameters should match", + listOf("--scan-all-unmanaged"), + retrievedConfig.additionalParameters + ) + + assertEquals("Settings map size should be 1", 1, settings.getAll().size) + assertTrue("Settings map should contain normalized path key", settings.getAll().containsKey(normalizedPath)) + } + + @Test + fun `addFolderConfig normalizes path with dot and double-dot segments`() { + val rawPath = "/test/projectB/./subfolder/../othersubfolder" + val expectedNormalizedPath = Paths.get(rawPath).normalize().toAbsolutePath().toString() + .suffixIfNot(File.separator) + + val config = FolderConfig(folderPath = rawPath, baseBranch = "develop") + settings.addFolderConfig(config) + + val retrievedConfig = settings.getFolderConfig(rawPath) + assertNotNull("Retrieved config should not be null", retrievedConfig) + assertEquals("Normalized path should match", expectedNormalizedPath, retrievedConfig.folderPath) + assertEquals("Base branch should match", "develop", retrievedConfig.baseBranch) + + val retrievedAgain = settings.getFolderConfig(expectedNormalizedPath) + assertNotNull("Retrieved again config should not be null", retrievedAgain) + assertEquals( + "Normalized path should match when retrieved again", + expectedNormalizedPath, + retrievedAgain.folderPath + ) + } + + @Test + fun `getFolderConfig retrieves config using equivalent normalized paths`() { + val path1 = "/my/project/folder" + val path2 = "/my/project/./folder" + val path3 = "/my/project/../project/folder" + val normalizedPath1 = Paths.get(path1).normalize().toAbsolutePath().toString().suffixIfNot(File.separator) + + val config = FolderConfig(folderPath = path1, baseBranch = "feature-branch") + settings.addFolderConfig(config) + + val retrievedConfig1 = settings.getFolderConfig(path1) + assertEquals("Path1 normalized path should match", normalizedPath1, retrievedConfig1.folderPath) + assertEquals("Path1 base branch should match", "feature-branch", retrievedConfig1.baseBranch) + + val retrievedConfig2 = settings.getFolderConfig(path2) + assertEquals("Path2 normalized path should match", normalizedPath1, retrievedConfig2.folderPath) + assertEquals("Path2 base branch should match", "feature-branch", retrievedConfig2.baseBranch) + + val retrievedConfig3 = settings.getFolderConfig(path3) + assertEquals("Path3 normalized path should match", normalizedPath1, retrievedConfig3.folderPath) + assertEquals("Path3 base branch should match", "feature-branch", retrievedConfig3.baseBranch) + } + + @Test + fun `addFolderConfig ignores empty or blank folderPaths`() { + settings.addFolderConfig(FolderConfig(folderPath = "", baseBranch = "main")) + assertEquals("Config with empty path should be ignored", 0, settings.getAll().size) + + settings.addFolderConfig(FolderConfig(folderPath = " ", baseBranch = "main")) + assertEquals("Config with blank path should be ignored", 0, settings.getAll().size) + } + + @Test + fun `getFolderConfig creates and stores new config if not found, with normalized path`() { + val rawPath = "/new/folder/./for/creation" + val expectedNormalizedPath = + Paths.get(rawPath).normalize().toAbsolutePath().toString().suffixIfNot(File.separator) + + assertTrue("Settings should be empty initially", settings.getAll().isEmpty()) + + val newConfig = settings.getFolderConfig(rawPath) + assertNotNull("New config should not be null", newConfig) + assertEquals("FolderPath in new config should be normalized", expectedNormalizedPath, newConfig.folderPath) + assertEquals("New config should have default baseBranch", "main", newConfig.baseBranch) + assertEquals( + "New config additionalParameters should be emptyList", + emptyList(), + newConfig.additionalParameters + ) + assertEquals("New config localBranches should be emptyList", emptyList(), newConfig.localBranches) + assertEquals("New config referenceFolderPath should be empty string", "", newConfig.referenceFolderPath) + assertEquals( + "New config scanCommandConfig should be emptyMap", + emptyMap(), + newConfig.scanCommandConfig + ) + + val allConfigs = settings.getAll() + assertEquals("A new config should have been added", 1, allConfigs.size) + assertTrue( + "Internal map should contain the new config with normalized path as key", + allConfigs.containsKey(expectedNormalizedPath) + ) + assertEquals( + "Stored config folderPath should match", + expectedNormalizedPath, + allConfigs[expectedNormalizedPath]?.folderPath + ) + } + + @Test + fun `addFolderConfig overwrites existing config with same normalized path`() { + val path = "/my/overwritable/folder" + val equivalentPath = "/my/overwritable/./folder/../folder" + val normalizedPath = Paths.get(path).normalize().toAbsolutePath().toString().suffixIfNot(File.separator) + + val config1 = FolderConfig(folderPath = path, baseBranch = "v1", additionalParameters = listOf("param1")) + settings.addFolderConfig(config1) + + var retrieved = settings.getFolderConfig(path) + assertEquals("Retrieved v1 baseBranch", "v1", retrieved.baseBranch) + assertEquals("Retrieved v1 additionalParameters", listOf("param1"), retrieved.additionalParameters) + assertEquals("Retrieved v1 normalizedPath", normalizedPath, retrieved.folderPath) + + val config2 = + FolderConfig(folderPath = equivalentPath, baseBranch = "v2", additionalParameters = listOf("param2")) + settings.addFolderConfig(config2) + + retrieved = settings.getFolderConfig(path) + assertEquals("BaseBranch should be from the overriding config", "v2", retrieved.baseBranch) + assertEquals( + "AdditionalParameters should be from the overriding config", + listOf("param2"), + retrieved.additionalParameters + ) + assertEquals("NormalizedPath should remain the same after overwrite", normalizedPath, retrieved.folderPath) + + assertEquals("Should still be only one entry in settings map", 1, settings.getAll().size) + } + + @Test + fun `paths are treated as case-sensitive by default by the underlying map`() { + val pathUpper = "/Case/Sensitive/Path" + val pathLower = "/case/sensitive/path" + + val normalizedUpper = Paths.get(pathUpper).normalize().toAbsolutePath().toString().suffixIfNot(File.separator) + val normalizedLower = Paths.get(pathLower).normalize().toAbsolutePath().toString().suffixIfNot(File.separator) + + val configUpper = FolderConfig(folderPath = pathUpper, baseBranch = "upper") + settings.addFolderConfig(configUpper) + + val configLower = FolderConfig(folderPath = pathLower, baseBranch = "lower") + settings.addFolderConfig(configLower) + + if (normalizedUpper.equals(normalizedLower, ignoreCase = true) && normalizedUpper != normalizedLower) { + assertEquals( + "Configs with paths differing only in case should be distinct if normalized strings differ", + 2, + settings.getAll().size + ) + assertEquals("BaseBranch for upper case path", "upper", settings.getFolderConfig(pathUpper).baseBranch) + assertEquals("BaseBranch for lower case path", "lower", settings.getFolderConfig(pathLower).baseBranch) + } else if (normalizedUpper == normalizedLower) { + assertEquals("If normalized paths are identical, one should overwrite the other", 1, settings.getAll().size) + assertEquals( + "Lower should overwrite if normalized paths are identical (upper retrieval)", + "lower", + settings.getFolderConfig(pathUpper).baseBranch + ) + assertEquals( + "Lower should overwrite if normalized paths are identical (lower retrieval)", + "lower", + settings.getFolderConfig(pathLower).baseBranch + ) + } else { + assertEquals("Distinct normalized paths should result in distinct entries", 2, settings.getAll().size) + assertEquals( + "BaseBranch for upper case path (distinct)", + "upper", + settings.getFolderConfig(pathUpper).baseBranch + ) + assertEquals( + "BaseBranch for lower case path (distinct)", + "lower", + settings.getFolderConfig(pathLower).baseBranch + ) + } + } + + @Test + fun `addFolderConfig with trailing slash is equivalent to without trailing slash`() { + val pathWithSlash = "/test/trailing/" + val pathWithoutSlash = "/test/trailing" + // For non-root paths, Paths.get().normalize() typically removes trailing slashes. + val expectedNormalizedPath = + Paths.get(pathWithoutSlash).normalize().toAbsolutePath().toString().suffixIfNot(File.separator) + + // Add with slash + val config1 = FolderConfig(folderPath = pathWithSlash, baseBranch = "main") + settings.addFolderConfig(config1) + + // Retrieve with and without slash + val retrieved1With = settings.getFolderConfig(pathWithSlash) + val retrieved1Without = settings.getFolderConfig(pathWithoutSlash) + + assertNotNull("Config should be retrievable with slash", retrieved1With) + assertEquals( + "Retrieved (with slash) path should be normalized", + expectedNormalizedPath, + retrieved1With.folderPath + ) + assertNotNull("Config should be retrievable without slash", retrieved1Without) + assertEquals( + "Retrieved (without slash) path should be normalized", + expectedNormalizedPath, + retrieved1Without.folderPath + ) + assertEquals("Both retrievals should yield the same object instance", retrieved1With, retrieved1Without) + assertEquals("Only one config should be stored", 1, settings.getAll().size) + assertTrue("Map key should be the normalized path", settings.getAll().containsKey(expectedNormalizedPath)) + + // Clear and test adding without slash first + settings.clear() + val config2 = FolderConfig(folderPath = pathWithoutSlash, baseBranch = "develop") + settings.addFolderConfig(config2) + + val retrieved2With = settings.getFolderConfig(pathWithSlash) + val retrieved2Without = settings.getFolderConfig(pathWithoutSlash) + + assertNotNull("Config (added without slash) should be retrievable with slash", retrieved2With) + assertEquals( + "Retrieved (with slash) path should be normalized (added without)", + expectedNormalizedPath, + retrieved2With.folderPath + ) + assertEquals("develop", retrieved2With.baseBranch) // Ensure correct config is retrieved + assertNotNull("Config (added without slash) should be retrievable without slash", retrieved2Without) + assertEquals( + "Retrieved (without slash) path should be normalized (added without)", + expectedNormalizedPath, + retrieved2Without.folderPath + ) + assertEquals("develop", retrieved2Without.baseBranch) + assertEquals("Both retrievals should yield the same object instance", retrieved2With, retrieved2Without) + assertEquals("Only one config should be stored when adding without slash", 1, settings.getAll().size) + } + + @Test + fun `addFolderConfig with trailing slash on root path`() { + // Behavior of Paths.get for root might differ slightly, e.g. "/" vs "/." + // Note: Windows root "C:\\" vs "C:\" might also be relevant if testing on Windows. + // For simplicity, this test uses POSIX root. + val rootPathWithSlash = "/" + val rootPathNormalized = Paths.get(rootPathWithSlash).normalize().toAbsolutePath().toString() + + val config = FolderConfig(folderPath = rootPathWithSlash, baseBranch = "rootBranch") + settings.addFolderConfig(config) + val retrieved = settings.getFolderConfig(rootPathWithSlash) + assertNotNull("Retrieved config for root path should not be null", retrieved) + assertEquals("Retrieved root path should be normalized", rootPathNormalized, retrieved.folderPath) + assertEquals("Settings map size for root path should be 1", 1, settings.getAll().size) + + // Test with a path that might normalize to root, e.g., "/." + val retrievedDot = settings.getFolderConfig("/.") + assertNotNull("Retrieved config for '/.' should not be null", retrievedDot) + assertEquals( + "Retrieved path for '/.' should be normalized to root", + rootPathNormalized, + retrievedDot.folderPath + ) + assertEquals( + "Settings map size should still be 1 after retrieving '/.'", + 1, + settings.getAll().size + ) // Still one config + } +}