[vfs] more tests and comments

GitOrigin-RevId: bb8e979492b9b08a45fb98025c4aee04f6bdd574

platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/ArchiveFileSystem.java

@@ -1,4 +1,4 @@
-// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.openapi.vfs.newvfs;
 
 import com.intellij.analysis.AnalysisBundle;
@@ -8,7 +8,10 @@ import com.intellij.openapi.util.Key;
 import com.intellij.openapi.util.Pair;
 import com.intellij.openapi.util.io.FileAttributes;
 import com.intellij.openapi.util.text.StringUtil;
-import com.intellij.openapi.vfs.*;
+import com.intellij.openapi.vfs.LocalFileSystem;
+import com.intellij.openapi.vfs.StandardFileSystems;
+import com.intellij.openapi.vfs.VfsUtilCore;
+import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.impl.ArchiveHandler;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
@@ -36,6 +39,10 @@ public abstract class ArchiveFileSystem extends NewVirtualFileSystem {
     return isCorrectFileType(file) ? findFileByPath(getRootPathByLocal(file)) : null;
   }
 
+  /**
+   * Dresses a local file path to make it a suitable root path for this filesystem.
+   * E.g., VirtualFile("/x/y.jar") -> "/x/y.jar!/"
+   */
   public @NotNull String getRootPathByLocal(@NotNull VirtualFile file) {
     return composeRootPath(file.getPath());
   }

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

@@ -1675,8 +1675,6 @@ public final class PersistentFSImpl extends PersistentFS implements Disposable {
     }
 
     if (attributes == null || !attributes.isDirectory()) {
-      //FIXME RC: put special sentinel value into rootsByUrl & idToDirCache so next attempt to resolve the root
-      //          doesn't need to repeat the whole lookup process (which is slow)
       return null;
     }
     // assume roots have the FS default case sensitivity
@@ -1688,7 +1686,7 @@ public final class PersistentFSImpl extends PersistentFS implements Disposable {
       FileAttributes parentAttributes = loadAttributes(fs, parentPath);
       if (parentAttributes != null) {
         throw new IllegalArgumentException(
-          "Must pass FS root path, but got: '" + path + "' (url: " + rootUrl + "), " +
+          "Must pass FS root path, but got: '" + path + "' (url: '" + rootUrl + "'), " +
           "which has a parent '" + parentPath + "'. " +
           "Use NewVirtualFileSystem.extractRootPath() for obtaining root path");
       }
@@ -1804,17 +1802,20 @@ public final class PersistentFSImpl extends PersistentFS implements Disposable {
       return;
     }
 
-    String rootUrl = missedRootUrlRef.get();
+    //TODO RC: tuple (rootUrl, rootPath/name, rootFS) is better to be wrapped as 'record Root(url,path,fs)',
-    String rootName = vfsPeer.getName(rootId);
+    //         with all the normalization methods encapsulated. It will be much better than all the components
-    String rootPath = getRootPath(rootUrl, rootName);
+    //         dancing/messing around individually
-    NewVirtualFile root = ensureRootCached(rootPath, rootUrl);
+    String missedRootUrl = missedRootUrlRef.get();
+    String missedRootName = vfsPeer.getName(rootId);
+    String missedRootPath = getRootPath(missedRootUrl, missedRootName);
+    NewVirtualFile root = ensureRootCached(missedRootPath, missedRootUrl);
     if (root != null) {
       if (root.getId() != rootId) {
         //Diogen reports like 49432216: rootId is _found_ among existing roots, but somehow ensureRootCached(rootPath, rootUrl)
         // leads to insertion of a _new_ root.
         // I suspect this is a bug, and this check is to provide more diagnostics for it:
         throw new IllegalStateException(
-          "root[#" + rootId + "]{rootName:'" + rootName + "', rootPath:'" + rootPath + "'} cached to something else: " +
+          "root[#" + rootId + "]{rootName: '" + missedRootName + "', rootPath: '" + missedRootPath + "'} cached to something else: " +
           "cached [#" + root.getId() + "]" +
           "{rootName: '" + root.getName() + "', rootPath: '" + root.getPath() + "', rootUrl: '" + root.getUrl() + "'}"
         );
@@ -1823,10 +1824,11 @@ public final class PersistentFSImpl extends PersistentFS implements Disposable {
   }
 
   /**
-   * rootName == rootPath in case of roots that are not archives
+   * rootPath == rootName in case of roots that are not archives
-   * but for archives e.g. jars rootName will be just name (see {@link PersistentFSImpl#findRoot})
+   * But for archives e.g. jars rootName will be just file name (see {@link PersistentFSImpl#findRoot(String, NewVirtualFileSystem)})
-   * so we need to extract path from url (IDEA-341011)
+   * so we need to extract a path from url (IDEA-341011)
-   * Path should not end with '!' because then '!' won't be stripped and file won't be found (see {@link ArchiveFileSystem#findLocalByRootPath})
+   * (Path should not end with '!' because then '!' won't be stripped and the apt file won't be found, see
+   * {@link ArchiveFileSystem#findLocalByRootPath})
    */
   private static @NotNull String getRootPath(@NotNull String rootUrl, @NotNull String rootName) {
     NewVirtualFileSystem fs = detectFileSystem(rootUrl);
@@ -1837,8 +1839,8 @@ public final class PersistentFSImpl extends PersistentFS implements Disposable {
     return rootName;
   }
 
-  @VisibleForTesting
+  private NewVirtualFile ensureRootCached(@NotNull String missedRootPath,
-  NewVirtualFile ensureRootCached(@NotNull String missedRootPath, @NotNull String missedRootUrl) {
+                                          @NotNull String missedRootUrl) {
     NewVirtualFileSystem fs = detectFileSystem(missedRootUrl);
     if (fs == null) {
       return null;
@@ -1847,8 +1849,8 @@ public final class PersistentFSImpl extends PersistentFS implements Disposable {
     try {
       NewVirtualFile cachedRoot = findRoot(missedRootPath, fs);
       if (LOG.isTraceEnabled()) {
-        LOG.trace("\tforce caching " + missedRootUrl +
+        LOG.trace("\tforce caching " + missedRootUrl + " (protocol: " + fs.getProtocol() + ", path: " + missedRootPath + ")" +
-                  " (protocol: " + fs.getProtocol() + ", path: " + missedRootPath + ") -> " + cachedRoot);
+                  " -> " + cachedRoot);
       }
       return cachedRoot;
     }

platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/FSRecordsImplTest.java

@@ -14,6 +14,7 @@ import org.junit.jupiter.api.io.TempDir;
 import java.io.DataInputStream;
 import java.io.IOException;
 import java.nio.file.Path;
+import java.util.Arrays;
 import java.util.concurrent.ThreadLocalRandom;
 import java.util.stream.Stream;
 
@@ -23,7 +24,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 
 public class FSRecordsImplTest {
 
-
   private FSRecordsImpl vfs;
   private Path vfsDir;
 
@@ -33,21 +33,18 @@ public class FSRecordsImplTest {
     int[] rootIds = new int[totalRoots];
 
     //shuffle roots to make sure rootUrlIds are not sequential:
-    for (int i = 0; i < totalRoots; i++) {
-      String rootUrl = "file:///root/" + i;
-      rootIds[i] = vfs.findOrCreateRootRecord(rootUrl);
-    }
     for (int i = 0; i < totalRoots; i++) {
       String rootUrl = "file:///root/" + (totalRoots - i - 1);
       vfs.connection().names().enumerate(rootUrl);
     }
     for (int i = 0; i < totalRoots; i++) {
       String rootUrl = "file:///root/" + i;
-      vfs.connection().names().enumerate(rootUrl);
       rootIds[i] = vfs.findOrCreateRootRecord(rootUrl);
     }
 
     int[] rootIdsReadBack = vfs.listRoots();
+    Arrays.sort(rootIds);
+    Arrays.sort(rootIdsReadBack);
     assertArrayEquals(
       rootIds,
       rootIdsReadBack,
@@ -56,22 +53,24 @@ public class FSRecordsImplTest {
   }
 
   @Test
-  public void insertedRoots_CouldBeReadBack_AfterReinitialization() throws Exception {
+  public void insertedRoots_CouldBeReadBack_AfterVFSReinitialization() throws Exception {
     int totalRoots = 100_000;               //too many roots could exceed attribute storage max record size
     int[] rootIds = new int[totalRoots];
     //shuffle roots to make sure rootUrlIds are not sequential:
-    for (int i = 0; i < totalRoots; i++) {
-      String rootUrl = "file:///root/" + i;
-      rootIds[i] = vfs.findOrCreateRootRecord(rootUrl);
-    }
     for (int i = 0; i < totalRoots; i++) {
       String rootUrl = "file:///root/" + (totalRoots - i - 1);
       vfs.connection().names().enumerate(rootUrl);
     }
+    for (int i = 0; i < totalRoots; i++) {
+      String rootUrl = "file:///root/" + i;
+      rootIds[i] = vfs.findOrCreateRootRecord(rootUrl);
+    }
 
     vfs = reloadVFS();
 
     int[] rootIdsReadBack = vfs.listRoots();
+    Arrays.sort(rootIds);
+    Arrays.sort(rootIdsReadBack);
     assertArrayEquals(
       rootIds,
       rootIdsReadBack,
@@ -79,6 +78,33 @@ public class FSRecordsImplTest {
     );
   }
 
+  @Test
+  public void insertedRoots_CouldBeResolvedByUrl_AfterVFSReinitialization() throws Exception {
+    int totalRoots = 100_000;               //too many roots could exceed attribute storage max record size
+    int[] rootIds = new int[totalRoots];
+    //shuffle roots to make sure rootUrlIds are not sequential:
+    for (int i = 0; i < totalRoots; i++) {
+      String rootUrl = "file:///root/" + (totalRoots - i - 1);
+      vfs.connection().names().enumerate(rootUrl);
+    }
+    for (int i = 0; i < totalRoots; i++) {
+      String rootUrl = "file:///root/" + i;
+      rootIds[i] = vfs.findOrCreateRootRecord(rootUrl);
+    }
+
+    vfs = reloadVFS();
+
+    for (int i = 0; i < totalRoots; i++) {
+      String rootUrl = "file:///root/" + i;
+      int rootId = vfs.findOrCreateRootRecord(rootUrl);
+      assertEquals(
+        rootIds[i],
+        rootId,
+        "root[" + i + "](url:"+rootUrl+") must be resolved to rootId: #" + rootIds[i] + ", but got #" + rootId + " instead"
+      );
+    }
+  }
+
 
   @Test
   public void writtenFileContentIsDeduplicated_onlyUniqueContentsAreStored() throws IOException {