correctly synchronize accessing client when config changes (#640)

src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt

@@ -143,6 +143,7 @@ open class AllContexts(
 				this.client.get()
 			)
 			this.current?.close()
+			newClient.config.save().join()
 			all.clear() // causes reload of all contexts when accessed afterwards
 			val newCurrent = this.current // gets new current from all
 			if (toWatch != null) {
@@ -197,7 +198,6 @@ open class AllContexts(
 	private fun replaceClient(new: ClientAdapter<out KubernetesClient>, old: ClientAdapter<out KubernetesClient>?)
 			: ClientAdapter<out KubernetesClient> {
 		old?.close()
-		new.config.save()
 		this.client.set(new)
 		return new
 	}
@@ -247,14 +247,16 @@ open class AllContexts(
 	}
 
 	protected open fun onKubeConfigChanged(fileConfig: io.fabric8.kubernetes.api.model.Config) {
-		val client = client.get() ?: return
-		val clientConfig = client.config.configuration
-		if (ConfigHelper.areEqual(fileConfig, clientConfig)) {
-			return
+		synchronized(this) {
+			val client = client.get() ?: return
+			val clientConfig = client.config.configuration
+			if (ConfigHelper.areEqual(fileConfig, clientConfig)) {
+				return
+			}
+			this.client.reset() // create new client when accessed
+			client.close()
+			refresh()
 		}
-		client.close()
-		this.client.reset() // create new client when accessed
-		refresh()
 	}
 
 	/** for testing purposes */

src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt

@@ -10,19 +10,21 @@
  ******************************************************************************/
 package com.redhat.devtools.intellij.kubernetes.model.client
 
-import com.intellij.openapi.application.ApplicationManager
 import com.redhat.devtools.intellij.common.utils.ConfigHelper
+import com.redhat.devtools.intellij.kubernetes.CompletableFutureUtils.PLATFORM_EXECUTOR
 import io.fabric8.kubernetes.api.model.Context
 import io.fabric8.kubernetes.api.model.NamedContext
 import io.fabric8.kubernetes.client.Client
 import io.fabric8.kubernetes.client.Config
 import io.fabric8.kubernetes.client.internal.KubeConfigUtils
+import java.util.concurrent.CompletableFuture
+import java.util.concurrent.Executor
 
 /**
  * An adapter to access [io.fabric8.kubernetes.client.Config].
  * It also saves the kube config [KubeConfigUtils] when it changes the client config.
  */
-open class ClientConfig(private val client: Client) {
+open class ClientConfig(private val client: Client, private val executor: Executor = PLATFORM_EXECUTOR) {
 
 	open var currentContext: NamedContext?
 		get() {
@@ -45,26 +47,33 @@ open class ClientConfig(private val client: Client) {
 		KubeConfigAdapter()
 	}
 
-	fun save() {
-		runAsync {
-			if (!kubeConfig.exists()) {
-				return@runAsync
-			}
-			val fromFile = kubeConfig.load() ?: return@runAsync
-			val currentContextInFile = KubeConfigUtils.getCurrentContext(fromFile)
-			if (setCurrentContext(
-					currentContext,
-					currentContextInFile,
-					fromFile
-				).or( // no short-circuit
-				setCurrentNamespace(
-					currentContext?.context,
-					currentContextInFile?.context)
-				)
-			) {
-				kubeConfig.save(fromFile)
-			}
-		}
+	fun save(): CompletableFuture<Boolean> {
+		return CompletableFuture.supplyAsync(
+			{
+				if (!kubeConfig.exists()) {
+					return@supplyAsync false
+				}
+				val fromFile = kubeConfig.load() ?: return@supplyAsync false
+				val currentContextInFile = KubeConfigUtils.getCurrentContext(fromFile)
+				if (setCurrentContext(
+						currentContext,
+						currentContextInFile,
+						fromFile
+					).or( // no short-circuit
+						setCurrentNamespace(
+							currentContext?.context,
+							currentContextInFile?.context
+						)
+					)
+				) {
+					kubeConfig.save(fromFile)
+					return@supplyAsync true
+				} else {
+					return@supplyAsync false
+				}
+			},
+			executor
+		)
 	}
 
 	private fun setCurrentContext(
@@ -111,10 +120,4 @@ open class ClientConfig(private val client: Client) {
 	fun isCurrent(context: NamedContext): Boolean {
 		return context == currentContext
 	}
-
-	/** for testing purposes */
-	protected open fun runAsync(runnable: () -> Unit) {
-		ApplicationManager.getApplication().executeOnPooledThread(runnable)
-	}
-
 }

src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt

@@ -88,7 +88,7 @@ class ClientConfigTest {
 		doReturn(false)
 			.whenever(kubeConfig).exists()
 		// when
-		clientConfig.save()
+		clientConfig.save().join()
 		// then
 		verify(kubeConfig, never()).save(any())
 	}
@@ -97,7 +97,7 @@ class ClientConfigTest {
 	fun `#save should NOT save if kubeConfig has same current context same namespace and same current context as client config`() {
 		// given
 		// when
-		clientConfig.save()
+		clientConfig.save().join()
 		// then
 		verify(kubeConfig, never()).save(any())
 	}
@@ -108,7 +108,7 @@ class ClientConfigTest {
 		f8clientConfig.currentContext.name = namedContext3.name
 		assertThat(f8kubeConfig.currentContext).isNotEqualTo(f8clientConfig.currentContext.name)
 		// when
-		clientConfig.save()
+		clientConfig.save().join()
 		// then
 		verify(kubeConfig).save(any())
 	}
@@ -126,7 +126,7 @@ class ClientConfigTest {
 		newAllContexts.add(newCurrentContext)
 		f8kubeConfig.contexts = newAllContexts
 		// when
-		clientConfig.save()
+		clientConfig.save().join()
 		// then
 		verify(kubeConfig).save(any())
 	}
@@ -140,7 +140,7 @@ class ClientConfigTest {
 		assertThat(KubeConfigUtils.getCurrentContext(f8kubeConfig))
 			.isNotEqualTo(f8clientConfig.currentContext)
 		// when
-		clientConfig.save()
+		clientConfig.save().join()
 		// then
 		verify(kubeConfig).save(argThat {
 			this.currentContext == newCurrentContext.name
@@ -159,7 +159,7 @@ class ClientConfigTest {
 		assertThat(KubeConfigUtils.getCurrentContext(f8kubeConfig).context.namespace)
 			.isNotEqualTo(f8clientConfig.currentContext.context.namespace)
 		// when
-		clientConfig.save()
+		clientConfig.save().join()
 		// then
 		verify(kubeConfig).save(argThat {
 			this.currentContext == [email protected]
@@ -211,10 +211,6 @@ class ClientConfigTest {
 			.build()
 	}
 
-	private class TestableClientConfig(client: Client, override val kubeConfig: KubeConfigAdapter) : ClientConfig(client) {
-		override fun runAsync(runnable: () -> Unit) {
-			// dont use jetbrains application threadpool
-			runnable.invoke()
-		}
-	}
+	private class TestableClientConfig(client: Client, override val kubeConfig: KubeConfigAdapter)
+		: ClientConfig(client, { it.run() })
 }

src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt

@@ -38,6 +38,7 @@ import io.fabric8.kubernetes.client.Watch
 import io.fabric8.kubernetes.client.Watcher
 import io.fabric8.kubernetes.client.dsl.ExecWatch
 import io.fabric8.kubernetes.client.dsl.LogWatch
+import java.util.concurrent.CompletableFuture
 import org.mockito.Mockito
 
 object Mocks {
@@ -210,13 +211,15 @@ object Mocks {
         allContexts: List<NamedContext>,
         configuration: io.fabric8.kubernetes.client.Config = mock()
     ): ClientConfig {
+        val saveFuture: CompletableFuture<Boolean> = mock()
         return mock {
             on { this.currentContext } doReturn currentContext
             on { isCurrent(any()) } doAnswer { invocation ->
                 invocation.getArgument<NamedContext>(0) == mock.currentContext
             }
             on { this.allContexts } doReturn allContexts
             on { this.configuration } doReturn configuration
+            on { this.save() } doReturn saveFuture
         }
     }