Fix WindowsFS to support Windows (#14627)

Windows does not support file keys (the attribute returns null). Therefor WindowsFS cannot be used on Windows. Due to Windows 11 no longer preventing depleting of open files, we need to be able to use WindowsFS on Windows (irony), fix this to fallback to path as key. It should be good enough like an inode. Renaming/moving files is already handles by WindowsFS when it detects a change in key after rename/move.

Closes #11920

Author: uschindler

lucene/CHANGES.txt

@@ -58,6 +58,10 @@ Other
 
 * GITHUB#14516: Move sloppySin into SloppyMath from GeoUtils (Ankit Jain)
 
+* GITHUB#14627, GITHUB#11920: Fix mock filesystem WindowsFS to also work on Windows.
+  This is required to make tests pass on Windows 11 which no longer has
+  all limitations of previous Windows versions.  (Uwe Schindler)
+
 
 ======================= Lucene 10.2.1 =======================
 

lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java

@@ -89,7 +89,6 @@
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.store.IndexOutput;
 import org.apache.lucene.store.LockObtainFailedException;
-import org.apache.lucene.store.MMapDirectory;
 import org.apache.lucene.store.NIOFSDirectory;
 import org.apache.lucene.store.NoLockFactory;
 import org.apache.lucene.store.SimpleFSLockFactory;
@@ -108,7 +107,6 @@
 import org.apache.lucene.tests.util.TestUtil;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.BytesRef;
-import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.IOSupplier;
 import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.InfoStream;
@@ -1243,15 +1241,9 @@ public void testDeleteUnusedFiles() throws Exception {
       WindowsFS provider = new WindowsFS(path.getFileSystem());
       Path indexPath = provider.wrapPath(path);
 
-      // NOTE: on Unix, we cannot use MMapDir, because WindowsFS doesn't see/think it keeps file
-      // handles open. Yet, on Windows, we MUST use MMapDir because the Windows OS will in fact
-      // prevent file deletion for us, and fails otherwise:
-      FSDirectory dir;
-      if (Constants.WINDOWS) {
-        dir = new MMapDirectory(indexPath);
-      } else {
-        dir = new NIOFSDirectory(indexPath);
-      }
+      // NOTE: We cannot use MMapDir, because WindowsFS doesn't see/think it keeps file
+      // handles open.
+      FSDirectory dir = new NIOFSDirectory(indexPath);
 
       MergePolicy mergePolicy = newLogMergePolicy(true);
 
@@ -2879,9 +2871,6 @@ public void testCommitImmediatelyAfterNRTReopen() throws Exception {
   }
 
   public void testPendingDeleteDVGeneration() throws IOException {
-    // irony: currently we don't emulate windows well enough to work on windows!
-    assumeFalse("windows is not supported", Constants.WINDOWS);
-
     Path path = createTempDir();
 
     // Use WindowsFS to prevent open files from being deleted:
@@ -2947,9 +2936,6 @@ public void testPendingDeleteDVGeneration() throws IOException {
   }
 
   public void testPendingDeletionsRollbackWithReader() throws IOException {
-    // irony: currently we don't emulate Windows well enough to work on Windows!
-    assumeFalse("Windows is not supported", Constants.WINDOWS);
-
     Path path = createTempDir();
 
     // Use WindowsFS to prevent open files from being deleted:
@@ -2986,9 +2972,6 @@ public void testPendingDeletionsRollbackWithReader() throws IOException {
   }
 
   public void testWithPendingDeletions() throws Exception {
-    // irony: currently we don't emulate Windows well enough to work on Windows!
-    assumeFalse("Windows is not supported", Constants.WINDOWS);
-
     Path path = createTempDir();
 
     // Use WindowsFS to prevent open files from being deleted:
@@ -3038,8 +3021,6 @@ dir, new IndexWriterConfig(new MockAnalyzer(random())).setIndexCommit(indexCommi
 
   public void testPendingDeletesAlreadyWrittenFiles() throws IOException {
     Path path = createTempDir();
-    // irony: currently we don't emulate Windows well enough to work on Windows!
-    assumeFalse("Windows is not supported", Constants.WINDOWS);
 
     // Use WindowsFS to prevent open files from being deleted:
     WindowsFS provider = new WindowsFS(path.getFileSystem());

lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java

@@ -36,7 +36,6 @@
 import org.apache.lucene.tests.store.BaseDirectoryTestCase;
 import org.apache.lucene.tests.store.MockDirectoryWrapper;
 import org.apache.lucene.tests.util.TestUtil;
-import org.apache.lucene.util.Constants;
 
 public class TestFileSwitchDirectory extends BaseDirectoryTestCase {
 
@@ -172,7 +171,6 @@ protected Directory getDirectory(Path path) throws IOException {
   public void testDeleteAndList() throws IOException {
     // relies on windows semantics
     Path path = createTempDir();
-    assumeFalse("Irony we seem to not emulate windows well enough", Constants.WINDOWS);
     WindowsFS provider = new WindowsFS(path.getFileSystem());
     Path indexPath = provider.wrapPath(path);
     try (final FileSwitchDirectory dir =

lucene/misc/src/test/org/apache/lucene/misc/store/TestHardLinkCopyDirectoryWrapper.java

@@ -33,7 +33,6 @@
 import org.apache.lucene.store.NIOFSDirectory;
 import org.apache.lucene.tests.mockfile.WindowsFS;
 import org.apache.lucene.tests.store.BaseDirectoryTestCase;
-import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.IOUtils;
 
 // See: https://issues.apache.org/jira/browse/SOLR-12028 Tests cannot remove files on Windows
@@ -104,8 +103,6 @@ public void testCopyHardLinks() throws IOException {
   }
 
   public void testRenameWithHardLink() throws Exception {
-    // irony: currently we don't emulate windows well enough to work on windows!
-    assumeFalse("windows is not supported", Constants.WINDOWS);
     Path path = createTempDir();
     WindowsFS provider = new WindowsFS(path.getFileSystem());
     Directory dir1 = new NIOFSDirectory(provider.wrapPath(path));

lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsFS.java

@@ -21,10 +21,10 @@
 import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.attribute.BasicFileAttributeView;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 
 /**
  * FileSystem that (imperfectly) acts like windows.
@@ -55,10 +55,10 @@ public WindowsFS(FileSystem delegate) {
 
   /** Returns file "key" (e.g. inode) for the specified path */
   private Object getKey(Path existing) throws IOException {
-    BasicFileAttributeView view =
-        Files.getFileAttributeView(existing, BasicFileAttributeView.class);
-    BasicFileAttributes attributes = view.readAttributes();
-    return attributes.fileKey();
+    // the key may be null, e.g. on real Windows!
+    // in that case we fallback to the file path as key.
+    return Optional.ofNullable(Files.readAttributes(existing, BasicFileAttributes.class).fileKey())
+        .orElse(existing);
   }
 
   @Override

lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsPath.java

@@ -18,20 +18,17 @@
 
 import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.HashSet;
+import java.util.Set;
 
 class WindowsPath extends FilterPath {
 
-  static final HashSet<Character> RESERVED_CHARACTERS =
-      new HashSet<>(Arrays.asList('<', '>', ':', '\"', '\\', '|', '?', '*'));
+  static final Set<Character> RESERVED_CHARACTERS =
+      Set.of('<', '>', ':', '\"', '\\', '|', '?', '*');
 
-  static final HashSet<String> RESERVED_NAMES =
-      new HashSet<>(
-          Arrays.asList(
-              "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7",
-              "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8",
-              "LPT9"));
+  static final Set<String> RESERVED_NAMES =
+      Set.of(
+          "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7",
+          "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9");
 
   WindowsPath(Path path, FilterFileSystem fileSystem) {
     super(path, fileSystem);

lucene/test-framework/src/java/org/apache/lucene/tests/util/TestRuleTemporaryFilesCleanup.java

@@ -43,7 +43,6 @@
 import org.apache.lucene.tests.util.LuceneTestCase.SuppressFileSystems;
 import org.apache.lucene.tests.util.LuceneTestCase.SuppressFsync;
 import org.apache.lucene.tests.util.LuceneTestCase.SuppressTempFileChecks;
-import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.IOUtils;
 
 /**
@@ -157,11 +156,8 @@ private FileSystem initializeFileSystem() {
         fs = new HandleLimitFS(fs, limit).getFileSystem(null);
       }
       // windows is currently slow
-      if (random.nextInt(10) == 0) {
-        // don't try to emulate windows on windows: they don't get along
-        if (!Constants.WINDOWS && allowed(avoid, WindowsFS.class)) {
-          fs = new WindowsFS(fs).getFileSystem(null);
-        }
+      if (random.nextInt(10) == 0 && allowed(avoid, WindowsFS.class)) {
+        fs = new WindowsFS(fs).getFileSystem(null);
       }
       if (allowed(avoid, ExtrasFS.class)) {
         fs = new ExtrasFS(fs, random.nextInt(4) == 0, random.nextBoolean()).getFileSystem(null);

lucene/test-framework/src/test/org/apache/lucene/tests/mockfile/TestWindowsFS.java

@@ -29,6 +29,7 @@
 import java.util.Random;
 import java.util.concurrent.CyclicBarrier;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Predicate;
 import org.apache.lucene.util.Constants;
 
 /** Basic tests for WindowsFS */
@@ -37,11 +38,6 @@ public class TestWindowsFS extends MockFileSystemTestCase {
   @Override
   public void setUp() throws Exception {
     super.setUp();
-    // irony: currently we don't emulate windows well enough to work on windows!
-    // TODO: Can we fork this class and create a new class with only those tests that can run on
-    // Windows and then check if
-    // the Lucene WindowsFS error is same as the one OG Windows throws
-    assumeFalse("windows is not supported", Constants.WINDOWS);
   }
 
   @Override
@@ -188,8 +184,12 @@ public void testMove() throws IOException {
   }
 
   public void testFileName() {
-    Character[] reservedCharacters = WindowsPath.RESERVED_CHARACTERS.toArray(new Character[0]);
-    String[] reservedNames = WindowsPath.RESERVED_NAMES.toArray(new String[0]);
+    Character[] reservedCharacters =
+        WindowsPath.RESERVED_CHARACTERS.stream()
+            .filter(Predicate.not(Character.valueOf('\\')::equals))
+            .sorted()
+            .toArray(Character[]::new);
+    String[] reservedNames = WindowsPath.RESERVED_NAMES.stream().sorted().toArray(String[]::new);
     String fileName;
     Random r = random();
     Path dir = wrap(createTempDir());
@@ -211,7 +211,11 @@ public void testFileName() {
     expectThrows(InvalidPathException.class, () -> dir.resolve("foo:bar:tar"));
     expectThrows(InvalidPathException.class, () -> dir.resolve("foo?bar"));
     expectThrows(InvalidPathException.class, () -> dir.resolve("foo<bar"));
-    expectThrows(InvalidPathException.class, () -> dir.resolve("foo\\bar"));
+    if (!Constants.WINDOWS) {
+      // we need to exclude that test on Windows, because the backslash is resolved before our code
+      // checks the filename:
+      expectThrows(InvalidPathException.class, () -> dir.resolve("foo\\bar"));
+    }
     expectThrows(InvalidPathException.class, () -> dir.resolve("foo*bar|tar"));
     expectThrows(InvalidPathException.class, () -> dir.resolve("foo|bar?tar"));
   }