From e7e2d3fe6362bdf7f71c2270b48623d14c2cf362 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Thu, 21 Aug 2025 14:04:42 +0200 Subject: [PATCH 1/4] Improve error checks when we start a watch --- .../java/engineering/swat/watch/Watch.java | 52 ++++++++++++------- .../swat/watch/APIErrorsTests.java | 14 ++--- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/main/java/engineering/swat/watch/Watch.java b/src/main/java/engineering/swat/watch/Watch.java index 99723812..556213f5 100644 --- a/src/main/java/engineering/swat/watch/Watch.java +++ b/src/main/java/engineering/swat/watch/Watch.java @@ -26,9 +26,14 @@ */ package engineering.swat.watch; +import java.io.FileNotFoundException; import java.io.IOException; +import java.nio.file.FileSystemException; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.LinkOption; +import java.nio.file.NoSuchFileException; +import java.nio.file.NotDirectoryException; import java.nio.file.Path; import java.util.concurrent.Executor; import java.util.function.BiConsumer; @@ -76,28 +81,13 @@ private Watch(Path path, WatchScope scope) { * Watch a path for updates, optionally also get events for its children/descendants * @param path which absolute path to monitor, can be a file or a directory, but has to be absolute * @param scope for directories you can also choose to monitor it's direct children or all it's descendants - * @throws IllegalArgumentException in case a path is not supported (in relation to the scope) + * @throws IllegalArgumentException in case a path is not supported * @return watch builder that can be further configured and then started */ public static Watch build(Path path, WatchScope scope) { if (!path.isAbsolute()) { throw new IllegalArgumentException("We can only watch absolute paths"); } - switch (scope) { - case PATH_AND_CHILDREN: // intended fallthrough - case PATH_AND_ALL_DESCENDANTS: - if (!Files.isDirectory(path, LinkOption.NOFOLLOW_LINKS)) { - throw new IllegalArgumentException("Only directories are supported for this scope: " + scope); - } - break; - case PATH_ONLY: - if (Files.isSymbolicLink(path)) { - throw new IllegalArgumentException("Symlinks are not supported"); - } - break; - default: - throw new IllegalArgumentException("Unsupported scope: " + scope); - } return new Watch(path, scope); } @@ -192,16 +182,38 @@ public Watch onOverflow(Approximation whichFiles) { return this; } + private void validateOptions() throws IOException { + if (this.eventHandler == EMPTY_HANDLER) { + throw new IllegalStateException("There is no onEvent handler defined"); + } + if (!Files.exists(path)) { + throw new FileSystemException(path.toString(), null, "Cannot open a watch on a non-existing path"); + } + switch (scope) { + case PATH_AND_CHILDREN: // intended fallthrough + case PATH_AND_ALL_DESCENDANTS: + if (!Files.isDirectory(path, LinkOption.NOFOLLOW_LINKS)) { + throw new FileSystemException(path.toString(), null, "Only directories are supported for this scope: " + scope); + } + break; + case PATH_ONLY: + if (Files.isSymbolicLink(path)) { + throw new FileSystemException(path.toString(), null, "Symlinks are not supported"); + } + break; + default: + throw new IllegalArgumentException("Unsupported scope: " + scope); + } + } + /** * Start watch the path for events. * @return a subscription for the watch, when closed, new events will stop being registered to the worker pool. - * @throws IOException in case the starting of the watcher caused an underlying IO exception + * @throws IOException in case the starting of the watcher caused an underlying IO exception or we detect it is an invalid watch * @throws IllegalStateException the watchers is not configured correctly (for example, missing {@link #on(Consumer)}, or a watcher is started twice) */ public ActiveWatch start() throws IOException { - if (this.eventHandler == EMPTY_HANDLER) { - throw new IllegalStateException("There is no onEvent handler defined"); - } + validateOptions(); var executor = this.executor; if (executor == null) { executor = FALLBACK_EXECUTOR; diff --git a/src/test/java/engineering/swat/watch/APIErrorsTests.java b/src/test/java/engineering/swat/watch/APIErrorsTests.java index db0f9fa4..4d7dda61 100644 --- a/src/test/java/engineering/swat/watch/APIErrorsTests.java +++ b/src/test/java/engineering/swat/watch/APIErrorsTests.java @@ -29,6 +29,7 @@ import static org.junit.jupiter.api.Assertions.assertThrowsExactly; import java.io.IOException; +import java.nio.file.FileSystemException; import java.nio.file.Files; import org.awaitility.Awaitility; @@ -70,9 +71,11 @@ void noDuplicateEvents() { @Test void onlyDirectoryWatchingOnDirectories() { - assertThrowsExactly(IllegalArgumentException.class, () -> + assertThrowsExactly(FileSystemException.class, () -> Watch .build(testDir.getTestFiles().get(0), WatchScope.PATH_AND_CHILDREN) + .on(e -> {}) + .start() ); } @@ -92,17 +95,14 @@ void noRelativePaths() { assertThrowsExactly(IllegalArgumentException.class, () -> Watch .build(relativePath, WatchScope.PATH_AND_CHILDREN) - .start() ); } @Test void nonExistingDirectory() throws IOException { - var nonExistingDir = testDir.getTestDirectory().resolve("testd1"); - Files.createDirectory(nonExistingDir); - var w = Watch.build(nonExistingDir, WatchScope.PATH_AND_CHILDREN); - Files.delete(nonExistingDir); - assertThrowsExactly(IllegalStateException.class, w::start); + var nonExistingDir = testDir.getTestDirectory().resolve("test-not-existing"); + var w = Watch.build(nonExistingDir, WatchScope.PATH_AND_CHILDREN).on(e -> {}); + assertThrowsExactly(FileSystemException.class, w::start); } From 4dc4c073b63da78590a554a2cec6c9825e975c33 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Mon, 25 Aug 2025 19:49:58 +0200 Subject: [PATCH 2/4] Update src/main/java/engineering/swat/watch/Watch.java Co-authored-by: sungshik <16154899+sungshik@users.noreply.github.com> --- src/main/java/engineering/swat/watch/Watch.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/engineering/swat/watch/Watch.java b/src/main/java/engineering/swat/watch/Watch.java index 556213f5..5e5813ab 100644 --- a/src/main/java/engineering/swat/watch/Watch.java +++ b/src/main/java/engineering/swat/watch/Watch.java @@ -184,7 +184,7 @@ public Watch onOverflow(Approximation whichFiles) { private void validateOptions() throws IOException { if (this.eventHandler == EMPTY_HANDLER) { - throw new IllegalStateException("There is no onEvent handler defined"); + throw new IllegalStateException("There is no `on` handler defined"); } if (!Files.exists(path)) { throw new FileSystemException(path.toString(), null, "Cannot open a watch on a non-existing path"); From a2919dc646e8334b807b5bc19af35915c2352850 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 27 Aug 2025 09:43:00 +0200 Subject: [PATCH 3/4] Made exception more specific --- src/main/java/engineering/swat/watch/Watch.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/engineering/swat/watch/Watch.java b/src/main/java/engineering/swat/watch/Watch.java index 5e5813ab..eccad388 100644 --- a/src/main/java/engineering/swat/watch/Watch.java +++ b/src/main/java/engineering/swat/watch/Watch.java @@ -187,7 +187,7 @@ private void validateOptions() throws IOException { throw new IllegalStateException("There is no `on` handler defined"); } if (!Files.exists(path)) { - throw new FileSystemException(path.toString(), null, "Cannot open a watch on a non-existing path"); + throw new NoSuchFileException(path.toString(), null, "Cannot open a watch on a non-existing path"); } switch (scope) { case PATH_AND_CHILDREN: // intended fallthrough From 81a085aa7b4ae471e0a5295d1a7e422ec0e470aa Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 27 Aug 2025 09:50:47 +0200 Subject: [PATCH 4/4] Fixed CF config and test config --- src/main/checkerframework/nio-file.astub | 5 +++++ src/test/java/engineering/swat/watch/APIErrorsTests.java | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/checkerframework/nio-file.astub b/src/main/checkerframework/nio-file.astub index 12185ba8..5b9e27d3 100644 --- a/src/main/checkerframework/nio-file.astub +++ b/src/main/checkerframework/nio-file.astub @@ -12,3 +12,8 @@ public interface WatchService { public interface WatchEvent { @Nullable T context(); } + +public class NoSuchFileException extends FileSystemException { + public NoSuchFileException(String file, @Nullable String other, String reason); +} + diff --git a/src/test/java/engineering/swat/watch/APIErrorsTests.java b/src/test/java/engineering/swat/watch/APIErrorsTests.java index 4d7dda61..4bf618cc 100644 --- a/src/test/java/engineering/swat/watch/APIErrorsTests.java +++ b/src/test/java/engineering/swat/watch/APIErrorsTests.java @@ -30,7 +30,7 @@ import java.io.IOException; import java.nio.file.FileSystemException; -import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; @@ -102,7 +102,7 @@ void noRelativePaths() { void nonExistingDirectory() throws IOException { var nonExistingDir = testDir.getTestDirectory().resolve("test-not-existing"); var w = Watch.build(nonExistingDir, WatchScope.PATH_AND_CHILDREN).on(e -> {}); - assertThrowsExactly(FileSystemException.class, w::start); + assertThrowsExactly(NoSuchFileException.class, w::start); }