diff --git a/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/ArchiveFileSystem.java b/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/ArchiveFileSystem.java index a9a640fff7b8..ba2c4a386745 100644 --- a/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/ArchiveFileSystem.java +++ b/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()); } diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java index 3cc8edc82661..edb78d97108a 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java +++ b/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(); - String rootName = vfsPeer.getName(rootId); - String rootPath = getRootPath(rootUrl, rootName); - NewVirtualFile root = ensureRootCached(rootPath, rootUrl); + //TODO RC: tuple (rootUrl, rootPath/name, rootFS) is better to be wrapped as 'record Root(url,path,fs)', + // with all the normalization methods encapsulated. It will be much better than all the components + // dancing/messing around individually + 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 - * but for archives e.g. jars rootName will be just name (see {@link PersistentFSImpl#findRoot}) - * so we need to extract 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}) + * rootPath == rootName in case of roots that are not archives + * But for archives e.g. jars rootName will be just file name (see {@link PersistentFSImpl#findRoot(String, NewVirtualFileSystem)}) + * so we need to extract a path from url (IDEA-341011) + * (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 - NewVirtualFile ensureRootCached(@NotNull String missedRootPath, @NotNull String missedRootUrl) { + private NewVirtualFile ensureRootCached(@NotNull String missedRootPath, + @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 + - " (protocol: " + fs.getProtocol() + ", path: " + missedRootPath + ") -> " + cachedRoot); + LOG.trace("\tforce caching " + missedRootUrl + " (protocol: " + fs.getProtocol() + ", path: " + missedRootPath + ")" + + " -> " + cachedRoot); } return cachedRoot; } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/FSRecordsImplTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/FSRecordsImplTest.java index e48cf073942c..2309c044cca8 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/FSRecordsImplTest.java +++ b/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 {