Bug 176764 - [Undo] - DeleteResourcesOperation does not handle overlapped selections
diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/AbstractCopyOrMoveResourcesOperation.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/AbstractCopyOrMoveResourcesOperation.java
index 3238f65..49c0358 100644
--- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/AbstractCopyOrMoveResourcesOperation.java
+++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/AbstractCopyOrMoveResourcesOperation.java
@@ -46,7 +46,8 @@
*
* @param resources
* the resources to be moved or copied. May not contain null
- * resources.
+ * resources, or resources that are descendants of already
+ * included resources.
* @param destinationPaths
* the destination paths for the resources, including the name to
* be assigned to the resource at its new location. May not contain
@@ -58,13 +59,20 @@
AbstractCopyOrMoveResourcesOperation(IResource[] resources,
IPath[] destinationPaths, String label) {
super(resources, label);
- if (resources == null || destinationPaths == null)
+ // Check for null arguments
+ if (this.resources == null || destinationPaths == null)
throw new IllegalArgumentException("The resource and destination paths may not be null"); //$NON-NLS-1$
- if (resources.length != destinationPaths.length) {
+ // Special case to flag descendants. Note this would fail on the next check
+ // anyway, so we are first giving a more specific explanation.
+ // See bug #176764
+ if (this.resources.length != resources.length)
+ throw new IllegalArgumentException("The resource list contained descendants that cannot be moved to separate destination paths"); //$NON-NLS-1$
+ // Check for destination paths corresponding for each resource
+ if (this.resources.length != destinationPaths.length) {
throw new IllegalArgumentException("The resource and destination paths must be the same length"); //$NON-NLS-1$
}
- for (int i=0; i<resources.length; i++) {
- if (resources[i] == null) {
+ for (int i=0; i<this.resources.length; i++) {
+ if (this.resources[i] == null) {
throw new IllegalArgumentException("The resources array may not contain null resources"); //$NON-NLS-1$
}
if (destinationPaths[i] == null) {
diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/AbstractResourcesOperation.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/AbstractResourcesOperation.java
index d58d388..542104f 100644
--- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/AbstractResourcesOperation.java
+++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/AbstractResourcesOperation.java
@@ -11,6 +11,9 @@
package org.eclipse.ui.ide.undo;
+import java.util.ArrayList;
+import java.util.List;
+
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IAdaptable;
@@ -42,6 +45,16 @@
* restore overwritten resources.
*/
protected ResourceDescription[] resourceDescriptions;
+
+ /*
+ * Return true if the specified subResource is a descendant of the specified
+ * super resource. Used to remove descendants from the resource array when
+ * an operation is requested on a parent and its descendant.
+ */
+ private static boolean isDescendantOf(IResource subResource, IResource superResource) {
+ return ! subResource.equals(superResource) && superResource.getFullPath().isPrefixOf(subResource.getFullPath());
+ }
+
/**
* Create an Abstract Resources Operation
@@ -276,4 +289,33 @@
return MultiRule.combine(ruleArray);
}
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see org.eclipse.ui.ide.undo.AbstractWorkspaceOperation#setTargetResources(org.eclipse.core.resources.IResource[])
+ */
+ protected void setTargetResources(IResource[] resources) {
+ // Remove any descendants if the parent has also
+ // been specified.
+ List subResources = new ArrayList();
+ for (int i = 0; i < resources.length; i++) {
+ IResource subResource = resources[i];
+ for (int j = 0; j < resources.length; j++) {
+ IResource superResource = resources[j];
+ if (isDescendantOf(subResource, superResource))
+ subResources.add(subResource);
+ }
+ }
+ IResource[] nestedResourcesRemoved = new IResource[resources.length
+ - subResources.size()];
+ int j = 0;
+ for (int i = 0; i < resources.length; i++) {
+ if (!subResources.contains(resources[i])) {
+ nestedResourcesRemoved[j] = resources[i];
+ j++;
+ }
+ }
+ super.setTargetResources(nestedResourcesRemoved);
+ }
}
diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/CopyResourcesOperation.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/CopyResourcesOperation.java
index fb914f3..2df1426 100644
--- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/CopyResourcesOperation.java
+++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/CopyResourcesOperation.java
@@ -83,7 +83,7 @@
public CopyResourcesOperation(IResource[] resources, IPath destinationPath,
String label) {
super(resources, destinationPath, label);
- setOriginalResources(resources);
+ setOriginalResources(this.resources);
}
/**
@@ -105,7 +105,7 @@
public CopyResourcesOperation(IResource[] resources,
IPath[] destinationPaths, String label) {
super(resources, destinationPaths, label);
- setOriginalResources(resources);
+ setOriginalResources(this.resources);
}
/*
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/operations/WorkspaceOperationsTests.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/operations/WorkspaceOperationsTests.java
index 6249b12..6cf0664 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/operations/WorkspaceOperationsTests.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/operations/WorkspaceOperationsTests.java
@@ -1243,6 +1243,56 @@
assertTrue("Folder content was altered", snap.isValid(targetProject));
}
+ public void testRedundantSubFolderMoveUndoRedo() throws ExecutionException,
+ CoreException {
+ IPath targetPath = targetProject.getFullPath();
+ IPath targetPathWithName = targetPath.append(testFolder.getName());
+ MoveResourcesOperation op = new MoveResourcesOperation(new IResource[] {
+ testFolder, testSubFolder }, targetPath,
+ "testRedundantSubFolderMove");
+ FolderSnapshot snap = new FolderSnapshot(testFolder);
+ execute(op);
+ IFolder movedFolder = getWorkspaceRoot().getFolder(targetPathWithName);
+ assertTrue("Folder move failed", movedFolder.exists());
+ assertTrue("Folder content was altered", snap.isValid(targetProject));
+
+ undo();
+ movedFolder = getWorkspaceRoot().getFolder(targetPathWithName);
+ assertFalse("Move undo failed", movedFolder.exists());
+ assertTrue("Folder content was altered on undo", snap
+ .isValid(testProject));
+
+ redo();
+ movedFolder = getWorkspaceRoot().getFolder(targetPathWithName);
+ assertTrue("Folder move failed", movedFolder.exists());
+ assertTrue("Folder content was altered", snap.isValid(targetProject));
+ }
+
+ public void testRedundantFolderFileMoveUndoRedo()
+ throws ExecutionException, CoreException {
+ IPath targetPath = targetProject.getFullPath();
+ IPath targetPathWithName = targetPath.append(testFolder.getName());
+ MoveResourcesOperation op = new MoveResourcesOperation(new IResource[] {
+ testFolder, testFileWithContent }, targetPath,
+ "testRedundantFolderFileMove");
+ FolderSnapshot snap = new FolderSnapshot(testFolder);
+ execute(op);
+ IFolder movedFolder = getWorkspaceRoot().getFolder(targetPathWithName);
+ assertTrue("Folder move failed", movedFolder.exists());
+ assertTrue("Folder content was altered", snap.isValid(targetProject));
+
+ undo();
+ movedFolder = getWorkspaceRoot().getFolder(targetPathWithName);
+ assertFalse("Move undo failed", movedFolder.exists());
+ assertTrue("Folder content was altered on undo", snap
+ .isValid(testProject));
+
+ redo();
+ movedFolder = getWorkspaceRoot().getFolder(targetPathWithName);
+ assertTrue("Folder move failed", movedFolder.exists());
+ assertTrue("Folder content was altered", snap.isValid(targetProject));
+ }
+
public void testFolderCopyUndoRedo() throws ExecutionException,
CoreException {
// copying with same name to a new project
@@ -1354,6 +1404,41 @@
assertFalse("Redo delete failed", testSubFolder.exists());
}
+ public void testNestedRedundantFolderDeleteUndoRedo()
+ throws ExecutionException, CoreException {
+ DeleteResourcesOperation op = new DeleteResourcesOperation(
+ new IResource[] { testFolder, testSubFolder },
+ "testNestedRedundantFolderDelete", false);
+ FolderSnapshot snap = new FolderSnapshot(testFolder);
+ execute(op);
+ assertFalse("Folder delete failed", testFolder.exists());
+ undo();
+ assertTrue("Folder recreation failed", testFolder.exists());
+ assertTrue("SubFolder recreation failed", testSubFolder.exists());
+ assertTrue("Folder content was altered on undo", snap
+ .isValid(testFolder.getParent()));
+ redo();
+ assertFalse("Redo delete failed", testFolder.exists());
+ }
+
+ public void testNestedRedundantFileDeleteUndoRedo()
+ throws ExecutionException, CoreException {
+ DeleteResourcesOperation op = new DeleteResourcesOperation(
+ new IResource[] { testFolder, testFileWithContent },
+ "testNestedRedundantFileDelete", false);
+ FolderSnapshot snap = new FolderSnapshot(testFolder);
+ execute(op);
+ assertFalse("Folder delete failed", testFolder.exists());
+ undo();
+ assertTrue("Folder recreation failed", testFolder.exists());
+ assertTrue("SubFolder recreation failed", testSubFolder.exists());
+ assertTrue("File recreation failed", testFileWithContent.exists());
+ assertTrue("Folder content was altered on undo", snap
+ .isValid(testFolder.getParent()));
+ redo();
+ assertFalse("Redo delete failed", testFolder.exists());
+ }
+
public void testFolderDeleteLinkedUndoRedo() throws ExecutionException,
CoreException {
DeleteResourcesOperation op = new DeleteResourcesOperation(
@@ -1844,6 +1929,42 @@
.isValid(targetProject));
}
+ public void testRedundantFileAndFolderCopy() throws CoreException,
+ ExecutionException {
+ // copying a file which is a child of a folder, keeping same name to a new project
+ CopyResourcesOperation op = new CopyResourcesOperation(new IResource[] {
+ testFolder, testFileWithContent }, targetProject
+ .getFullPath(), "testRedundantFileAndFolderCopy");
+ FolderSnapshot snapFolder = new FolderSnapshot(testFolder);
+ FileSnapshot snapFile = new FileSnapshot(testFileWithContent);
+ execute(op);
+ IFolder copiedFolder = targetProject.getFolder(testFolder.getName());
+ assertTrue("Folder copy failed", copiedFolder.exists());
+ assertTrue("Source folder was altered", snapFolder.isValid(testProject));
+ assertTrue("Folder copy does not match", snapFolder
+ .isValid(targetProject));
+ IFile copiedFile = targetProject.getFile(testFileWithContent.getName());
+ assertFalse("Nested file should not have been copied to new location", copiedFile.exists());
+ copiedFile = testFolder.getFile(testFileWithContent.getName());
+ assertTrue("Nested file should have been copied to existing parent", copiedFile.exists());
+ assertTrue("Source file was altered", snapFile.isValid(testFolder));
+
+ undo();
+ assertFalse("Copy folder undo failed", copiedFolder.exists());
+ assertTrue("Source file was altered during undo", snapFile
+ .isValid(testFolder));
+ assertTrue("Source folder was altered during undo", snapFolder
+ .isValid(testProject));
+
+ redo();
+ assertTrue("Source folder was altered during redo", snapFolder
+ .isValid(testProject));
+ assertTrue("Folder copy does not match on redo", snapFolder
+ .isValid(targetProject));
+ assertTrue("Source file was altered during redo", snapFile
+ .isValid(testFolder));
+ }
+
public void testFileAndFolderCopySameDests() throws ExecutionException,
CoreException {
// copying a file and folder, keeping same name to a new project
@@ -1953,19 +2074,19 @@
// The operation should know that undoing is dangerous
undoExpectFail(op);
}
-
+
public void test162655() throws ExecutionException, CoreException {
DeleteResourcesOperation op = new DeleteResourcesOperation(
new IResource[] { testProject }, "testProjectDelete", false);
execute(op);
assertFalse("Project delete failed", testProject.exists());
-
+
// recreate outside the scope of undo
testProject = getWorkspace().getRoot().getProject(TEST_PROJECT_NAME);
testProject.create(getMonitor());
testProject.open(getMonitor());
assertTrue("Project creation failed", testProject.exists());
-
+
// Now that project exists again, the undo should fail.
undoExpectFail(op);
}
