From 2117070fd8ff3cb34fde3d046cb7a65480ebaa3f Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Fri, 17 Jul 2026 11:16:49 +0200 Subject: [PATCH 1/4] feature: run against IDEA using runIdeIdea (crw-11741) Signed-off-by: Andre Dietisheim Co-authored-by: Cursor --- ...ests.run.xml => Run Plugin (IDEA).run.xml} | 9 +++-- build.gradle.kts | 35 +++++++++++-------- gradle.properties | 2 -- 3 files changed, 25 insertions(+), 21 deletions(-) rename .run/{Run IDE for UI Tests.run.xml => Run Plugin (IDEA).run.xml} (77%) diff --git a/.run/Run IDE for UI Tests.run.xml b/.run/Run Plugin (IDEA).run.xml similarity index 77% rename from .run/Run IDE for UI Tests.run.xml rename to .run/Run Plugin (IDEA).run.xml index ee99b7ed..9b0cae16 100644 --- a/.run/Run IDE for UI Tests.run.xml +++ b/.run/Run Plugin (IDEA).run.xml @@ -1,5 +1,5 @@ - + - true true false - false - \ No newline at end of file + diff --git a/build.gradle.kts b/build.gradle.kts index b1f66122..a17eee0c 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,5 +1,6 @@ import org.jetbrains.changelog.Changelog import org.jetbrains.changelog.markdownToHTML +import org.jetbrains.intellij.platform.gradle.IntelliJPlatformType import org.jetbrains.intellij.platform.gradle.TestFrameworkType import org.gradle.api.tasks.testing.TestDescriptor import org.gradle.api.tasks.testing.TestResult @@ -18,6 +19,19 @@ plugins { group = providers.gradleProperty("pluginGroup").get() version = providers.gradleProperty("pluginVersion").get() +// -Pidea switches from Gateway (GW) to IntelliJ IDEA (IU) +val isIdea = providers.gradleProperty("idea").isPresent +val resolvedPlatformType = if (isIdea) { + "IU" +} else { + providers.gradleProperty("platformType").get() +} +val resolvedBundledPlugins = if (isIdea) { + "com.jetbrains.gateway" +} else { + providers.gradleProperty("platformBundledPlugins").get() +} + // Set the JVM language level used to build the project. kotlin { jvmToolchain(21) @@ -56,10 +70,10 @@ dependencies { // IntelliJ Platform Gradle Plugin Dependencies Extension - read more: https://plugins.jetbrains.com/docs/intellij/tools-intellij-platform-gradle-plugin-dependencies-extension.html intellijPlatform { - create(providers.gradleProperty("platformType"), providers.gradleProperty("platformVersion")) + create(resolvedPlatformType, providers.gradleProperty("platformVersion")) // Plugin Dependencies. Uses `platformBundledPlugins` property from the gradle.properties file for bundled IntelliJ Platform plugins. - bundledPlugins(providers.gradleProperty("platformBundledPlugins").map { it.split(',') }) + bundledPlugins(resolvedBundledPlugins.split(',').filter { it.isNotBlank() }) // Plugin Dependencies. Uses `platformPlugins` property from the gradle.properties file for plugin from JetBrains Marketplace. plugins(providers.gradleProperty("platformPlugins").map { it.split(',') }) @@ -227,20 +241,13 @@ tasks { intellijPlatformTesting { runIde { - register("runIdeForUiTests") { - task { - jvmArgumentProviders += CommandLineArgumentProvider { - listOf( - "-Drobot-server.port=8082", - "-Dide.mac.message.dialogs.as.sheets=false", - "-Djb.privacy.policy.text=", - "-Djb.consents.confirmation.enabled=false", - ) - } - } + // Visible next to runIde under "intellij platform" in the Gradle tool window + register("runIdeIdea") { + type = IntelliJPlatformType.IntellijIdeaUltimate + version = providers.gradleProperty("platformVersion") plugins { - robotServerPlugin() + bundledPlugin("com.jetbrains.gateway") } } } diff --git a/gradle.properties b/gradle.properties index 1ded4a0e..536d762a 100644 --- a/gradle.properties +++ b/gradle.properties @@ -8,7 +8,6 @@ pluginVersion = 0.0.18 # Supported build number ranges and IntelliJ Platform versions -> https://plugins.jetbrains.com/docs/intellij/build-number-ranges.html pluginSinceBuild = 251 -#pluginUntilBuild = 252.* # IntelliJ Platform Properties -> https://plugins.jetbrains.com/docs/intellij/tools-gradle-intellij-plugin.html#configuration-intellij-extension platformType = GW @@ -20,7 +19,6 @@ platformVersion = 2025.1.1 platformPlugins = # Example: platformBundledPlugins = com.intellij.java platformBundledPlugins = - # Gradle Releases -> https://github.com/gradle/gradle/releases gradleVersion = 8.10.2 From 49b74b39f658cec82119709f8016a260fd318400 Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Sat, 18 Jul 2026 21:56:39 +0200 Subject: [PATCH 2/4] fix: async kubeconfig refreshes overwrite cluster list with an empty/partial (crw-11741) Signed-off-by: Andre Dietisheim Co-authored-by: Cursor --- .../gateway/kubeconfig/FileWatcher.kt | 130 ++++++++--- .../gateway/kubeconfig/KubeConfigMonitor.kt | 72 ++++-- .../gateway/kubeconfig/FileWatcherTest.kt | 206 ++++++++++++------ .../kubeconfig/KubeConfigMonitorTest.kt | 168 +++++++++----- 4 files changed, 403 insertions(+), 173 deletions(-) diff --git a/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcher.kt b/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcher.kt index 2910cda7..8de6131c 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcher.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcher.kt @@ -14,6 +14,8 @@ package com.redhat.devtools.gateway.kubeconfig import kotlinx.coroutines.* import java.nio.file.* import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.TimeUnit +import java.util.concurrent.locks.ReentrantLock import kotlin.io.path.exists import kotlin.io.path.isRegularFile @@ -23,80 +25,144 @@ class FileWatcher( private val watchService: WatchService = FileSystems.getDefault().newWatchService() ) { private var onFileChanged: ((Path) -> Unit)? = null - private val registeredKeys = ConcurrentHashMap() + private val registeredDirectories = ConcurrentHashMap() private val monitoredFiles = ConcurrentHashMap.newKeySet() + private val notifyLock = ReentrantLock() private var watchJob: Job? = null fun start() { this.watchJob = scope.launch(dispatcher) { - try { - while (isActive) { - val key = watchService.poll(100, java.util.concurrent.TimeUnit.MILLISECONDS) - if (key == null) { - @Suppress("ConvertLongToDuration") - delay(100) - continue - } - val dir = registeredKeys[key] ?: continue - pollEvents(key, dir) - key.reset() + pollDirectoryEvents() + } + } + + private suspend fun CoroutineScope.pollDirectoryEvents() { + try { + while (isActive) { + val key = watchService.poll(100, TimeUnit.MILLISECONDS) + if (key == null) { + @Suppress("ConvertLongToDuration") + delay(100) + continue } - } catch (e: ClosedWatchServiceException) { - // Watch service was closed, exit gracefully + val dir = registeredDirectories[key] ?: continue + handleDirectoryEvent(key, dir) + key.reset() } + } catch (_: ClosedWatchServiceException) { + // Watch service was closed, exit gracefully } } fun stop() { watchJob?.cancel() watchJob = null + registeredDirectories.keys.forEach { it.cancel() } + registeredDirectories.clear() + monitoredFiles.clear() try { watchService.close() - } catch (e: Exception) { + } catch (_: Exception) { // Ignore exceptions when closing } } + /** + * Registers [path] for watching and invokes the [onFileChanged] callback if this is + * a new file that was not previously monitored. For non-existent files, the callback + * is invoked so listeners can track them for a later CREATE event. If [path] is + * already in the monitored set, no callback is issued — this avoids redundant refreshes. + * + * @param path the file to monitor; may be non-existent (e.g. a kubeconfig file expected + * to appear later). If [path.parent] is null, the path will not be registered. + * @return this instance for chaining. + */ fun addFile(path: Path): FileWatcher { - if (!path.exists() - || !path.isRegularFile()) { - return this - } val parentDir = path.parent if (parentDir != null && !monitoredFiles.contains(path)) { - val watchKey = parentDir.register(watchService, - StandardWatchEventKinds.ENTRY_CREATE, - StandardWatchEventKinds.ENTRY_MODIFY, - StandardWatchEventKinds.ENTRY_DELETE - ) - registeredKeys[watchKey] = parentDir + registerDirectory(parentDir) monitoredFiles.add(path) - onFileChanged?.invoke(path) + invokeOnFileChanged(path) } return this } fun removeFile(path: Path): FileWatcher { - monitoredFiles.remove(path) + if (!monitoredFiles.remove(path)) { + return this + } + val parentDir = path.parent ?: return this + if (monitoredFiles.none { it.parent == parentDir }) { + unregisterDirectory(parentDir) + } return this } + fun getMonitoredFiles(): Set = monitoredFiles + + /** Parent directories that currently have an active [WatchKey]. For tests. */ + internal fun getWatchedDirectories(): Set = registeredDirectories.values.toSet() + fun onFileChanged(action: ((Path) -> Unit)?) { - this.onFileChanged = action + notifyLock.lock() + try { + this.onFileChanged = action + } finally { + notifyLock.unlock() + } } - private fun pollEvents(key: WatchKey, dir: Path) { + private fun registerDirectory(directory: Path) { + if (registeredDirectories.values.any { it == directory }) { + return + } + val watchKey = directory.register( + watchService, + StandardWatchEventKinds.ENTRY_CREATE, + StandardWatchEventKinds.ENTRY_MODIFY, + StandardWatchEventKinds.ENTRY_DELETE + ) + registeredDirectories[watchKey] = directory + } + + private fun unregisterDirectory(directory: Path) { + val keys = registeredDirectories.filterValues { it == directory }.keys + keys.forEach { key -> + registeredDirectories.remove(key) + key.cancel() + } + } + + private fun handleDirectoryEvent(key: WatchKey, dir: Path) { for (event in key.pollEvents()) { val relativePath = event.context() as? Path ?: continue val changedFile = dir.resolve(relativePath) + val kind = event.kind() - if (monitoredFiles.contains(changedFile) - && event.kind() != StandardWatchEventKinds.OVERFLOW + if (!monitoredFiles.contains(changedFile) + || kind == StandardWatchEventKinds.OVERFLOW ) { - onFileChanged?.invoke(changedFile) + continue + } + + // DELETE: file is already gone — still notify so callers can refresh. + // CREATE/MODIFY: only notify for an existing regular file. + val shouldNotify = kind == StandardWatchEventKinds.ENTRY_DELETE + || (changedFile.exists() && changedFile.isRegularFile()) + + if (shouldNotify) { + invokeOnFileChanged(changedFile) } } } + private fun invokeOnFileChanged(path: Path) { + notifyLock.lock() + try { + onFileChanged?.invoke(path) + } finally { + notifyLock.unlock() + } + } } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitor.kt b/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitor.kt index 434ca530..8d6e9794 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitor.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitor.kt @@ -14,10 +14,14 @@ package com.redhat.devtools.gateway.kubeconfig import com.intellij.openapi.diagnostic.thisLogger import com.redhat.devtools.gateway.openshift.Cluster import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import java.nio.file.Path +import java.util.concurrent.atomic.AtomicLong class KubeConfigMonitor( private val scope: CoroutineScope, @@ -26,17 +30,19 @@ class KubeConfigMonitor( ) { private val logger = thisLogger() - private val _clusters = MutableSharedFlow>(replay = 1) - private val clusters = _clusters.asSharedFlow() + /** null until the first successful refresh — avoids pushing an empty list to late collectors. */ + private val _clusters = MutableStateFlow?>(null) + private val clusters = _clusters.asStateFlow() - private val monitoredPaths = mutableSetOf() + private val refreshMutex = Mutex() + private val refreshGeneration = AtomicLong(0) /** - * Runs the given action for each collected cluster. + * Runs the given action for each collected cluster list. */ suspend fun onClustersCollected(action: suspend (clusters: List) -> Unit) { - logger.info("Setting up SharedFlow collection for cluster updates") - clusters.collect { collected -> + logger.info("Setting up StateFlow collection for cluster updates") + clusters.filterNotNull().collect { collected -> logger.info("Found ${collected.size} clusters") action(collected) } @@ -47,7 +53,7 @@ class KubeConfigMonitor( * * @see [onClustersCollected] */ - internal fun getCurrentClusters(): List = _clusters.replayCache.firstOrNull() ?: emptyList() + internal fun getCurrentClusters(): List = _clusters.value.orEmpty() fun start() { fileWatcher.onFileChanged(::onFileChanged) @@ -55,6 +61,7 @@ class KubeConfigMonitor( fileWatcher.start() } updateMonitoredPaths() + // Barrier refresh: ensures a generation after all sync addFile() notifies. refreshClusters() } @@ -64,38 +71,59 @@ class KubeConfigMonitor( } internal fun updateMonitoredPaths() { - val newPaths = mutableSetOf() - newPaths.addAll(kubeConfigUtils.getAllConfigFiles()) + val newPaths = kubeConfigUtils.getAllConfigFiles().toSet() stopWatchingRemoved(newPaths) startWatchingNew(newPaths) - - monitoredPaths.clear() - monitoredPaths.addAll(newPaths) - logger.info("Monitored paths: $monitoredPaths") + logger.info("Monitored paths: ${fileWatcher.getMonitoredFiles()}") } private fun stopWatchingRemoved(newPaths: Set) { - (monitoredPaths - newPaths).forEach { path -> + (fileWatcher.getMonitoredFiles() - newPaths).forEach { path -> fileWatcher.removeFile(path) logger.info("Stopped monitoring kubeconfig file: $path") } } private fun startWatchingNew(newPaths: Set) { - (newPaths - monitoredPaths).forEach { path -> + (newPaths - fileWatcher.getMonitoredFiles()).forEach { path -> fileWatcher.addFile(path) logger.info("Started monitoring kubeconfig file: $path") } } + /** + * Schedules a cluster refresh. Multiple rapid calls (e.g. per-file [FileWatcher.addFile] + * notifies during bootstrap) coalesce: only the latest generation parses and emits. + * Paths are snapshotted when the work runs, not when it is scheduled, so a burst of + * sync registrations yields one full-list emit. + * + * [MutableStateFlow] skips collector notifications when the new list equals the previous. + */ internal fun refreshClusters() { - logger.info("Reparsing kubeconfig files. Monitored paths: $monitoredPaths") - val allClusters = kubeConfigUtils.getClusters(monitoredPaths.toList()) + val gen = refreshGeneration.incrementAndGet() scope.launch { - logger.info("Emitting ${allClusters.size} clusters to SharedFlow") - _clusters.emit(allClusters) + refreshMutex.withLock { + if (gen != refreshGeneration.get()) { + return@withLock + } + val monitored = fileWatcher.getMonitoredFiles().toList() + logger.info("Reparsing kubeconfig files. Monitored paths: $monitored") + val allClusters = kubeConfigUtils.getClusters(monitored) + if (gen != refreshGeneration.get()) { + return@withLock + } + val previous = _clusters.value + _clusters.value = allClusters + if (previous == allClusters) { + logger.info("Skipping notify; clusters unchanged (${allClusters.size})") + } else { + logger.info( + "Updated clusters (${allClusters.size}): " + + allClusters.map { "${it.name}@${it.url}" } + ) + } + } } - logger.info("Reparsed kubeconfig files. Found ${allClusters.size} clusters: ${allClusters.map { "${it.name}@${it.url}" }}") } fun onFileChanged(filePath: Path) { diff --git a/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcherTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcherTest.kt index c77c9f15..f539322f 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcherTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcherTest.kt @@ -12,23 +12,27 @@ package com.redhat.devtools.gateway.kubeconfig import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import kotlinx.coroutines.withTimeout - - import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test - import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import kotlin.io.path.createFile +import kotlin.io.path.deleteExisting import kotlin.io.path.writeText @ExperimentalCoroutinesApi @@ -57,136 +61,217 @@ class FileWatcherTest { @Test fun `#addFile() invokes callback when a file is added`() = runTest { - // given - var onFileChangedCount = 0 - watcher.onFileChanged { onFileChangedCount++ } + var onFileChangedPath: Path? = null + watcher.onFileChanged { path -> onFileChangedPath = path } - // when watcher.addFile(testFile) advanceUntilIdle() - // then - Initial notification when file is added - assertThat(onFileChangedCount).isEqualTo(1) + assertThat(onFileChangedPath).isEqualTo(testFile) + assertThat(watcher.getMonitoredFiles()).containsExactly(testFile) } @Test - fun `#addFile() does not invoke callback for non-existent files`() = runTest { - // given + fun `#addFile() invokes callback for non-existent files that are tracked`() = runTest { var onFileChangedCount = 0 watcher.onFileChanged { onFileChangedCount++ } val nonExistentFile = tempDir.resolve("non-existent.yaml") - // when watcher.addFile(nonExistentFile) advanceUntilIdle() - // then - No callback should be invoked for non-existent files - assertThat(onFileChangedCount).isEqualTo(0) + assertThat(onFileChangedCount).isEqualTo(1) + assertThat(watcher.getMonitoredFiles()).containsExactly(nonExistentFile) } @Test - fun `#addFile() invokes callback when multiple files are added`() = runTest { - // given + fun `#addFile() invokes callback for each file when multiple are added`() = runTest { var onFileChangedCount = 0 watcher.onFileChanged { onFileChangedCount++ } - val secondFile = createFile("second-file.yaml","second content") + val secondFile = createFile("second-file.yaml", "second content") - // when watcher .addFile(testFile) .addFile(secondFile) advanceUntilIdle() - // then - Callback should be invoked for both files assertThat(onFileChangedCount).isEqualTo(2) + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(testFile, secondFile) } @Test - fun `#removeFile() removes file from monitoring`() = runTest { - // given + fun `#addFile() notifies again when re-adding after remove`() = runTest { var onFileChangedCount = 0 watcher.onFileChanged { onFileChangedCount++ } - // when - add file and verify it triggers callback watcher.addFile(testFile) advanceUntilIdle() assertThat(onFileChangedCount).isEqualTo(1) - // when - remove file from monitoring watcher.removeFile(testFile) advanceUntilIdle() + assertThat(watcher.getMonitoredFiles()).isEmpty() + + watcher.addFile(testFile) + advanceUntilIdle() + assertThat(onFileChangedCount).isEqualTo(2) + assertThat(watcher.getMonitoredFiles()).containsExactly(testFile) + } + + @Test + fun `#addFile() does not invoke callback when file is already monitored`() = runTest { + var onFileChangedPath: Path? = null + watcher.onFileChanged { path -> onFileChangedPath = path } + + watcher.addFile(testFile) + advanceUntilIdle() - val initialOnFileChangedCount = onFileChangedCount - - // Re-adding the same file should trigger callback again since it was removed + onFileChangedPath = null watcher.addFile(testFile) advanceUntilIdle() - - assertThat(onFileChangedCount).isEqualTo(initialOnFileChangedCount + 1) + + assertThat(onFileChangedPath).isNull() + assertThat(watcher.getMonitoredFiles()).containsExactly(testFile) } @Test fun `#removeFile() handles non-existent files gracefully`() = runTest { - // given val nonExistentFile = tempDir.resolve("never-added.yaml") - // when - removing a file that was never added val result = watcher.removeFile(nonExistentFile) - // then - should return the watcher instance and not throw any exceptions assertThat(result).isSameAs(watcher) } + @Test + fun `#removeFile() unregisters directory when last file is removed`() = runTest { + watcher.addFile(testFile) + advanceUntilIdle() + assertThat(watcher.getWatchedDirectories()).containsExactly(testFile.parent) + + watcher.removeFile(testFile) + advanceUntilIdle() + + assertThat(watcher.getMonitoredFiles()).isEmpty() + assertThat(watcher.getWatchedDirectories()).isEmpty() + } + + @Test + fun `#removeFile() keeps directory watch while sibling files remain`() = runTest { + val secondFile = createFile("second-file.yaml", "second content") + + watcher.addFile(testFile).addFile(secondFile) + advanceUntilIdle() + assertThat(watcher.getWatchedDirectories()).containsExactly(testFile.parent) + + watcher.removeFile(testFile) + advanceUntilIdle() + + assertThat(watcher.getMonitoredFiles()).containsExactly(secondFile) + assertThat(watcher.getWatchedDirectories()).containsExactly(testFile.parent) + } + + @Test + fun `#addFile() registers a directory watch only once for siblings`() = runTest { + val secondFile = createFile("second-file.yaml", "second content") + + watcher.addFile(testFile).addFile(secondFile) + advanceUntilIdle() + + assertThat(watcher.getWatchedDirectories()).containsExactly(testFile.parent) + } + @Test fun `#removeFile() keeps other files monitored`() = runTest { - // given val secondFile = createFile("second-file.yaml", "second content") val thirdFile = createFile("third-file.yaml", "third content") - var onFileChangedCount = 0 - watcher.onFileChanged { onFileChangedCount++ } - // when - add multiple files watcher .addFile(testFile) .addFile(secondFile) .addFile(thirdFile) advanceUntilIdle() - - // then - all files should trigger callbacks - assertThat(onFileChangedCount).isEqualTo(3) - // when - remove one file + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(testFile, secondFile, thirdFile) + watcher.removeFile(secondFile) advanceUntilIdle() - // then - other files should still be monitored - val initialOnFileChangedCount = onFileChangedCount - watcher.addFile(testFile) // Re-adding should not trigger (already added) - watcher.addFile(thirdFile) // Re-adding should not trigger (already added) - watcher.addFile(secondFile) // Adding should trigger (was removed) - advanceUntilIdle() + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(testFile, thirdFile) - assertThat(onFileChangedCount).isEqualTo(initialOnFileChangedCount + 1) + watcher.addFile(secondFile) + advanceUntilIdle() + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(testFile, secondFile, thirdFile) } @Test - fun `#onFileChanged() is invoked when a watched file is modified`() = runTest { - // given - val callbackReceived = CompletableDeferred() - watcher.onFileChanged { - callbackReceived.complete(Unit) - } + fun `#getMonitoredFiles() includes non-existent files`() = runTest { + val existingFile = createFile("existing.yaml", "content") + val nonExistentFile = tempDir.resolve("never-exists.yaml") - watcher.start() - watcher.addFile(testFile) + watcher.addFile(existingFile) + watcher.addFile(nonExistentFile) advanceUntilIdle() - // when - testFile.writeText("updated content") + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(existingFile, nonExistentFile) + } - // then - withTimeout(1000) { - callbackReceived.await() + @Test + fun `#onFileChanged() is invoked when a watched file is modified`() = runBlocking { + val ioScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + val ioWatcher = FileWatcher(ioScope, Dispatchers.IO) + try { + val callbackReceived = CompletableDeferred() + var notifyCount = 0 + ioWatcher.onFileChanged { + notifyCount++ + // First notify is from addFile; complete on a subsequent FS event. + if (notifyCount > 1) { + callbackReceived.complete(Unit) + } + } + ioWatcher.start() + ioWatcher.addFile(testFile) + delay(200) + + testFile.writeText("updated content") + + @Suppress("ConvertLongToDuration") + withTimeout(5_000) { + callbackReceived.await() + } + } finally { + ioWatcher.stop() + ioScope.cancel() + } + } + + @Test + fun `#onFileChanged() is invoked when a watched file is deleted`() = runBlocking { + val ioScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + val ioWatcher = FileWatcher(ioScope, Dispatchers.IO) + try { + val callbackReceived = CompletableDeferred() + var notifyCount = 0 + ioWatcher.onFileChanged { path -> + notifyCount++ + if (notifyCount > 1) { + callbackReceived.complete(path) + } + } + ioWatcher.start() + ioWatcher.addFile(testFile) + delay(200) + + testFile.deleteExisting() + + @Suppress("ConvertLongToDuration") + withTimeout(5_000) { + assertThat(callbackReceived.await()).isEqualTo(testFile) + } + } finally { + ioWatcher.stop() + ioScope.cancel() } } @@ -196,5 +281,4 @@ class FileWatcherTest { file.writeText(content) return file } - -} \ No newline at end of file +} diff --git a/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitorTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitorTest.kt index aea169d6..4d426f13 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitorTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitorTest.kt @@ -11,16 +11,17 @@ */ package com.redhat.devtools.gateway.kubeconfig -import com.redhat.devtools.gateway.kubeconfig.FileWatcher -import com.redhat.devtools.gateway.kubeconfig.KubeConfigUtils -import com.redhat.devtools.gateway.kubeconfig.KubeConfigMonitor import com.redhat.devtools.gateway.openshift.Cluster import io.mockk.every +import io.mockk.justRun import io.mockk.mockk +import io.mockk.spyk import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.cancel +import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -35,14 +36,13 @@ import kotlin.io.path.createFile @ExperimentalCoroutinesApi class KubeConfigMonitorTest { - // notsecret — cluster token literals in this class are invented test fixtures only. - @TempDir lateinit var tempDir: Path + private lateinit var testDispatcher: TestDispatcher private lateinit var testScope: TestScope - private lateinit var mockFileWatcher: FileWatcher - private lateinit var mockKubeConfigBuilder: KubeConfigUtils + private lateinit var fileWatcher: FileWatcher + private lateinit var mockKubeConfigUtils: KubeConfigUtils private lateinit var kubeconfigMonitor: KubeConfigMonitor private lateinit var kubeconfigPath1: Path @@ -50,15 +50,18 @@ class KubeConfigMonitorTest { @BeforeEach fun beforeEach() { - // given - testScope = TestScope(StandardTestDispatcher()) - mockFileWatcher = mockk(relaxed = true) - mockKubeConfigBuilder = mockk(relaxed = true) + testDispatcher = StandardTestDispatcher() + testScope = TestScope(testDispatcher) + // Real FileWatcher for path tracking; stub start() so its infinite poll/delay loop + // does not run on the test dispatcher (advanceUntilIdle would never complete). + fileWatcher = spyk(FileWatcher(testScope, testDispatcher)) + justRun { fileWatcher.start() } + mockKubeConfigUtils = mockk(relaxed = true) kubeconfigPath1 = tempDir.resolve("kubeconfig1.yaml").createFile() kubeconfigPath2 = tempDir.resolve("kubeconfig2.yaml").createFile() - kubeconfigMonitor = KubeConfigMonitor(testScope, mockFileWatcher, mockKubeConfigBuilder) + kubeconfigMonitor = KubeConfigMonitor(testScope, fileWatcher, mockKubeConfigUtils) } @AfterEach @@ -66,93 +69,142 @@ class KubeConfigMonitorTest { kubeconfigMonitor.stop() } + /** Subscribe to [KubeConfigMonitor.onClustersCollected] before start/refresh so emissions are counted. */ + private fun collectClusterEmissions(): MutableList> { + val emissions = mutableListOf>() + testScope.launch { + kubeconfigMonitor.onClustersCollected { emissions.add(it) } + } + return emissions + } + + @Test + fun `#start should initially parse and publish clusters`() = runTest(testDispatcher) { + val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + + val emissions = collectClusterEmissions() + advanceUntilIdle() + + kubeconfigMonitor.start() + advanceUntilIdle() + + assertThat(emissions).containsExactly(listOf(cluster1)) + assertThat(fileWatcher.getMonitoredFiles()).containsExactly(kubeconfigPath1) + // addFile notify + barrier refreshClusters coalesce to one parse of the full set + verify(exactly = 1) { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } + } + @Test - fun `#start should initially parse and publish clusters`() = testScope.runTest { - // given - val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) - every { mockKubeConfigBuilder.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + fun `#start coalesces multi-file addFile notifies into one full-list parse`() = runTest(testDispatcher) { + val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) + val cluster2 = Cluster(name = "obi-wan", url = "url2", token = null) + + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1, kubeconfigPath2) + every { + mockKubeConfigUtils.getClusters(match { it.toSet() == setOf(kubeconfigPath1, kubeconfigPath2) }) + } returns listOf(cluster1, cluster2) + + val emissions = collectClusterEmissions() + advanceUntilIdle() - // when kubeconfigMonitor.start() advanceUntilIdle() - // then - assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) - verify(exactly = 1) { mockFileWatcher.addFile(kubeconfigPath1) } - verify(exactly = 1) { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } + verify(exactly = 0) { mockKubeConfigUtils.getClusters(emptyList()) } + verify(exactly = 0) { + mockKubeConfigUtils.getClusters(match { it.size == 1 }) + } + verify(exactly = 1) { + mockKubeConfigUtils.getClusters(match { it.toSet() == setOf(kubeconfigPath1, kubeconfigPath2) }) + } + assertThat(emissions).containsExactly(listOf(cluster1, cluster2)) + assertThat(fileWatcher.getMonitoredFiles()).containsExactlyInAnyOrder(kubeconfigPath1, kubeconfigPath2) } @Test - fun `#onFileChanged should reparse and publish updated clusters`() = testScope.runTest { - // given + fun `#onFileChanged should reparse and publish updated clusters`() = runTest(testDispatcher) { val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) val cluster1Updated = Cluster(name = "skywalker", url = "url1", token = "token1") - every { mockKubeConfigBuilder.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + + val emissions = collectClusterEmissions() + advanceUntilIdle() kubeconfigMonitor.start() advanceUntilIdle() - assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) + assertThat(emissions).containsExactly(listOf(cluster1)) - // when - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1Updated) - kubeconfigMonitor.onFileChanged(kubeconfigPath1) // Simulate watcher callback + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1Updated) + kubeconfigMonitor.onFileChanged(kubeconfigPath1) advanceUntilIdle() - // then - assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1Updated) - verify(exactly = 2) { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } // Initial + update + assertThat(emissions).containsExactly(listOf(cluster1), listOf(cluster1Updated)) + verify(exactly = 2) { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } } @Test - fun `#updateMonitoredPaths should add and remove files based on KUBECONFIG env var`() = testScope.runTest { - // given + fun `#refreshClusters skips emit when cluster list is unchanged`() = runTest(testDispatcher) { + val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + + val emissions = collectClusterEmissions() + advanceUntilIdle() + + kubeconfigMonitor.start() + advanceUntilIdle() + assertThat(emissions).containsExactly(listOf(cluster1)) + + kubeconfigMonitor.refreshClusters() + advanceUntilIdle() + + assertThat(emissions).containsExactly(listOf(cluster1)) + verify(exactly = 2) { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } + } + + @Test + fun `#updateMonitoredPaths should add and remove files based on KUBECONFIG env var`() = runTest(testDispatcher) { val cluster1 = Cluster(name = "skywalker", url = "url1") val cluster2 = Cluster(name = "obi-wan", url = "url2") - // Initial KUBECONFIG - every { mockKubeConfigBuilder.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + + val emissions = collectClusterEmissions() + advanceUntilIdle() + kubeconfigMonitor.start() advanceUntilIdle() - assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) - verify(exactly = 1) { mockFileWatcher.addFile(kubeconfigPath1) } + assertThat(emissions).containsExactly(listOf(cluster1)) + assertThat(fileWatcher.getMonitoredFiles()).containsExactly(kubeconfigPath1) - // when: Change KUBECONFIG to include kubeconfigPath2 and remove kubeconfigPath1 - every { mockKubeConfigBuilder.getAllConfigFiles(any()) } returns listOf(kubeconfigPath2) - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath2)) } returns listOf(cluster2) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath2) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath2)) } returns listOf(cluster2) - // Manually trigger updateMonitoredPaths and reparse kubeconfigMonitor.updateMonitoredPaths() kubeconfigMonitor.refreshClusters() advanceUntilIdle() - // then - assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster2) - verify(exactly = 1) { - mockFileWatcher.removeFile(kubeconfigPath1) - mockFileWatcher.addFile(kubeconfigPath2) - } - verify(exactly = 1) { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath2)) } + assertThat(emissions).containsExactly(listOf(cluster1), listOf(cluster2)) + assertThat(fileWatcher.getMonitoredFiles()).containsExactly(kubeconfigPath2) + verify(exactly = 1) { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath2)) } } - @Test - fun `#stop should not cancel the provided scope`() = testScope.runTest { - // given + fun `#stop should not cancel the provided scope`() = runTest(testDispatcher) { val mockScope = mockk(relaxed = true) - val monitor = KubeConfigMonitor(mockScope, mockFileWatcher, mockKubeConfigBuilder) + val monitor = KubeConfigMonitor(mockScope, fileWatcher, mockKubeConfigUtils) every { mockScope.cancel() } returns Unit - // when monitor.stop() advanceUntilIdle() - // then verify(exactly = 0) { mockScope.cancel() } } -} \ No newline at end of file +} From 75649614a0682c1fe8bdf2065ed18807ec9b79bb Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Mon, 20 Jul 2026 17:52:37 +0200 Subject: [PATCH 3/4] fix: avoid IndexOutOfBounds after wizard dispose (crw-11741) Late kubeconfig cluster updates raced dispose clearing steps; stop monitoring on dispose and skip nav updates when the step list is empty. Signed-off-by: Andre Dietisheim Co-authored-by: Cursor --- .../gateway/view/DevSpacesWizardView.kt | 27 ++++++++++++-- .../view/steps/DevSpacesServerStepView.kt | 16 +++++++-- .../view/DevSpacesWizardViewDisposeTest.kt | 35 +++++++++++++++++++ 3 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 src/test/kotlin/com/redhat/devtools/gateway/view/DevSpacesWizardViewDisposeTest.kt diff --git a/src/main/kotlin/com/redhat/devtools/gateway/view/DevSpacesWizardView.kt b/src/main/kotlin/com/redhat/devtools/gateway/view/DevSpacesWizardView.kt index da69cd13..b9103b65 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/view/DevSpacesWizardView.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/view/DevSpacesWizardView.kt @@ -41,7 +41,9 @@ class DevSpacesWizardView(devSpacesContext: DevSpacesContext) : BorderLayoutPane devSpacesContext = devSpacesContext, enableNextButton = { enableNavigationButtons() }, triggerNextAction = { nextStep() }, - ) + ).also { + Disposer.register(this, it) + } ) steps.add( DevSpacesWorkspacesStepView(devSpacesContext) @@ -55,6 +57,13 @@ class DevSpacesWizardView(devSpacesContext: DevSpacesContext) : BorderLayoutPane } override fun dispose() { + // Children registered with Disposer are disposed before this runs. + // Call onDispose for any step that is not itself a Disposable. + steps.forEach { step -> + if (step !is Disposable) { + step.onDispose() + } + } steps.clear() } @@ -83,6 +92,7 @@ class DevSpacesWizardView(devSpacesContext: DevSpacesContext) : BorderLayoutPane } private fun nextStep() { + if (currentStep !in steps.indices) return val step = steps[currentStep] if (!step.isNavigationEnabled()) return @@ -107,6 +117,7 @@ class DevSpacesWizardView(devSpacesContext: DevSpacesContext) : BorderLayoutPane } private fun previousStep() { + if (currentStep !in steps.indices) return WizardAsyncWork.invalidatePending() if (!steps[currentStep].onPrevious()) return @@ -118,10 +129,12 @@ class DevSpacesWizardView(devSpacesContext: DevSpacesContext) : BorderLayoutPane } private fun applyStep(shift: Int) { + if (currentStep !in steps.indices) return remove(steps[currentStep].component) updateUI() currentStep += shift + if (currentStep !in steps.indices) return steps[currentStep].apply { addToCenter(component) nextButton.text = nextActionText @@ -133,6 +146,9 @@ class DevSpacesWizardView(devSpacesContext: DevSpacesContext) : BorderLayoutPane } private fun enableNavigationButtons(enabled: Boolean? = null) { + if (!canEnableNavigationButtons(steps.size, currentStep)) { + return + } val step = steps[currentStep] val navigationEnabled = enabled ?: step.isNavigationEnabled() previousButton.isEnabled = navigationEnabled @@ -146,4 +162,11 @@ class DevSpacesWizardView(devSpacesContext: DevSpacesContext) : BorderLayoutPane private fun isLastStep(): Boolean { return currentStep == steps.size - 1 } -} \ No newline at end of file +} + +/** + * Returns true when [currentStep] is a valid index into a wizard step list of [stepCount]. + * Used to ignore navigation updates after [DevSpacesWizardView.dispose] clears steps. + */ +internal fun canEnableNavigationButtons(stepCount: Int, currentStep: Int): Boolean = + currentStep in 0 until stepCount diff --git a/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesServerStepView.kt b/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesServerStepView.kt index 0cb04479..d5fcc4d2 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesServerStepView.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesServerStepView.kt @@ -11,6 +11,7 @@ */ package com.redhat.devtools.gateway.view.steps +import com.intellij.openapi.Disposable import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.ModalityState import com.intellij.openapi.application.PathManager @@ -61,7 +62,7 @@ class DevSpacesServerStepView( private var devSpacesContext: DevSpacesContext, private val enableNextButton: (() -> Unit)?, private val triggerNextAction: (() -> Unit)? = null, -) : DevSpacesWizardStep { +) : DevSpacesWizardStep, Disposable { private lateinit var allClusters: List @@ -83,6 +84,8 @@ class DevSpacesServerStepView( private lateinit var kubeconfigScope: CoroutineScope private lateinit var kubeconfigMonitor: KubeConfigMonitor private var kubeconfigMonitoringActive = false + @Volatile + private var disposed = false private val saveConfigCheckbox = JBCheckBox( DevSpacesBundle.message("connector.wizard_step.openshift_connection.checkbox.save_configuration") @@ -281,6 +284,11 @@ class DevSpacesServerStepView( findStrategy()?.startMonitoring(component) } + override fun dispose() { + disposed = true + onDispose() + } + override fun onDispose() { findStrategy()?.stopMonitoring() @@ -391,7 +399,8 @@ class DevSpacesServerStepView( } } - private fun onClustersChanged(): suspend (List) -> Unit = { updatedClusters -> + private fun onClustersChanged(): suspend (List) -> Unit = action@{ updatedClusters -> + if (disposed) return@action this.allClusters = updatedClusters if (updatedClusters.isNotEmpty()) { val kubeConfigCurrentCluster = withContext(Dispatchers.IO) { @@ -399,6 +408,7 @@ class DevSpacesServerStepView( } ApplicationManager.getApplication().invokeLater( { + if (disposed) return@invokeLater currentContextClusterName = kubeConfigCurrentCluster val previouslySelected = tfServer.selectedItem as? Cluster? setClusters(updatedClusters) @@ -409,7 +419,7 @@ class DevSpacesServerStepView( setSelectedAuthTab() enableSaveConfigCheckbox() }, - ModalityState.stateForComponent(component) + ModalityState.any() ) } } diff --git a/src/test/kotlin/com/redhat/devtools/gateway/view/DevSpacesWizardViewDisposeTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/view/DevSpacesWizardViewDisposeTest.kt new file mode 100644 index 00000000..1404cef5 --- /dev/null +++ b/src/test/kotlin/com/redhat/devtools/gateway/view/DevSpacesWizardViewDisposeTest.kt @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2026 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package com.redhat.devtools.gateway.view + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +class DevSpacesWizardViewDisposeTest { + + @Test + fun `canEnableNavigationButtons is false when steps were cleared`() { + assertThat(canEnableNavigationButtons(stepCount = 0, currentStep = 0)).isFalse() + } + + @Test + fun `canEnableNavigationButtons is false when currentStep is out of range`() { + assertThat(canEnableNavigationButtons(stepCount = 2, currentStep = 2)).isFalse() + assertThat(canEnableNavigationButtons(stepCount = 2, currentStep = -1)).isFalse() + } + + @Test + fun `canEnableNavigationButtons is true for a valid step index`() { + assertThat(canEnableNavigationButtons(stepCount = 2, currentStep = 0)).isTrue() + assertThat(canEnableNavigationButtons(stepCount = 2, currentStep = 1)).isTrue() + } +} From 0c580863743a329adc4b9890765881459c3b879b Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Mon, 20 Jul 2026 17:52:37 +0200 Subject: [PATCH 4/4] refactor: improve thin-client connection lifecycle (crw-11741) Clear workspace tracking on disconnect before remote stop waits; gate Connect on Running only. Keep name/namespace bookkeeping. Extract connect() sub-methods to shorten the 130-line connect() flow: - waitUntilServerReady: poll workspace & remote IDE readiness - setupPortForwarding: find free port, forward to pod - startThinClient: create client, wire presence/closed advisors - waitForThinClientConnect: timeout-guarded presence wait Signed-off-by: Andre Dietisheim Co-authored-by: Cursor --- .../devtools/gateway/DevSpacesConnection.kt | 337 ++++++++++++------ .../devtools/gateway/DevSpacesContext.kt | 22 +- .../gateway/devworkspace/DevWorkspace.kt | 9 +- .../gateway/server/RemoteIDEServer.kt | 51 ++- .../view/steps/DevSpacesServerStepView.kt | 2 +- .../view/steps/DevSpacesWorkspacesStepView.kt | 15 +- .../devtools/gateway/DevSpacesContextTest.kt | 78 ++++ .../gateway/devworkspace/DevWorkspaceTest.kt | 23 +- .../gateway/server/RemoteIDEServerTest.kt | 2 +- 9 files changed, 398 insertions(+), 141 deletions(-) create mode 100644 src/test/kotlin/com/redhat/devtools/gateway/DevSpacesContextTest.kt diff --git a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt index 6fb78c74..b8510955 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024-2025 Red Hat, Inc. + * Copyright (c) 2024-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -18,6 +18,7 @@ import com.intellij.openapi.project.ex.ProjectManagerEx import com.jetbrains.gateway.thinClientLink.LinkedClientManager import com.jetbrains.gateway.thinClientLink.ThinClientHandle import com.jetbrains.rd.util.lifetime.Lifetime +import com.redhat.devtools.gateway.devworkspace.DevWorkspace import com.redhat.devtools.gateway.devworkspace.DevWorkspacePatch import com.redhat.devtools.gateway.devworkspace.DevWorkspaceRestart import com.redhat.devtools.gateway.devworkspace.DevWorkspaces @@ -29,6 +30,7 @@ import com.redhat.devtools.gateway.util.ProgressCountdown import com.redhat.devtools.gateway.util.isCancellationException import com.redhat.devtools.gateway.view.ui.Dialogs import io.kubernetes.client.openapi.ApiClient +import io.kubernetes.client.openapi.models.V1Pod import kotlinx.coroutines.* import java.io.Closeable import java.io.IOException @@ -37,12 +39,29 @@ import java.net.URI import java.util.concurrent.CancellationException import java.util.concurrent.atomic.AtomicBoolean +/** + * Thin-client connection lifecycle. + * + * Thin-client close always ends the connect wait; [tearDownConnection] runs only if the + * connection is already live ([connectionLive]). Failures during connect are cleaned up by + * [connect]'s catch path. + * + * Connect enablement in the wizard is based on workspace Running state (not + * [DevSpacesContext.activeWorkspaces]), because IDEA often keeps the connector view + * after Guest close and thin-client signals are unreliable for UI gating. + * + * Still clear [DevSpacesContext.activeWorkspaces] when the connection ends (before optional + * remote stop) for tooltips / bookkeeping. + */ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { + /** Ensures [tearDownConnection] runs at most once for this connect attempt. */ + private val tearDownStarted = AtomicBoolean(false) + @Throws(Exception::class) @Suppress("UnstableApiUsage") suspend fun connect( onConnected: () -> Unit, - onDisconnected: () -> Unit, + onConnectionEnded: () -> Unit, onDevWorkspaceStopped: () -> Unit, onProgress: ((value: ProgressCountdown.ProgressEvent) -> Unit)? = null, checkCancelled: (() -> Unit)? = null, @@ -50,115 +69,39 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { registerRestartWatcher: Boolean? = true ): ThinClientHandle { val workspace = devSpacesContext.devWorkspace + tearDownStarted.set(false) devSpacesContext.addWorkspace(workspace) var remoteIdeServer: RemoteIDEServer? = null var forwarder: Closeable? = null var client: ThinClientHandle? = null + val connectionLive = AtomicBoolean(false) return try { - var remoteIdeServerStatus: RemoteIDEServerStatus = RemoteIDEServerStatus.empty() - while (!remoteIdeServerStatus.isReady) { - checkCancelled?.invoke() - onProgress?.invoke(ProgressCountdown.ProgressEvent( - message = "Waiting for the workspace to get started...", - countdownSeconds = DevWorkspaces.RUNNING_TIMEOUT)) - - DevWorkspaces(devSpacesContext.client) - .startAndWait( - devSpacesContext.devWorkspace.namespace, - devSpacesContext.devWorkspace.name, - checkCancelled = checkCancelled) - - checkCancelled?.invoke() - onProgress?.invoke(ProgressCountdown.ProgressEvent( - message = "Waiting for the workspace to get ready...", - countdownSeconds = RemoteIDEServer.readyTimeout)) - - remoteIdeServer = RemoteIDEServer(devSpacesContext) - remoteIdeServerStatus = runCatching { - remoteIdeServer.apply { waitServerReady(checkCancelled) }.getStatus(checkCancelled) - }.getOrElse { e -> - if (e.isCancellationException()) throw e - RemoteIDEServerStatus.empty() - } - - checkCancelled?.invoke() - if (!remoteIdeServerStatus.isReady) { - val restartWorkspace = Dialogs.ideNotResponding(modalityState) - - if (restartWorkspace) { - // User chose "Restart Pod": stop the Pod and try starting from scratch - DevWorkspaces(devSpacesContext.client).stopAndWait( - devSpacesContext.devWorkspace.namespace, - devSpacesContext.devWorkspace.name, - checkCancelled = checkCancelled - ) - continue - } else { - // User chose "Cancel Connection" - throw CancellationException("User cancelled the operation") - } - } - } + remoteIdeServer = waitUntilServerReady(checkCancelled, onProgress, modalityState) checkCancelled?.invoke() - val joinLink = remoteIdeServerStatus.joinLink + val joinLink = remoteIdeServer.getStatus(checkCancelled).joinLink ?: throw IOException("Could not connect, workspace IDE is not ready. No join link present.") - check(remoteIdeServer != null) checkCancelled?.invoke() onProgress?.invoke(ProgressCountdown.ProgressEvent( message = "Waiting for the workspace IDE client to start...")) - val pods = DevWorkspacePods(devSpacesContext.client) - val localPort = findFreePort() - forwarder = pods.forward(remoteIdeServer.pod, localPort, 5990) - pods.waitForForwardReady(localPort) + val (fwd, localPort) = setupPortForwarding(remoteIdeServer.pod) + forwarder = fwd val effectiveJoinLink = joinLink.replace(":5990", ":$localPort") - - val lifetimeDef = Lifetime.Eternal.createNested() - lifetimeDef.lifetime.onTermination { onClientClosed( client, onDisconnected, onDevWorkspaceStopped, remoteIdeServer, forwarder) } - - val finished = AtomicBoolean(false) + val connectWaitDone = AtomicBoolean(false) checkCancelled?.invoke() - client = LinkedClientManager - .getInstance() - .startNewClient( - Lifetime.Eternal, - URI(effectiveJoinLink), - "", - onConnected, // Triggers enableButtons() via view - false - ) - - client.onClientPresenceChanged.advise(client.lifetime) { finished.set(true) } - client.clientClosed.advise(client.lifetime) { - onClientClosed(client, onDisconnected, onDevWorkspaceStopped, remoteIdeServer, forwarder) - finished.set(true) - } - client.clientFailedToOpenProject.advise(client.lifetime) { - onClientClosed(client, onDisconnected, onDevWorkspaceStopped, remoteIdeServer, forwarder) - finished.set(true) - } + client = startThinClient( + URI(effectiveJoinLink), workspace, onConnected, onConnectionEnded, onDevWorkspaceStopped, + remoteIdeServer, forwarder, connectWaitDone, connectionLive + ) - @Suppress("ConvertLongToDuration") - val success = withTimeoutOrNull(60_000L) { - while (!finished.get()) { - checkCancelled?.invoke() - delay(200L) - } - true - } ?: false + waitForThinClientConnect(client, connectWaitDone, checkCancelled) - // Check if the thin client has opened - check(success && client.clientPresent) { - "Could not connect, workspace IDE is not ready." - } - - // Watch for restart annotation on the DevWorkspace if (registerRestartWatcher == true) { watchRestartAnnotation( workspace.namespace, @@ -168,17 +111,54 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { ) } + connectionLive.set(true) onConnected() client } catch (e: Exception) { runCatching { client?.close() } - onClientClosed(client, onDisconnected, onDevWorkspaceStopped, remoteIdeServer, forwarder) + tearDownConnection( + client, workspace, onConnectionEnded, onDevWorkspaceStopped, remoteIdeServer, forwarder + ) throw e } } + /** + * Thin client closed or failed to open. + * Always ends the connect-wait loop. Calls [tearDownConnection] only if the connection is + * already live; failures during connect are cleaned up by [connect]'s catch. + */ + @Suppress("UnstableApiUsage") + private fun onThinClientClosed( + connectWaitDone: AtomicBoolean, + connectionLive: AtomicBoolean, + thinClient: ThinClientHandle, + workspace: DevWorkspace, + onConnectionEnded: () -> Unit, + onDevWorkspaceStopped: () -> Unit, + remoteIdeServer: RemoteIDEServer?, + forwarder: Closeable?, + ) { + connectWaitDone.set(true) + if (connectionLive.get()) { + tearDownConnection( + thinClient, + workspace, + onConnectionEnded, + onDevWorkspaceStopped, + remoteIdeServer, + forwarder + ) + } + } + @Suppress("UnstableApiUsage") - private fun watchRestartAnnotation(namespace: String, workspaceName: String, kubeClient: ApiClient, thinClient: ThinClientHandle) { + private fun watchRestartAnnotation( + namespace: String, + workspaceName: String, + kubeClient: ApiClient, + thinClient: ThinClientHandle + ) { val restartWatchScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) RestartDevWorkspaceAnnotationWatch( onRestartAnnotated(thinClient), @@ -203,17 +183,34 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { } } + /** + * Ends the local connection: clear "already connected" tracking, notify the UI, then + * optionally stop the DevWorkspace (restart annotation / Close and Stop). + * + * Uses the [workspace] captured at [connect] start so teardown stays correct if the wizard + * selection changes while the Guest is open. + * + * Idempotent across thin-client close, connect failure, and duplicate signals. + */ @Suppress("UnstableApiUsage") - private fun onClientClosed( + private fun tearDownConnection( client: ThinClientHandle? = null, - onDisconnected: () -> Unit, + workspace: DevWorkspace, + onConnectionEnded: () -> Unit, onDevWorkspaceStopped: () -> Unit, remoteIdeServer: RemoteIDEServer?, forwarder: Closeable? ) { + if (!tearDownStarted.compareAndSet(false, true)) { + return + } + // Clear tracking + refresh UI before any remote wait so Connect is not stuck + // behind waitServerTerminated (up to 10s) when the wizard stays open in IDEA. + devSpacesContext.removeWorkspace(workspace) + runCatching { onConnectionEnded() } + CoroutineScope(Dispatchers.IO).launch { runCatching { client?.close() } - val workspace = devSpacesContext.devWorkspace val workspacePatch = DevWorkspacePatch( workspace.namespace, workspace.name, @@ -224,28 +221,15 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { ) try { if (workspacePatch.hasRestartAnnotation()) { - /** - * user triggered restart - * logic to restart workspace is in [onRestartAnnotated] - * The annotation will be cleaned up by DevWorkspaceRestart - */ closeAllProjects() } else if (true == remoteIdeServer?.waitServerTerminated()) { - /** - * user closed IDE and clicked "Close and Stop" - */ DevWorkspaces(devSpacesContext.client) .stop(workspace.namespace, workspace.name) .also { onDevWorkspaceStopped() } } } finally { - runCatching { - forwarder?.close() - }.onFailure { e -> - thisLogger().debug("Failed to close port forwarder", e) - } - devSpacesContext.removeWorkspace(workspace) - runCatching { onDisconnected() } + runCatching { forwarder?.close() } + .onFailure { e -> thisLogger().debug("Failed to close port forwarder", e) } } } } @@ -270,4 +254,137 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { return socket.localPort } } + + /** + * Handles the case where the remote IDE server is not ready after starting the workspace. + * Shows a dialog and either restarts the workspace (return true) or indicates cancellation (return false). + */ + private fun handleServerNotReady( + checkCancelled: (() -> Unit)?, + modalityState: ModalityState? + ): Boolean { + val restartWorkspace = Dialogs.ideNotResponding(modalityState) + if (restartWorkspace) { + DevWorkspaces(devSpacesContext.client).stopAndWait( + devSpacesContext.devWorkspace.namespace, + devSpacesContext.devWorkspace.name, + checkCancelled = checkCancelled + ) + } + return restartWorkspace + } + + private suspend fun waitUntilServerReady( + checkCancelled: (() -> Unit)?, + onProgress: ((ProgressCountdown.ProgressEvent) -> Unit)?, + modalityState: ModalityState? + ): RemoteIDEServer { + var remoteIdeServerStatus: RemoteIDEServerStatus = RemoteIDEServerStatus.empty() + var remoteIdeServer: RemoteIDEServer? = null + + while (!remoteIdeServerStatus.isReady) { + checkCancelled?.invoke() + onProgress?.invoke(ProgressCountdown.ProgressEvent( + message = "Waiting for the workspace to get started...", + countdownSeconds = DevWorkspaces.RUNNING_TIMEOUT)) + + DevWorkspaces(devSpacesContext.client) + .startAndWait( + devSpacesContext.devWorkspace.namespace, + devSpacesContext.devWorkspace.name, + checkCancelled = checkCancelled) + + checkCancelled?.invoke() + onProgress?.invoke(ProgressCountdown.ProgressEvent( + message = "Waiting for the workspace to get ready...", + countdownSeconds = RemoteIDEServer.readyTimeout)) + + remoteIdeServer = RemoteIDEServer(devSpacesContext) + remoteIdeServerStatus = runCatching { + remoteIdeServer.apply { waitServerReady(checkCancelled) }.getStatus(checkCancelled) + }.getOrElse { e -> + if (e.isCancellationException()) throw e + RemoteIDEServerStatus.empty() + } + + checkCancelled?.invoke() + if (!remoteIdeServerStatus.isReady) { + if (handleServerNotReady(checkCancelled, modalityState)) { + continue + } else { + throw CancellationException("User cancelled the operation") + } + } + } + return remoteIdeServer!! + } + + private fun setupPortForwarding(pod: V1Pod): Pair { + val pods = DevWorkspacePods(devSpacesContext.client) + val localPort = findFreePort() + val forwarder = pods.forward(pod, localPort, 5990) + pods.waitForForwardReady(localPort) + return forwarder to localPort + } + + @Suppress("UnstableApiUsage") + private fun startThinClient( + effectiveJoinLink: URI, + workspace: DevWorkspace, + onConnected: () -> Unit, + onConnectionEnded: () -> Unit, + onDevWorkspaceStopped: () -> Unit, + remoteIdeServer: RemoteIDEServer?, + forwarder: Closeable?, + connectWaitDone: AtomicBoolean, + connectionLive: AtomicBoolean, + ): ThinClientHandle { + val thinClient = LinkedClientManager + .getInstance() + .startNewClient( + Lifetime.Eternal, + effectiveJoinLink, + "", + onConnected, + false + ) + + thinClient.onClientPresenceChanged.advise(thinClient.lifetime) { connectWaitDone.set(true) } + + fun notifyThinClientClosed() { + onThinClientClosed( + connectWaitDone, + connectionLive, + thinClient, + workspace, + onConnectionEnded, + onDevWorkspaceStopped, + remoteIdeServer, + forwarder + ) + } + thinClient.clientClosed.advise(thinClient.lifetime) { notifyThinClientClosed() } + thinClient.clientFailedToOpenProject.advise(thinClient.lifetime) { notifyThinClientClosed() } + + return thinClient + } + + private suspend fun waitForThinClientConnect( + thinClient: ThinClientHandle, + connectWaitDone: AtomicBoolean, + checkCancelled: (() -> Unit)? + ) { + @Suppress("ConvertLongToDuration") + val success = withTimeoutOrNull(60_000L) { + while (!connectWaitDone.get()) { + checkCancelled?.invoke() + delay(200L) + } + true + } ?: false + + check(success && thinClient.clientPresent) { + "Could not connect, workspace IDE is not ready." + } + } } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt index 4641c179..8c8e3867 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt @@ -11,6 +11,7 @@ */ package com.redhat.devtools.gateway +import com.intellij.openapi.diagnostic.thisLogger import com.redhat.devtools.gateway.devworkspace.DevWorkspace import com.redhat.devtools.gateway.openshift.Cluster import io.kubernetes.client.openapi.ApiClient @@ -37,17 +38,34 @@ class DevSpacesContext { fun addWorkspace(workspace: DevWorkspace) { synchronized(activeWorkspaces) { - if (activeWorkspaces.contains(workspace)) { + if (activeWorkspaces.any { sameWorkspace(it, workspace) }) { return } activeWorkspaces.add(workspace) + thisLogger().info( + "Tracking active connection to workspace ${workspace.namespace}/${workspace.name}" + ) } } fun removeWorkspace(currentWorkspace: DevWorkspace) { synchronized(activeWorkspaces) { - activeWorkspaces.remove(currentWorkspace) + val removed = activeWorkspaces.removeAll { sameWorkspace(it, currentWorkspace) } + if (removed) { + thisLogger().info( + "Stopped tracking connection to workspace " + + "${currentWorkspace.namespace}/${currentWorkspace.name}" + ) + } + } + } + + fun isWorkspaceActive(workspace: DevWorkspace): Boolean { + synchronized(activeWorkspaces) { + return activeWorkspaces.any { sameWorkspace(it, workspace) } } } + private fun sameWorkspace(a: DevWorkspace, b: DevWorkspace): Boolean = + a.namespace == b.namespace && a.name == b.name } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspace.kt b/src/main/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspace.kt index 4a65fe0a..e6581ba6 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspace.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspace.kt @@ -80,15 +80,12 @@ data class DevWorkspace( other as DevWorkspace return metadata.name == other.metadata.name && - metadata.namespace == other.metadata.namespace && - metadata.annotations == other.metadata.annotations && - labels == other.labels + metadata.namespace == other.metadata.namespace } override fun hashCode(): Int { - var result = metadata.hashCode() - result = 31 * result + spec.hashCode() - result = 31 * result + status.hashCode() + var result = metadata.name.hashCode() + result = 31 * result + metadata.namespace.hashCode() return result } } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt b/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt index a854eb71..abf1a06d 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024-2025 Red Hat, Inc. + * Copyright (c) 2024-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -30,7 +30,17 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { private var container: V1Container companion object { - var readyTimeout: Long = 60 // seconds + /** Overall wait for remote IDE readiness (wizard "Checking workspace IDE Status..."). */ + var readyTimeout: Long = 120 // seconds + + /** + * Per-probe exec budget while polling. Must stay well below [readyTimeout] so a slow/hung + * status exec cannot burn the entire wait (CRW-11119). + */ + const val STATUS_EXEC_TIMEOUT: Long = 15 // seconds + + /** Number of consecutive pod-refresh failures before emitting a warning. */ + const val REFRESH_FAILURE_WARNING_THRESHOLD: Int = 10 } init { @@ -63,7 +73,7 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { "-c", "/idea-server/bin/remote-dev-server.sh status \$PROJECT_SOURCE | awk '/STATUS:/{p=1; next} p'" ), - timeout = readyTimeout + timeout = STATUS_EXEC_TIMEOUT ).trim() checkCancelled?.invoke() @@ -84,7 +94,7 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { return doWaitServerState(true, timeout, checkCancelled) .also { if (!it) throw IOException( - "Workspace IDE is not ready after $readyTimeout seconds.", + "Workspace IDE is not ready after $timeout seconds.", ) } } @@ -92,9 +102,28 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { @Throws(CancellationException::class) private suspend fun isServerState( isReadyState: Boolean, - checkCancelled: (() -> Unit)? = null + checkCancelled: (() -> Unit)? = null, + refreshPodBeforeCheck: Boolean = false, + refreshFailures: IntArray = intArrayOf(0), ): Boolean { return try { + if (refreshPodBeforeCheck) { + runCatching { + pod = findPod() + container = findContainer() + }.onFailure { e -> + if (e.isCancellationException()) throw e + refreshFailures[0]++ + thisLogger().debug("Failed to refresh workspace pod during IDE state check", e) + if (refreshFailures[0] == REFRESH_FAILURE_WARNING_THRESHOLD) { + thisLogger().warn( + "Pod/container refresh has failed ${refreshFailures[0]} consecutive times; " + + "stale pod references may cause incorrect status checks" + ) + } + return false + }.onSuccess { refreshFailures[0] = 0 } + } getStatus(checkCancelled).isReady == isReadyState } catch (e: Exception) { if (e.isCancellationException()) throw e @@ -110,7 +139,7 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { /** * Waits for the server to have or not have projects according to the given parameter. - * Times out the wait if the expected state is not reached within specified timeout (default is 60 seconds). + * Times out the wait if the expected state is not reached within specified timeout. * * @param isReadyState True if server up and running with the projects all set are expected, False otherwise, * @return True if the expected state is achieved within the timeout, False otherwise. @@ -123,9 +152,17 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { ): Boolean = @Suppress("ConvertLongToDuration") withTimeoutOrNull(timeout * 1000L) { + val refreshFailures = intArrayOf(0) while (true) { checkCancelled?.invoke() - if (isServerState(isReadyState, checkCancelled)) { + if (isServerState( + isReadyState, + checkCancelled, + // Re-resolve pod while waiting for ready so a recycled pod is not missed. + refreshPodBeforeCheck = isReadyState, + refreshFailures, + ) + ) { return@withTimeoutOrNull true } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesServerStepView.kt b/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesServerStepView.kt index d5fcc4d2..22ec64ca 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesServerStepView.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesServerStepView.kt @@ -419,7 +419,7 @@ class DevSpacesServerStepView( setSelectedAuthTab() enableSaveConfigCheckbox() }, - ModalityState.any() + ModalityState.stateForComponent(component) ) } } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesWorkspacesStepView.kt b/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesWorkspacesStepView.kt index 274153ca..a1966ee3 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesWorkspacesStepView.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/view/steps/DevSpacesWorkspacesStepView.kt @@ -129,6 +129,7 @@ class DevSpacesWorkspacesStepView( watchManager = WorkspacesWatch(devSpacesContext.client, listDWDataModel) refreshAndWatchAllDevWorkspaces() + enableButtons() } override fun onPrevious(): Boolean { @@ -310,8 +311,13 @@ class DevSpacesWorkspacesStepView( val remoteIdeServer = RemoteIDEServer(devSpacesContext) status = runBlocking { + // Progress text stays visible for the whole wait; update so a long poll + // does not look frozen while RemoteIDEServer probes status. + progressIndicator.text = + "Waiting for workspace IDE to become ready (up to ${RemoteIDEServer.readyTimeout}s)..." remoteIdeServer.waitServerReady(checkCancelled) - remoteIdeServer.getStatus() + progressIndicator.text = "Reading workspace IDE status..." + remoteIdeServer.getStatus(checkCancelled) } } catch (e: Exception) { if (e.isCancellationException()) { @@ -423,7 +429,10 @@ class DevSpacesWorkspacesStepView( override fun isNextEnabled(): Boolean { val workspace = getSelectedWorkspace() ?: return false - return isRunning(workspace) && !isAlreadyConnected(workspace) + // Do not gate on "already connected": in IDEA the wizard often stays open after + // Guest close, and thin-client / activeWorkspaces tracking is too unreliable to + // keep Connect disabled. Tooltip still reflects isAlreadyConnected. + return isRunning(workspace) } private fun isStopped(workspace: DevWorkspace?): Boolean { @@ -439,7 +448,7 @@ class DevSpacesWorkspacesStepView( */ private fun isAlreadyConnected(workspace: DevWorkspace?): Boolean { if (workspace == null) return false - return devSpacesContext.activeWorkspaces.any { it.namespace == workspace.namespace && it.name == workspace.name } + return devSpacesContext.isWorkspaceActive(workspace) } class DevWorkspaceListRenderer : ColoredListCellRenderer() { diff --git a/src/test/kotlin/com/redhat/devtools/gateway/DevSpacesContextTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/DevSpacesContextTest.kt new file mode 100644 index 00000000..6775f256 --- /dev/null +++ b/src/test/kotlin/com/redhat/devtools/gateway/DevSpacesContextTest.kt @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2026 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package com.redhat.devtools.gateway + +import com.redhat.devtools.gateway.devworkspace.DevWorkspace +import com.redhat.devtools.gateway.devworkspace.DevWorkspaceObjectMeta +import com.redhat.devtools.gateway.devworkspace.DevWorkspaceSpec +import com.redhat.devtools.gateway.devworkspace.DevWorkspaceStatus +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +class DevSpacesContextTest { + + @Test + fun `removeWorkspace clears entry when instance differs but name and namespace match`() { + val context = DevSpacesContext() + val connected = createDevWorkspace( + name = "ws", + namespace = "ns", + phase = "Running", + annotations = mapOf("a" to "1") + ) + val refreshed = createDevWorkspace( + name = "ws", + namespace = "ns", + phase = "Stopped", + annotations = mapOf("a" to "2") + ) + + context.addWorkspace(connected) + assertThat(context.isWorkspaceActive(refreshed)).isTrue() + + context.removeWorkspace(refreshed) + + assertThat(context.activeWorkspaces).isEmpty() + assertThat(context.isWorkspaceActive(connected)).isFalse() + } + + @Test + fun `addWorkspace dedupes by name and namespace`() { + val context = DevSpacesContext() + val first = createDevWorkspace(name = "ws", namespace = "ns", phase = "Running") + val second = createDevWorkspace(name = "ws", namespace = "ns", phase = "Starting") + + context.addWorkspace(first) + context.addWorkspace(second) + + assertThat(context.activeWorkspaces).hasSize(1) + } + + private fun createDevWorkspace( + name: String, + namespace: String, + phase: String = "Running", + annotations: Map = emptyMap() + ): DevWorkspace { + return DevWorkspace( + DevWorkspaceObjectMeta( + name = name, + namespace = namespace, + uid = "uid-$name", + annotations = annotations, + labels = emptyMap() + ), + DevWorkspaceSpec(started = phase != "Stopped"), + DevWorkspaceStatus(phase = phase) + ) + } +} diff --git a/src/test/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspaceTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspaceTest.kt index a76060e1..dbda52cd 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspaceTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspaceTest.kt @@ -74,29 +74,28 @@ class DevWorkspaceTest { } @Test - fun `equals returns true for workspaces with same name, namespace, annotations and labels`() { + fun `equals returns true for workspaces with same name and namespace`() { // given - val annotations = mapOf("key1" to "value1") - val labels = mapOf("label1" to "labelValue1") val workspace1 = createDevWorkspace( name = "test-workspace", namespace = "test-ns", - annotations = annotations, - labels = labels + annotations = mapOf("key1" to "value1"), + labels = mapOf("label1" to "labelValue1") ) val workspace2 = createDevWorkspace( name = "test-workspace", namespace = "test-ns", - annotations = annotations, - labels = labels + annotations = mapOf("key1" to "value2"), + labels = mapOf("label1" to "other") ) // when/then assertThat(workspace1).isEqualTo(workspace2) + assertThat(workspace1.hashCode()).isEqualTo(workspace2.hashCode()) } @Test - fun `equals returns false for workspaces with different annotations`() { + fun `equals returns true when annotations differ but name and namespace match`() { // given val workspace1 = createDevWorkspace( name = "test-workspace", @@ -112,11 +111,12 @@ class DevWorkspaceTest { ) // when/then - assertThat(workspace1).isNotEqualTo(workspace2) + assertThat(workspace1).isEqualTo(workspace2) + assertThat(workspace1.hashCode()).isEqualTo(workspace2.hashCode()) } @Test - fun `equals returns false for workspaces with different labels`() { + fun `equals returns true when labels differ but name and namespace match`() { // given val workspace1 = createDevWorkspace( name = "test-workspace", @@ -132,7 +132,8 @@ class DevWorkspaceTest { ) // when/then - assertThat(workspace1).isNotEqualTo(workspace2) + assertThat(workspace1).isEqualTo(workspace2) + assertThat(workspace1.hashCode()).isEqualTo(workspace2.hashCode()) } @Test diff --git a/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt index 73059c28..6001cd34 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt @@ -210,7 +210,7 @@ class RemoteIDEServerTest { remoteIDEServer.getStatus() } - // then — exec should never be called, preventing 60s hang on stuck pod + // then — exec should never be called, preventing a long hang on stuck pod assertThat(result).isEqualTo(RemoteIDEServerStatus.empty()) coVerify(exactly = 0) { anyConstructed().exec(any(), any(), any(), any(), any())