summaryrefslogtreecommitdiffstatsabout
diff options
context:
space:
mode:
authorFrançois Rey2013-05-01 18:33:39 (EDT)
committer Robin Stocker2013-05-03 10:10:55 (EDT)
commitdadf0386a08feff68b62bf1a1b506611ec7fce6b (patch)
tree4c014390e8ee5d92662dfe6494d711725d1c5009
parent0a93dc5de1c889d388a1c9eb5a0b6702ae6c5daf (diff)
downloadegit-dadf0386a08feff68b62bf1a1b506611ec7fce6b.zip
egit-dadf0386a08feff68b62bf1a1b506611ec7fce6b.tar.gz
egit-dadf0386a08feff68b62bf1a1b506611ec7fce6b.tar.bz2
Check target of linked resources in action handler isEnabledrefs/changes/43/12443/3
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>
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/actions/LinkedResourcesTest.java138
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/nonswt/AllNonSWTTests.java2
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesActionHandler.java14
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/IgnoreActionHandler.java8
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/RepositoryActionHandler.java46
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 0000000..7ce5e77
--- /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 296475d..a845868 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 689fe8a..455e178 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 d8ebf50..49c9b79 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 1380d97..a213b24 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;
+ }
}