From 9437eec89706fe3c7a01d50e3fb453ad23985640 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Wed, 5 Mar 2025 16:54:06 +0100 Subject: [PATCH 01/16] Add `IndexingRescanner` --- .../java/engineering/swat/watch/Watcher.java | 3 + .../impl/overflows/IndexingRescanner.java | 137 ++++++++++++++++++ .../impl/overflows/MemorylessRescanner.java | 49 ++++--- 3 files changed, 165 insertions(+), 24 deletions(-) create mode 100644 src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java diff --git a/src/main/java/engineering/swat/watch/Watcher.java b/src/main/java/engineering/swat/watch/Watcher.java index 98fd0a12..a77a6aa1 100644 --- a/src/main/java/engineering/swat/watch/Watcher.java +++ b/src/main/java/engineering/swat/watch/Watcher.java @@ -42,6 +42,7 @@ import engineering.swat.watch.impl.jdk.JDKDirectoryWatch; import engineering.swat.watch.impl.jdk.JDKFileWatch; import engineering.swat.watch.impl.jdk.JDKRecursiveDirectoryWatch; +import engineering.swat.watch.impl.overflows.IndexingRescanner; import engineering.swat.watch.impl.overflows.MemorylessRescanner; /** @@ -213,6 +214,8 @@ private BiConsumer applyApproximateOnOverflow() return eventHandler; case ALL: return new MemorylessRescanner(executor).andThen(eventHandler); + case DIRTY: + return new IndexingRescanner(executor, path, scope).andThen(eventHandler); default: throw new UnsupportedOperationException("No event handler has been defined yet for this overflow policy"); } diff --git a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java new file mode 100644 index 00000000..50636640 --- /dev/null +++ b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java @@ -0,0 +1,137 @@ +/* + * 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 java.io.IOException; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import engineering.swat.watch.WatchEvent; +import engineering.swat.watch.WatchScope; +import engineering.swat.watch.impl.EventHandlingWatch; + +public class IndexingRescanner extends MemorylessRescanner { + private final Logger logger = LogManager.getLogger(); + private final Map index = new ConcurrentHashMap<>(); + + public IndexingRescanner(Executor exec, Path path, WatchScope scope) { + super(exec); + rescan(path, scope); // Make an initial scan to populate the index + } + + // -- MemorylessRescanner -- + + @Override + protected MemorylessRescanner.FileVisitor newFileVisitor() { + return new FileVisitor(); + } + + protected class FileVisitor extends MemorylessRescanner.FileVisitor { + private Set visited = new HashSet<>(); + + @Override + protected void addEvents(Path path, BasicFileAttributes attrs) { + visited.add(path); + var lastModifiedTimeOld = index.get(path); + var lastModifiedTimeNew = attrs.lastModifiedTime(); + + // The path isn't indexed yet + if (lastModifiedTimeOld == null) { + index.put(path, lastModifiedTimeNew); + super.addEvents(path, attrs); + } + + // The path is already indexed, and the old last-modified-time is + // strictly before the new-last-modified-time + else if (lastModifiedTimeOld.compareTo(lastModifiedTimeNew) < 0) { + index.put(path, lastModifiedTimeNew); + events.add(new WatchEvent(WatchEvent.Kind.MODIFIED, path)); + } + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + // If the visitor is back at the start, then the time is right to + // issue `DELETED` events based on the set of `visited` paths. + if (dir.equals(start)) { + var i = index.keySet().iterator(); + while (i.hasNext()) { + var p = i.next(); + if (p.startsWith(start) && !visited.contains(p)) { + events.add(new WatchEvent(WatchEvent.Kind.DELETED, p)); + i.remove(); // Remove `p` from `index` + } + } + } + + return super.postVisitDirectory(dir, exc); + } + } + + @Override + public void accept(EventHandlingWatch watch, WatchEvent event) { + var fullPath = event.calculateFullPath(); + switch (event.getKind()) { + + case MODIFIED: + // If a modified event happens for a path that's not in the + // index yet, then a create event might have been missed. + if (!index.containsKey(fullPath)) { + var created = new WatchEvent(WatchEvent.Kind.CREATED, fullPath); + watch.handleEvent(created); + } + // Fallthrough intended + + case CREATED: + try { + index.put(fullPath, Files.getLastModifiedTime(fullPath)); + } catch (IOException e) { + logger.error("Could not get modification time of: {} ({})", fullPath, e); + } + break; + + case DELETED: + index.remove(fullPath); + break; + + case OVERFLOW: + super.accept(watch, event); + break; + } + } +} diff --git a/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java index 8cdebd12..30c82d83 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java @@ -59,11 +59,18 @@ public MemorylessRescanner(Executor exec) { * `MODIFIED` events (not `DELETED` events) for each file. This method * should typically be executed asynchronously (using `exec`). */ - protected void rescan(EventHandlingWatch watch) { - var start = watch.getPath(); + protected void rescanAndHandle(EventHandlingWatch watch) { + rescan(watch.getPath(), watch.getScope()) + .stream() + .map(watch::relativize) + .forEach(watch::handleEvent); + } + + protected List rescan(Path path, WatchScope scope) { + var start = path; var options = EnumSet.noneOf(FileVisitOption.class); - var maxDepth = watch.getScope() == WatchScope.PATH_AND_ALL_DESCENDANTS ? Integer.MAX_VALUE : 1; - var visitor = new FileVisitor(watch); + var maxDepth = scope == WatchScope.PATH_AND_ALL_DESCENDANTS ? Integer.MAX_VALUE : 1; + var visitor = newFileVisitor(); try { Files.walkFileTree(start, options, maxDepth, visitor); @@ -71,41 +78,35 @@ protected void rescan(EventHandlingWatch watch) { logger.error("Could not walk: {} ({})", start, e); } - for (var e : visitor.getEvents()) { - watch.handleEvent(e); - } + return visitor.getEvents(); } - private class FileVisitor extends SimpleFileVisitor { - private final EventHandlingWatch watch; - private final List events; + protected FileVisitor newFileVisitor() { + return new FileVisitor(); + } - public FileVisitor(EventHandlingWatch watch) { - this.watch = watch; - this.events = new ArrayList<>(); - } + protected class FileVisitor extends SimpleFileVisitor { + protected final List events = new ArrayList<>(); + protected Path start; public List getEvents() { return events; } - private void addEvents(Path path, BasicFileAttributes attrs) { - events.add(newEvent(WatchEvent.Kind.CREATED, path)); + protected void addEvents(Path path, BasicFileAttributes attrs) { + events.add(new WatchEvent(WatchEvent.Kind.CREATED, path)); if (attrs.isRegularFile() && attrs.size() > 0) { - events.add(newEvent(WatchEvent.Kind.MODIFIED, path)); + events.add(new WatchEvent(WatchEvent.Kind.MODIFIED, path)); } } - private WatchEvent newEvent(WatchEvent.Kind kind, Path fullPath) { - var event = new WatchEvent(kind, fullPath); - return watch.relativize(event); - } - // -- SimpleFileVisitor -- @Override public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { - if (!watch.getPath().equals(dir)) { + if (start == null) { + start = dir; + } else { addEvents(dir, attrs); } return FileVisitResult.CONTINUE; @@ -137,7 +138,7 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx @Override public void accept(EventHandlingWatch watch, WatchEvent event) { if (event.getKind() == WatchEvent.Kind.OVERFLOW) { - exec.execute(() -> rescan(watch)); + exec.execute(() -> rescanAndHandle(watch)); } } } From b85b7bb1febe1da4576b4a177e89c016c74651a0 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Wed, 5 Mar 2025 16:54:27 +0100 Subject: [PATCH 02/16] Add test for `IndexingRescanner` --- .../swat/watch/SingleDirectoryTests.java | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java index eedf2e92..4a0a1e68 100644 --- a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java +++ b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java @@ -162,4 +162,50 @@ void memorylessRescanOnOverflow() throws IOException, InterruptedException { .until(nOverflow::get, Predicate.isEqual(1)); } } + + @Test + void indexingRescanOnOverflow() throws IOException, InterruptedException { + var directory = testDir.getTestDirectory(); + Files.writeString(directory.resolve("a.txt"), "foo"); + Files.writeString(directory.resolve("b.txt"), "bar"); + + var nCreated = new AtomicInteger(); + var nModified = new AtomicInteger(); + var watchConfig = Watcher.watch(directory, WatchScope.PATH_AND_CHILDREN) + .approximate(OnOverflow.DIRTY) + .on(e -> { + switch (e.getKind()) { + case CREATED: + nCreated.incrementAndGet(); + break; + case MODIFIED: + nModified.incrementAndGet(); + break; + default: + break; + } + }); + + try (var watch = watchConfig.start()) { + var overflow = new WatchEvent(WatchEvent.Kind.OVERFLOW, directory); + ((EventHandlingWatch) watch).handleEvent(overflow); + + Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); + await("Overflow shouldn't trigger created events") + .until(nCreated::get, Predicate.isEqual(0)); + await("Overflow shouldn't trigger modified events") + .until(nModified::get, Predicate.isEqual(0)); + + Files.writeString(directory.resolve("b.txt"), "baz"); + await("File write should trigger modified event") + .until(nModified::get, Predicate.isEqual(1)); + + ((EventHandlingWatch) watch).handleEvent(overflow); + Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); + await("Overflow shouldn't trigger created event after file write (and index updated)") + .until(nCreated::get, Predicate.isEqual(0)); + await("Overflow shouldn't trigger modified event after file write (and index updated)") + .until(nModified::get, Predicate.isEqual(1)); // Still 1 (because of the real MODIFIED) + } + } } From 8ffde3b792a95320dd0ad9bb786a1def0d1c4fcd Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Wed, 5 Mar 2025 16:58:24 +0100 Subject: [PATCH 03/16] Fix typo --- .../swat/watch/impl/overflows/IndexingRescanner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java index 50636640..43956081 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java @@ -77,7 +77,7 @@ protected void addEvents(Path path, BasicFileAttributes attrs) { } // The path is already indexed, and the old last-modified-time is - // strictly before the new-last-modified-time + // strictly before the new last-modified-time else if (lastModifiedTimeOld.compareTo(lastModifiedTimeNew) < 0) { index.put(path, lastModifiedTimeNew); events.add(new WatchEvent(WatchEvent.Kind.MODIFIED, path)); From fd7035995b8133caa9870934eb9bf8a3faa33782 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 11:32:23 +0100 Subject: [PATCH 04/16] Switch order of overflow auto-handler and user-defined event handler --- src/main/java/engineering/swat/watch/Watcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/engineering/swat/watch/Watcher.java b/src/main/java/engineering/swat/watch/Watcher.java index a77a6aa1..c84d9049 100644 --- a/src/main/java/engineering/swat/watch/Watcher.java +++ b/src/main/java/engineering/swat/watch/Watcher.java @@ -215,7 +215,7 @@ private BiConsumer applyApproximateOnOverflow() case ALL: return new MemorylessRescanner(executor).andThen(eventHandler); case DIRTY: - return new IndexingRescanner(executor, path, scope).andThen(eventHandler); + return eventHandler.andThen(new IndexingRescanner(executor, path, scope)); default: throw new UnsupportedOperationException("No event handler has been defined yet for this overflow policy"); } From a826e442a9e273fca86163bd2b94c23ca8e0cf33 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 13:24:00 +0100 Subject: [PATCH 05/16] Improve comments --- .../swat/watch/impl/overflows/IndexingRescanner.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java index 43956081..d38022d4 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java @@ -62,6 +62,9 @@ protected MemorylessRescanner.FileVisitor newFileVisitor() { } protected class FileVisitor extends MemorylessRescanner.FileVisitor { + // Field to keep track of the paths that are visited during the current + // rescan. Subsequently, the `DELETED` events since the previous rescan + // can be approximated. private Set visited = new HashSet<>(); @Override @@ -76,8 +79,8 @@ protected void addEvents(Path path, BasicFileAttributes attrs) { super.addEvents(path, attrs); } - // The path is already indexed, and the old last-modified-time is - // strictly before the new last-modified-time + // The path is already indexed, and the previous last-modified-time + // is older than the current last-modified-time else if (lastModifiedTimeOld.compareTo(lastModifiedTimeNew) < 0) { index.put(path, lastModifiedTimeNew); events.add(new WatchEvent(WatchEvent.Kind.MODIFIED, path)); @@ -86,8 +89,9 @@ else if (lastModifiedTimeOld.compareTo(lastModifiedTimeNew) < 0) { @Override public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { - // If the visitor is back at the start, then the time is right to - // issue `DELETED` events based on the set of `visited` paths. + // 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(start)) { var i = index.keySet().iterator(); while (i.hasNext()) { From d832746b1a86315bde0c419973a9f7accb6825fb Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 13:37:51 +0100 Subject: [PATCH 06/16] Improve comments --- .../impl/overflows/IndexingRescanner.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java index d38022d4..baa83d38 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java @@ -109,18 +109,23 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx @Override public void accept(EventHandlingWatch watch, WatchEvent event) { + // Auto-handle `OVERFLOW` events + super.accept(watch, event); + + // In addition to auto-handling `OVERFLOW` events, extra processing is + // needed to update the index when `CREATED`, `MODIFIED`, and `DELETED` + // events happen. var fullPath = event.calculateFullPath(); switch (event.getKind()) { - case MODIFIED: - // If a modified event happens for a path that's not in the - // index yet, then a create event might have been missed. + // If a `MODIFIED` event happens for a path that's not in the + // index yet, then a `CREATED` event has somehow been missed. + // Just in case, it's issued synthetically here. if (!index.containsKey(fullPath)) { var created = new WatchEvent(WatchEvent.Kind.CREATED, fullPath); watch.handleEvent(created); } // Fallthrough intended - case CREATED: try { index.put(fullPath, Files.getLastModifiedTime(fullPath)); @@ -128,13 +133,11 @@ public void accept(EventHandlingWatch watch, WatchEvent event) { logger.error("Could not get modification time of: {} ({})", fullPath, e); } break; - case DELETED: index.remove(fullPath); break; - - case OVERFLOW: - super.accept(watch, event); + default: + logger.error("Could not auto-handle event of kind: {}", event.getKind()); break; } } From d5192e74b0bacc807fcb0b8719ca69a109fc3cc9 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 13:52:40 +0100 Subject: [PATCH 07/16] Improve comments and rename method --- .../watch/impl/overflows/IndexingRescanner.java | 14 ++++++-------- .../watch/impl/overflows/MemorylessRescanner.java | 6 +++--- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java index baa83d38..6909e5e6 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java @@ -63,12 +63,12 @@ protected MemorylessRescanner.FileVisitor newFileVisitor() { protected class FileVisitor extends MemorylessRescanner.FileVisitor { // Field to keep track of the paths that are visited during the current - // rescan. Subsequently, the `DELETED` events since the previous rescan - // can be approximated. + // rescan. After the visit, the `DELETED` events that happened since the + // previous rescan can be approximated. private Set visited = new HashSet<>(); @Override - protected void addEvents(Path path, BasicFileAttributes attrs) { + protected void generateEvents(Path path, BasicFileAttributes attrs) { visited.add(path); var lastModifiedTimeOld = index.get(path); var lastModifiedTimeNew = attrs.lastModifiedTime(); @@ -76,7 +76,7 @@ protected void addEvents(Path path, BasicFileAttributes attrs) { // The path isn't indexed yet if (lastModifiedTimeOld == null) { index.put(path, lastModifiedTimeNew); - super.addEvents(path, attrs); + super.generateEvents(path, attrs); } // The path is already indexed, and the previous last-modified-time @@ -102,7 +102,6 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx } } } - return super.postVisitDirectory(dir, exc); } } @@ -112,9 +111,8 @@ public void accept(EventHandlingWatch watch, WatchEvent event) { // Auto-handle `OVERFLOW` events super.accept(watch, event); - // In addition to auto-handling `OVERFLOW` events, extra processing is - // needed to update the index when `CREATED`, `MODIFIED`, and `DELETED` - // events happen. + // Additional processing is needed to update the index when `CREATED`, + // `MODIFIED`, and `DELETED` events happen. var fullPath = event.calculateFullPath(); switch (event.getKind()) { case MODIFIED: diff --git a/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java index 30c82d83..d51bfd95 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java @@ -93,7 +93,7 @@ public List getEvents() { return events; } - protected void addEvents(Path path, BasicFileAttributes attrs) { + protected void generateEvents(Path path, BasicFileAttributes attrs) { events.add(new WatchEvent(WatchEvent.Kind.CREATED, path)); if (attrs.isRegularFile() && attrs.size() > 0) { events.add(new WatchEvent(WatchEvent.Kind.MODIFIED, path)); @@ -107,14 +107,14 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) th if (start == null) { start = dir; } else { - addEvents(dir, attrs); + generateEvents(dir, attrs); } return FileVisitResult.CONTINUE; } @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { - addEvents(file, attrs); + generateEvents(file, attrs); return FileVisitResult.CONTINUE; } From eb13d91500b67b7d42c9c93c01f4126ebce7aa48 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 15:26:27 +0100 Subject: [PATCH 08/16] Extract a base file visitor from the visitor in `MemorylessRescanner` --- .../watch/impl/overflows/BaseFileVisitor.java | 88 +++++++++++++++++++ .../impl/overflows/MemorylessRescanner.java | 68 ++++---------- 2 files changed, 105 insertions(+), 51 deletions(-) create mode 100644 src/main/java/engineering/swat/watch/impl/overflows/BaseFileVisitor.java diff --git a/src/main/java/engineering/swat/watch/impl/overflows/BaseFileVisitor.java b/src/main/java/engineering/swat/watch/impl/overflows/BaseFileVisitor.java new file mode 100644 index 00000000..4ca9b71d --- /dev/null +++ b/src/main/java/engineering/swat/watch/impl/overflows/BaseFileVisitor.java @@ -0,0 +1,88 @@ +/* + * 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 java.io.IOException; +import java.nio.file.FileVisitOption; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.util.EnumSet; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import engineering.swat.watch.WatchEvent; +import engineering.swat.watch.WatchScope; + +/** + * Base extension of {@link SimpleFileVisitor}, intended to be further + * specialized by subclasses to auto-handle {@link WatchEvent.Kind#OVERFLOW} + * events. In particular, method {@link #walkFileTree} of this class internally + * calls {@link Files#walkFileTree} to visit the file tree that starts at + * {@link #path}, with a maximum depth inferred from {@link #scope}. Subclasses + * can be specialized, for instance, to generate synthetic events or index a + * file tree. + */ +public class BaseFileVisitor extends SimpleFileVisitor { + private final Logger logger = LogManager.getLogger(); + + protected final Path path; + protected final WatchScope scope; + + public BaseFileVisitor(Path path, WatchScope scope) { + this.path = path; + this.scope = scope; + } + + public void walkFileTree() { + var options = EnumSet.noneOf(FileVisitOption.class); + var maxDepth = scope == WatchScope.PATH_AND_ALL_DESCENDANTS ? Integer.MAX_VALUE : 1; + try { + Files.walkFileTree(path, options, maxDepth, this); + } catch (IOException e) { + logger.error("Could not walk: {} ({})", path, e); + } + } + + // -- SimpleFileVisitor -- + + @Override + public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException { + logger.error("Could not walk regular file: {} ({})", file, exc); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + if (exc != null) { + logger.error("Could not walk directory: {} ({})", dir, exc); + } + return FileVisitResult.CONTINUE; + } +} diff --git a/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java index d51bfd95..da156898 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java @@ -27,27 +27,20 @@ package engineering.swat.watch.impl.overflows; import java.io.IOException; -import java.nio.file.FileVisitOption; import java.nio.file.FileVisitResult; -import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; -import java.util.EnumSet; import java.util.List; import java.util.concurrent.Executor; import java.util.function.BiConsumer; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; +import java.util.stream.Stream; import engineering.swat.watch.WatchEvent; import engineering.swat.watch.WatchScope; import engineering.swat.watch.impl.EventHandlingWatch; public class MemorylessRescanner implements BiConsumer { - private final Logger logger = LogManager.getLogger(); private final Executor exec; public MemorylessRescanner(Executor exec) { @@ -59,38 +52,27 @@ public MemorylessRescanner(Executor exec) { * `MODIFIED` events (not `DELETED` events) for each file. This method * should typically be executed asynchronously (using `exec`). */ - protected void rescanAndHandle(EventHandlingWatch watch) { - rescan(watch.getPath(), watch.getScope()) - .stream() + protected void rescan(EventHandlingWatch watch) { + var generator = newGenerator(watch.getPath(), watch.getScope()); + generator.walkFileTree(); + generator.eventStream() .map(watch::relativize) .forEach(watch::handleEvent); } - protected List rescan(Path path, WatchScope scope) { - var start = path; - var options = EnumSet.noneOf(FileVisitOption.class); - var maxDepth = scope == WatchScope.PATH_AND_ALL_DESCENDANTS ? Integer.MAX_VALUE : 1; - var visitor = newFileVisitor(); - - try { - Files.walkFileTree(start, options, maxDepth, visitor); - } catch (IOException e) { - logger.error("Could not walk: {} ({})", start, e); - } - - return visitor.getEvents(); + protected Generator newGenerator(Path path, WatchScope scope) { + return new Generator(path, scope); } - protected FileVisitor newFileVisitor() { - return new FileVisitor(); - } - - protected class FileVisitor extends SimpleFileVisitor { + protected class Generator extends BaseFileVisitor { protected final List events = new ArrayList<>(); - protected Path start; - public List getEvents() { - return events; + public Generator(Path path, WatchScope scope) { + super(path, scope); + } + + public Stream eventStream() { + return events.stream(); } protected void generateEvents(Path path, BasicFileAttributes attrs) { @@ -100,13 +82,11 @@ protected void generateEvents(Path path, BasicFileAttributes attrs) { } } - // -- SimpleFileVisitor -- + // -- BaseFileVisitor -- @Override public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { - if (start == null) { - start = dir; - } else { + if (!path.equals(dir)) { generateEvents(dir, attrs); } return FileVisitResult.CONTINUE; @@ -117,20 +97,6 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO generateEvents(file, attrs); return FileVisitResult.CONTINUE; } - - @Override - public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException { - logger.error("Could not generate events for file: {} ({})", file, exc); - return FileVisitResult.CONTINUE; - } - - @Override - public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { - if (exc != null) { - logger.error("Could not successfully walk: {} ({})", dir, exc); - } - return FileVisitResult.CONTINUE; - } } // -- BiConsumer -- @@ -138,7 +104,7 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx @Override public void accept(EventHandlingWatch watch, WatchEvent event) { if (event.getKind() == WatchEvent.Kind.OVERFLOW) { - exec.execute(() -> rescanAndHandle(watch)); + exec.execute(() -> rescan(watch)); } } } From 285d92a3956aa9b8f706ec8ea0878f77d7bd94c1 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 15:27:36 +0100 Subject: [PATCH 09/16] Integrate the usage of the base file visitor in `IndexingRescanner` --- .../impl/overflows/IndexingRescanner.java | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java index 6909e5e6..d2e6f160 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java @@ -51,22 +51,48 @@ public class IndexingRescanner extends MemorylessRescanner { public IndexingRescanner(Executor exec, Path path, WatchScope scope) { super(exec); - rescan(path, scope); // Make an initial scan to populate the index + new Indexer(path, scope).walkFileTree(); // Make an initial scan to populate the index + } + + private class Indexer extends BaseFileVisitor { + public Indexer(Path path, WatchScope scope) { + super(path, scope); + } + + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { + if (!path.equals(dir)) { + index.put(dir, attrs.lastModifiedTime()); + } + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + index.put(file, attrs.lastModifiedTime()); + return FileVisitResult.CONTINUE; + } } // -- MemorylessRescanner -- @Override - protected MemorylessRescanner.FileVisitor newFileVisitor() { - return new FileVisitor(); + protected MemorylessRescanner.Generator newGenerator(Path path, WatchScope scope) { + return new Generator(path, scope); } - protected class FileVisitor extends MemorylessRescanner.FileVisitor { + 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 visited = new HashSet<>(); + public Generator(Path path, WatchScope scope) { + super(path, scope); + } + + // -- MemorylessRescanner.Generator -- + @Override protected void generateEvents(Path path, BasicFileAttributes attrs) { visited.add(path); @@ -75,14 +101,12 @@ protected void generateEvents(Path path, BasicFileAttributes attrs) { // The path isn't indexed yet if (lastModifiedTimeOld == null) { - index.put(path, lastModifiedTimeNew); super.generateEvents(path, attrs); } // The path is already indexed, and the previous last-modified-time // is older than the current last-modified-time else if (lastModifiedTimeOld.compareTo(lastModifiedTimeNew) < 0) { - index.put(path, lastModifiedTimeNew); events.add(new WatchEvent(WatchEvent.Kind.MODIFIED, path)); } } @@ -92,11 +116,11 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx // 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(start)) { + if (dir.equals(path)) { var i = index.keySet().iterator(); while (i.hasNext()) { var p = i.next(); - if (p.startsWith(start) && !visited.contains(p)) { + if (p.startsWith(path) && !visited.contains(p)) { events.add(new WatchEvent(WatchEvent.Kind.DELETED, p)); i.remove(); // Remove `p` from `index` } @@ -106,6 +130,8 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx } } + // -- MemorylessRescanner + @Override public void accept(EventHandlingWatch watch, WatchEvent event) { // Auto-handle `OVERFLOW` events @@ -134,8 +160,7 @@ public void accept(EventHandlingWatch watch, WatchEvent event) { case DELETED: index.remove(fullPath); break; - default: - logger.error("Could not auto-handle event of kind: {}", event.getKind()); + case OVERFLOW: // Already auto-handled above break; } } From 3581cb6ed461793970a1d8e42eeb9977de3e5b72 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 15:39:50 +0100 Subject: [PATCH 10/16] Simplify generation of DELETED events --- .../swat/watch/impl/overflows/BaseFileVisitor.java | 1 - .../swat/watch/impl/overflows/IndexingRescanner.java | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/overflows/BaseFileVisitor.java b/src/main/java/engineering/swat/watch/impl/overflows/BaseFileVisitor.java index 4ca9b71d..e3233ebe 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/BaseFileVisitor.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/BaseFileVisitor.java @@ -51,7 +51,6 @@ */ public class BaseFileVisitor extends SimpleFileVisitor { private final Logger logger = LogManager.getLogger(); - protected final Path path; protected final WatchScope scope; diff --git a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java index d2e6f160..678fd9e9 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java @@ -117,12 +117,9 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx // is right to issue `DELETED` events based on the set of `visited` // paths. if (dir.equals(path)) { - var i = index.keySet().iterator(); - while (i.hasNext()) { - var p = i.next(); + for (var p : index.keySet()) { if (p.startsWith(path) && !visited.contains(p)) { events.add(new WatchEvent(WatchEvent.Kind.DELETED, p)); - i.remove(); // Remove `p` from `index` } } } @@ -130,7 +127,7 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx } } - // -- MemorylessRescanner + // -- MemorylessRescanner -- @Override public void accept(EventHandlingWatch watch, WatchEvent event) { From 5b942252a475cee2a7ebc9e1351696ae3699d586 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 16:56:35 +0100 Subject: [PATCH 11/16] Extend `indexingRescanOnOverflow` test --- .../swat/watch/SingleDirectoryTests.java | 105 ++++++++++++++---- 1 file changed, 83 insertions(+), 22 deletions(-) diff --git a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java index 4a0a1e68..88f41f9d 100644 --- a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java +++ b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java @@ -26,10 +26,12 @@ */ package engineering.swat.watch; +import static engineering.swat.watch.WatchEvent.Kind.OVERFLOW; import static org.awaitility.Awaitility.await; import java.io.IOException; import java.nio.file.Files; +import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; @@ -165,47 +167,106 @@ void memorylessRescanOnOverflow() throws IOException, InterruptedException { @Test void indexingRescanOnOverflow() throws IOException, InterruptedException { + // Preface: This test looks a bit hacky because there's no API to + // directly manipulate, or prevent the auto-manipulation of, the index + // inside a watch. I've added some comments below to make it make sense. + var directory = testDir.getTestDirectory(); - Files.writeString(directory.resolve("a.txt"), "foo"); - Files.writeString(directory.resolve("b.txt"), "bar"); + var semaphore = new Semaphore(0); var nCreated = new AtomicInteger(); var nModified = new AtomicInteger(); + var nDeleted = new AtomicInteger(); + var watchConfig = Watcher.watch(directory, WatchScope.PATH_AND_CHILDREN) .approximate(OnOverflow.DIRTY) .on(e -> { - switch (e.getKind()) { - case CREATED: - nCreated.incrementAndGet(); - break; - case MODIFIED: - nModified.incrementAndGet(); - break; - default: - break; + var kind = e.getKind(); + if (kind != OVERFLOW) { + // Threads can handle non-`OVERFLOW` events *only after* + // everything is "ready" for that (in which case a token is + // released to the semaphore, which is initially empty). See + // below for an explanation of "readiness". + semaphore.acquireUninterruptibly(); + switch (e.getKind()) { + case CREATED: + nCreated.incrementAndGet(); + break; + case MODIFIED: + nModified.incrementAndGet(); + break; + case DELETED: + nDeleted.incrementAndGet(); + break; + default: + break; + } + semaphore.release(); } }); try (var watch = watchConfig.start()) { + // At this point, the index of last-modified-times inside `watch` is + // populated with initial values. + + Files.writeString(directory.resolve("a.txt"), "foo"); + Files.writeString(directory.resolve("b.txt"), "bar"); + Files.delete(directory.resolve("c.txt")); + Files.createFile(directory.resolve("d.txt")); + // At this point, regular events have been generated for a.txt, + // b.txt, c.txt, and d.txt by the file system. These events won't be + // handled by `watch` just yet, though, because the semaphore is + // still empty (i.e., event-handling threads are blocked from making + // progress). Thus, the index inside `watch` still contains the + // initial last-modified-times. (Warning: The blockade works only + // when the rescanner runs after the user-defined event-handler. + // Currently, this is the case, but changing their order probably + // breaks this test.) + var overflow = new WatchEvent(WatchEvent.Kind.OVERFLOW, directory); ((EventHandlingWatch) watch).handleEvent(overflow); - Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); - await("Overflow shouldn't trigger created events") - .until(nCreated::get, Predicate.isEqual(0)); - await("Overflow shouldn't trigger modified events") - .until(nModified::get, Predicate.isEqual(0)); + // At this point, the current thread has presumably slept long + // enough for the `OVERFLOW` event to have been handled by the + // rescanner. This means that synthetic events must have been issued + // (because the index still contained the initial last-modified + // times). + + // Readiness achieved: Threads can now start handling non-`OVERFLOW` + // events. + semaphore.release(); + await("Overflow should trigger created events") + .until(nCreated::get, Predicate.isEqual(2)); // 1 synthetic event + 1 regular event + await("Overflow should trigger modified events") + .until(nModified::get, Predicate.isEqual(4)); // 2 synthetic events + 2 regular events + await("Overflow should trigger deleted events") + .until(nDeleted::get, Predicate.isEqual(2)); // 1 synthetic event + 1 regular event + + // Let's do some more file operations, trigger another `OVERFLOW` + // event, and observe that synthetic events *aren't* issued this + // time (because the index was already updated when the regular + // events were handled). Files.writeString(directory.resolve("b.txt"), "baz"); - await("File write should trigger modified event") - .until(nModified::get, Predicate.isEqual(1)); + Files.createFile(directory.resolve("c.txt")); + Files.delete(directory.resolve("d.txt")); + + await("File create should trigger regular created event") + .until(nCreated::get, Predicate.isEqual(3)); + await("File write should trigger regular modified event") + .until(nModified::get, Predicate.isEqual(5)); + await("File delete should trigger regular deleted event") + .until(nDeleted::get, Predicate.isEqual(3)); ((EventHandlingWatch) watch).handleEvent(overflow); Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); - await("Overflow shouldn't trigger created event after file write (and index updated)") - .until(nCreated::get, Predicate.isEqual(0)); - await("Overflow shouldn't trigger modified event after file write (and index updated)") - .until(nModified::get, Predicate.isEqual(1)); // Still 1 (because of the real MODIFIED) + + await("Overflow shouldn't trigger synthetic created event after file create (and index updated)") + .until(nCreated::get, Predicate.isEqual(3)); + await("Overflow shouldn't trigger synthetic modified event after file write (and index updated)") + .until(nModified::get, Predicate.isEqual(5)); + await("Overflow shouldn't trigger synthetic deleted event after file delete (and index updated)") + .until(nDeleted::get, Predicate.isEqual(3)); } } } From 94c3aeebf80d2285f9eef630491966c28d6890ad Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 7 Mar 2025 17:30:35 +0100 Subject: [PATCH 12/16] Increase sleep time in test --- src/test/java/engineering/swat/watch/SingleDirectoryTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java index 88f41f9d..196ea11a 100644 --- a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java +++ b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java @@ -225,7 +225,7 @@ void indexingRescanOnOverflow() throws IOException, InterruptedException { var overflow = new WatchEvent(WatchEvent.Kind.OVERFLOW, directory); ((EventHandlingWatch) watch).handleEvent(overflow); - Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); + Thread.sleep(TestHelper.LONG_WAIT.toMillis()); // At this point, the current thread has presumably slept long // enough for the `OVERFLOW` event to have been handled by the // rescanner. This means that synthetic events must have been issued From 25d5eb746cf65433e505e7e542bc5324cd935f3e Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Mon, 10 Mar 2025 13:49:50 +0100 Subject: [PATCH 13/16] Add comment to explain why `events` is thread-safe --- .../swat/watch/impl/overflows/MemorylessRescanner.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java index da156898..dad8528b 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/MemorylessRescanner.java @@ -65,6 +65,10 @@ protected Generator newGenerator(Path path, WatchScope scope) { } protected class Generator extends BaseFileVisitor { + // When this class is used as intended, `events` is accessed only by one + // thread (the one that executes `Files.walkFileTree` via + // `BaseFileVisitor.walkFileTree`), so no additional thread-safety + // measures are needed to protect it from concurrent accesses. protected final List events = new ArrayList<>(); public Generator(Path path, WatchScope scope) { From b19cdafc0d80a7131f9bbb792b563f4d5b19bc22 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Mon, 10 Mar 2025 16:06:06 +0100 Subject: [PATCH 14/16] Improve event handler of `IndexingRescanner` (avoid double map lookup) --- .../impl/overflows/IndexingRescanner.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java index 678fd9e9..1af09527 100644 --- a/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java +++ b/src/main/java/engineering/swat/watch/impl/overflows/IndexingRescanner.java @@ -136,20 +136,22 @@ public void accept(EventHandlingWatch watch, WatchEvent event) { // Additional processing is needed to update the index when `CREATED`, // `MODIFIED`, and `DELETED` events happen. + var kind = event.getKind(); var fullPath = event.calculateFullPath(); - switch (event.getKind()) { - case MODIFIED: - // If a `MODIFIED` event happens for a path that's not in the - // index yet, then a `CREATED` event has somehow been missed. - // Just in case, it's issued synthetically here. - if (!index.containsKey(fullPath)) { - var created = new WatchEvent(WatchEvent.Kind.CREATED, fullPath); - watch.handleEvent(created); - } - // Fallthrough intended + switch (kind) { case CREATED: + case MODIFIED: try { - index.put(fullPath, Files.getLastModifiedTime(fullPath)); + var lastModifiedTimeNew = Files.getLastModifiedTime(fullPath); + var lastModifiedTimeOld = index.put(fullPath, lastModifiedTimeNew); + + // If a `MODIFIED` event happens for a path that wasn't in + // the index yet, then a `CREATED` event has somehow been + // missed. Just in case, it's issued synthetically here. + if (lastModifiedTimeOld == null && kind == WatchEvent.Kind.MODIFIED) { + var created = new WatchEvent(WatchEvent.Kind.CREATED, fullPath); + watch.handleEvent(created); + } } catch (IOException e) { logger.error("Could not get modification time of: {} ({})", fullPath, e); } From 924492d351bbefb455a08284e82ede91347178b5 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Mon, 10 Mar 2025 16:06:21 +0100 Subject: [PATCH 15/16] Fix test: don't rely on exact numbers of events --- .../swat/watch/SingleDirectoryTests.java | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java index 196ea11a..ef0210a7 100644 --- a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java +++ b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java @@ -172,8 +172,8 @@ void indexingRescanOnOverflow() throws IOException, InterruptedException { // inside a watch. I've added some comments below to make it make sense. var directory = testDir.getTestDirectory(); - var semaphore = new Semaphore(0); + var nCreated = new AtomicInteger(); var nModified = new AtomicInteger(); var nDeleted = new AtomicInteger(); @@ -225,7 +225,7 @@ void indexingRescanOnOverflow() throws IOException, InterruptedException { var overflow = new WatchEvent(WatchEvent.Kind.OVERFLOW, directory); ((EventHandlingWatch) watch).handleEvent(overflow); - Thread.sleep(TestHelper.LONG_WAIT.toMillis()); + Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); // At this point, the current thread has presumably slept long // enough for the `OVERFLOW` event to have been handled by the // rescanner. This means that synthetic events must have been issued @@ -237,11 +237,16 @@ void indexingRescanOnOverflow() throws IOException, InterruptedException { semaphore.release(); await("Overflow should trigger created events") - .until(nCreated::get, Predicate.isEqual(2)); // 1 synthetic event + 1 regular event + .until(nCreated::get, n -> n >= 2); // 1 synthetic event + >=1 regular event await("Overflow should trigger modified events") - .until(nModified::get, Predicate.isEqual(4)); // 2 synthetic events + 2 regular events + .until(nModified::get, n -> n >= 4); // 2 synthetic events + >=2 regular events await("Overflow should trigger deleted events") - .until(nDeleted::get, Predicate.isEqual(2)); // 1 synthetic event + 1 regular event + .until(nDeleted::get, n -> n >= 2); // 1 synthetic event + >=1 regular event + + // Reset counters for next phase of the test + nCreated.set(0); + nModified.set(0); + nDeleted.set(0); // Let's do some more file operations, trigger another `OVERFLOW` // event, and observe that synthetic events *aren't* issued this @@ -252,21 +257,25 @@ void indexingRescanOnOverflow() throws IOException, InterruptedException { Files.delete(directory.resolve("d.txt")); await("File create should trigger regular created event") - .until(nCreated::get, Predicate.isEqual(3)); + .until(nCreated::get, n -> n >= 1); await("File write should trigger regular modified event") - .until(nModified::get, Predicate.isEqual(5)); + .until(nModified::get, n -> n >= 1); await("File delete should trigger regular deleted event") - .until(nDeleted::get, Predicate.isEqual(3)); + .until(nDeleted::get, n -> n >= 1); + + var nCreatedBeforeOverflow = nCreated.get(); + var nModifiedBeforeOverflow = nModified.get(); + var nDeletedBeforeOverflow = nDeleted.get(); ((EventHandlingWatch) watch).handleEvent(overflow); Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); await("Overflow shouldn't trigger synthetic created event after file create (and index updated)") - .until(nCreated::get, Predicate.isEqual(3)); + .until(nCreated::get, Predicate.isEqual(nCreatedBeforeOverflow)); await("Overflow shouldn't trigger synthetic modified event after file write (and index updated)") - .until(nModified::get, Predicate.isEqual(5)); + .until(nModified::get, Predicate.isEqual(nModifiedBeforeOverflow)); await("Overflow shouldn't trigger synthetic deleted event after file delete (and index updated)") - .until(nDeleted::get, Predicate.isEqual(3)); + .until(nDeleted::get, Predicate.isEqual(nDeletedBeforeOverflow)); } } } From 493165d35dd6f2f80e34e5f7f0c260317a2f4a5d Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Tue, 11 Mar 2025 12:26:40 +0100 Subject: [PATCH 16/16] Fix `indexingRescanOnOverflow` test --- src/test/java/engineering/swat/watch/SingleDirectoryTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java index ef0210a7..00159970 100644 --- a/src/test/java/engineering/swat/watch/SingleDirectoryTests.java +++ b/src/test/java/engineering/swat/watch/SingleDirectoryTests.java @@ -206,6 +206,7 @@ void indexingRescanOnOverflow() throws IOException, InterruptedException { }); try (var watch = watchConfig.start()) { + Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); // At this point, the index of last-modified-times inside `watch` is // populated with initial values. @@ -213,6 +214,7 @@ void indexingRescanOnOverflow() throws IOException, InterruptedException { Files.writeString(directory.resolve("b.txt"), "bar"); Files.delete(directory.resolve("c.txt")); Files.createFile(directory.resolve("d.txt")); + Thread.sleep(TestHelper.NORMAL_WAIT.toMillis()); // At this point, regular events have been generated for a.txt, // b.txt, c.txt, and d.txt by the file system. These events won't be // handled by `watch` just yet, though, because the semaphore is