Bug 508260 - A lot of unnecessary file I/O when deleting a linked folder
pointing to a deleted file system directory.
Change-Id: I2ba4b13274e97ad26a7bc92d7c0301f8dbecd924
Signed-off-by: Sergey Prigogin <eclipse.sprigogin@gmail.com>
diff --git a/org.eclipse.ltk.core.refactoring.tests/src/org/eclipse/ltk/core/refactoring/tests/resource/ResourceRefactoringUndoTests.java b/org.eclipse.ltk.core.refactoring.tests/src/org/eclipse/ltk/core/refactoring/tests/resource/ResourceRefactoringUndoTests.java
index 47c3e07..736f357 100644
--- a/org.eclipse.ltk.core.refactoring.tests/src/org/eclipse/ltk/core/refactoring/tests/resource/ResourceRefactoringUndoTests.java
+++ b/org.eclipse.ltk.core.refactoring.tests/src/org/eclipse/ltk/core/refactoring/tests/resource/ResourceRefactoringUndoTests.java
@@ -4,17 +4,19 @@
* 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:
* IBM Corporation - initial API and implementation
*******************************************************************************/
package org.eclipse.ltk.core.refactoring.tests.resource;
import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.URI;
+import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
@@ -63,7 +65,6 @@
import junit.framework.TestSuite;
public class ResourceRefactoringUndoTests extends TestCase {
-
private static final String TEST_NEWPROJECT_NAME= "projectTestNew";
private static final String TEST_FOLDER_NAME= "test";
private static final String TEST_NEWFOLDER_NAME= "testNew";
@@ -73,6 +74,7 @@
private static final String TEST_LINKEDFILE_NAME= "linkedFile.txt";
private static final String TEST_SUBFOLDER_NAME= "subFolder";
private static List<String> fileNameExcludes= new ArrayList<>();
+
static {
fileNameExcludes.add(".project");
}
@@ -190,7 +192,7 @@
desc.setResourcePath(fProject.getProject().getFullPath());
desc.setNewName(TEST_NEWPROJECT_NAME);
PerformRefactoringOperation op= new PerformRefactoringOperation(desc.createRefactoringContext(new RefactoringStatus()), CheckConditionsOperation.ALL_CONDITIONS);
-
+
ProjectSnapshot snap= new ProjectSnapshot(fProject.getProject());
execute(op);
renamedProject= getWorkspaceRoot().getProject(TEST_NEWPROJECT_NAME);
@@ -283,6 +285,36 @@
assertFalse("Redo delete failed", testLinkedFolder.exists());
}
+ public void testFolderDeleteLinkedDeletedOnFilesystemUndoRedoLTK() throws ExecutionException, CoreException {
+ RefactoringContribution deleteContribution= RefactoringCore.getRefactoringContribution(DeleteResourcesDescriptor.ID);
+ DeleteResourcesDescriptor desc= (DeleteResourcesDescriptor) deleteContribution.createDescriptor();
+ desc.setResourcePaths(new IPath[] { testLinkedFolder.getFullPath() });
+
+ PerformRefactoringOperation op= new PerformRefactoringOperation(
+ desc.createRefactoringContext(new RefactoringStatus()),
+ CheckConditionsOperation.ALL_CONDITIONS);
+
+ FolderSnapshot snap= new FolderSnapshot(testLinkedFolder);
+
+ // Create a subfolder containing a file under the linked folder.
+ IFolder subfolder= testLinkedFolder.getFolder("A");
+ subfolder.create(true, true, getMonitor());
+ IFile file= subfolder.getFile("test.txt");
+ file.create(new ByteArrayInputStream("test contents".getBytes(StandardCharsets.UTF_8)), true, getMonitor());
+ // Delete the target of the linked folder on the file system making the linked folder out of sync
+ // with the file system.
+ IFileStore folderStore= EFS.getStore(testLinkedFolder.getLocationURI());
+ folderStore.delete(EFS.NONE, getMonitor()); // Delete the target folder on the file system.
+
+ execute(op);
+ assertFalse("Folder delete failed", testLinkedFolder.exists());
+ undo();
+ assertTrue("Folder recreation failed", testLinkedFolder.exists());
+ assertTrue("Folder CONTENT was altered on undo", snap.isValid(testLinkedFolder.getParent()));
+ redo();
+ assertFalse("Redo delete failed", testLinkedFolder.exists());
+ }
+
public void testProjectDeleteUndoRedoLTK() throws ExecutionException, CoreException {
RefactoringContribution renameContribution= RefactoringCore.getRefactoringContribution(DeleteResourcesDescriptor.ID);
DeleteResourcesDescriptor desc= (DeleteResourcesDescriptor) renameContribution.createDescriptor();
@@ -430,10 +462,10 @@
}
/**
- * Returns a FileStore instance backed by storage in a temporary location.
- * The returned store will not exist, but will belong to an existing parent.
- * The tearDown method in this class will ensure the location is deleted
- * after the test is completed.
+ * Returns a FileStore instance backed by storage in a temporary location. The returned store
+ * will not exist, but will belong to an existing parent. The tearDown method in this class will
+ * ensure the location is deleted after the test is completed.
+ *
* @return The temp filestore to use
*/
private IFileStore getTempStore() {
diff --git a/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/resource/undostates/AbstractResourceUndoState.java b/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/resource/undostates/AbstractResourceUndoState.java
index 1face25..20ab87e 100644
--- a/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/resource/undostates/AbstractResourceUndoState.java
+++ b/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/resource/undostates/AbstractResourceUndoState.java
@@ -57,9 +57,13 @@
protected AbstractResourceUndoState(IResource resource) {
parent= resource.getParent();
if (resource.isAccessible()) {
- modificationStamp= resource.getModificationStamp();
- localTimeStamp= resource.getLocalTimeStamp();
resourceAttributes= resource.getResourceAttributes();
+ // If resourceAttruibutes is null, the resource doesn't exist on disk, so leave
+ // modificationStamp and localTimeStamp set to IResource.NULL_STAMP.
+ if (resourceAttributes != null) {
+ modificationStamp= resource.getModificationStamp();
+ localTimeStamp= resource.getLocalTimeStamp();
+ }
try {
IMarker[] markers= resource.findMarkers(null, true, IResource.DEPTH_INFINITE);
markerDescriptions= new MarkerUndoState[markers.length];
diff --git a/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/resource/undostates/ContainerUndoState.java b/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/resource/undostates/ContainerUndoState.java
index 42fa0b0..b8e1ea0 100644
--- a/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/resource/undostates/ContainerUndoState.java
+++ b/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/resource/undostates/ContainerUndoState.java
@@ -12,6 +12,8 @@
package org.eclipse.ltk.internal.core.refactoring.resource.undostates;
import java.net.URI;
+import java.util.ArrayList;
+import java.util.List;
import org.eclipse.core.runtime.Assert;
import org.eclipse.core.runtime.CoreException;
@@ -44,7 +46,7 @@
protected URI location;
private String defaultCharSet;
- private AbstractResourceUndoState[] members;
+ private List<AbstractResourceUndoState> members;
/**
* Create a container description from the specified container handle that
@@ -132,10 +134,13 @@
if (container.isAccessible()) {
defaultCharSet = container.getDefaultCharset(false);
IResource[] resourceMembers = container.members();
- members = new AbstractResourceUndoState[resourceMembers.length];
- for (int i = 0; i < resourceMembers.length; i++) {
- members[i] = (AbstractResourceUndoState) ResourceUndoState
- .fromResource(resourceMembers[i]);
+ members = new ArrayList<>(resourceMembers.length);
+ for (IResource resourceMember : resourceMembers) {
+ // Add a member only if its container exists on disk or if the member is a linked resource.
+ // Otherwise avoid wasting time. See http://bugs.eclipse.org/508260
+ if (localTimeStamp != IResource.NULL_STAMP || resourceMember.isLinked()) {
+ members.add((AbstractResourceUndoState) ResourceUndoState.fromResource(resourceMember));
+ }
}
}
} catch (CoreException e) {
@@ -161,11 +166,10 @@
IProgressMonitor monitor, int ticks) throws CoreException {
// restore any children
- if (members != null && members.length > 0) {
- for (int i = 0; i < members.length; i++) {
- members[i].parent = parentHandle;
- members[i].createResource(new SubProgressMonitor(monitor, ticks
- / members.length));
+ if (members != null) {
+ for (AbstractResourceUndoState member : members) {
+ member.parent = parentHandle;
+ member.createResource(new SubProgressMonitor(monitor, ticks / members.size()));
}
}
}
@@ -176,21 +180,17 @@
monitor.beginTask(
RefactoringCoreMessages.FolderDescription_SavingUndoInfoProgress, 100);
if (members != null) {
- for (int i = 0; i < members.length; i++) {
- if (members[i] instanceof FileUndoState) {
- IPath path = resource.getFullPath().append(
- ((FileUndoState) members[i]).name);
- IFile fileHandle = resource.getWorkspace().getRoot().getFile(
- path);
- members[i].recordStateFromHistory(fileHandle,
- new SubProgressMonitor(monitor, 100 / members.length));
- } else if (members[i] instanceof FolderUndoState) {
- IPath path = resource.getFullPath().append(
- ((FolderUndoState) members[i]).name);
- IFolder folderHandle = resource.getWorkspace().getRoot()
- .getFolder(path);
- members[i].recordStateFromHistory(folderHandle,
- new SubProgressMonitor(monitor, 100 / members.length));
+ for (AbstractResourceUndoState member : members) {
+ if (member instanceof FileUndoState) {
+ IPath path = resource.getFullPath().append(((FileUndoState) member).name);
+ IFile fileHandle = resource.getWorkspace().getRoot().getFile(path);
+ member.recordStateFromHistory(fileHandle,
+ new SubProgressMonitor(monitor, 100 / members.size()));
+ } else if (member instanceof FolderUndoState) {
+ IPath path = resource.getFullPath().append(((FolderUndoState) member).name);
+ IFolder folderHandle = resource.getWorkspace().getRoot().getFolder(path);
+ member.recordStateFromHistory(folderHandle,
+ new SubProgressMonitor(monitor, 100 / members.size()));
}
}
}
@@ -215,13 +215,13 @@
*/
public ContainerUndoState getFirstLeafFolder() {
// If there are no members, this is a leaf
- if (members == null || members.length == 0) {
+ if (members == null || members.isEmpty()) {
return this;
}
// Traverse the members and find the first potential leaf
- for (int i = 0; i < members.length; i++) {
- if (members[i] instanceof ContainerUndoState) {
- return ((ContainerUndoState) members[i]).getFirstLeafFolder();
+ for (AbstractResourceUndoState member : members) {
+ if (member instanceof ContainerUndoState) {
+ return ((ContainerUndoState) member).getFirstLeafFolder();
}
}
// No child folders were found, this is a leaf
@@ -238,13 +238,9 @@
*/
public void addMember(AbstractResourceUndoState member) {
if (members == null) {
- members = new AbstractResourceUndoState[] { member };
- } else {
- AbstractResourceUndoState[] expandedMembers = new AbstractResourceUndoState[members.length + 1];
- System.arraycopy(members, 0, expandedMembers, 0, members.length);
- expandedMembers[members.length] = member;
- members = expandedMembers;
+ members = new ArrayList<>();
}
+ members.add(member);
}
@Override
@@ -274,9 +270,9 @@
if (existence) {
if (checkMembers) {
// restore any children
- if (members != null && members.length > 0) {
- for (int i = 0; i < members.length; i++) {
- if (!members[i].verifyExistence(checkMembers)) {
+ if (members != null) {
+ for (AbstractResourceUndoState member : members) {
+ if (!member.verifyExistence(checkMembers)) {
return false;
}
}
