From c1d3622e39bcd80c03fb8abee5ce1b5afe843560 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Tue, 18 Feb 2025 10:36:56 +0100 Subject: [PATCH 01/10] Simplify `JDKFileWatch` by moving the creation (but not start) of its parent watch to the constructor --- .../swat/watch/impl/jdk/JDKFileWatch.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java index bd27b83c..5fa9ffa1 100644 --- a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java +++ b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java @@ -33,7 +33,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.Nullable; import engineering.swat.watch.WatchEvent; @@ -45,16 +44,17 @@ */ public class JDKFileWatch extends JDKBaseWatch { private final Logger logger = LogManager.getLogger(); - private final Path parent; private final Path fileName; - private volatile @MonotonicNonNull JDKDirectoryWatch parentWatch; + private final Path parent; + private final JDKBaseWatch parentWatch; public JDKFileWatch(Path file, Executor exec, Consumer eventHandler) { super(file, exec, eventHandler); var message = "The root path is not a valid path for a file watch"; - this.parent = requireNonNull(path.getParent(), message); this.fileName = requireNonNull(path.getFileName(), message); + this.parent = requireNonNull(path.getParent(), message); + this.parentWatch = new JDKDirectoryWatch(parent, exec, this::filter); assert !parent.equals(path); } @@ -75,15 +75,11 @@ private void filter(WatchEvent event) { @Override public synchronized void close() throws IOException { - if (parentWatch != null) { - parentWatch.close(); - } + parentWatch.close(); } @Override protected synchronized void start() throws IOException { - assert parentWatch == null; - parentWatch = new JDKDirectoryWatch(parent, exec, this::filter); parentWatch.open(); logger.debug("File watch (for: {}) is in reality a directory watch (for: {}) with a filter (for: {})", path, parent, fileName); } From 27cad3078b4cea55e1471deac947cb4dbc83f494 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 21 Feb 2025 14:44:46 +0100 Subject: [PATCH 02/10] Update workflow trigger for target branch `improved-overflow-support-main` --- .github/workflows/build.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 21fa1ca7..01ee331f 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -8,6 +8,7 @@ on: pull_request: branches: - main + - improved-overflow-support-main jobs: test: From 9fe5be04cd43fd72fb125d2a6fa92755212a7613 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 21 Feb 2025 14:53:50 +0100 Subject: [PATCH 03/10] Simplify class `WatchEvent` by adding a 2-arg constructor (instead of using the 3-arg constructor with a `null` arg) --- src/main/java/engineering/swat/watch/WatchEvent.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/engineering/swat/watch/WatchEvent.java b/src/main/java/engineering/swat/watch/WatchEvent.java index 4d4874d5..79cc11ca 100644 --- a/src/main/java/engineering/swat/watch/WatchEvent.java +++ b/src/main/java/engineering/swat/watch/WatchEvent.java @@ -28,8 +28,6 @@ import java.nio.file.Path; -import org.checkerframework.checker.nullness.qual.Nullable; - /** * The library publishes these events to all subscribers, they are immutable and safe to share around. */ @@ -68,10 +66,14 @@ public enum Kind { private final Path rootPath; private final Path relativePath; - public WatchEvent(Kind kind, Path rootPath, @Nullable Path relativePath) { + public WatchEvent(Kind kind, Path rootPath) { + this(kind, rootPath, Path.of("")); + } + + public WatchEvent(Kind kind, Path rootPath, Path relativePath) { this.kind = kind; this.rootPath = rootPath; - this.relativePath = relativePath == null ? Path.of("") : relativePath; + this.relativePath = relativePath; } public Kind getKind() { From f559c1c4408d2e56a4f90a7cef33b2d605f91cc5 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 21 Feb 2025 14:58:39 +0100 Subject: [PATCH 04/10] Rename method to more clearly convey what it does --- .../engineering/swat/watch/impl/jdk/JDKDirectoryWatch.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/jdk/JDKDirectoryWatch.java b/src/main/java/engineering/swat/watch/impl/jdk/JDKDirectoryWatch.java index fcb6004e..82a8d097 100644 --- a/src/main/java/engineering/swat/watch/impl/jdk/JDKDirectoryWatch.java +++ b/src/main/java/engineering/swat/watch/impl/jdk/JDKDirectoryWatch.java @@ -58,7 +58,7 @@ public JDKDirectoryWatch(Path directory, Executor exec, Consumer eve this.nativeRecursive = nativeRecursive; } - private void handleChanges(List> events) { + private void handleJDKEvents(List> events) { exec.execute(() -> { for (var ev : events) { try { @@ -85,6 +85,6 @@ public synchronized void close() throws IOException { protected synchronized void start() throws IOException { assert bundledJDKWatcher == null; var key = new SubscriptionKey(path, nativeRecursive); - bundledJDKWatcher = BUNDLED_JDK_WATCHERS.subscribe(key, this::handleChanges); + bundledJDKWatcher = BUNDLED_JDK_WATCHERS.subscribe(key, this::handleJDKEvents); } } From a00cc9a3942f138e535c3dbfa4fe99358eff6839 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 21 Feb 2025 15:31:42 +0100 Subject: [PATCH 05/10] Reduce diff of `JDKFileWatch` simplifications --- .../swat/watch/impl/jdk/JDKFileWatch.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java index 5fa9ffa1..5083ed92 100644 --- a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java +++ b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java @@ -44,18 +44,19 @@ */ public class JDKFileWatch extends JDKBaseWatch { private final Logger logger = LogManager.getLogger(); - private final Path fileName; private final Path parent; - private final JDKBaseWatch parentWatch; + private final Path fileName; + private final JDKBaseWatch delegate; public JDKFileWatch(Path file, Executor exec, Consumer eventHandler) { super(file, exec, eventHandler); var message = "The root path is not a valid path for a file watch"; - this.fileName = requireNonNull(path.getFileName(), message); this.parent = requireNonNull(path.getParent(), message); - this.parentWatch = new JDKDirectoryWatch(parent, exec, this::filter); + this.fileName = requireNonNull(path.getFileName(), message); assert !parent.equals(path); + + this.delegate = new JDKDirectoryWatch(parent, exec, this::filter, false); } private static Path requireNonNull(@Nullable Path p, String message) { @@ -75,12 +76,12 @@ private void filter(WatchEvent event) { @Override public synchronized void close() throws IOException { - parentWatch.close(); + delegate.close(); } @Override protected synchronized void start() throws IOException { - parentWatch.open(); + delegate.open(); logger.debug("File watch (for: {}) is in reality a directory watch (for: {}) with a filter (for: {})", path, parent, fileName); } } From bead33e9f5aadfae4ee03f54b8b8ef39a3c21cc3 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 21 Feb 2025 15:42:48 +0100 Subject: [PATCH 06/10] Reduce diff of `JDKFileWatch` simplifications --- .../engineering/swat/watch/impl/jdk/JDKFileWatch.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java index 5083ed92..36d2d177 100644 --- a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java +++ b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java @@ -46,7 +46,7 @@ public class JDKFileWatch extends JDKBaseWatch { private final Logger logger = LogManager.getLogger(); private final Path parent; private final Path fileName; - private final JDKBaseWatch delegate; + private final JDKBaseWatch internal; public JDKFileWatch(Path file, Executor exec, Consumer eventHandler) { super(file, exec, eventHandler); @@ -56,7 +56,7 @@ public JDKFileWatch(Path file, Executor exec, Consumer eventHandler) this.fileName = requireNonNull(path.getFileName(), message); assert !parent.equals(path); - this.delegate = new JDKDirectoryWatch(parent, exec, this::filter, false); + this.internal = new JDKDirectoryWatch(parent, exec, this::filter); } private static Path requireNonNull(@Nullable Path p, String message) { @@ -76,12 +76,12 @@ private void filter(WatchEvent event) { @Override public synchronized void close() throws IOException { - delegate.close(); + internal.close(); } @Override protected synchronized void start() throws IOException { - delegate.open(); + internal.open(); logger.debug("File watch (for: {}) is in reality a directory watch (for: {}) with a filter (for: {})", path, parent, fileName); } } From 6b92693939a6684d768ea25957043e6ac172f949 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 21 Feb 2025 16:13:20 +0100 Subject: [PATCH 07/10] Fix Checker Framework issue in `JDKFileWatch` --- .../swat/watch/impl/jdk/JDKFileWatch.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java index 36d2d177..638583cd 100644 --- a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java +++ b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java @@ -52,11 +52,17 @@ public JDKFileWatch(Path file, Executor exec, Consumer eventHandler) super(file, exec, eventHandler); var message = "The root path is not a valid path for a file watch"; - this.parent = requireNonNull(path.getParent(), message); - this.fileName = requireNonNull(path.getFileName(), message); - assert !parent.equals(path); + var parent = requireNonNull(file.getParent(), message); + var fileName = requireNonNull(file.getFileName(), message); + assert !parent.equals(file); - this.internal = new JDKDirectoryWatch(parent, exec, this::filter); + this.internal = new JDKDirectoryWatch(parent, exec, e -> { + if (fileName.equals(e.getRelativePath())) { + eventHandler.accept(e); + } + }); + + logger.debug("File watch (for: {}) is in reality a directory watch (for: {}) with a filter (for: {})", file, parent, fileName); } private static Path requireNonNull(@Nullable Path p, String message) { @@ -66,12 +72,6 @@ private static Path requireNonNull(@Nullable Path p, String message) { return p; } - private void filter(WatchEvent event) { - if (fileName.equals(event.getRelativePath())) { - eventHandler.accept(event); - } - } - // -- JDKBaseWatch -- @Override @@ -82,6 +82,5 @@ public synchronized void close() throws IOException { @Override protected synchronized void start() throws IOException { internal.open(); - logger.debug("File watch (for: {}) is in reality a directory watch (for: {}) with a filter (for: {})", path, parent, fileName); } } From eab398fb7cef280029de736898046e6b513362a9 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 21 Feb 2025 16:15:42 +0100 Subject: [PATCH 08/10] Fix Checker Framework issue in `JDKBaseWatch` --- .../swat/watch/impl/jdk/JDKBaseWatch.java | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/jdk/JDKBaseWatch.java b/src/main/java/engineering/swat/watch/impl/jdk/JDKBaseWatch.java index fb90755a..febe5917 100644 --- a/src/main/java/engineering/swat/watch/impl/jdk/JDKBaseWatch.java +++ b/src/main/java/engineering/swat/watch/impl/jdk/JDKBaseWatch.java @@ -90,27 +90,32 @@ protected boolean startIfFirstTime() throws IOException { } protected WatchEvent translate(java.nio.file.WatchEvent jdkEvent) { - WatchEvent.Kind kind; - if (jdkEvent.kind() == StandardWatchEventKinds.ENTRY_CREATE) { - kind = WatchEvent.Kind.CREATED; - } - else if (jdkEvent.kind() == StandardWatchEventKinds.ENTRY_MODIFY) { - kind = WatchEvent.Kind.MODIFIED; - } - else if (jdkEvent.kind() == StandardWatchEventKinds.ENTRY_DELETE) { - kind = WatchEvent.Kind.DELETED; - } - else if (jdkEvent.kind() == StandardWatchEventKinds.OVERFLOW) { - kind = WatchEvent.Kind.OVERFLOW; - } - else { - throw new IllegalArgumentException("Unexpected watch event: " + jdkEvent); - } + var jdkKind = jdkEvent.kind(); + var context = jdkKind == StandardWatchEventKinds.OVERFLOW ? null : jdkEvent.context(); + + var kind = translate(jdkKind); var rootPath = path; - var relativePath = kind == WatchEvent.Kind.OVERFLOW ? Path.of("") : (@Nullable Path)jdkEvent.context(); + var relativePath = context == null ? Path.of("") : (Path) context; var event = new WatchEvent(kind, rootPath, relativePath); logger.trace("Translated: {} to {}", jdkEvent, event); return event; } + + private WatchEvent.Kind translate(java.nio.file.WatchEvent.Kind jdkKind) { + if (jdkKind == StandardWatchEventKinds.ENTRY_CREATE) { + return WatchEvent.Kind.CREATED; + } + if (jdkKind == StandardWatchEventKinds.ENTRY_MODIFY) { + return WatchEvent.Kind.MODIFIED; + } + if (jdkKind == StandardWatchEventKinds.ENTRY_DELETE) { + return WatchEvent.Kind.DELETED; + } + if (jdkKind == StandardWatchEventKinds.OVERFLOW) { + return WatchEvent.Kind.OVERFLOW; + } + + throw new IllegalArgumentException("Unexpected watch kind: " + jdkKind); + } } From ff91702b9545c56a7c1c7872b87495fc38fcc906 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 21 Feb 2025 16:18:33 +0100 Subject: [PATCH 09/10] Remove unused fields in `JDKFileWatch` --- src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java index 638583cd..ae572d00 100644 --- a/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java +++ b/src/main/java/engineering/swat/watch/impl/jdk/JDKFileWatch.java @@ -44,8 +44,6 @@ */ public class JDKFileWatch extends JDKBaseWatch { private final Logger logger = LogManager.getLogger(); - private final Path parent; - private final Path fileName; private final JDKBaseWatch internal; public JDKFileWatch(Path file, Executor exec, Consumer eventHandler) { From c365b4ac8f216c5dffe2998dd94347cf9738c86c Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Mon, 24 Feb 2025 09:16:28 +0100 Subject: [PATCH 10/10] Move null preprocessing of relative path back to the constructor of `WatchEvent` (because having it outside didn't lead to significantly simpler code overall) --- src/main/java/engineering/swat/watch/WatchEvent.java | 8 +++++--- .../engineering/swat/watch/impl/jdk/JDKBaseWatch.java | 7 ++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/main/java/engineering/swat/watch/WatchEvent.java b/src/main/java/engineering/swat/watch/WatchEvent.java index 79cc11ca..3edb87c4 100644 --- a/src/main/java/engineering/swat/watch/WatchEvent.java +++ b/src/main/java/engineering/swat/watch/WatchEvent.java @@ -28,6 +28,8 @@ import java.nio.file.Path; +import org.checkerframework.checker.nullness.qual.Nullable; + /** * The library publishes these events to all subscribers, they are immutable and safe to share around. */ @@ -67,13 +69,13 @@ public enum Kind { private final Path relativePath; public WatchEvent(Kind kind, Path rootPath) { - this(kind, rootPath, Path.of("")); + this(kind, rootPath, null); } - public WatchEvent(Kind kind, Path rootPath, Path relativePath) { + public WatchEvent(Kind kind, Path rootPath, @Nullable Path relativePath) { this.kind = kind; this.rootPath = rootPath; - this.relativePath = relativePath; + this.relativePath = relativePath == null ? Path.of("") : relativePath; } public Kind getKind() { diff --git a/src/main/java/engineering/swat/watch/impl/jdk/JDKBaseWatch.java b/src/main/java/engineering/swat/watch/impl/jdk/JDKBaseWatch.java index febe5917..d0dc6c67 100644 --- a/src/main/java/engineering/swat/watch/impl/jdk/JDKBaseWatch.java +++ b/src/main/java/engineering/swat/watch/impl/jdk/JDKBaseWatch.java @@ -90,12 +90,9 @@ protected boolean startIfFirstTime() throws IOException { } protected WatchEvent translate(java.nio.file.WatchEvent jdkEvent) { - var jdkKind = jdkEvent.kind(); - var context = jdkKind == StandardWatchEventKinds.OVERFLOW ? null : jdkEvent.context(); - - var kind = translate(jdkKind); + var kind = translate(jdkEvent.kind()); var rootPath = path; - var relativePath = context == null ? Path.of("") : (Path) context; + var relativePath = kind == WatchEvent.Kind.OVERFLOW ? null : (@Nullable Path) jdkEvent.context(); var event = new WatchEvent(kind, rootPath, relativePath); logger.trace("Translated: {} to {}", jdkEvent, event);