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 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/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/BaseClientBuilder.kt b/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/BaseClientBuilder.kt index 33437d08..d9fe8bb5 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/BaseClientBuilder.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/BaseClientBuilder.kt @@ -14,6 +14,7 @@ package com.redhat.devtools.gateway.openshift.apiclient import io.kubernetes.client.openapi.ApiClient import okhttp3.OkHttpClient import okhttp3.Protocol +import java.net.Proxy import java.util.concurrent.TimeUnit import javax.net.ssl.SSLContext import javax.net.ssl.X509TrustManager @@ -55,9 +56,11 @@ abstract class BaseClientBuilder : OpenShiftClientBuilder { /** * Creates an [OkHttpClient] with the given [sslContext] and [trustManager]. * Uses HTTP/1.1 (not HTTP/2): some OpenShift clusters hang on HTTP/2. + * Bypasses JVM proxy settings — the Kubernetes API server is always accessed directly. */ protected fun createHttpClient(sslContext: SSLContext, trustManager: X509TrustManager): OkHttpClient = OkHttpClient.Builder() + .proxy(Proxy.NO_PROXY) .sslSocketFactory(sslContext.socketFactory, trustManager) .connectTimeout(DEFAULT_HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) .readTimeout(DEFAULT_HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) diff --git a/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/LinkClientBuilder.kt b/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/LinkClientBuilder.kt index 65f971f2..4b4000c3 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/LinkClientBuilder.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/LinkClientBuilder.kt @@ -15,6 +15,7 @@ import com.intellij.openapi.diagnostic.thisLogger import com.redhat.devtools.gateway.kubeconfig.KubeConfigUtils import io.kubernetes.client.util.ClientBuilder import io.kubernetes.client.openapi.ApiClient +import java.net.Proxy /** * Builder for API clients used in the deep-link flow (no wizard). @@ -27,25 +28,32 @@ class LinkClientBuilder( val paths = configUtils.getAllConfigFiles() if (paths.isEmpty()) { thisLogger().debug("No effective kubeconfig found. Falling back to default ApiClient.") - return ClientBuilder.defaultClient() + return bypassProxy(ClientBuilder.defaultClient()) } return try { val allConfigs = configUtils.getAllConfigs(paths) if (allConfigs.isEmpty()) { thisLogger().debug("No valid kubeconfig content found. Falling back to default ApiClient.") - return ClientBuilder.defaultClient() + return bypassProxy(ClientBuilder.defaultClient()) } val kubeConfig = configUtils.mergeConfigs(allConfigs) - val client = ClientBuilder.kubeconfig(kubeConfig).build() + val client = bypassProxy(ClientBuilder.kubeconfig(kubeConfig).build()) applyReadTimeout(client) } catch (e: Exception) { thisLogger().debug( "Failed to build effective Kube config from discovered files due to error: ${e.message}. " + "Falling back to the default ApiClient." ) - ClientBuilder.defaultClient() + bypassProxy(ClientBuilder.defaultClient()) } } + + private fun bypassProxy(client: ApiClient): ApiClient { + client.httpClient = client.httpClient.newBuilder() + .proxy(Proxy.NO_PROXY) + .build() + return client + } } 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..1038cf0e 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,16 @@ */ 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.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -35,14 +35,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 +49,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 @@ -67,92 +69,117 @@ class KubeConfigMonitorTest { } @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 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) - // when kubeconfigMonitor.start() advanceUntilIdle() - // then assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) - verify(exactly = 1) { mockFileWatcher.addFile(kubeconfigPath1) } - verify(exactly = 1) { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } + 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 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) + + kubeconfigMonitor.start() + advanceUntilIdle() + + 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(kubeconfigMonitor.getCurrentClusters()).containsExactlyInAnyOrder(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) kubeconfigMonitor.start() advanceUntilIdle() assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(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 + 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) + + kubeconfigMonitor.start() + advanceUntilIdle() + assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) + + kubeconfigMonitor.refreshClusters() + advanceUntilIdle() + + assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(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) kubeconfigMonitor.start() advanceUntilIdle() assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) - verify(exactly = 1) { mockFileWatcher.addFile(kubeconfigPath1) } + 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(fileWatcher.getMonitoredFiles()).containsExactly(kubeconfigPath2) + verify(atLeast = 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 +}