Check target of linked resources in action handler isEnabled

The fix provides stronger checks on selected linked resources before
enabling menu items. In particular, linked resources that point to a
file that is part of a project shared with egit are allowed to be
operated on. Otherwise team actions are disabled if the selection
contains a linked resource pointing to a file outside any project and
repository. A default isEnabled() implementation is provided for this
behavior. After adding a test case a couple more handlers have been
amended and made compliant.

Bug: 406922
Change-Id: Id9fd5ea0c31a78b84796638570d214d14cc3c4fd
Signed-off-by: François Rey <eclipse.org@francois.rey.name>
Signed-off-by: Robin Stocker <robin@nibor.org>

5 files changed, 189 insertions, 19 deletions

diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/LinkedResourcesTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/LinkedResourcesTest.java
new file mode 100644
index 0000000000..7ce5e77939
--- /dev/null
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/LinkedResourcesTest.java
@@ -0,0 +1,138 @@
+/*******************************************************************************
+ * Copyright (C) 2013, François Rey <eclipse.org_@_francois_._rey_._name>
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    François Rey - First implementation as part of handling linked resources
+ ******************************************************************************/
+package org.eclipse.egit.ui.internal.actions;
+
+import static org.eclipse.jgit.junit.JGitTestUtil.write;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.ResourcesPlugin;
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.IConfigurationElement;
+import org.eclipse.core.runtime.IExtensionPoint;
+import org.eclipse.core.runtime.IExtensionRegistry;
+import org.eclipse.core.runtime.Platform;
+import org.eclipse.egit.ui.common.LocalRepositoryTestCase;
+import org.eclipse.jface.viewers.StructuredSelection;
+import org.eclipse.jgit.util.FS;
+import org.eclipse.jgit.util.FileUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class LinkedResourcesTest extends LocalRepositoryTestCase {
+
+	// the standalone temporary directory
+	private static File standaloneDirectory;
+
+	private static final String LINKED_FILE = "LinkedFile";
+
+	private static final String STANDALONE_FOLDER = "StandaloneFolder";
+
+	private File gitDir;
+
+	private IProject project;
+
+	@Before
+	public void setUp() throws Exception {
+		gitDir = createProjectAndCommitToRepository();
+		project = ResourcesPlugin.getWorkspace().getRoot().getProject(PROJ1);
+		// create our standalone temporary directory in the user space
+		File userHome = FS.DETECTED.userHome();
+		standaloneDirectory = new File(userHome, STANDALONE_FOLDER);
+		if (standaloneDirectory.exists())
+			FileUtils.delete(standaloneDirectory, FileUtils.RECURSIVE
+					| FileUtils.RETRY);
+		if (!standaloneDirectory.exists())
+			FileUtils.mkdir(standaloneDirectory, true);
+	}
+
+	@After
+	public void tearDown() throws Exception {
+		deleteAllProjects();
+		shutDownRepositories();
+		FileUtils.delete(gitDir.getParentFile(), FileUtils.RECURSIVE
+				| FileUtils.RETRY);
+		FileUtils.delete(standaloneDirectory, FileUtils.RECURSIVE
+				| FileUtils.RETRY);
+	}
+
+	private List<RepositoryActionHandler> getRepositoryActionHandlerList()
+			throws CoreException {
+		IExtensionRegistry registry = Platform.getExtensionRegistry();
+		IExtensionPoint point = registry.getExtensionPoint("org.eclipse.ui.commands");
+		IConfigurationElement[] elements = point.getConfigurationElements();
+		ArrayList<RepositoryActionHandler> result = new ArrayList<RepositoryActionHandler>();
+		for (IConfigurationElement e: elements) {
+			String categoryId = e.getAttribute("categoryId");
+			if ("org.eclipse.egit.ui.commandCategory".equals(categoryId)) {
+				if (e.getAttribute("defaultHandler") != null) {
+					Object o = e.createExecutableExtension("defaultHandler");
+					if (o instanceof RepositoryActionHandler)
+						result.add((RepositoryActionHandler) o);
+				}
+			}
+		}
+		return result;
+	}
+
+	@Test
+	public void testSomeActionsWithoutLinkedResources() throws Exception {
+		List<RepositoryActionHandler> handlers = getRepositoryActionHandlerList();
+		int count = 0;
+		IFile selection = project.getFile(FILE1);
+		for (RepositoryActionHandler handler: handlers) {
+			handler.setSelection(new StructuredSelection(selection));
+			if (handler.isEnabled())
+				count++;
+		}
+		// This sorts of test this test case: if no action is enabled,
+		// there's something wrong in our testing approach.
+		assertTrue(
+				"Some EGit action should be enabled, please review this test.",
+				count > 0);
+	}
+
+	@Test
+	public void testNoActionOnLinkedResources() throws Exception {
+		List<RepositoryActionHandler> handlers = getRepositoryActionHandlerList();
+		// Create a file outside any project or repository
+		File standaloneFile = new File(standaloneDirectory, LINKED_FILE);
+		write(standaloneFile, "Something");
+		// Create linked file in project that points the file above
+		IFile linkedFile = project.getFile(LINKED_FILE);
+		linkedFile.createLink(standaloneFile.toURI(),
+				IResource.ALLOW_MISSING_LOCAL, null);
+		// Prepare a mixed selection
+		Object[] mixedSelection = { linkedFile,
+				project.getFile(FILE1), project.getFile(FILE2) };
+		for (RepositoryActionHandler handler : handlers) {
+			handler.setSelection(new StructuredSelection(linkedFile));
+			assertFalse(
+					handler.getClass().getSimpleName()
+							+ " is enabled on a linked resource pointing outside any project and repository.",
+					handler.isEnabled());
+			handler.setSelection(new StructuredSelection(mixedSelection));
+			assertFalse(
+					handler.getClass().getSimpleName()
+							+ " is enabled when selection contains a linked resource pointing outside any project and repository.",
+					handler.isEnabled());
+		}
+	}
+}
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/nonswt/AllNonSWTTests.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/nonswt/AllNonSWTTests.java
index 296475d331..a8458685b1 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/nonswt/AllNonSWTTests.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/nonswt/AllNonSWTTests.java
@@ -8,6 +8,7 @@
  *******************************************************************************/
 package org.eclipse.egit.ui.test.nonswt;
 
+import org.eclipse.egit.ui.internal.actions.LinkedResourcesTest;
 import org.eclipse.egit.ui.internal.decorators.DecoratableResourceAdapterTest;
 import org.eclipse.egit.ui.internal.synchronize.model.AllGitModelTests;
 import org.junit.runner.RunWith;
@@ -16,6 +17,7 @@ import org.junit.runners.Suite.SuiteClasses;
 
 @RunWith(Suite.class)
 @SuiteClasses({ DecoratableResourceAdapterTest.class,
+		LinkedResourcesTest.class,
 		AllGitModelTests.class })
 public class AllNonSWTTests {
 	// Empty class
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesActionHandler.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesActionHandler.java
index 689fe8a86e..455e178869 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesActionHandler.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesActionHandler.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (C) 2010, Roland Grunberg <rgrunber@redhat.com>
+ * Copyright (C) 2010, 2013 Roland Grunberg <rgrunber@redhat.com>
  *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -8,12 +8,12 @@
  *
  * Contributors:
  *   Benjamin Muskalla (Tasktop Technologies Inc.) - support for model scoping
+ *   François Rey <eclipse.org_@_francois_._rey_._name> - handling of linked resources
  *******************************************************************************/
 package org.eclipse.egit.ui.internal.actions;
 
 import org.eclipse.core.commands.ExecutionEvent;
 import org.eclipse.core.commands.ExecutionException;
-import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -75,12 +75,10 @@ public class DiscardChangesActionHandler extends RepositoryActionHandler {
 
 	@Override
 	public boolean isEnabled() {
-		for (IResource res : getSelectedResources()) {
-			IProject[] proj = new IProject[] { res.getProject() };
-			Repository[] repositories = getRepositoriesFor(proj);
-			if (repositories.length == 0)
-				return false;
-			Repository repository = repositories[0];
+		Repository[] repositories = getRepositories();
+		if (repositories.length == 0)
+			return false;
+		for (Repository repository : repositories) {
 			if (!repository.getRepositoryState().equals(RepositoryState.SAFE))
 				return false;
 		}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/IgnoreActionHandler.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/IgnoreActionHandler.java
index d8ebf50e51..49c9b79410 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/IgnoreActionHandler.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/IgnoreActionHandler.java
@@ -2,6 +2,7 @@
  * Copyright (C) 2009, Alex Blewitt <alex.blewitt@gmail.com>
  * Copyright (C) 2010, Jens Baumgart <jens.baumgart@sap.com>
  * Copyright (C) 2012, Robin Stocker <robin@nibor.org>
+ * Copyright (C) 2013, François Rey <eclipse.org_@_francois_._rey_._name>
  *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -33,11 +34,4 @@ public class IgnoreActionHandler extends RepositoryActionHandler {
 		operation.run();
 		return null;
 	}
-
-	@Override
-	public boolean isEnabled() {
-		// Do not consult Team.isIgnoredHint here because the user
-		// should be allowed to add ignored resources to .gitignore
-		return true;
-	}
 }
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/RepositoryActionHandler.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/RepositoryActionHandler.java
index 1380d97589..a213b249ba 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/RepositoryActionHandler.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/RepositoryActionHandler.java
@@ -5,7 +5,7 @@
  * Copyright (C) 2010, Mathias Kinzler <mathias.kinzler@sap.com>
  * Copyright (C) 2011, Dariusz Luksza <dariusz@luksza.org>
  * Copyright (C) 2012, 2013 Robin Stocker <robin@nibor.org>
- * Copyright (C) 2012, François Rey <eclipse.org_@_francois_._rey_._name>
+ * Copyright (C) 2012, 2013 François Rey <eclipse.org_@_francois_._rey_._name>
  *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -93,6 +93,12 @@ abstract class RepositoryActionHandler extends AbstractHandler {
 	}
 
 	/**
+	 * Retrieve the list of projects that contains the given resources. All
+	 * resources must actually map to a project shared with egit, otherwise an
+	 * empty array is returned. In case of a linked resource, the project
+	 * returned is the one that contains the link target and is shared with
+	 * egit, if any, otherwise an empty array is also returned.
+	 *
 	 * @param selection
 	 * @return the projects hosting the selected resources
 	 */
@@ -100,8 +106,13 @@ abstract class RepositoryActionHandler extends AbstractHandler {
 			IStructuredSelection selection) {
 		Set<IProject> ret = new LinkedHashSet<IProject>();
 		for (IResource resource : (IResource[]) getSelectedAdaptables(
-				selection, IResource.class))
-			ret.add(resource.getProject());
+				selection, IResource.class)) {
+			RepositoryMapping mapping = RepositoryMapping.getMapping(resource);
+			if (mapping != null && (mapping.getContainer() instanceof IProject))
+				ret.add((IProject) mapping.getContainer());
+			else
+				return new IProject[0];
+		}
 		ret.addAll(extractProjectsFromMappings(selection));
 
 		return ret.toArray(new IProject[ret.size()]);
@@ -128,6 +139,12 @@ abstract class RepositoryActionHandler extends AbstractHandler {
 	}
 
 	/**
+	 * Retrieve the list of projects that contains the selected resources. All
+	 * resources must actually map to a project shared with egit, otherwise an
+	 * empty array is returned. In case of a linked resource, the project
+	 * returned is the one that contains the link target and is shared with
+	 * egit, if any, otherwise an empty array is also returned.
+	 *
 	 * @param event
 	 * @return the projects hosting the selected resources
 	 * @throws ExecutionException
@@ -138,6 +155,15 @@ abstract class RepositoryActionHandler extends AbstractHandler {
 		return getProjectsForSelectedResources(selection);
 	}
 
+	/**
+	 * Retrieve the list of projects that contains the selected resources. All
+	 * resources must actually map to a project shared with egit, otherwise an
+	 * empty array is returned. In case of a linked resource, the project
+	 * returned is the one that contains the link target and is shared with
+	 * egit, if any, otherwise an empty array is also returned.
+	 *
+	 * @return the projects hosting the selected resources
+	 */
 	protected IProject[] getProjectsForSelectedResources() {
 		IStructuredSelection selection = getSelection();
 		return getProjectsForSelectedResources(selection);
@@ -146,7 +172,7 @@ abstract class RepositoryActionHandler extends AbstractHandler {
 	/**
 	 * @param projects
 	 *            a list of projects
-	 * @return the repositories that projects map to iff all projects are mapped
+	 * @return the repositories that projects map to if all projects are mapped
 	 */
 	protected Repository[] getRepositoriesFor(final IProject[] projects) {
 		Set<Repository> ret = new LinkedHashSet<Repository>();
@@ -732,4 +758,16 @@ abstract class RepositoryActionHandler extends AbstractHandler {
 			this.path = path;
 		}
 	}
+
+	/**
+	 * By default egit operates only on resources that map to a project shared
+	 * with egit. For linked resources the project that contains the link
+	 * target, if any, must be shared with egit.
+	 *
+	 * @return the projects hosting the selected resources
+	 */
+	@Override
+	public boolean isEnabled() {
+		return getProjectsForSelectedResources().length > 0;
+	}
 }