Merge pull request #28 from SWAT-engineering/improved-overflow-support/fix-delete-detection-in-indexing-rescanner

Improved overflow support: Fix detection of `DELETED` events in `IndexingRescanner`

Author: sungshik

src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java

@@ -32,6 +32,8 @@
 import java.nio.file.Path;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.nio.file.attribute.FileTime;
+import java.util.ArrayDeque;
+import java.util.Deque;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -82,20 +84,29 @@ protected MemorylessRescanner.Generator newGenerator(Path path, WatchScope scope
     }
 
     protected class Generator extends MemorylessRescanner.Generator {
-        // Field to keep track of the paths that are visited during the current
-        // rescan. After the visit, the `DELETED` events that happened since the
-        // previous rescan can be approximated.
-        private Set<Path> visited = new HashSet<>();
+        // Field to keep track of (a stack of) the paths that are visited during
+        // the current rescan (one frame for each nested subdirectory), to
+        // approximate `DELETED` events that happened since the previous rescan.
+        // Instances of this class are supposed to be used non-concurrently, so
+        // no synchronization to access this field is needed.
+        private final Deque<Set<Path>> visited = new ArrayDeque<>();
 
         public Generator(Path path, WatchScope scope) {
             super(path, scope);
+            this.visited.push(new HashSet<>()); // Initial set for content of `path`
+        }
+
+        private <T> void addToPeeked(Deque<Set<T>> deque, T t) {
+            var peeked = deque.peek();
+            if (peeked != null) {
+                peeked.add(t);
+            }
         }
 
         // -- MemorylessRescanner.Generator --
 
         @Override
         protected void generateEvents(Path path, BasicFileAttributes attrs) {
-            visited.add(path);
             var lastModifiedTimeOld = index.get(path);
             var lastModifiedTimeNew = attrs.lastModifiedTime();
 
@@ -111,14 +122,26 @@ else if (lastModifiedTimeOld.compareTo(lastModifiedTimeNew) < 0) {
             }
         }
 
+        @Override
+        public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
+            addToPeeked(visited, dir);
+            visited.push(new HashSet<>());
+            return super.preVisitDirectory(dir, attrs);
+        }
+
+        @Override
+        public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+            addToPeeked(visited, file);
+            return super.visitFile(file, attrs);
+        }
+
         @Override
         public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
-            // If the visitor is back at the root of the rescan, then the time
-            // is right to issue `DELETED` events based on the set of `visited`
-            // paths.
-            if (dir.equals(path)) {
+            // Issue `DELETED` events based on the set of paths visited in `dir`
+            var visitedInDir = visited.pop();
+            if (visitedInDir != null) {
                 for (var p : index.keySet()) {
-                    if (p.startsWith(path) && !visited.contains(p)) {
+                    if (dir.equals(p.getParent()) && !visitedInDir.contains(p)) {
                         events.add(new WatchEvent(WatchEvent.Kind.DELETED, p));
                     }
                 }

src/test/java/engineering/swat/watch/RecursiveWatchTests.java

@@ -140,5 +140,4 @@ void deleteOfFileInDirectoryShouldBeVisible() throws IOException {
                 .untilTrue(seen);
         }
     }
-
 }

src/test/java/engineering/swat/watch/TestDirectory.java

@@ -37,12 +37,11 @@
 import java.util.Comparator;
 import java.util.List;
 
-class TestDirectory implements Closeable {
+public class TestDirectory implements Closeable {
     private final Path testDirectory;
     private final List<Path> testFiles;
 
-
-    TestDirectory() throws IOException {
+    public TestDirectory() throws IOException {
         testDirectory = Files.createTempDirectory("java-watch-test");
         List<Path> testFiles = new ArrayList<>();
         add3Files(testFiles, testDirectory);

src/test/java/engineering/swat/watch/impl/overflows/IndexingRescannerTests.java

@@ -0,0 +1,105 @@
+/*
+ * BSD 2-Clause License
+ *
+ * Copyright (c) 2023, Swat.engineering
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ *    list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package engineering.swat.watch.impl.overflows;
+
+import static org.awaitility.Awaitility.await;
+
+import java.io.IOException;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.awaitility.Awaitility;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import engineering.swat.watch.OnOverflow;
+import engineering.swat.watch.TestDirectory;
+import engineering.swat.watch.TestHelper;
+import engineering.swat.watch.WatchEvent;
+import engineering.swat.watch.WatchScope;
+import engineering.swat.watch.Watcher;
+import engineering.swat.watch.impl.EventHandlingWatch;
+
+class IndexingRescannerTests {
+
+    private TestDirectory testDir;
+
+    @BeforeEach
+    void setup() throws IOException {
+        testDir = new TestDirectory();
+    }
+
+    @AfterEach
+    void cleanup() {
+        if (testDir != null) {
+            testDir.close();
+        }
+    }
+
+    @BeforeAll
+    static void setupEverything() {
+        Awaitility.setDefaultTimeout(TestHelper.NORMAL_WAIT);
+    }
+
+    @Test
+    void onlyEventsForFilesInScopeAreIssued() throws IOException, InterruptedException {
+        var path = testDir.getTestDirectory();
+
+        // Configure a non-recursive directory watch that monitors only the
+        // children (not all descendants) of `path`
+        var eventsOnlyForChildren = new AtomicBoolean(true);
+        var watchConfig = Watcher.watch(path, WatchScope.PATH_AND_CHILDREN)
+            .approximate(OnOverflow.NONE) // Disable the auto-handler here; we'll have an explicit one below
+            .on(e -> {
+                if (e.getRelativePath().getNameCount() > 1) {
+                    eventsOnlyForChildren.set(false);
+                }
+            });
+
+        try (var watch = (EventHandlingWatch) watchConfig.start()) {
+            // Create a rescanner that initially indexes all descendants (not
+            // only the children) of `path`. The resulting initial index is an
+            // overestimation of the files monitored by the watch.
+            var rescanner = new IndexingRescanner(
+                ForkJoinPool.commonPool(), path,
+                WatchScope.PATH_AND_ALL_DESCENDANTS);
+
+            // Trigger a rescan. Because only the children (not all descendants)
+            // of `path` are watched, the rescan should issue events only for
+            // those children (even though the initial index contains entries
+            // for all descendants).
+            var overflow = new WatchEvent(WatchEvent.Kind.OVERFLOW, path);
+            rescanner.accept(watch, overflow);
+            Thread.sleep(TestHelper.SHORT_WAIT.toMillis());
+
+            await("No events for non-children descendants should have been issued")
+                .until(eventsOnlyForChildren::get);
+        }
+    }
+}