From fc2864e8d910fb60acf4f0907ddec9ae2e3c525b Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Fri, 26 Apr 2024 15:16:37 +0200 Subject: [PATCH] fix: dont refresh tree if binary is not downloaded (#802) Signed-off-by: Andre Dietisheim --- build.gradle | 2 +- .../openshift/utils/ToolFactoryTest.java | 10 +-- .../openshift/utils/helm/HelmCliTest.java | 16 +++-- .../openshift/utils/odo/OdoCliTest.java | 5 +- .../openshift/actions/HelmAction.java | 2 +- .../application/ApplicationRootNodeOdo.java | 12 +++- .../application/ApplicationsRootNode.java | 66 +++++++++++-------- .../ApplicationsTreeStructure.java | 4 +- .../intellij/openshift/utils/ToolFactory.java | 50 +++++++++----- .../openshift/actions/ActionTest.java | 17 ++--- .../ApplicationRootNodeOdoTest.java | 2 +- 11 files changed, 116 insertions(+), 70 deletions(-) diff --git a/build.gradle b/build.gradle index f8fa8ee77..e9b80164c 100644 --- a/build.gradle +++ b/build.gradle @@ -140,7 +140,7 @@ dependencies { 'io.fabric8:openshift-client:6.12.0', 'org.apache.commons:commons-compress:1.26.1', 'org.apache.commons:commons-exec:1.4.0', - 'com.redhat.devtools.intellij:intellij-common:1.9.5', + 'com.redhat.devtools.intellij:intellij-common:1.9.6-SNAPSHOT', 'io.jsonwebtoken:jjwt-impl:0.12.5', 'io.jsonwebtoken:jjwt-jackson:0.12.5', 'org.keycloak:keycloak-installed-adapter:24.0.3', diff --git a/src/it/java/org/jboss/tools/intellij/openshift/utils/ToolFactoryTest.java b/src/it/java/org/jboss/tools/intellij/openshift/utils/ToolFactoryTest.java index a44ab782f..371f0a850 100644 --- a/src/it/java/org/jboss/tools/intellij/openshift/utils/ToolFactoryTest.java +++ b/src/it/java/org/jboss/tools/intellij/openshift/utils/ToolFactoryTest.java @@ -11,20 +11,22 @@ package org.jboss.tools.intellij.openshift.utils; import com.intellij.testFramework.fixtures.BasePlatformTestCase; +import java.util.concurrent.ExecutionException; +import org.jboss.tools.intellij.openshift.utils.ToolFactory.Tool; import org.jboss.tools.intellij.openshift.utils.helm.Helm; import org.jboss.tools.intellij.openshift.utils.odo.Odo; -import java.util.concurrent.ExecutionException; - public class ToolFactoryTest extends BasePlatformTestCase { public void testGetOdo() throws ExecutionException, InterruptedException { - Odo odo = ToolFactory.getInstance().createOdo(getProject()).get(); + Tool tool = ToolFactory.getInstance().createOdo(getProject()).get(); + Odo odo = tool.get(); assertNotNull(odo); } public void testGetHelm() throws ExecutionException, InterruptedException { - Helm helm = ToolFactory.getInstance().createHelm(getProject()).get(); + Tool tool = ToolFactory.getInstance().createHelm(getProject()).get(); + Helm helm = tool.get(); assertNotNull(helm); } diff --git a/src/it/java/org/jboss/tools/intellij/openshift/utils/helm/HelmCliTest.java b/src/it/java/org/jboss/tools/intellij/openshift/utils/helm/HelmCliTest.java index 4b0cd33f3..ee14949e8 100644 --- a/src/it/java/org/jboss/tools/intellij/openshift/utils/helm/HelmCliTest.java +++ b/src/it/java/org/jboss/tools/intellij/openshift/utils/helm/HelmCliTest.java @@ -11,12 +11,12 @@ package org.jboss.tools.intellij.openshift.utils.helm; import com.intellij.testFramework.fixtures.BasePlatformTestCase; +import java.util.Random; import org.jboss.tools.intellij.openshift.utils.OdoCluster; import org.jboss.tools.intellij.openshift.utils.ToolFactory; +import org.jboss.tools.intellij.openshift.utils.ToolFactory.Tool; import org.jboss.tools.intellij.openshift.utils.odo.Odo; -import java.util.Random; - public abstract class HelmCliTest extends BasePlatformTestCase { protected Helm helm; @@ -26,17 +26,21 @@ public abstract class HelmCliTest extends BasePlatformTestCase { @Override protected void setUp() throws Exception { super.setUp(); - Odo odo = ToolFactory.getInstance().createOdo(getProject()).get(); + Tool odoTool = ToolFactory.getInstance().createOdo(getProject()).get(); + Odo odo = odoTool.get(); OdoCluster.INSTANCE.login(odo); odo.createProject(projectName); - this.helm = ToolFactory.getInstance().createHelm(getProject()).get(); + Tool helmTool = ToolFactory.getInstance().createHelm(getProject()).get(); + this.helm = helmTool.get(); Charts.addRepository(Charts.REPOSITORY_STABLE, helm); } @Override protected void tearDown() throws Exception { - Odo odo = ToolFactory.getInstance().createOdo(getProject()).get(); - odo.deleteProject(projectName); + ToolFactory.Tool tool = ToolFactory.getInstance().createOdo(getProject()).getNow(null); + if (tool != null) { + tool.get().deleteProject(projectName); + } super.tearDown(); } diff --git a/src/it/java/org/jboss/tools/intellij/openshift/utils/odo/OdoCliTest.java b/src/it/java/org/jboss/tools/intellij/openshift/utils/odo/OdoCliTest.java index f2b2362c1..18cbb0cd9 100644 --- a/src/it/java/org/jboss/tools/intellij/openshift/utils/odo/OdoCliTest.java +++ b/src/it/java/org/jboss/tools/intellij/openshift/utils/odo/OdoCliTest.java @@ -85,8 +85,9 @@ protected void tearDown() throws Exception { } private CompletableFuture getOdo() { - return ToolFactory.getInstance().createOdo(getProject()) - .thenApply(odoDelegate -> new ApplicationRootNodeOdo(odoDelegate, mock(ApplicationsRootNode.class), processHelper)); + return ToolFactory.getInstance() + .createOdo(getProject()) + .thenApply(tool -> new ApplicationRootNodeOdo(tool.get(), false, mock(ApplicationsRootNode.class), processHelper)); } protected void createProject(String project) throws IOException, ExecutionException, InterruptedException { diff --git a/src/main/java/org/jboss/tools/intellij/openshift/actions/HelmAction.java b/src/main/java/org/jboss/tools/intellij/openshift/actions/HelmAction.java index 630d4a899..58ab0731b 100644 --- a/src/main/java/org/jboss/tools/intellij/openshift/actions/HelmAction.java +++ b/src/main/java/org/jboss/tools/intellij/openshift/actions/HelmAction.java @@ -45,7 +45,7 @@ public void actionPerformed(AnActionEvent anActionEvent, TreePath path, Object s protected Helm getHelm(AnActionEvent anActionEvent) { try { - return ActionUtils.getApplicationRootNode(anActionEvent).getHelm(true).getNow(null); + return ActionUtils.getApplicationRootNode(anActionEvent).getHelm(true); } catch (Exception e) { LOGGER.warn("Could not get helm: " + e.getMessage(), e); return null; diff --git a/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationRootNodeOdo.java b/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationRootNodeOdo.java index 2416e6343..699df1ae9 100644 --- a/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationRootNodeOdo.java +++ b/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationRootNodeOdo.java @@ -52,16 +52,18 @@ public class ApplicationRootNodeOdo implements Odo { private final Odo delegate; + private final boolean isDownloaded; private final OdoProcessHelper processHelper; private final ApplicationsRootNode root; private final FileOperations fileOperations; - public ApplicationRootNodeOdo(Odo delegate, ApplicationsRootNode root, OdoProcessHelper processHelper) { - this(delegate, processHelper, root, new FileOperations()); + public ApplicationRootNodeOdo(Odo delegate, boolean isDownloaded, ApplicationsRootNode root, OdoProcessHelper processHelper) { + this(delegate, isDownloaded, processHelper, root, new FileOperations()); } - ApplicationRootNodeOdo(Odo delegate, OdoProcessHelper processHelper, ApplicationsRootNode root, FileOperations fileOperations) { + ApplicationRootNodeOdo(Odo delegate, boolean isDownloaded, OdoProcessHelper processHelper, ApplicationsRootNode root, FileOperations fileOperations) { this.delegate = delegate; + this.isDownloaded = isDownloaded; this.processHelper = processHelper; this.root = root; this.fileOperations = fileOperations; @@ -374,6 +376,10 @@ public void migrateComponent(String name) { delegate.migrateComponent(name); } + public boolean isDownloaded() { + return isDownloaded; + } + /** for testing purposes **/ protected static class FileOperations { protected File createTempDir(String prefix) throws IOException { diff --git a/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationsRootNode.java b/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationsRootNode.java index 04dd449fd..0cfe5b61b 100644 --- a/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationsRootNode.java +++ b/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationsRootNode.java @@ -23,9 +23,17 @@ import com.redhat.devtools.intellij.common.utils.ConfigWatcher; import com.redhat.devtools.intellij.common.utils.ExecHelper; import io.fabric8.kubernetes.api.model.Config; +import java.io.File; +import java.io.IOException; +import java.nio.file.Paths; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; import org.jboss.tools.intellij.openshift.actions.NotificationUtils; import org.jboss.tools.intellij.openshift.utils.ProjectUtils; import org.jboss.tools.intellij.openshift.utils.ToolFactory; +import org.jboss.tools.intellij.openshift.utils.ToolFactory.Tool; import org.jboss.tools.intellij.openshift.utils.helm.Helm; import org.jboss.tools.intellij.openshift.utils.odo.ComponentDescriptor; import org.jboss.tools.intellij.openshift.utils.odo.Odo; @@ -34,15 +42,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.File; -import java.io.IOException; -import java.nio.file.Paths; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.CompletableFuture; -import java.util.function.BiConsumer; - public class ApplicationsRootNode implements ModuleListener, ConfigWatcher.Listener, ProcessingNode, StructureAwareNode, ParentableNode, Disposable { @@ -51,8 +50,8 @@ public class ApplicationsRootNode private final ApplicationsTreeStructure structure; private final ProcessingNodeImpl processingNode = new ProcessingNodeImpl(); private final Map components = new HashMap<>(); - private CompletableFuture odoFuture; - private CompletableFuture helmFuture; + private CompletableFuture odoFuture; + private CompletableFuture> helmFuture; private boolean logged; private Config config; private final OdoProcessHelper processHelper; @@ -81,39 +80,53 @@ public void setLogged(boolean logged) { this.logged = logged; } - private CompletableFuture getOdo(BiConsumer whenComplete) { + private CompletableFuture doGetOdo() { if (odoFuture == null) { this.odoFuture = ToolFactory.getInstance() - .createOdo(project) - .thenApply(odo -> (Odo) new ApplicationRootNodeOdo(odo, this, processHelper)) - .whenComplete((odo, err) -> loadProjectModel(odo, project)) - .whenComplete(whenComplete); + .createOdo(project) + .thenApply(tool -> { + ApplicationRootNodeOdo odo = new ApplicationRootNodeOdo(tool.get(), tool.isDownloaded(), this, processHelper); + loadProjectModel(odo, project); + return odo; + }); } return odoFuture; } - public CompletableFuture getOdo() { - return getOdo((odo, err) -> structure.fireModified(this)); + public CompletableFuture getOdo() { + return doGetOdo() + .whenComplete((ApplicationRootNodeOdo odo, Throwable err) -> { + if (odo.isDownloaded()) { + structure.fireModified(this); + } + }); } public void resetOdo() { this.odoFuture = null; } - public CompletableFuture getHelm(boolean notify) { + public CompletableFuture> getHelmTool(boolean notify) { if (helmFuture == null) { this.helmFuture = ToolFactory.getInstance() .createHelm(project) - .whenComplete((odo, err) -> { - if (notify) { + .whenComplete((tool, err) -> { + if (notify && tool.isDownloaded()) { structure.fireModified(this); } - } - ); + }); } return helmFuture; } + public Helm getHelm(boolean notify) { + Tool tool = getHelmTool(notify).getNow(null); + if (tool == null) { + return null; + } + return tool.get(); + } + public Project getProject() { return project; } @@ -159,8 +172,7 @@ private void addContextToSettings(String path, ComponentDescriptor descriptor) { } private void migrateOdo(ComponentDescriptor descriptor) { - getOdo() - .thenAccept(odo -> { + doGetOdo().whenComplete((odo, err) -> { if (odo != null) { odo.migrateComponent(descriptor.getName()); } @@ -228,7 +240,9 @@ public void onUpdate(ConfigWatcher source, Config config) { public synchronized void refresh() { resetOdo(); - getOdo((odo, err) -> structure.fireModified(ApplicationsRootNode.this)); + doGetOdo().whenComplete((odo, err) -> + structure.fireModified(ApplicationsRootNode.this) + ); } @Override diff --git a/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationsTreeStructure.java b/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationsTreeStructure.java index 9c41d9d03..04f7ce62e 100644 --- a/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationsTreeStructure.java +++ b/src/main/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationsTreeStructure.java @@ -167,7 +167,7 @@ private Object getCurrentNamespace(ApplicationsRootNode element) { } private Object[] createHelmRepositoriesChildren(HelmRepositoriesNode parent) { - Helm helm = root.getHelm(true).getNow(null); + Helm helm = root.getHelm(true); if (helm == null) { return new Object[] { new MessageNode<>(root, parent, "Could not list repositories: Helm binary missing.") }; } @@ -226,7 +226,7 @@ private List> getServices(NamespaceNode namespaceNode, Odo odo) { } private List> getHelmReleases(NamespaceNode namespaceNode) { - Helm helm = namespaceNode.getRoot().getHelm(true).getNow(null); + Helm helm = namespaceNode.getRoot().getHelm(true); if (helm == null) { return List.of(new MessageNode<>(root, namespaceNode, "Could not get chart releases")); } diff --git a/src/main/java/org/jboss/tools/intellij/openshift/utils/ToolFactory.java b/src/main/java/org/jboss/tools/intellij/openshift/utils/ToolFactory.java index cebb98224..8dd687a5f 100644 --- a/src/main/java/org/jboss/tools/intellij/openshift/utils/ToolFactory.java +++ b/src/main/java/org/jboss/tools/intellij/openshift/utils/ToolFactory.java @@ -12,17 +12,16 @@ import com.intellij.openapi.project.Project; import com.redhat.devtools.intellij.common.utils.DownloadHelper; +import java.net.URL; +import java.util.concurrent.CompletableFuture; +import java.util.function.BiFunction; import org.jboss.tools.intellij.openshift.utils.helm.Helm; import org.jboss.tools.intellij.openshift.utils.helm.HelmCli; import org.jboss.tools.intellij.openshift.utils.odo.Odo; import org.jboss.tools.intellij.openshift.utils.odo.OdoCli; -import java.util.concurrent.CompletableFuture; -import java.util.function.BiFunction; - public class ToolFactory { - private static final String TOOLS_JSON = "/tools.json"; private static ToolFactory INSTANCE; @@ -34,41 +33,60 @@ public static ToolFactory getInstance() { return INSTANCE; } - private final Tool odo = new Tool<>("odo", OdoCli::new); - private final Tool helm = new Tool<>("helm", (project, command) -> new HelmCli(command)); + private final Factory odo = new Factory<>("odo", OdoCli::new); + private final Factory helm = new Factory<>("helm", (project, command) -> new HelmCli(command)); private ToolFactory() { } - public CompletableFuture createOdo(Project project) { + public CompletableFuture> createOdo(Project project) { return odo.create(project); } - public CompletableFuture createHelm(Project project) { + public CompletableFuture> createHelm(Project project) { return helm.create(project); } - private static class Tool { + public static class Tool { + private final T tool; + private final boolean isDownloaded; + + private Tool(T tool, boolean isDownloaded) { + this.tool = tool; + this.isDownloaded = isDownloaded; + } + + public T get() { + return tool; + } + + public boolean isDownloaded() { + return isDownloaded; + } + } + + private static class Factory { private final String name; + private final URL url = ToolFactory.class.getResource(TOOLS_JSON); private final BiFunction toolFactory; - private Tool(String name, BiFunction toolFactory) { + private Factory(String name, BiFunction toolFactory) { this.name = name; this.toolFactory = toolFactory; } - private CompletableFuture create(Project project) { + private CompletableFuture> create(Project project) { return create(name, toolFactory, project); } - private CompletableFuture create(String name, BiFunction toolFactory, Project project) { + private CompletableFuture> create(String name, BiFunction toolFactory, Project project) { return DownloadHelper.getInstance() - .downloadIfRequiredAsync(name, ToolFactory.class.getResource(TOOLS_JSON)) - .thenApply(command -> { - if (command != null) { - return toolFactory.apply(project, command); + .downloadIfRequiredAsync(name, url) + .thenApply(toolInstance -> { + if (toolInstance != null) { + return new Tool<>(toolFactory.apply(project, toolInstance.getCommand()), toolInstance.isDownloaded()); } else { return null; } diff --git a/src/test/java/org/jboss/tools/intellij/openshift/actions/ActionTest.java b/src/test/java/org/jboss/tools/intellij/openshift/actions/ActionTest.java index 15c89edba..377e25178 100644 --- a/src/test/java/org/jboss/tools/intellij/openshift/actions/ActionTest.java +++ b/src/test/java/org/jboss/tools/intellij/openshift/actions/ActionTest.java @@ -16,6 +16,9 @@ import com.intellij.openapi.actionSystem.Presentation; import com.intellij.testFramework.fixtures.BasePlatformTestCase; import com.intellij.ui.treeStructure.Tree; +import java.util.concurrent.CompletableFuture; +import javax.swing.tree.TreePath; +import javax.swing.tree.TreeSelectionModel; import org.jboss.tools.intellij.openshift.Constants; import org.jboss.tools.intellij.openshift.tree.application.ApplicationsRootNode; import org.jboss.tools.intellij.openshift.tree.application.ApplicationsTreeStructure; @@ -35,11 +38,8 @@ import org.jboss.tools.intellij.openshift.utils.odo.URL; import org.jetbrains.annotations.NotNull; -import javax.swing.tree.TreePath; -import javax.swing.tree.TreeSelectionModel; -import java.util.concurrent.CompletableFuture; - import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -104,11 +104,11 @@ protected void verifyLoggedOutCluster(boolean visible) { } @NotNull - protected static ApplicationsRootNode createApplicationRootNode(CompletableFuture odoFuture) { + protected static ApplicationsRootNode createApplicationRootNode(CompletableFuture odoFuture) { ApplicationsRootNode applicationsRootNode = mock(ApplicationsRootNode.class); - when(applicationsRootNode.isLogged()).thenReturn(true); - when(applicationsRootNode.getRoot()).thenReturn(applicationsRootNode); - when(applicationsRootNode.getOdo()).thenReturn(odoFuture); + doReturn(true).when(applicationsRootNode).isLogged(); + doReturn(applicationsRootNode).when(applicationsRootNode).getRoot(); + doReturn(odoFuture).when(applicationsRootNode).getOdo(); return applicationsRootNode; } @@ -116,6 +116,7 @@ protected static ApplicationsRootNode createApplicationRootNode(CompletableFutur protected static CompletableFuture createOdoFuture(boolean isOpenShift) { Odo odo = mock(Odo.class); when(odo.isOpenShift()).thenReturn(isOpenShift); + CompletableFuture odoFuture = mock(CompletableFuture.class); when(odoFuture.getNow(any())).thenReturn(odo); return odoFuture; diff --git a/src/test/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationRootNodeOdoTest.java b/src/test/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationRootNodeOdoTest.java index 19613c55e..f9eff63cb 100644 --- a/src/test/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationRootNodeOdoTest.java +++ b/src/test/java/org/jboss/tools/intellij/openshift/tree/application/ApplicationRootNodeOdoTest.java @@ -61,7 +61,7 @@ public void before() throws IOException { this.rootNode = mock(ApplicationsRootNode.class); this.fileOperations = mockFileOperations(tempDir, destinationDir); OdoProcessHelper processHelper = mock(OdoProcessHelper.class); - this.rootNodeOdo = new ApplicationRootNodeOdo(odo, processHelper, rootNode, fileOperations); + this.rootNodeOdo = new ApplicationRootNodeOdo(odo, false, processHelper, rootNode, fileOperations); } @Test