test fix, correctly remove root after deleting, correctly find attributes for special files

platform/platform-impl/src/com/intellij/openapi/vfs/impl/local/LocalFileSystemBase.java

@@ -197,16 +197,15 @@ public abstract class LocalFileSystemBase extends LocalFileSystem {
   protected String normalize(@NotNull String path) {
     if (path.isEmpty()) {
       try {
-        return new File("").getCanonicalPath();
+        path = new File("").getCanonicalPath();
       }
       catch (IOException e) {
         return path;
       }
     }
-
-    if (SystemInfo.isWindows) {
+    else if (SystemInfo.isWindows) {
       if (path.charAt(0) == '/' && !path.startsWith("//")) {
-        path = path.substring(1);  // hack over new File(path).toUrl().getFile()
+        path = path.substring(1);  // hack over new File(path).toURI().toURL().getFile()
       }
 
       if (path.contains("~")) {
@@ -218,15 +217,25 @@ public abstract class LocalFileSystemBase extends LocalFileSystem {
         }
       }
     }
-    else {
-      if (!StringUtil.startsWithChar(path, '/')) {
-        path = new File(path).getAbsolutePath();
-      }
+    File file = new File(path);
+
+    if (!isAbsoluteFileOrDriveLetter(file)) {
+      path = file.getAbsolutePath();
     }
 
     return FileUtil.normalize(path);
   }
 
+  private static boolean isAbsoluteFileOrDriveLetter(File file) {
+    String path = file.getPath();
+    if (SystemInfo.isWindows && path.length() == 2 && path.charAt(1) == ':') {
+      // just drive letter.
+      // return true, despite the fact that technically it's not an absolute path
+      return true;
+    }
+    return file.isAbsolute();
+  }
+
   @Override
   public VirtualFile refreshAndFindFileByIoFile(@NotNull File file) {
     String path = file.getAbsolutePath();
@@ -688,12 +697,20 @@ public abstract class LocalFileSystemBase extends LocalFileSystem {
 
   @Override
   public FileAttributes getAttributes(@NotNull final VirtualFile file) {
-    String path = normalize(file.getPath());
-    if (path == null) return null;
-    if (StringUtil.isEmpty(path) || file.getParent() == null && path.startsWith("//")) {
+    String path = file.getPath();
+
+    // ordinarily, usual valid file path is always absolute.
+    // however, some artificially constructed files may appear here which are not absolute, e.g. new FakeVirtualFile(root, "x.txt")
+    // they shall not pass
+    File ioFile = new File(path);
+    if (!isAbsoluteFileOrDriveLetter(ioFile)) return null;
+
+    if (file.getParent() == null && path.startsWith("//")) {
+      // UNC path
       return FAKE_ROOT_ATTRIBUTES;  // fake Windows roots
     }
-    return FileSystemUtil.getAttributes(FileUtil.toSystemDependentName(path));
+    // ioFile already normalized
+    return FileSystemUtil.getAttributes(ioFile);
   }
 
   @Override

platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java

@@ -21,6 +21,7 @@ import com.intellij.openapi.components.ApplicationComponent;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.util.ShutDownTracker;
 import com.intellij.openapi.util.io.*;
+import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.openapi.vfs.*;
 import com.intellij.openapi.vfs.newvfs.*;
 import com.intellij.openapi.vfs.newvfs.events.*;
@@ -753,12 +754,13 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone
   @Override
   @Nullable
   public VirtualFileSystemEntry findRoot(@NotNull String basePath, @NotNull NewVirtualFileSystem fs) {
-    String rootUrl = fs.getProtocol() + "://" + VfsImplUtil.normalize(fs, basePath);
+    String rootUrl = normalizeRootUrl(fs, basePath);
 
-    VirtualFileSystemEntry root;
+    boolean isFakeRoot = basePath.isEmpty();
     myRootsLock.readLock().lock();
+    VirtualFileSystemEntry root;
     try {
-      root = basePath.isEmpty() ? myFakeRoot : myRoots.get(rootUrl);
+      root = isFakeRoot ? myFakeRoot : myRoots.get(rootUrl);
       if (root != null) return root;
     }
     finally {
@@ -767,14 +769,14 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone
 
     myRootsLock.writeLock().lock();
     try {
-      root = basePath.isEmpty() ? myFakeRoot : myRoots.get(rootUrl);
+      root = isFakeRoot ? myFakeRoot : myRoots.get(rootUrl);
       if (root != null) return root;
 
       int rootId = FSRecords.findRootRecord(rootUrl);
       root = myRootsById.get(rootId);
       if (root != null) return root;
 
-      if (basePath.isEmpty()) {
+      if (isFakeRoot) {
         // fake super-root
         root = new VirtualDirectoryImpl("", null, fs, rootId, 0) {
           @SuppressWarnings("NonSynchronizedMethodOverridesSynchronizedMethod")
@@ -809,22 +811,19 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone
         root = new VirtualDirectoryImpl(basePath, null, fs, rootId, 0);
       }
 
-      final FileAttributes attributes = fs.getAttributes(root);
-      if (attributes == null) {
-        return null;
-      }
-
-      final boolean newRoot = writeAttributesToRecord(rootId, 0, root, fs, attributes);
-      if (!newRoot) {
-        if (attributes.lastModified != FSRecords.getTimestamp(rootId)) {
-          root.markDirtyRecursively();
-        }
-      }
-
-      if (basePath.isEmpty()) {
+      if (isFakeRoot) {
         myFakeRoot = root;
       }
       else {
+        FileAttributes attributes = fs.getAttributes(root);
+        if (attributes == null || !attributes.isDirectory()) {
+          return null;
+        }
+        final boolean newRoot = writeAttributesToRecord(rootId, 0, root, fs, attributes);
+        if (!newRoot && attributes.lastModified != FSRecords.getTimestamp(rootId)) {
+          root.markDirtyRecursively();
+        }
+
         myRoots.put(rootUrl, root);
         myRootsById.put(rootId, root);
 
@@ -838,6 +837,12 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone
     }
   }
 
+  @NotNull
+  private static String normalizeRootUrl(@NotNull NewVirtualFileSystem fs, @NotNull String basePath) {
+    String url = fs.getProtocol() + "://" + VfsImplUtil.normalize(fs, basePath);
+    return StringUtil.trimEnd(url, "/");
+  }
+
   @Override
   public void clearIdCache() {
     myIdToDirCache.clear();
@@ -1048,7 +1053,7 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone
     if (parentId == 0) {
       myRootsLock.writeLock().lock();
       try {
-        String rootUrl = file.getUrl();
+        String rootUrl = normalizeRootUrl((NewVirtualFileSystem)file.getFileSystem(), file.getPath());
         myRoots.remove(rootUrl);
         myRootsById.remove(id);
         FSRecords.deleteRootRecord(id);

platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/LocalFileSystemTest.java

@@ -22,9 +22,11 @@ import com.intellij.openapi.util.io.FileUtil;
 import com.intellij.openapi.util.io.IoTestUtil;
 import com.intellij.openapi.vfs.*;
 import com.intellij.openapi.vfs.newvfs.ManagingFS;
+import com.intellij.openapi.vfs.newvfs.NewVirtualFile;
 import com.intellij.openapi.vfs.newvfs.NewVirtualFileSystem;
 import com.intellij.openapi.vfs.newvfs.RefreshQueue;
 import com.intellij.openapi.vfs.newvfs.events.VFileDeleteEvent;
+import com.intellij.openapi.vfs.newvfs.impl.FakeVirtualFile;
 import com.intellij.openapi.vfs.newvfs.impl.VirtualDirectoryImpl;
 import com.intellij.openapi.vfs.newvfs.impl.VirtualFileSystemEntry;
 import com.intellij.openapi.vfs.newvfs.persistent.PersistentFS;
@@ -342,4 +344,35 @@ public class LocalFileSystemTest extends PlatformLangTestCase {
     final VirtualFile vFile = LocalFileSystem.getInstance().refreshAndFindFileByIoFile(file);
     assertNull(vFile);
   }
+
+  public void testGetAttributesConvertsToAbsolute() throws Exception {
+    PersistentFS fs = PersistentFS.getInstance();
+    LocalFileSystem lfs = LocalFileSystem.getInstance();
+    NewVirtualFile fakeRoot = fs.findRoot("", lfs);
+    assertNotNull(fakeRoot);
+    File userDir = new File(System.getProperty("user.dir"));
+    File[] files = userDir.listFiles();
+    File fileToQuery;
+    if (files != null && files.length != 0) {
+      fileToQuery = files[0];
+    }
+    else if (userDir.isDirectory()) {
+      fileToQuery = FileUtil.createTempFile(userDir, getTestName(false), "", true);
+      myFilesToDelete.add(fileToQuery);
+    }
+    else {
+      // can't test
+      return;
+    }
+
+    FileAttributes attributes = lfs.getAttributes(new FakeVirtualFile(fakeRoot, fileToQuery.getName()));
+    assertNull(attributes);
+
+    attributes = lfs.getAttributes(new FakeVirtualFile(fakeRoot, "windows"));
+    assertNull(attributes);
+    attributes = lfs.getAttributes(new FakeVirtualFile(fakeRoot, "usr"));
+    assertNull(attributes);
+    attributes = lfs.getAttributes(new FakeVirtualFile(fakeRoot, "Users"));
+    assertNull(attributes);
+  }
 }

platform/platform-tests/testSrc/com/intellij/util/io/PersistentFSTest.java

@@ -15,6 +15,10 @@
  */
 package com.intellij.util.io;
 
+import com.intellij.openapi.util.SystemInfo;
+import com.intellij.openapi.util.io.FileUtil;
+import com.intellij.openapi.util.io.IoTestUtil;
+import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.openapi.vfs.LocalFileSystem;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.VirtualFileWithId;
@@ -23,6 +27,10 @@ import com.intellij.openapi.vfs.newvfs.persistent.PersistentFS;
 import com.intellij.testFramework.PlatformTestCase;
 
 import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+
+import static org.junit.Assume.assumeTrue;
 
 public class PersistentFSTest extends PlatformTestCase {
   @Override
@@ -59,7 +67,7 @@ public class PersistentFSTest extends PlatformTestCase {
     VirtualFile[] roots = fs.getRoots(LocalFileSystem.getInstance());
     for (VirtualFile root : roots) {
       int rid = fs.getId(fakeRoot, root.getName(), LocalFileSystem.getInstance());
-      assertTrue(root.getPath(), 0 != rid);
+      assertTrue(root.getPath()+"; Roots:"+ Arrays.toString(roots), 0 != rid);
     }
 
     NewVirtualFile c = fakeRoot.refreshAndFindChild("Users");
@@ -71,4 +79,29 @@ public class PersistentFSTest extends PlatformTestCase {
     c = fakeRoot.refreshAndFindChild("Windows");
     assertNull(c);
   }
+
+  public void testDeleteSubstRoots() throws IOException, InterruptedException {
+    assumeTrue(SystemInfo.isWindows);
+
+    File tempDirectory = FileUtil.createTempDirectory(getTestName(false), null);
+    File substRoot = IoTestUtil.createSubst(tempDirectory.getPath());
+    VirtualFile subst = LocalFileSystem.getInstance().refreshAndFindFileByIoFile(substRoot);
+    assertNotNull(subst);
+    try {
+      final File[] children = substRoot.listFiles();
+      assertNotNull(children);
+    }
+    finally {
+      IoTestUtil.deleteSubst(substRoot.getPath());
+    }
+    subst.refresh(false, true);
+    PersistentFS fs = PersistentFS.getInstance();
+
+    VirtualFile[] roots = fs.getRoots(LocalFileSystem.getInstance());
+    for (VirtualFile root : roots) {
+      String rootPath = root.getPath();
+      String prefix = StringUtil.commonPrefix(rootPath, substRoot.getPath());
+      assertEmpty(prefix);
+    }
+  }
 }