From 09e521727f4c27f2df0d1c3798ca4a36f208995f Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sun, 22 Oct 2023 13:13:07 +0100 Subject: [PATCH] Make mount error messages a bit more consistent - Move most error message constants to a new MountHelpers class. - Be a little more consistent in when we throw "No such file" vs "Not a file/directory" messages. --- .../shared/computer/core/ResourceMount.java | 2 + .../filesystem/AbstractInMemoryMount.java | 10 ++- .../core/filesystem/ArchiveMount.java | 2 - .../core/filesystem/FileMount.java | 27 ++---- .../core/filesystem/FileSystem.java | 22 ++--- .../core/filesystem/FileSystemException.java | 4 +- .../core/filesystem/JarMount.java | 7 +- .../core/filesystem/MemoryMount.java | 17 ++-- .../core/filesystem/MountHelpers.java | 88 +++++++++++++++++++ .../core/filesystem/MountWrapper.java | 23 ++--- .../core/filesystem/WritableFileMount.java | 29 +++--- .../core/filesystem/MemoryMountTest.java | 2 + .../resources/test-rom/spec/apis/fs_spec.lua | 8 +- .../resources/test-rom/spec/apis/io_spec.lua | 2 +- .../test/core/filesystem/MountContract.java | 35 +++++++- .../filesystem/WritableMountContract.java | 5 +- 16 files changed, 197 insertions(+), 86 deletions(-) create mode 100644 projects/core/src/main/java/dan200/computercraft/core/filesystem/MountHelpers.java diff --git a/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ResourceMount.java b/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ResourceMount.java index b213588c2..0a69cdc7e 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ResourceMount.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ResourceMount.java @@ -21,6 +21,8 @@ import java.util.HashMap; import java.util.Map; +import static dan200.computercraft.core.filesystem.MountHelpers.NO_SUCH_FILE; + /** * A mount backed by Minecraft's {@link ResourceManager}. * diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/AbstractInMemoryMount.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/AbstractInMemoryMount.java index 58c34ba62..fdf619d8d 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/AbstractInMemoryMount.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/AbstractInMemoryMount.java @@ -17,14 +17,14 @@ import java.util.Map; import java.util.function.Function; +import static dan200.computercraft.core.filesystem.MountHelpers.*; + /** * An abstract mount which stores its file tree in memory. * * @param The type of file. */ public abstract class AbstractInMemoryMount> implements Mount { - protected static final String NO_SUCH_FILE = "No such file"; - @Nullable protected T root; @@ -57,7 +57,8 @@ public final boolean isDirectory(String path) { @Override public final void list(String path, List contents) throws IOException { var file = get(path); - if (file == null || file.children == null) throw new FileOperationException(path, "Not a directory"); + if (file == null) throw new FileOperationException(path, NO_SUCH_FILE); + if (file.children == null) throw new FileOperationException(path, NOT_A_DIRECTORY); contents.addAll(file.children.keySet()); } @@ -82,7 +83,8 @@ public final long getSize(String path) throws IOException { @Override public final SeekableByteChannel openForRead(String path) throws IOException { var file = get(path); - if (file == null || file.isDirectory()) throw new FileOperationException(path, NO_SUCH_FILE); + if (file == null) throw new FileOperationException(path, NO_SUCH_FILE); + if (file.isDirectory()) throw new FileOperationException(path, NOT_A_FILE); return openForRead(path, file); } diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/ArchiveMount.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/ArchiveMount.java index 8d47d7f33..5b22a998e 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/ArchiveMount.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/ArchiveMount.java @@ -21,8 +21,6 @@ * @param The type of file. */ public abstract class ArchiveMount> extends AbstractInMemoryMount { - protected static final String NO_SUCH_FILE = "No such file"; - /** * Limit the entire cache to 64MiB. */ 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 087464cd7..d75065a50 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 @@ -17,6 +17,9 @@ import java.util.List; import java.util.Set; +import static dan200.computercraft.core.filesystem.MountHelpers.NOT_A_FILE; +import static dan200.computercraft.core.filesystem.MountHelpers.NO_SUCH_FILE; + /** * A {@link Mount} implementation which provides read-only access to a directory. */ @@ -84,7 +87,9 @@ public BasicFileAttributes getAttributes(String path) throws FileOperationExcept @Override public SeekableByteChannel openForRead(String path) throws FileOperationException { var file = resolvePath(path); - if (!Files.isRegularFile(file)) throw new FileOperationException(path, "No such file"); + if (!Files.isRegularFile(file)) { + throw new FileOperationException(path, Files.exists(file) ? NOT_A_FILE : NO_SUCH_FILE); + } try { return Files.newByteChannel(file, READ_OPTIONS); @@ -103,7 +108,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 ? "Access denied" : exn.getMessage()); + : new FileOperationException(fallbackPath, exn.getMessage() == null ? MountHelpers.ACCESS_DENIED : exn.getMessage()); } /** @@ -115,7 +120,7 @@ protected FileOperationException remapException(String fallbackPath, IOException * @return The wrapped exception. */ protected FileOperationException remapException(String fallbackPath, FileSystemException exn) { - var reason = getReason(exn); + var reason = MountHelpers.getReason(exn); var failedFile = exn.getFile(); if (failedFile == null) return new FileOperationException(fallbackPath, reason); @@ -125,20 +130,4 @@ protected FileOperationException remapException(String fallbackPath, FileSystemE ? new FileOperationException(Joiner.on('/').join(root.relativize(failedPath)), reason) : new FileOperationException(fallbackPath, reason); } - - /** - * Get the user-friendly reason for a {@link FileSystemException}. - * - * @param exn The exception that occurred. - * @return The friendly reason for this exception. - */ - protected String getReason(FileSystemException exn) { - if (exn instanceof FileAlreadyExistsException) return "File exists"; - if (exn instanceof NoSuchFileException) return "No such file"; - if (exn instanceof NotDirectoryException) return "Not a directory"; - if (exn instanceof AccessDeniedException) return "Access denied"; - - var reason = exn.getReason(); - return reason != null ? reason.trim() : "Operation failed"; - } } diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java index 139e69e2f..0d1876923 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java @@ -24,6 +24,8 @@ import java.util.function.Function; import java.util.regex.Pattern; +import static dan200.computercraft.core.filesystem.MountHelpers.*; + public class FileSystem { /** * Maximum depth that {@link #copyRecursive(String, MountWrapper, String, MountWrapper, int)} will descend into. @@ -206,9 +208,9 @@ public synchronized void move(String sourcePath, String destPath) throws FileSys sourcePath = sanitizePath(sourcePath); destPath = sanitizePath(destPath); - if (isReadOnly(sourcePath) || isReadOnly(destPath)) throw new FileSystemException("Access denied"); - if (!exists(sourcePath)) throw new FileSystemException("No such file"); - if (exists(destPath)) throw new FileSystemException("File exists"); + if (isReadOnly(sourcePath) || isReadOnly(destPath)) throw new FileSystemException(ACCESS_DENIED); + if (!exists(sourcePath)) throw new FileSystemException(NO_SUCH_FILE); + if (exists(destPath)) throw new FileSystemException(FILE_EXISTS); if (contains(sourcePath, destPath)) throw new FileSystemException("Can't move a directory inside itself"); var mount = getMount(sourcePath); @@ -223,15 +225,9 @@ public synchronized void move(String sourcePath, String destPath) throws FileSys public synchronized void copy(String sourcePath, String destPath) throws FileSystemException { sourcePath = sanitizePath(sourcePath); destPath = sanitizePath(destPath); - if (isReadOnly(destPath)) { - throw new FileSystemException("/" + destPath + ": Access denied"); - } - if (!exists(sourcePath)) { - throw new FileSystemException("/" + sourcePath + ": No such file"); - } - if (exists(destPath)) { - throw new FileSystemException("/" + destPath + ": File exists"); - } + if (isReadOnly(destPath)) throw new FileSystemException("/" + destPath + ": " + ACCESS_DENIED); + if (!exists(sourcePath)) throw new FileSystemException("/" + sourcePath + ": " + NO_SUCH_FILE); + if (exists(destPath)) throw new FileSystemException("/" + destPath + ": " + FILE_EXISTS); if (contains(sourcePath, destPath)) { throw new FileSystemException("/" + sourcePath + ": Can't copy a directory inside itself"); } @@ -264,7 +260,7 @@ private synchronized void copyRecursive(String sourcePath, MountWrapper sourceMo // Copy bytes as fast as we can ByteStreams.copy(source, destination); } catch (AccessDeniedException e) { - throw new FileSystemException("Access denied"); + throw new FileSystemException(ACCESS_DENIED); } catch (IOException e) { throw FileSystemException.of(e); } diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystemException.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystemException.java index ecce695ff..8828218c7 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystemException.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystemException.java @@ -7,6 +7,8 @@ import java.io.IOException; import java.io.Serial; +import static dan200.computercraft.core.filesystem.MountHelpers.ACCESS_DENIED; + public class FileSystemException extends Exception { @Serial private static final long serialVersionUID = -2500631644868104029L; @@ -20,6 +22,6 @@ public static FileSystemException of(IOException e) { } public static String getMessage(IOException e) { - return e.getMessage() == null ? "Access denied" : e.getMessage(); + return e.getMessage() == null ? ACCESS_DENIED : e.getMessage(); } } diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/JarMount.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/JarMount.java index 10c4959d1..4711333b3 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/JarMount.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/JarMount.java @@ -14,11 +14,12 @@ import java.io.IOException; import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.FileTime; -import java.time.Instant; import java.util.HashMap; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; +import static dan200.computercraft.core.filesystem.MountHelpers.NO_SUCH_FILE; + /** * A mount which reads zip/jar files. */ @@ -95,9 +96,7 @@ void setup(ZipEntry entry) { } } - private static final FileTime EPOCH = FileTime.from(Instant.EPOCH); - private static FileTime orEpoch(@Nullable FileTime time) { - return time == null ? EPOCH : time; + return time == null ? MountHelpers.EPOCH : time; } } diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/MemoryMount.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/MemoryMount.java index 702cfbe79..5310c3812 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/MemoryMount.java +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/MemoryMount.java @@ -22,14 +22,13 @@ import java.time.Instant; import java.util.*; -import static dan200.computercraft.core.filesystem.WritableFileMount.MINIMUM_FILE_SIZE; +import static dan200.computercraft.core.filesystem.MountHelpers.*; /** * A basic {@link Mount} which stores files and directories in-memory. */ public final class MemoryMount extends AbstractInMemoryMount implements WritableMount { private static final byte[] EMPTY = new byte[0]; - private static final FileTime EPOCH = FileTime.from(Instant.EPOCH); private final long capacity; @@ -80,7 +79,7 @@ protected long getSize(String path, FileEntry file) { @Override protected SeekableByteChannel openForRead(String path, FileEntry file) throws IOException { - if (file.contents == null) throw new FileOperationException(path, "File is a directory"); + if (file.contents == null) throw new FileOperationException(path, NOT_A_FILE); return new EntryChannel(file, 0); } @@ -119,7 +118,7 @@ public void makeDirectory(String path) throws IOException { if (nextEntry == null) { lastEntry.children.put(part, nextEntry = FileEntry.newDir()); } else if (nextEntry.children == null) { - throw new FileOperationException(path, "File exists"); + throw new FileOperationException(path, FILE_EXISTS); } lastEntry = nextEntry; @@ -129,7 +128,7 @@ public void makeDirectory(String path) throws IOException { @Override public void delete(String path) throws IOException { - if (path.isEmpty()) throw new AccessDeniedException("Access denied"); + if (path.isEmpty()) throw new AccessDeniedException(ACCESS_DENIED); var node = getParentAndName(path); if (node != null) node.parent().remove(node.name()); } @@ -139,23 +138,23 @@ public void rename(String source, String dest) throws IOException { if (dest.startsWith(source)) throw new FileOperationException(source, "Cannot move a directory inside itself"); var sourceParent = getParentAndName(source); - if (sourceParent == null || !sourceParent.exists()) throw new FileOperationException(source, "No such file"); + if (sourceParent == null || !sourceParent.exists()) throw new FileOperationException(source, NO_SUCH_FILE); var destParent = getParentAndName(dest); if (destParent == null) throw new FileOperationException(dest, "Parent directory does not exist"); - if (destParent.exists()) throw new FileOperationException(dest, "File exists"); + if (destParent.exists()) throw new FileOperationException(dest, FILE_EXISTS); destParent.put(sourceParent.parent().remove(sourceParent.name())); } private FileEntry getForWrite(String path) throws FileOperationException { - if (path.isEmpty()) throw new FileOperationException(path, "Cannot write to directory"); + if (path.isEmpty()) throw new FileOperationException(path, CANNOT_WRITE_TO_DIRECTORY); var parent = getParentAndName(path); if (parent == null) throw new FileOperationException(path, "Parent directory does not exist"); var file = parent.get(); - if (file != null && file.isDirectory()) throw new FileOperationException(path, "Cannot write to directory"); + if (file != null && file.isDirectory()) throw new FileOperationException(path, CANNOT_WRITE_TO_DIRECTORY); if (file == null) parent.put(file = FileEntry.newFile()); return file; diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountHelpers.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountHelpers.java new file mode 100644 index 000000000..3e3b701af --- /dev/null +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountHelpers.java @@ -0,0 +1,88 @@ +// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers +// +// SPDX-License-Identifier: MPL-2.0 + +package dan200.computercraft.core.filesystem; + +import dan200.computercraft.api.filesystem.Mount; +import dan200.computercraft.api.filesystem.WritableMount; + +import java.nio.file.FileSystemException; +import java.nio.file.*; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; +import java.time.Instant; +import java.util.List; + +/** + * Useful constants and helper functions for working with mounts. + */ +public final class MountHelpers { + /** + * A {@link FileTime} set to the Unix EPOCH, intended for {@link BasicFileAttributes}'s file times. + */ + public static final FileTime EPOCH = FileTime.from(Instant.EPOCH); + + /** + * The minimum size of a file for file {@linkplain WritableMount#getCapacity() capacity calculations}. + */ + public static final long MINIMUM_FILE_SIZE = 500; + + /** + * The error message used when the file does not exist. + */ + public static final String NO_SUCH_FILE = "No such file"; + + /** + * The error message used when trying to use a file as a directory (for instance when + * {@linkplain Mount#list(String, List) listing its contents}). + */ + public static final String NOT_A_DIRECTORY = "Not a directory"; + + /** + * The error message used when trying to use a directory as a file (for instance when + * {@linkplain Mount#openForRead(String) opening for reading}). + */ + public static final String NOT_A_FILE = "Not a file"; + + /** + * The error message used when attempting to modify a read-only file or mount. + */ + public static final String ACCESS_DENIED = "Access denied"; + + /** + * The error message used when trying to overwrite a file (for instance when + * {@linkplain WritableMount#rename(String, String) renaming files} or {@linkplain WritableMount#makeDirectory(String) + * creating directories}). + */ + public static final String FILE_EXISTS = "File exists"; + + /** + * The error message used when trying to {@linkplain WritableMount#openForWrite(String) opening a directory to read}. + */ + public static final String CANNOT_WRITE_TO_DIRECTORY = "Cannot write to directory"; + + /** + * The error message used when the mount runs out of space. + */ + public static final String OUT_OF_SPACE = "Out of space"; + + private MountHelpers() { + } + + /** + * Get the user-friendly reason for a {@link java.nio.file.FileSystemException}. + * + * @param exn The exception that occurred. + * @return The friendly reason for this exception. + */ + public static String getReason(FileSystemException exn) { + if (exn instanceof FileAlreadyExistsException) return FILE_EXISTS; + if (exn instanceof NoSuchFileException) return NO_SUCH_FILE; + if (exn instanceof NotDirectoryException) return NOT_A_DIRECTORY; + if (exn instanceof AccessDeniedException) return ACCESS_DENIED; + + var reason = exn.getReason(); + return reason != null ? reason.trim() : "Operation failed"; + } +} 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 f3938b4a9..2f72a0c21 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 @@ -15,6 +15,8 @@ import java.util.List; import java.util.OptionalLong; +import static dan200.computercraft.core.filesystem.MountHelpers.*; + class MountWrapper { private final String label; private final String location; @@ -87,9 +89,8 @@ public boolean isDirectory(String path) throws FileSystemException { public void list(String path, List contents) throws FileSystemException { path = toLocal(path); try { - if (!mount.exists(path) || !mount.isDirectory(path)) { - throw localExceptionOf(path, "Not a directory"); - } + if (!mount.exists(path)) throw localExceptionOf(path, NO_SUCH_FILE); + if (!mount.isDirectory(path)) throw localExceptionOf(path, NOT_A_DIRECTORY); mount.list(path, contents); } catch (IOException e) { @@ -125,7 +126,7 @@ public SeekableByteChannel openForRead(String path) throws FileSystemException { } public void makeDirectory(String path) throws FileSystemException { - if (writableMount == null) throw exceptionOf(path, "Access denied"); + if (writableMount == null) throw exceptionOf(path, ACCESS_DENIED); path = toLocal(path); try { @@ -136,7 +137,7 @@ public void makeDirectory(String path) throws FileSystemException { } public void delete(String path) throws FileSystemException { - if (writableMount == null) throw exceptionOf(path, "Access denied"); + if (writableMount == null) throw exceptionOf(path, ACCESS_DENIED); path = toLocal(path); try { @@ -147,7 +148,7 @@ public void delete(String path) throws FileSystemException { } public void rename(String source, String dest) throws FileSystemException { - if (writableMount == null) throw exceptionOf(source, "Access denied"); + if (writableMount == null) throw exceptionOf(source, ACCESS_DENIED); source = toLocal(source); dest = toLocal(dest); @@ -164,12 +165,12 @@ public void rename(String source, String dest) throws FileSystemException { } public SeekableByteChannel openForWrite(String path) throws FileSystemException { - if (writableMount == null) throw exceptionOf(path, "Access denied"); + if (writableMount == null) throw exceptionOf(path, ACCESS_DENIED); path = toLocal(path); try { if (mount.exists(path) && mount.isDirectory(path)) { - throw localExceptionOf(path, "Cannot write to directory"); + throw localExceptionOf(path, CANNOT_WRITE_TO_DIRECTORY); } else { if (!path.isEmpty()) { var dir = FileSystem.getDirectory(path); @@ -185,7 +186,7 @@ public SeekableByteChannel openForWrite(String path) throws FileSystemException } public SeekableByteChannel openForAppend(String path) throws FileSystemException { - if (writableMount == null) throw exceptionOf(path, "Access denied"); + if (writableMount == null) throw exceptionOf(path, ACCESS_DENIED); path = toLocal(path); try { @@ -198,7 +199,7 @@ public SeekableByteChannel openForAppend(String path) throws FileSystemException } return writableMount.openForWrite(path); } else if (mount.isDirectory(path)) { - throw localExceptionOf(path, "Cannot write to directory"); + throw localExceptionOf(path, CANNOT_WRITE_TO_DIRECTORY); } else { return writableMount.openForAppend(path); } @@ -220,7 +221,7 @@ private FileSystemException localExceptionOf(String localPath, IOException e) { // 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 = "Access denied"; + if (message == null) message = ACCESS_DENIED; return localExceptionOf(localPath, message); } 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 47ac4482c..b3626acef 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 @@ -20,13 +20,14 @@ import java.nio.file.attribute.BasicFileAttributes; import java.util.Set; +import static dan200.computercraft.core.filesystem.MountHelpers.*; + /** * A {@link WritableFileMount} implementation which provides read-write access to a directory. */ public class WritableFileMount extends FileMount implements WritableMount { private static final Logger LOG = LoggerFactory.getLogger(WritableFileMount.class); - static final long MINIMUM_FILE_SIZE = 500; private static final Set WRITE_OPTIONS = Set.of(StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); private static final Set APPEND_OPTIONS = Set.of(StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.APPEND); @@ -49,7 +50,7 @@ private void create() throws FileOperationException { try { Files.createDirectories(root); } catch (IOException e) { - throw new FileOperationException("Access denied"); + throw new FileOperationException(ACCESS_DENIED); } } @@ -78,7 +79,7 @@ public void makeDirectory(String path) throws IOException { create(); var file = resolveFile(path); if (file.exists()) { - if (!file.isDirectory()) throw new FileOperationException(path, "File exists"); + if (!file.isDirectory()) throw new FileOperationException(path, FILE_EXISTS); return; } @@ -90,19 +91,19 @@ public void makeDirectory(String path) throws IOException { } if (getRemainingSpace() < dirsToCreate * MINIMUM_FILE_SIZE) { - throw new FileOperationException(path, "Out of space"); + throw new FileOperationException(path, OUT_OF_SPACE); } if (file.mkdirs()) { usedSpace += dirsToCreate * MINIMUM_FILE_SIZE; } else { - throw new FileOperationException(path, "Access denied"); + throw new FileOperationException(path, ACCESS_DENIED); } } @Override public void delete(String path) throws IOException { - if (path.isEmpty()) throw new FileOperationException(path, "Access denied"); + if (path.isEmpty()) throw new FileOperationException(path, ACCESS_DENIED); if (created()) { var file = resolveFile(path); @@ -125,7 +126,7 @@ private void deleteRecursively(File file) throws IOException { if (success) { usedSpace -= Math.max(MINIMUM_FILE_SIZE, fileSize); } else { - throw new IOException("Access denied"); + throw new IOException(ACCESS_DENIED); } } @@ -133,8 +134,8 @@ private void deleteRecursively(File file) throws IOException { public void rename(String source, String dest) throws FileOperationException { var sourceFile = resolvePath(source); var destFile = resolvePath(dest); - if (!Files.exists(sourceFile)) throw new FileOperationException(source, "No such file"); - if (Files.exists(destFile)) throw new FileOperationException(dest, "File exists"); + if (!Files.exists(sourceFile)) throw new FileOperationException(source, NO_SUCH_FILE); + if (Files.exists(destFile)) throw new FileOperationException(dest, FILE_EXISTS); if (destFile.startsWith(sourceFile)) { throw new FileOperationException(source, "Cannot move a directory inside itself"); @@ -164,9 +165,9 @@ public SeekableByteChannel openForWrite(String path) throws FileOperationExcepti var file = resolvePath(path); var attributes = tryGetAttributes(path, file); if (attributes == null) { - if (getRemainingSpace() < MINIMUM_FILE_SIZE) throw new FileOperationException(path, "Out of space"); + if (getRemainingSpace() < MINIMUM_FILE_SIZE) throw new FileOperationException(path, OUT_OF_SPACE); } else if (attributes.isDirectory()) { - throw new FileOperationException(path, "Cannot write to directory"); + throw new FileOperationException(path, CANNOT_WRITE_TO_DIRECTORY); } else { usedSpace -= Math.max(attributes.size(), MINIMUM_FILE_SIZE); } @@ -187,9 +188,9 @@ public SeekableByteChannel openForAppend(String path) throws FileOperationExcept var file = resolvePath(path); var attributes = tryGetAttributes(path, file); if (attributes == null) { - if (getRemainingSpace() < MINIMUM_FILE_SIZE) throw new FileOperationException(path, "Out of space"); + if (getRemainingSpace() < MINIMUM_FILE_SIZE) throw new FileOperationException(path, OUT_OF_SPACE); } else if (attributes.isDirectory()) { - throw new FileOperationException(path, "Cannot write to directory"); + throw new FileOperationException(path, CANNOT_WRITE_TO_DIRECTORY); } // Allowing seeking when appending is not recommended, so we use a separate channel. @@ -228,7 +229,7 @@ void count(long n) throws IOException { ignoredBytesLeft = 0; var bytesLeft = capacity - usedSpace; - if (newBytes > bytesLeft) throw new IOException("Out of space"); + if (newBytes > bytesLeft) throw new IOException(OUT_OF_SPACE); usedSpace += newBytes; } } diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/MemoryMountTest.java b/projects/core/src/test/java/dan200/computercraft/core/filesystem/MemoryMountTest.java index 7b03c14b2..0d456a51d 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/filesystem/MemoryMountTest.java +++ b/projects/core/src/test/java/dan200/computercraft/core/filesystem/MemoryMountTest.java @@ -10,6 +10,8 @@ import dan200.computercraft.test.core.filesystem.WritableMountContract; import org.opentest4j.TestAbortedException; +import static dan200.computercraft.core.filesystem.MountHelpers.EPOCH; + public class MemoryMountTest implements MountContract, WritableMountContract { @Override public Mount createSkeleton() { diff --git a/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua b/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua index 156dafdd5..6cc5c5516 100644 --- a/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua +++ b/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua @@ -82,8 +82,8 @@ describe("The fs library", function() end) it("fails on non-existent nodes", function() - expect.error(fs.list, "rom/x"):eq("/rom/x: Not a directory") - expect.error(fs.list, "x"):eq("/x: Not a directory") + expect.error(fs.list, "rom/x"):eq("/rom/x: No such file") + expect.error(fs.list, "x"):eq("/x: No such file") end) end) @@ -160,8 +160,8 @@ describe("The fs library", function() describe("fs.open", function() describe("reading", function() it("fails on directories", function() - expect { fs.open("rom", "r") }:same { nil, "/rom: No such file" } - expect { fs.open("", "r") }:same { nil, "/: No such file" } + expect { fs.open("rom", "r") }:same { nil, "/rom: Not a file" } + expect { fs.open("", "r") }:same { nil, "/: Not a file" } end) it("fails on non-existent nodes", function() diff --git a/projects/core/src/test/resources/test-rom/spec/apis/io_spec.lua b/projects/core/src/test/resources/test-rom/spec/apis/io_spec.lua index 1512928e1..e5d7667e7 100644 --- a/projects/core/src/test/resources/test-rom/spec/apis/io_spec.lua +++ b/projects/core/src/test/resources/test-rom/spec/apis/io_spec.lua @@ -75,7 +75,7 @@ describe("The io library", function() describe("io.lines", function() it("validates arguments", function() io.lines(nil) - expect.error(io.lines, ""):eq("/: No such file") + expect.error(io.lines, ""):eq("/: Not a file") expect.error(io.lines, false):eq("bad argument #1 (string expected, got boolean)") end) diff --git a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MountContract.java b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MountContract.java index a43fece23..3b6fa3122 100644 --- a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MountContract.java +++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MountContract.java @@ -6,6 +6,8 @@ import dan200.computercraft.api.filesystem.FileOperationException; import dan200.computercraft.api.filesystem.Mount; +import dan200.computercraft.test.core.ReplaceUnderscoresDisplayNameGenerator; +import org.junit.jupiter.api.DisplayNameGeneration; import org.junit.jupiter.api.Test; import java.io.IOException; @@ -19,6 +21,7 @@ import java.util.Comparator; import java.util.List; +import static dan200.computercraft.core.filesystem.MountHelpers.*; import static org.junit.jupiter.api.Assertions.*; /** @@ -26,8 +29,8 @@ * * @see WritableMountContract */ +@DisplayNameGeneration(ReplaceUnderscoresDisplayNameGenerator.class) public interface MountContract { - FileTime EPOCH = FileTime.from(Instant.EPOCH); FileTime MODIFY_TIME = FileTime.from(Instant.EPOCH.plus(2, ChronoUnit.DAYS)); /** @@ -84,6 +87,24 @@ default void testList() throws IOException { assertEquals(List.of("dir", "f.lua"), list); } + @Test + default void testListMissing() throws IOException { + var mount = createSkeleton(); + + var error = assertThrows(FileOperationException.class, () -> mount.list("no_such_file", new ArrayList<>())); + assertEquals("no_such_file", error.getFilename()); + assertEquals(NO_SUCH_FILE, error.getMessage()); + } + + @Test + default void testListFile() throws IOException { + var mount = createSkeleton(); + + var error = assertThrows(FileOperationException.class, () -> mount.list("dir/file.lua", new ArrayList<>())); + assertEquals("dir/file.lua", error.getFilename()); + assertEquals(NOT_A_DIRECTORY, error.getMessage()); + } + @Test default void testOpenFile() throws IOException { var mount = createSkeleton(); @@ -97,7 +118,7 @@ default void testOpenFile() throws IOException { } @Test - default void testOpenFileFails() throws IOException { + default void openForRead_fails_on_missing_file() throws IOException { var mount = createSkeleton(); var exn = assertThrows(FileOperationException.class, () -> mount.openForRead("doesnt/exist")); @@ -105,6 +126,16 @@ default void testOpenFileFails() throws IOException { assertEquals("No such file", exn.getMessage()); } + + @Test + default void openForRead_fails_on_directory() throws IOException { + var mount = createSkeleton(); + + var error = assertThrows(FileOperationException.class, () -> mount.openForRead("dir").close()); + assertEquals("dir", error.getFilename()); + assertEquals(NOT_A_FILE, error.getMessage()); + } + @Test default void testSize() throws IOException { var mount = createSkeleton(); diff --git a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/WritableMountContract.java b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/WritableMountContract.java index a69668dee..a9e97b201 100644 --- a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/WritableMountContract.java +++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/WritableMountContract.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.util.stream.Stream; +import static dan200.computercraft.core.filesystem.MountHelpers.MINIMUM_FILE_SIZE; import static org.junit.jupiter.api.Assertions.*; /** @@ -74,7 +75,7 @@ default void Make_directory_recursive() throws IOException { assertTrue(mount.isDirectory("a/b/c")); - assertEquals(CAPACITY - 500 * 3, mount.getRemainingSpace()); + assertEquals(CAPACITY - MINIMUM_FILE_SIZE * 3, mount.getRemainingSpace()); assertEquals(access.computeRemainingSpace(), access.mount().getRemainingSpace(), "Free space is inconsistent"); } @@ -108,7 +109,7 @@ default void Write_updates_size_and_free_space() throws IOException { Mounts.writeFile(mount, "hello.txt", ""); assertEquals(0, mount.getSize("hello.txt")); - assertEquals(CAPACITY - 500, mount.getRemainingSpace()); + assertEquals(CAPACITY - MINIMUM_FILE_SIZE, mount.getRemainingSpace()); assertEquals(access.computeRemainingSpace(), access.mount().getRemainingSpace(), "Free space is inconsistent"); }