From 18fbd96c10baf1dad49debc554d49b1f7934c6e6 Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sat, 10 Dec 2022 12:54:49 +0000 Subject: [PATCH] Some further improvemnets to mount error handling - Correctly handle FileOperationExceptions for the root mount. - Remove some checks from MountWrapper: Mount/WritableMount should do these already! - Normalise file paths, always using a '/'. --- .../api/filesystem/WritableMount.java | 3 +- .../core/filesystem/FileMount.java | 5 ++-- .../core/filesystem/MountWrapper.java | 30 ++++++------------- .../core/filesystem/WritableFileMount.java | 2 +- 4 files changed, 15 insertions(+), 25 deletions(-) diff --git a/projects/core-api/src/main/java/dan200/computercraft/api/filesystem/WritableMount.java b/projects/core-api/src/main/java/dan200/computercraft/api/filesystem/WritableMount.java index 85cfd2774..761437344 100644 --- a/projects/core-api/src/main/java/dan200/computercraft/api/filesystem/WritableMount.java +++ b/projects/core-api/src/main/java/dan200/computercraft/api/filesystem/WritableMount.java @@ -32,7 +32,8 @@ public interface WritableMount extends Mount { void makeDirectory(String path) throws IOException; /** - * Deletes a directory at a given path inside the virtual file system. + * Deletes a directory at a given path inside the virtual file system. If the file does not exist, this method + * should do nothing. * * @param path A file path in normalised format, relative to the mount location. ie: "programs/myoldprograms". * @throws IOException If the file does not exist or could not be deleted. diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileMount.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileMount.java index 288160e76..309a107d5 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileMount.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileMount.java @@ -5,6 +5,7 @@ */ package dan200.computercraft.core.filesystem; +import com.google.common.base.Joiner; import dan200.computercraft.api.filesystem.FileAttributes; import dan200.computercraft.api.filesystem.FileOperationException; import dan200.computercraft.api.filesystem.Mount; @@ -104,7 +105,7 @@ public SeekableByteChannel openForRead(String path) throws FileOperationExceptio protected FileOperationException remapException(String fallbackPath, IOException exn) { return exn instanceof FileSystemException fsExn ? remapException(fallbackPath, fsExn) - : new FileOperationException(fallbackPath, exn.getMessage() == null ? "Operation failed" : exn.getMessage()); + : new FileOperationException(fallbackPath, exn.getMessage() == null ? "Access denied" : exn.getMessage()); } /** @@ -123,7 +124,7 @@ protected FileOperationException remapException(String fallbackPath, FileSystemE var failedPath = Path.of(failedFile); return failedPath.startsWith(root) - ? new FileOperationException(root.relativize(failedPath).toString(), reason) + ? new FileOperationException(Joiner.on('/').join(root.relativize(failedPath)), reason) : new FileOperationException(fallbackPath, reason); } diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountWrapper.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountWrapper.java index f22d87990..ff491e21d 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountWrapper.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountWrapper.java @@ -79,7 +79,7 @@ public boolean exists(String path) throws FileSystemException { public boolean isDirectory(String path) throws FileSystemException { path = toLocal(path); try { - return mount.exists(path) && mount.isDirectory(path); + return mount.isDirectory(path); } catch (IOException e) { throw localExceptionOf(path, e); } @@ -101,8 +101,7 @@ public void list(String path, List contents) throws FileSystemException public long getSize(String path) throws FileSystemException { path = toLocal(path); try { - if (!mount.exists(path)) throw localExceptionOf(path, "No such file"); - return mount.isDirectory(path) ? 0 : mount.getSize(path); + return mount.getSize(path); } catch (IOException e) { throw localExceptionOf(path, e); } @@ -111,7 +110,6 @@ public long getSize(String path) throws FileSystemException { public BasicFileAttributes getAttributes(String path) throws FileSystemException { path = toLocal(path); try { - if (!mount.exists(path)) throw localExceptionOf(path, "No such file"); return mount.getAttributes(path); } catch (IOException e) { throw localExceptionOf(path, e); @@ -121,11 +119,7 @@ public BasicFileAttributes getAttributes(String path) throws FileSystemException public SeekableByteChannel openForRead(String path) throws FileSystemException { path = toLocal(path); try { - if (mount.exists(path) && !mount.isDirectory(path)) { - return mount.openForRead(path); - } else { - throw localExceptionOf(path, "No such file"); - } + return mount.openForRead(path); } catch (IOException e) { throw localExceptionOf(path, e); } @@ -136,11 +130,7 @@ public void makeDirectory(String path) throws FileSystemException { path = toLocal(path); try { - if (mount.exists(path)) { - if (!mount.isDirectory(path)) throw localExceptionOf(path, "File exists"); - } else { - writableMount.makeDirectory(path); - } + writableMount.makeDirectory(path); } catch (IOException e) { throw localExceptionOf(path, e); } @@ -151,9 +141,7 @@ public void delete(String path) throws FileSystemException { path = toLocal(path); try { - if (mount.exists(path)) { - writableMount.delete(path); - } + writableMount.delete(path); } catch (IOException e) { throw localExceptionOf(path, e); } @@ -224,8 +212,8 @@ private String toLocal(String path) { return FileSystem.toLocal(path, location); } - private FileSystemException localExceptionOf(@Nullable String localPath, IOException e) { - if (!location.isEmpty() && e instanceof FileOperationException ex) { + private FileSystemException localExceptionOf(String localPath, IOException e) { + if (e instanceof FileOperationException ex) { if (ex.getFilename() != null) return localExceptionOf(ex.getFilename(), FileSystemException.getMessage(ex)); } @@ -233,8 +221,8 @@ private FileSystemException localExceptionOf(@Nullable String localPath, IOExcep // This error will contain the absolute path, leaking information about where MC is installed. We drop that, // just taking the reason. We assume that the error refers to the input path. var message = ex.getReason(); - if (message == null) message = "Failed"; - return localPath == null ? new FileSystemException(message) : localExceptionOf(localPath, message); + if (message == null) message = "Access denied"; + return localExceptionOf(localPath, message); } return FileSystemException.of(e); diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/WritableFileMount.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/WritableFileMount.java index 07f51e2ea..be055a293 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/WritableFileMount.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/WritableFileMount.java @@ -183,7 +183,7 @@ public SeekableByteChannel openForWrite(String path) throws FileOperationExcepti } @Override - public SeekableByteChannel openForAppend(String path) throws IOException { + public SeekableByteChannel openForAppend(String path) throws FileOperationException { create(); var file = resolvePath(path);