[vfs][refactoring] redefine BatchingFileSystem contract: always return case-sensitive maps

... thus it is responsibility of a caller to convert returned map to case-insensitive, if needed, and deal with possible inconsistencies during that conversion

GitOrigin-RevId: 83b2c714a4e9cba62281d2bf2a05ebd04f643225
This commit is contained in:
Ruslan Cheremin
2025-07-27 15:22:38 +00:00
committed by intellij-monorepo-bot
parent 66f1d90df4
commit 48e5349bbe
5 changed files with 37 additions and 13 deletions
@@ -165,10 +165,11 @@ public abstract class ArchiveFileSystem extends NewVirtualFileSystem implements
ArchiveHandler handler = getHandler(dir);
if (childNames == null) {
childNames = createFilePathSet(handler.list(directoryRelativePath), isCaseSensitive());
childNames = createFilePathSet(handler.list(directoryRelativePath), /*isCaseSensitive: */ true);
}
Map<String, FileAttributes> childrenWithAttributes = createFilePathMap(childNames.size(), isCaseSensitive());
//We must return 'normal' (case-sensitive) map from this method, see BatchingFileSystem.listWithAttributes() contract:
Map<String, FileAttributes> childrenWithAttributes = createFilePathMap(childNames.size(), /*isCaseSensitive: */ true);
for (String childName : childNames) {
String childRelativePath = normalizedDirectoryPath + childName;
@@ -18,12 +18,20 @@ import java.util.Set;
@ApiStatus.Internal
public interface BatchingFileSystem {
/**
* Returns a list of files in a directory along with their attributes.
* When the {@code childrenNames} set is {@code null}, all files should be returned.
* TODO RC: specify, should returned map be case-(in)sensitive, according to dir.isCaseSensitive(), or it should be a plain
* case-sensitive map, and it is up to client code to transform it, if needed?
* @return directory's children, in form of Map[childName-> FileAttributes]
* When the {@code childrenNames} is not-null, only those children's data should be returned (if exists!),
* when {@code childrenNames} is null -> all directory's children should be returned.
* Returned map should be a 'normal' (i.e., case-sensitive) map -- it is up to calling code to covert it to case-insensitive, if
* needed.
*/
@NotNull Map<@NotNull String, @NotNull FileAttributes> listWithAttributes(@NotNull VirtualFile dir, @Nullable Set<String> childrenNames);
//Why contract defines returned map to be case-sensitive? Because dir.isCaseSensitive() is tricky, and sometimes it's value could
// be unreliable/outdated -- in which case this method would provide incorrect info, be it defined to return map with case-sensitivity
// =dir.isCaseSensitive().
// But the responsibility of (Batching)FileSystem is _only_ to be golden-source of info about actual file system state -- it is
// the responsibility of the caller (=VFS) to deal with (possible) inconsistencies between VFS state and actual FS state.
//MAYBE RC: return List<Pair<String,FileAttributes>>? this way question of case-sensitivity is not even on the table
@NotNull Map<@NotNull String, @NotNull FileAttributes> listWithAttributes(@NotNull VirtualFile dir,
@Nullable Set<String> childrenNames);
default @NotNull Map<@NotNull String, @NotNull FileAttributes> listWithAttributes(@NotNull VirtualFile dir) {
return listWithAttributes(dir, null);
@@ -372,7 +372,8 @@ public class LocalFileSystemImpl extends LocalFileSystemBase implements Disposab
return Collections.emptyMap();
}
try {
Map<String, FileAttributes> childrenWithAttributes = createFilePathMap(10, dir.isCaseSensitive());
//We must return 'normal' (case-sensitive) map from this method, see BatchingFileSystem.listWithAttributes() contract:
Map<String, FileAttributes> childrenWithAttributes = createFilePathMap(10, /*caseSensitive: */true);
PlatformNioHelper.visitDirectory(Path.of(toIoPath(dir)), filter, (file, ioAttributesHolder) -> {
try {
@@ -255,6 +255,11 @@ final class RefreshWorker {
for (String name : childrenNames) {
childrenWithAttributes.put(name, null);
}
if(childrenWithAttributes.size()!=childrenNames.length){
//TODO RC: seems like dir.isCaseSensitive() is wrong/outdated (i.e. actual dir case-sensitivity is different from
// FS-default, and it wasn't yet determined).
// We should re-query dir.case-sensitivity
}
}
myIoTime.addAndGet(System.nanoTime() - t);
@@ -401,15 +406,23 @@ final class RefreshWorker {
}
/** Converts a case-sensitive rawDirList map into case-insensitive, if toCaseSensitive=false, leaves the map as-is otherwise */
private static @NotNull Map<String, FileAttributes> adjustCaseSensitivity(@NotNull Map<String, FileAttributes> rawDirList,
private static @NotNull Map<String, FileAttributes> adjustCaseSensitivity(@NotNull Map<String, FileAttributes> childrenWithAttributes,
boolean toCaseSensitive) {
if (toCaseSensitive) {
return rawDirList;
return childrenWithAttributes;
}
else {
Map<String, FileAttributes> filtered = createFilePathMap(rawDirList.size(), /*caseSensitive: */ false);
filtered.putAll(rawDirList);
return filtered;
Map<String, FileAttributes> childrenWithAttributesCaseInsensitive = createFilePathMap(
childrenWithAttributes.size(),
/*caseSensitive: */ false
);
childrenWithAttributesCaseInsensitive.putAll(childrenWithAttributes);
if (childrenWithAttributesCaseInsensitive.size() != childrenWithAttributes.size()) {
//TODO RC: seems like a conflict if dir.isCaseSensitive() is wrong/outdated (i.e. actual dir case-sensitivity
// is different from FS-default, and it wasn't yet determined).
// We should re-query dir.case-sensitivity
}
return childrenWithAttributesCaseInsensitive;
}
}
@@ -456,6 +456,7 @@ public final class PersistentFSImpl extends PersistentFS implements Disposable {
//TODO RC: we use a map here to prevent duplicates -- but we still add those duplicates to childrenToAdd
// -- what's the point?
//MAYBE RC: duplicates may indicate wrongly-detected dir.caseSensitivity -- so we should consider re-detect it?
ChildInfo newChild = justCreated.computeIfAbsent(
newChildName,
_newChildName -> makeChildRecord(dir, dirId, _newChildName, childData, fs, null)