From 8f71ec55edb6bb3952092e13a59635eb90155e89 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Tue, 28 Apr 2026 18:34:05 +0200 Subject: [PATCH] Fix CloseUnrelatedProjectsAction enablement after closing all unrelated projects After PR #3871, updateSelection() only checked whether the selection contained an open project, so the action stayed enabled even when no unrelated projects were left to close. PR #3871 avoided calling buildConnectedComponents() on every selection change, since IProject#getReferencedProjects() can resolve classpath containers and block the IDE. Cache the workspace project graph and invalidate it from resourceChanged() on project open/description changes. The graph is now built at most once per project-state change rather than once per selection event, while updateSelection() can again check whether any open unrelated project exists. Add CloseUnrelatedProjectsActionEnablementTest covering the regression. --- .../actions/CloseUnrelatedProjectsAction.java | 111 ++++----- .../tests/navigator/NavigatorTestSuite.java | 2 + ...UnrelatedProjectsActionEnablementTest.java | 223 ++++++++++++++++++ 3 files changed, 281 insertions(+), 55 deletions(-) create mode 100644 tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/resources/CloseUnrelatedProjectsActionEnablementTest.java diff --git a/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CloseUnrelatedProjectsAction.java b/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CloseUnrelatedProjectsAction.java index 54eebcb244b..cb997022c53 100644 --- a/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CloseUnrelatedProjectsAction.java +++ b/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CloseUnrelatedProjectsAction.java @@ -19,7 +19,9 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; @@ -30,7 +32,6 @@ import org.eclipse.jface.dialogs.IDialogConstants; import org.eclipse.jface.dialogs.MessageDialogWithToggle; import org.eclipse.jface.preference.IPreferenceStore; -import org.eclipse.jface.viewers.IStructuredSelection; import org.eclipse.jface.window.IShellProvider; import org.eclipse.osgi.util.NLS; import org.eclipse.swt.widgets.Shell; @@ -63,10 +64,12 @@ public class CloseUnrelatedProjectsAction extends CloseResourceAction { private List projectsToClose = new ArrayList<>(); - private boolean selectionDirty = true; - private List oldSelection = Collections.emptyList(); + // Cached workspace project graph. Built lazily, reused across selection + // changes, invalidated when project open state or description changes. + private DisjointSet projectGraph; + /** * Builds the connected component set for the input projects. @@ -131,25 +134,6 @@ public CloseUnrelatedProjectsAction(IShellProvider provider){ initAction(); } - /** - * Overrides to avoid calling the expensive - * {@code computeRelated(List)} during selection changes. Uses only - * the raw selection to determine enablement. - */ - @Override - protected boolean updateSelection(IStructuredSelection s) { - selectionDirty = true; - if (!selectionIsOfType(IResource.PROJECT)) { - return false; - } - for (IResource resource : super.getSelectedResources()) { - if (resource instanceof IProject project && project.isOpen()) { - return true; - } - } - return false; - } - @Override public void run() { if (promptForConfirmation()) { @@ -207,46 +191,60 @@ private void initAction(){ setId(ID); setToolTipText(IDEWorkbenchMessages.CloseUnrelatedProjectsAction_toolTip); PlatformUI.getWorkbench().getHelpSystem().setHelp(this, IIDEHelpContextIds.CLOSE_UNRELATED_PROJECTS_ACTION); - } - - @Override - protected void clearCache() { - super.clearCache(); - oldSelection = Collections.emptyList(); - selectionDirty = true; + ResourcesPlugin.getWorkspace().addResourceChangeListener(this, IResourceChangeEvent.POST_CHANGE); } /** - * Computes the related projects of the selection. + * Computes the projects unrelated to the given selection. */ private List computeRelated(List selection) { if (selection.contains(ResourcesPlugin.getWorkspace().getRoot())) { return new ArrayList<>(); } - //build the connected component set for all projects in the workspace - DisjointSet set = buildConnectedComponents(ResourcesPlugin.getWorkspace().getRoot().getProjects()); - //remove the connected components that the selected projects are in + DisjointSet set = getProjectGraph(); + Set excludedRoots = new HashSet<>(); for (IResource resource : selection) { IProject project = resource.getProject(); if (project != null) { - set.removeSet(project); + IProject root = set.findSet(project); + if (root != null) { + excludedRoots.add(root); + } } } - //the remainder of the projects in the disjoint set are unrelated to the selection + List all = new ArrayList<>(); + set.toList(all); List projects = new ArrayList<>(); - set.toList(projects); + for (IProject project : all) { + IProject root = set.findSet(project); + if (root != null && !excludedRoots.contains(root)) { + projects.add(project); + } + } return projects; } + private DisjointSet getProjectGraph() { + DisjointSet graph = projectGraph; + if (graph == null) { + graph = buildConnectedComponents(ResourcesPlugin.getWorkspace().getRoot().getProjects()); + projectGraph = graph; + } + return graph; + } + + private void invalidateProjectGraph() { + projectGraph = null; + oldSelection = Collections.emptyList(); + clearCache(); + } + @Override protected List getSelectedResources() { - if (selectionDirty) { - List newSelection = super.getSelectedResources(); - if (!oldSelection.equals(newSelection)) { - oldSelection = newSelection; - projectsToClose = computeRelated(newSelection); - } - selectionDirty = false; + List newSelection = super.getSelectedResources(); + if (!oldSelection.equals(newSelection)) { + oldSelection = newSelection; + projectsToClose = computeRelated(newSelection); } return projectsToClose; } @@ -259,19 +257,22 @@ protected List getSelectedResources() { * the selection when the open state or description of any project changes. */ @Override - public void resourceChanged(IResourceChangeEvent event) { - // don't bother looking at delta if selection not applicable - if (selectionIsOfType(IResource.PROJECT)) { - IResourceDelta delta = event.getDelta(); - if (delta != null) { - IResourceDelta[] projDeltas = delta.getAffectedChildren(IResourceDelta.CHANGED); - for (IResourceDelta projDelta : projDeltas) { - //changing either the description or the open state can affect enablement - if ((projDelta.getFlags() & (IResourceDelta.OPEN | IResourceDelta.DESCRIPTION)) != 0) { - selectionChanged(getStructuredSelection()); - return; - } + public synchronized void resourceChanged(IResourceChangeEvent event) { + IResourceDelta delta = event.getDelta(); + if (delta == null) { + return; + } + IResourceDelta[] projDeltas = delta + .getAffectedChildren(IResourceDelta.ADDED | IResourceDelta.REMOVED | IResourceDelta.CHANGED); + for (IResourceDelta projDelta : projDeltas) { + int kind = projDelta.getKind(); + if (kind == IResourceDelta.ADDED || kind == IResourceDelta.REMOVED + || (projDelta.getFlags() & (IResourceDelta.OPEN | IResourceDelta.DESCRIPTION)) != 0) { + invalidateProjectGraph(); + if (selectionIsOfType(IResource.PROJECT)) { + selectionChanged(getStructuredSelection()); } + return; } } } diff --git a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/NavigatorTestSuite.java b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/NavigatorTestSuite.java index ed64af58737..1398af4a2b9 100644 --- a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/NavigatorTestSuite.java +++ b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/NavigatorTestSuite.java @@ -21,6 +21,7 @@ import org.eclipse.ui.tests.navigator.cdt.CdtTest; import org.eclipse.ui.tests.navigator.jst.JstPipelineTest; +import org.eclipse.ui.tests.navigator.resources.CloseUnrelatedProjectsActionEnablementTest; import org.eclipse.ui.tests.navigator.resources.FoldersAsProjectsContributionTest; import org.eclipse.ui.tests.navigator.resources.NestedResourcesTests; import org.eclipse.ui.tests.navigator.resources.PathComparatorTest; @@ -36,6 +37,7 @@ LabelProviderTest.class, SorterTest.class, ViewerTest.class, CdtTest.class, M12Tests.class, FirstClassM1Tests.class, LinkHelperTest.class, ShowInTest.class, ResourceTransferTest.class, EvaluationCacheTest.class, ResourceMgmtActionProviderTests.class, + CloseUnrelatedProjectsActionEnablementTest.class, NestedResourcesTests.class, PathComparatorTest.class, FoldersAsProjectsContributionTest.class, GoBackForwardsTest.class, CopyPasteActionTest.class // DnDTest.class, // DnDTest.testSetDragOperation() fails diff --git a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/resources/CloseUnrelatedProjectsActionEnablementTest.java b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/resources/CloseUnrelatedProjectsActionEnablementTest.java new file mode 100644 index 00000000000..ab225f29bd3 --- /dev/null +++ b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/resources/CloseUnrelatedProjectsActionEnablementTest.java @@ -0,0 +1,223 @@ +/******************************************************************************* + * Copyright (c) 2026 Lars Vogel and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.ui.tests.navigator.resources; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.eclipse.core.resources.IProject; +import org.eclipse.core.resources.IProjectDescription; +import org.eclipse.core.resources.IWorkspace; +import org.eclipse.core.resources.ResourcesPlugin; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.jface.preference.IPreferenceStore; +import org.eclipse.jface.viewers.StructuredSelection; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.ui.actions.CloseUnrelatedProjectsAction; +import org.eclipse.ui.internal.ide.IDEInternalPreferences; +import org.eclipse.ui.internal.ide.IDEWorkbenchPlugin; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class CloseUnrelatedProjectsActionEnablementTest { + + private IProject a; + private IProject b; + private IProject c; + private IProject d; + private boolean oldCloseUnrelated; + private Shell shell; + + @BeforeEach + public void setUp() throws CoreException { + IPreferenceStore store = IDEWorkbenchPlugin.getDefault().getPreferenceStore(); + oldCloseUnrelated = store.getBoolean(IDEInternalPreferences.CLOSE_UNRELATED_PROJECTS); + store.setValue(IDEInternalPreferences.CLOSE_UNRELATED_PROJECTS, true); + IWorkspace ws = ResourcesPlugin.getWorkspace(); + long suffix = System.nanoTime(); + a = ws.getRoot().getProject("CUPA_A_" + suffix); + b = ws.getRoot().getProject("CUPA_B_" + suffix); + c = ws.getRoot().getProject("CUPA_C_" + suffix); + d = ws.getRoot().getProject("CUPA_D_" + suffix); + a.create(null); + a.open(null); + b.create(null); + b.open(null); + c.create(null); + c.open(null); + + IProjectDescription aDesc = a.getDescription(); + aDesc.setReferencedProjects(new IProject[] { b }); + a.setDescription(aDesc, null); + + shell = new Shell(Display.getDefault()); + } + + @AfterEach + public void tearDown() throws CoreException { + if (shell != null && !shell.isDisposed()) { + shell.dispose(); + } + IDEWorkbenchPlugin.getDefault().getPreferenceStore().setValue(IDEInternalPreferences.CLOSE_UNRELATED_PROJECTS, + oldCloseUnrelated); + for (IProject p : new IProject[] { a, b, c, d }) { + if (p != null && p.exists()) { + p.delete(true, true, null); + } + } + } + + @Test + public void testDisabledAfterAllUnrelatedProjectsClosedAndSelectionChanges() throws CoreException { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + + action.selectionChanged(new StructuredSelection(a)); + assertTrue(action.isEnabled(), "action must be enabled while unrelated open project C exists"); + + c.close(null); + + action.selectionChanged(new StructuredSelection(b)); + assertFalse(action.isEnabled(), "action must be disabled when no unrelated open project remains"); + } + + @Test + public void testDisabledAfterAllUnrelatedProjectsAreDeleted() throws CoreException { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + + action.selectionChanged(new StructuredSelection(a)); + assertTrue(action.isEnabled(), "action must be enabled while unrelated open project C exists"); + + c.delete(true, true, null); + + action.selectionChanged(new StructuredSelection(a)); + assertFalse(action.isEnabled(), "action must be disabled when no unrelated open project remains"); + } + + @Test + public void testDoNotCloseDeleted() throws CoreException { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + + action.selectionChanged(new StructuredSelection(a)); + assertTrue(action.isEnabled(), "action must be enabled while unrelated open project C exists"); + + c.delete(true, true, null); + + action.selectionChanged(new StructuredSelection(a)); + assertFalse(action.isEnabled(), "action must be disabled when no unrelated open project remains"); + action.run(); // should not throw + } + + @Test + public void testDisabledAfterUnrelatedProjectCreated() throws CoreException { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + c.close(null); + action.selectionChanged(new StructuredSelection(b)); + assertFalse(action.isEnabled(), "action must be disabled when no unrelated open project remains"); + + d.create(null); + d.open(null); + + action.selectionChanged(new StructuredSelection(b)); + assertTrue(action.isEnabled(), "action must be enabled unrelated project is created"); + } + + @Test + public void testEnabledAfterDeleteAndReopen() throws CoreException { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + action.selectionChanged(new StructuredSelection(b)); + assertTrue(action.isEnabled(), "action must be enabled when and unrelated open project remains"); + action.run(); // should not throw + + d.create(null); + d.open(null); + + action.selectionChanged(new StructuredSelection(b)); + assertTrue(action.isEnabled(), "action must be enabled when unrelated project is created"); + + d.delete(true, true, null); + c.open(null); + + action.selectionChanged(new StructuredSelection(b)); + assertTrue(action.isEnabled(), "action must be enabled when unrelated project is reopened"); + action.run(); // should not throw + } + + @Test + public void testDisabledAfterRunAndUnrelatedProjectCreated() throws CoreException { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + action.selectionChanged(new StructuredSelection(b)); + assertTrue(action.isEnabled(), "action must be enabled while unrelated open projects exist"); + action.run(); + + d.create(null); + d.open(null); + + action.selectionChanged(new StructuredSelection(b)); + assertTrue(action.isEnabled(), "action must be enabled unrelated project is created"); + } + + @Test + public void testDisabledAfterAllUnrelatedProjectsClosed() throws CoreException { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + + action.selectionChanged(new StructuredSelection(a)); + assertTrue(action.isEnabled(), + "action must be enabled while unrelated open project C exists"); + + c.close(null); + + action.selectionChanged(new StructuredSelection(b)); + assertFalse(action.isEnabled(), + "action must be disabled when no unrelated open project remains"); + } + + @Test + public void testEnabledWhenUnrelatedOpenProjectExists() { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + + action.selectionChanged(new StructuredSelection(a)); + assertTrue(action.isEnabled(), "expected enabled when unrelated open project C exists"); + + action.selectionChanged(new StructuredSelection(b)); + assertTrue(action.isEnabled(), + "expected enabled when unrelated open project C exists (selection B)"); + } + + @Test + public void testDisabledWhenSelectionCoversAllOpenProjects() throws CoreException { + c.close(null); + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + action.selectionChanged(new StructuredSelection(new Object[] { a, b })); + assertFalse(action.isEnabled(), + "action must be disabled when selection plus its references covers all open projects"); + } + + @Test + public void testListenerInvalidatesGraphOnProjectClose() throws CoreException { + CloseUnrelatedProjectsAction action = new CloseUnrelatedProjectsAction(() -> shell); + + action.selectionChanged(new StructuredSelection(a)); + assertTrue(action.isEnabled(), + "action must be enabled while unrelated open project C exists"); + + // Closing C fires a POST_CHANGE event; the registered listener + // invalidates the cached graph and re-evaluates enablement. + c.close(null); + + // Re-select A without manually calling selectionChanged first — + // the graph must already be invalidated by the listener. + action.selectionChanged(new StructuredSelection(a)); + assertFalse(action.isEnabled(), + "action must be disabled after listener-driven graph invalidation"); + } +}