From de965742f030d3e7e72e6a232aca63b18ad8ff8a Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Sun, 20 Jan 2019 02:55:35 +0300 Subject: [PATCH] directory creation should not lead to deeper file pointer invalidation --- .../openapi/vfs/impl/FilePointerPartNode.java | 21 ++++--- .../impl/VirtualFilePointerManagerImpl.java | 43 +++++++++----- .../vfs/impl/VirtualFilePointerTest.java | 57 ++++++++++++++++++- 3 files changed, 96 insertions(+), 25 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/FilePointerPartNode.java b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/FilePointerPartNode.java index 60ce1838462d..a2f76d170f74 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/FilePointerPartNode.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/FilePointerPartNode.java @@ -142,13 +142,20 @@ class FilePointerPartNode { void addRelevantPointersFrom(@Nullable VirtualFile parent, boolean separator, @NotNull CharSequence childName, - @NotNull List out) { + @NotNull List out, boolean addSubdirectoryPointers) { CharSequence parentName = parent == null ? null : parent.getNameSequence(); FilePointerPartNode[] outNode = new FilePointerPartNode[1]; int position = position(parent, parentName, separator, childName, 0, outNode, out); if (position != -1) { FilePointerPartNode node = outNode[0]; - addAllPointersUnder(node, out); + boolean matches = position == node.part.length() || addSubdirectoryPointers; + if (matches && node.leaves != null) { + out.add(node); + } + if (addSubdirectoryPointers) { + // when "a/b" changed, treat all "a/b/*" virtual file pointers as changed because that's what happens on directory rename "a"->"newA": "a" deleted and "newA" created + addAllPointersStrictlyUnder(node, out); + } } } @@ -164,12 +171,12 @@ class FilePointerPartNode { return false; } - private static void addAllPointersUnder(@NotNull FilePointerPartNode node, @NotNull List out) { - if (node.leaves != null) { - out.add(node); - } + private static void addAllPointersStrictlyUnder(@NotNull FilePointerPartNode node, @NotNull List out) { for (FilePointerPartNode child : node.children) { - addAllPointersUnder(child, out); + if (child.leaves != null) { + out.add(child); + } + addAllPointersStrictlyUnder(child, out); } } diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java index 760981693120..5e08ce42458a 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java @@ -34,6 +34,11 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; +import java.io.IOException; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.*; import java.util.concurrent.ConcurrentMap; @@ -97,18 +102,19 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp } @TestOnly - synchronized VirtualFilePointer[] getPointersUnder(VirtualFile parent, String childName) { + @NotNull + synchronized VirtualFilePointer[] getPointersUnder(@NotNull VirtualFile parent, @NotNull String childName) { List nodes = new ArrayList<>(); - addRelevantPointers(parent, true, childName, nodes); + addRelevantPointers(parent, true, childName, nodes, true); return toPointers(nodes); } private void addRelevantPointers(VirtualFile parent, boolean separator, @NotNull CharSequence childName, - @NotNull List out) { + @NotNull List out, boolean addSubdirectoryPointers) { for (FilePointerPartNode root : myPointers.values()) { - root.addRelevantPointersFrom(parent, separator, childName, out); + root.addRelevantPointersFrom(parent, separator, childName, out, addSubdirectoryPointers); } } @@ -274,7 +280,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp private synchronized void assertAllPointersDisposed() { for (FilePointerPartNode root : myPointers.values()) { List left = new ArrayList<>(); - root.addRelevantPointersFrom(null, false, "", left); + root.addRelevantPointersFrom(null, false, "", left, true); List pointers = new ArrayList<>(); for (FilePointerPartNode node : left) { node.addAllPointersTo(pointers); @@ -303,9 +309,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp @TestOnly synchronized void addAllPointersTo(@NotNull Collection pointers) { List out = new ArrayList<>(); - for (FilePointerPartNode root : myPointers.values()) { - root.addRelevantPointersFrom(null, false, "", out); - } + addRelevantPointers(null, false, "", out, true); for (FilePointerPartNode node : out) { node.addAllPointersTo(pointers); } @@ -353,6 +357,14 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp private List myNodesToUpdateUrl = Collections.emptyList(); private List myNodesToFire = Collections.emptyList(); + private static boolean isEmptyDir(@NotNull String path) { + try (DirectoryStream stream = Files.newDirectoryStream(Paths.get(path))) { + return !stream.iterator().hasNext(); + } + catch (IOException e) { + return false; + } + } @Override public void before(@NotNull final List events) { ApplicationManager.getApplication().assertIsDispatchThread(); // guarantees no attempts to get read action lock under "this" lock @@ -366,23 +378,24 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp for (VFileEvent event : events) { if (event instanceof VFileDeleteEvent) { final VFileDeleteEvent deleteEvent = (VFileDeleteEvent)event; - addRelevantPointers(deleteEvent.getFile(), false, "", toFireEvents); + addRelevantPointers(deleteEvent.getFile(), false, "", toFireEvents, true); } else if (event instanceof VFileCreateEvent) { final VFileCreateEvent createEvent = (VFileCreateEvent)event; - addRelevantPointers(createEvent.getParent(), true, createEvent.getChildName(), toFireEvents); + boolean fireSubdirectoryPointers = createEvent.isDirectory() && !isEmptyDir(createEvent.getPath()); + addRelevantPointers(createEvent.getParent(), true, createEvent.getChildName(), toFireEvents, fireSubdirectoryPointers); } else if (event instanceof VFileCopyEvent) { final VFileCopyEvent copyEvent = (VFileCopyEvent)event; - addRelevantPointers(copyEvent.getNewParent(), true, copyEvent.getNewChildName(), toFireEvents); + addRelevantPointers(copyEvent.getNewParent(), true, copyEvent.getNewChildName(), toFireEvents, true); } else if (event instanceof VFileMoveEvent) { final VFileMoveEvent moveEvent = (VFileMoveEvent)event; VirtualFile eventFile = moveEvent.getFile(); - addRelevantPointers(moveEvent.getNewParent(), true, eventFile.getName(), toFireEvents); + addRelevantPointers(moveEvent.getNewParent(), true, eventFile.getName(), toFireEvents, true); List nodes = new ArrayList<>(); - addRelevantPointers(eventFile, false, "", nodes); + addRelevantPointers(eventFile, false, "", nodes, true); toFireEvents.addAll(nodes); // files deleted from eventFile and created in moveEvent.getNewParent() collectNodes(nodes, toUpdateUrl); } @@ -392,10 +405,10 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp && !Comparing.equal(change.getOldValue(), change.getNewValue())) { VirtualFile eventFile = change.getFile(); VirtualFile parent = eventFile.getParent(); // e.g. for LightVirtualFiles - addRelevantPointers(parent, true, change.getNewValue().toString(), toFireEvents); + addRelevantPointers(parent, true, change.getNewValue().toString(), toFireEvents, true); List nodes = new ArrayList<>(); - addRelevantPointers(eventFile, false, "", nodes); + addRelevantPointers(eventFile, false, "", nodes, true); collectNodes(nodes, toUpdateUrl); } } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java index c6ceb219fdbd..4215361a1670 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java @@ -9,6 +9,7 @@ import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.application.WriteAction; import com.intellij.openapi.application.ex.PathManagerEx; +import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.vfs.*; @@ -16,13 +17,17 @@ import com.intellij.openapi.vfs.newvfs.ManagingFS; import com.intellij.openapi.vfs.pointers.VirtualFilePointer; import com.intellij.openapi.vfs.pointers.VirtualFilePointerListener; import com.intellij.openapi.vfs.pointers.VirtualFilePointerManager; -import com.intellij.testFramework.*; +import com.intellij.testFramework.LightPlatformTestCase; +import com.intellij.testFramework.PlatformTestUtil; +import com.intellij.testFramework.Timings; +import com.intellij.testFramework.VfsTestUtil; import com.intellij.util.ConcurrencyUtil; import com.intellij.util.ExceptionUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.TimeoutUtil; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.ui.UIUtil; +import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import java.io.File; @@ -633,9 +638,10 @@ public class VirtualFilePointerTest extends LightPlatformTestCase { myVirtualFilePointerManager.create(vDir.getUrl() + "/d1/subdir", disposable, listener); myVirtualFilePointerManager.create(vDir.getUrl() + "/d2/subdir", disposable, listener); - File dir = newFolder("d1"); + File dir = new File(vDir.getPath()+"/d1"); + FileUtil.createDirectory(dir); getVirtualFile(dir).getChildren(); - assertEquals("[before:false, after:false]", listener.log.toString()); + assertEquals("[]", listener.log.toString()); listener.log.clear(); File subDir = new File(dir, "subdir"); @@ -697,4 +703,49 @@ public class VirtualFilePointerTest extends LightPlatformTestCase { assertTrue(file == null || file.isValid()); } } + + @NotNull + protected static VirtualFile createChildDirectory(@NotNull final VirtualFile dir, @NotNull @NonNls final String name) { + try { + return WriteAction.computeAndWait(() -> + // requestor must be notnull + dir.createChildDirectory(dir, name)); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + protected static void rename(@NotNull final VirtualFile vFile1, @NotNull final String newName) { + try { + WriteCommandAction.writeCommandAction(null).run(() -> vFile1.rename(vFile1, newName)); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + + public void testVirtualPointerForSubdirMustNotFireWhenSuperDirectoryCreated() throws IOException { + VirtualFile vDir = getVirtualTempRoot(); + assertNotNull(vDir); + vDir.getChildren(); + vDir.refresh(false, true); + + LoggingListener listener = new LoggingListener(); + VirtualFilePointer subPtr = myVirtualFilePointerManager.create(vDir.getUrl() + "/cmake/subdir", disposable, listener); + + VirtualFile cmake = createChildDirectory(vDir, "cmake"); + assertEquals("[]", listener.log.toString()); + listener.log.clear(); + + createChildDirectory(cmake, "subdir"); + assertEquals("[before:false, after:true]", listener.log.toString()); + assertTrue(subPtr.isValid()); + + listener.log.clear(); + FileUtil.rename(new File(cmake.getPath()), "newCmake"); + vDir.refresh(false, true); + assertEquals("[before:true, after:false]", listener.log.toString()); + assertFalse(subPtr.isValid()); + } } \ No newline at end of file