From 367773e173b2a135f213c19bb3df949ea43b9dfa Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Fri, 9 Dec 2022 22:01:01 +0000 Subject: [PATCH] Some refactoring of mounts - Separate FileMount into separate FileMount and WritableFileMount classes. This separates the (relatively simple) read-only code from the (soon to be even more complex) read/write code. It also allows you to create read-only mounts which don't bother with filesystem accounting, which is nice. - Make openForWrite/openForAppend always return a SeekableFileHandle. Appendable files still cannot be seeked within, but that check is now done on the FS side. - Refactor the various mount tests to live in test contract interfaces, allowing us to reuse them between mounts. - Clean up our error handling a little better. (Most) file-specific code has been moved to FileMount, and ArchiveMount-derived classes now throw correct path-localised exceptions. --- .../computercraft/api/ComputerCraftAPI.java | 17 +- .../impl/ComputerCraftAPIService.java | 1 - projects/common/build.gradle.kts | 1 + .../impl/AbstractComputerCraftAPI.java | 18 +- .../shared/computer/core/ResourceMount.java | 15 +- .../network/server/UploadFileMessage.java | 7 +- .../diskdrive/DiskDriveBlockEntity.java | 1 + .../computercraft/TestPlatformHelper.java | 7 - .../computer/core/ResourceMountTest.java | 82 ++-- .../api/filesystem/WritableMount.java | 12 +- .../dan200/computercraft/core/apis/FSAPI.java | 4 +- .../apis/handles/BinaryWritableHandle.java | 31 +- .../core/apis/handles/HandleGeneric.java | 13 - .../core/computer/ComputerEnvironment.java | 4 +- .../core/filesystem/ArchiveMount.java | 12 +- .../core/filesystem/FileMount.java | 423 ++++-------------- .../core/filesystem/FileSystem.java | 3 +- .../core/filesystem/JarMount.java | 23 +- .../core/filesystem/MountWrapper.java | 30 +- .../core/filesystem/WritableFileMount.java | 322 +++++++++++++ .../core/ComputerTestDelegate.java | 4 +- .../core/computer/ComputerBootstrap.java | 3 +- .../core/filesystem/FileMountTest.java | 79 ++-- .../core/filesystem/FileSystemTest.java | 4 +- .../core/filesystem/JarMountTest.java | 57 +-- .../filesystem/WritableFileMountTest.java | 67 +++ .../computercraft/test/core/CloseScope.java | 61 +++ .../test/core/computer/BasicEnvironment.java | 5 +- .../test/core/filesystem/MemoryMount.java | 79 +--- .../test/core/filesystem/MountContract.java | 128 ++++++ .../filesystem/ReadOnlyWritableMount.java | 92 ++++ .../filesystem/WritableMountContract.java | 6 +- .../impl/ComputerCraftAPIImpl.java | 10 - .../impl/ComputerCraftAPIImpl.java | 11 - tools/parse-reports.py | 2 +- 35 files changed, 978 insertions(+), 656 deletions(-) create mode 100644 projects/core/src/main/java/dan200/computercraft/core/filesystem/WritableFileMount.java create mode 100644 projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableFileMountTest.java create mode 100644 projects/core/src/testFixtures/java/dan200/computercraft/test/core/CloseScope.java create mode 100644 projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MountContract.java create mode 100644 projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/ReadOnlyWritableMount.java rename projects/core/src/{test/java/dan200/computercraft => testFixtures/java/dan200/computercraft/test}/core/filesystem/WritableMountContract.java (95%) diff --git a/projects/common-api/src/main/java/dan200/computercraft/api/ComputerCraftAPI.java b/projects/common-api/src/main/java/dan200/computercraft/api/ComputerCraftAPI.java index 90533ee19..1c61f3351 100644 --- a/projects/common-api/src/main/java/dan200/computercraft/api/ComputerCraftAPI.java +++ b/projects/common-api/src/main/java/dan200/computercraft/api/ComputerCraftAPI.java @@ -61,22 +61,25 @@ public static int createUniqueNumberedSaveDir(MinecraftServer server, String par /** * Creates a file system mount that maps to a subfolder of the save directory for a given world, and returns it. *

- * Use in conjunction with IComputerAccess.mount() or IComputerAccess.mountWritable() to mount a folder from the - * users save directory onto a computers file system. + * Use in conjunction with Use {@link IComputerAccess#mount(String, Mount)} or {@link IComputerAccess#mountWritable(String, WritableMount)} + * to mount this on a computer's file system. + *

+ * If the same folder may be mounted on multiple computers at once (for instance, if you provide a network file share), + * the same mount instance should be used for all computers. You should NOT have multiple mount instances for the + * same folder. * * @param server The server which the save dir can be found. - * @param subPath The folder path within the save directory that the mount should map to. eg: "computer/disk/42". - * Use createUniqueNumberedSaveDir() to create a new numbered folder to use. + * @param subPath The folder path within the save directory that the mount should map to. eg: "disk/42". + * Use {@link #createUniqueNumberedSaveDir(MinecraftServer, String)} to create a new numbered folder + * to use. * @param capacity The amount of data that can be stored in the directory before it fills up, in bytes. - * @return The mount, or null if it could be created for some reason. Use IComputerAccess.mount() or IComputerAccess.mountWritable() - * to mount this on a Computers' file system. + * @return The newly created mount. * @see #createUniqueNumberedSaveDir(MinecraftServer, String) * @see IComputerAccess#mount(String, Mount) * @see IComputerAccess#mountWritable(String, WritableMount) * @see Mount * @see WritableMount */ - @Nullable public static WritableMount createSaveDirMount(MinecraftServer server, String subPath, long capacity) { return getInstance().createSaveDirMount(server, subPath, capacity); } diff --git a/projects/common-api/src/main/java/dan200/computercraft/impl/ComputerCraftAPIService.java b/projects/common-api/src/main/java/dan200/computercraft/impl/ComputerCraftAPIService.java index 73a3dfd0d..020510308 100644 --- a/projects/common-api/src/main/java/dan200/computercraft/impl/ComputerCraftAPIService.java +++ b/projects/common-api/src/main/java/dan200/computercraft/impl/ComputerCraftAPIService.java @@ -43,7 +43,6 @@ static ComputerCraftAPIService get() { int createUniqueNumberedSaveDir(MinecraftServer server, String parentSubPath); - @Nullable WritableMount createSaveDirMount(MinecraftServer server, String subPath, long capacity); @Nullable diff --git a/projects/common/build.gradle.kts b/projects/common/build.gradle.kts index 644f02a6f..0efbaa220 100644 --- a/projects/common/build.gradle.kts +++ b/projects/common/build.gradle.kts @@ -24,6 +24,7 @@ dependencies { compileOnly(libs.mixin) annotationProcessorEverywhere(libs.autoService) + testFixturesAnnotationProcessor(libs.autoService) testImplementation(testFixtures(project(":core"))) testImplementation(libs.bundles.test) diff --git a/projects/common/src/main/java/dan200/computercraft/impl/AbstractComputerCraftAPI.java b/projects/common/src/main/java/dan200/computercraft/impl/AbstractComputerCraftAPI.java index d02b2ca89..7aca97e73 100644 --- a/projects/common/src/main/java/dan200/computercraft/impl/AbstractComputerCraftAPI.java +++ b/projects/common/src/main/java/dan200/computercraft/impl/AbstractComputerCraftAPI.java @@ -7,6 +7,7 @@ import dan200.computercraft.api.detail.BlockReference; import dan200.computercraft.api.detail.DetailRegistry; +import dan200.computercraft.api.filesystem.Mount; import dan200.computercraft.api.filesystem.WritableMount; import dan200.computercraft.api.lua.GenericSource; import dan200.computercraft.api.lua.ILuaAPIFactory; @@ -18,9 +19,10 @@ import dan200.computercraft.api.turtle.TurtleRefuelHandler; import dan200.computercraft.core.apis.ApiFactories; import dan200.computercraft.core.asm.GenericMethod; -import dan200.computercraft.core.filesystem.FileMount; +import dan200.computercraft.core.filesystem.WritableFileMount; import dan200.computercraft.impl.detail.DetailRegistryImpl; import dan200.computercraft.impl.network.wired.WiredNodeImpl; +import dan200.computercraft.shared.computer.core.ResourceMount; import dan200.computercraft.shared.computer.core.ServerContext; import dan200.computercraft.shared.details.BlockDetails; import dan200.computercraft.shared.details.ItemDetails; @@ -57,13 +59,15 @@ public final int createUniqueNumberedSaveDir(MinecraftServer server, String pare } @Override - public final @Nullable WritableMount createSaveDirMount(MinecraftServer server, String subPath, long capacity) { + public final WritableMount createSaveDirMount(MinecraftServer server, String subPath, long capacity) { var root = ServerContext.get(server).storageDir().toFile(); - try { - return new FileMount(new File(root, subPath), capacity); - } catch (Exception e) { - return null; - } + return new WritableFileMount(new File(root, subPath), capacity); + } + + @Override + public final @Nullable Mount createResourceMount(MinecraftServer server, String domain, String subPath) { + var mount = ResourceMount.get(domain, subPath, server.getResourceManager()); + return mount.exists("") ? mount : null; } @Override 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 27bc269ac..cc7423561 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 @@ -5,6 +5,8 @@ */ package dan200.computercraft.shared.computer.core; +import com.google.common.annotations.VisibleForTesting; +import dan200.computercraft.api.filesystem.FileOperationException; import dan200.computercraft.core.filesystem.ArchiveMount; import dan200.computercraft.core.filesystem.FileSystem; import net.minecraft.ResourceLocationException; @@ -16,7 +18,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.FileNotFoundException; import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -49,7 +50,8 @@ public static ResourceMount get(String namespace, String subPath, ResourceManage } } - private ResourceMount(String namespace, String subPath, ResourceManager manager) { + @VisibleForTesting + ResourceMount(String namespace, String subPath, ResourceManager manager) { this.namespace = namespace; this.subPath = subPath; load(manager); @@ -59,7 +61,7 @@ private void load(ResourceManager manager) { var hasAny = false; String existingNamespace = null; - var newRoot = new FileEntry(new ResourceLocation(namespace, subPath)); + var newRoot = new FileEntry("", new ResourceLocation(namespace, subPath)); for (var file : manager.listResources(subPath, s -> true).keySet()) { existingNamespace = file.getNamespace(); @@ -100,7 +102,7 @@ private void create(FileEntry lastEntry, String path) { LOG.warn("Cannot create resource location for {} ({})", part, e.getMessage()); return; } - lastEntry.children.put(part, nextEntry = new FileEntry(childPath)); + lastEntry.children.put(part, nextEntry = new FileEntry(path, childPath)); } lastEntry = nextEntry; @@ -129,7 +131,7 @@ public long getSize(FileEntry file) { @Override public byte[] getContents(FileEntry file) throws IOException { var resource = manager.getResource(file.identifier).orElse(null); - if (resource == null) throw new FileNotFoundException(NO_SUCH_FILE); + if (resource == null) throw new FileOperationException(file.path, NO_SUCH_FILE); try (var stream = resource.open()) { return stream.readAllBytes(); @@ -139,7 +141,8 @@ public byte[] getContents(FileEntry file) throws IOException { protected static class FileEntry extends ArchiveMount.FileEntry { final ResourceLocation identifier; - FileEntry(ResourceLocation identifier) { + FileEntry(String path, ResourceLocation identifier) { + super(path); this.identifier = identifier; } } diff --git a/projects/common/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java b/projects/common/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java index 8aa2afbb6..560244c19 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java @@ -25,6 +25,7 @@ public class UploadFileMessage extends ComputerServerMessage { public static final int MAX_SIZE = 512 * 1024; static final int MAX_PACKET_SIZE = 30 * 1024; // Max packet size is 32767. + private static final int HEADER_SIZE = 16 + 1; // 16 bytes for the UUID, 4 for the flag. public static final int MAX_FILES = 32; public static final int MAX_FILE_NAME = 128; @@ -120,8 +121,8 @@ public void toBytes(FriendlyByteBuf buf) { public static void send(AbstractContainerMenu container, List files, Consumer send) { var uuid = UUID.randomUUID(); - var remaining = MAX_PACKET_SIZE; - for (var file : files) remaining -= file.getName().length() * 4 + FileUpload.CHECKSUM_LENGTH; + var remaining = MAX_PACKET_SIZE - HEADER_SIZE; + for (var file : files) remaining -= 4 + file.getName().length() * 4 + FileUpload.CHECKSUM_LENGTH; var first = true; List slices = new ArrayList<>(files.size()); @@ -137,7 +138,7 @@ public static void send(AbstractContainerMenu container, List files, ? new UploadFileMessage(container, uuid, FLAG_FIRST, files, new ArrayList<>(slices)) : new UploadFileMessage(container, uuid, 0, null, new ArrayList<>(slices))); slices.clear(); - remaining = MAX_PACKET_SIZE; + remaining = MAX_PACKET_SIZE - HEADER_SIZE; first = false; } diff --git a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java index 76b6f7605..c93c4b16d 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java @@ -202,6 +202,7 @@ void ejectDisk() { ejectQueued.set(true); } + @GuardedBy("this") private void mountDisk(IComputerAccess computer, MountInfo info, MediaStack disk) { var mount = disk.getMount((ServerLevel) getLevel()); if (mount != null) { diff --git a/projects/common/src/test/java/dan200/computercraft/TestPlatformHelper.java b/projects/common/src/test/java/dan200/computercraft/TestPlatformHelper.java index 9c12600f3..fcb17f904 100644 --- a/projects/common/src/test/java/dan200/computercraft/TestPlatformHelper.java +++ b/projects/common/src/test/java/dan200/computercraft/TestPlatformHelper.java @@ -8,7 +8,6 @@ import com.google.auto.service.AutoService; import com.mojang.authlib.GameProfile; import com.mojang.brigadier.arguments.ArgumentType; -import dan200.computercraft.api.filesystem.Mount; import dan200.computercraft.api.network.wired.WiredElement; import dan200.computercraft.api.peripheral.IPeripheral; import dan200.computercraft.impl.AbstractComputerCraftAPI; @@ -221,12 +220,6 @@ public String getInstalledVersion() { return "1.0"; } - @Nullable - @Override - public Mount createResourceMount(MinecraftServer server, String domain, String subPath) { - throw new UnsupportedOperationException("Cannot create resource mount"); - } - @Nullable @Override public T tryGetRegistryObject(ResourceKey> registry, ResourceLocation id) { diff --git a/projects/common/src/test/java/dan200/computercraft/shared/computer/core/ResourceMountTest.java b/projects/common/src/test/java/dan200/computercraft/shared/computer/core/ResourceMountTest.java index c060c9a85..e9c2fb1d7 100644 --- a/projects/common/src/test/java/dan200/computercraft/shared/computer/core/ResourceMountTest.java +++ b/projects/common/src/test/java/dan200/computercraft/shared/computer/core/ResourceMountTest.java @@ -5,73 +5,59 @@ */ package dan200.computercraft.shared.computer.core; +import com.google.common.io.MoreFiles; +import com.google.common.io.RecursiveDeleteOption; import dan200.computercraft.api.filesystem.Mount; +import dan200.computercraft.test.core.CloseScope; +import dan200.computercraft.test.core.filesystem.MountContract; import net.minecraft.Util; import net.minecraft.server.packs.PackType; import net.minecraft.server.packs.PathPackResources; import net.minecraft.server.packs.resources.ReloadableResourceManager; import net.minecraft.util.Unit; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.AfterEach; import java.io.IOException; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Comparator; +import java.nio.file.Files; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; -import static org.junit.jupiter.api.Assertions.*; +public class ResourceMountTest implements MountContract { + private final CloseScope toClose = new CloseScope(); -public class ResourceMountTest { - private Mount mount; + @Override + public Mount createSkeleton() throws IOException { + var path = Files.createTempDirectory("cctweaked-test"); + toClose.add(() -> MoreFiles.deleteRecursively(path, RecursiveDeleteOption.ALLOW_INSECURE)); + + Files.createDirectories(path.resolve("data/computercraft/rom/dir")); + try (var writer = Files.newBufferedWriter(path.resolve("data/computercraft/rom/dir/file.lua"))) { + writer.write("print('testing')"); + } + Files.newBufferedWriter(path.resolve("data/computercraft/rom/f.lua")).close(); - @BeforeEach - public void before() { var manager = new ReloadableResourceManager(PackType.SERVER_DATA); - var done = new CompletableFuture(); - manager.createReload(Util.backgroundExecutor(), Util.backgroundExecutor(), done, List.of( - new PathPackResources("resources", Path.of("../core/src/main/resources"), false) + var reload = manager.createReload(Util.backgroundExecutor(), Util.backgroundExecutor(), CompletableFuture.completedFuture(Unit.INSTANCE), List.of( + new PathPackResources("resources", path, false) )); - mount = ResourceMount.get("computercraft", "lua/rom", manager); + try { + reload.done().get(); + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException("Failed to load resources", e); + } + + return new ResourceMount("computercraft", "rom", manager); } - @Test - public void testList() throws IOException { - List files = new ArrayList<>(); - mount.list("", files); - files.sort(Comparator.naturalOrder()); - - assertEquals( - Arrays.asList("apis", "autorun", "help", "modules", "motd.txt", "programs", "startup.lua"), - files - ); + @Override + public boolean hasFileTimes() { + return false; } - @Test - public void testExists() throws IOException { - assertTrue(mount.exists("")); - assertTrue(mount.exists("startup.lua")); - assertTrue(mount.exists("programs/fun/advanced/paint.lua")); - - assertFalse(mount.exists("programs/fun/advance/paint.lua")); - assertFalse(mount.exists("programs/fun/advanced/paint.lu")); - } - - @Test - public void testIsDir() throws IOException { - assertTrue(mount.isDirectory("")); - } - - @Test - public void testIsFile() throws IOException { - assertFalse(mount.isDirectory("startup.lua")); - } - - @Test - public void testSize() throws IOException { - assertNotEquals(mount.getSize("startup.lua"), 0); + @AfterEach + public void after() throws Exception { + toClose.close(); } } 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 e2660b60c..85cfd2774 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 @@ -9,7 +9,7 @@ import java.io.IOException; import java.io.OutputStream; -import java.nio.channels.WritableByteChannel; +import java.nio.channels.SeekableByteChannel; /** * Represents a part of a virtual filesystem that can be mounted onto a computer using {@link IComputerAccess#mount(String, Mount)} @@ -54,21 +54,19 @@ public interface WritableMount extends Mount { * Opens a file with a given path, and returns an {@link OutputStream} for writing to it. * * @param path A file path in normalised format, relative to the mount location. ie: "programs/myprogram". - * @return A stream for writing to. If the channel implements {@link java.nio.channels.SeekableByteChannel}, one - * will be able to seek to arbitrary positions when using binary mode. + * @return A stream for writing to. * @throws IOException If the file could not be opened for writing. */ - WritableByteChannel openForWrite(String path) throws IOException; + SeekableByteChannel openForWrite(String path) throws IOException; /** * Opens a file with a given path, and returns an {@link OutputStream} for appending to it. * * @param path A file path in normalised format, relative to the mount location. ie: "programs/myprogram". - * @return A stream for writing to. If the channel implements {@link java.nio.channels.SeekableByteChannel}, one - * will be able to seek to arbitrary positions when using binary mode. + * @return A stream for writing to. * @throws IOException If the file could not be opened for writing. */ - WritableByteChannel openForAppend(String path) throws IOException; + SeekableByteChannel openForAppend(String path) throws IOException; /** * Get the amount of free space on the mount, in bytes. You should decrease this value as the user writes to the diff --git a/projects/core/src/main/java/dan200/computercraft/core/apis/FSAPI.java b/projects/core/src/main/java/dan200/computercraft/core/apis/FSAPI.java index ed87c5e90..eeb3b4b95 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/apis/FSAPI.java +++ b/projects/core/src/main/java/dan200/computercraft/core/apis/FSAPI.java @@ -391,12 +391,12 @@ public final Object[] open(String path, String mode) throws LuaException { case "wb" -> { // Open the file for binary writing, then create a wrapper around the writer var writer = getFileSystem().openForWrite(path, false, Function.identity()); - return new Object[]{ BinaryWritableHandle.of(writer.get(), writer) }; + return new Object[]{ BinaryWritableHandle.of(writer.get(), writer, true) }; } case "ab" -> { // Open the file for binary appending, then create a wrapper around the reader var writer = getFileSystem().openForWrite(path, true, Function.identity()); - return new Object[]{ BinaryWritableHandle.of(writer.get(), writer) }; + return new Object[]{ BinaryWritableHandle.of(writer.get(), writer, false) }; } default -> throw new LuaException("Unsupported mode"); } diff --git a/projects/core/src/main/java/dan200/computercraft/core/apis/handles/BinaryWritableHandle.java b/projects/core/src/main/java/dan200/computercraft/core/apis/handles/BinaryWritableHandle.java index bf85a9042..12e9e829b 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/apis/handles/BinaryWritableHandle.java +++ b/projects/core/src/main/java/dan200/computercraft/core/apis/handles/BinaryWritableHandle.java @@ -10,14 +10,12 @@ import dan200.computercraft.api.lua.LuaFunction; import dan200.computercraft.api.lua.LuaValues; import dan200.computercraft.core.filesystem.TrackingCloseable; -import dan200.computercraft.core.util.Nullability; import javax.annotation.Nullable; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.nio.channels.SeekableByteChannel; -import java.nio.channels.WritableByteChannel; import java.util.Optional; /** @@ -27,23 +25,16 @@ * @cc.module fs.BinaryWriteHandle */ public class BinaryWritableHandle extends HandleGeneric { - private final WritableByteChannel writer; - final @Nullable SeekableByteChannel seekable; + final SeekableByteChannel channel; private final ByteBuffer single = ByteBuffer.allocate(1); - protected BinaryWritableHandle(WritableByteChannel writer, @Nullable SeekableByteChannel seekable, TrackingCloseable closeable) { + protected BinaryWritableHandle(SeekableByteChannel channel, TrackingCloseable closeable) { super(closeable); - this.writer = writer; - this.seekable = seekable; + this.channel = channel; } - public static BinaryWritableHandle of(WritableByteChannel channel, TrackingCloseable closeable) { - var seekable = asSeekable(channel); - return seekable == null ? new BinaryWritableHandle(channel, null, closeable) : new Seekable(seekable, closeable); - } - - public static BinaryWritableHandle of(WritableByteChannel channel) { - return of(channel, new TrackingCloseable.Impl(channel)); + public static BinaryWritableHandle of(SeekableByteChannel channel, TrackingCloseable closeable, boolean canSeek) { + return canSeek ? new Seekable(channel, closeable) : new BinaryWritableHandle(channel, closeable); } /** @@ -66,9 +57,9 @@ public final void write(IArguments arguments) throws LuaException { single.put((byte) number); single.flip(); - writer.write(single); + channel.write(single); } else if (arg instanceof String) { - writer.write(arguments.getBytes(0)); + channel.write(arguments.getBytes(0)); } else { throw LuaValues.badArgumentOf(0, "string or number", arg); } @@ -87,15 +78,15 @@ public final void flush() throws LuaException { checkOpen(); try { // Technically this is not needed - if (writer instanceof FileChannel channel) channel.force(false); + if (channel instanceof FileChannel channel) channel.force(false); } catch (IOException e) { throw new LuaException(e.getMessage()); } } public static class Seekable extends BinaryWritableHandle { - public Seekable(SeekableByteChannel seekable, TrackingCloseable closeable) { - super(seekable, seekable, closeable); + public Seekable(SeekableByteChannel channel, TrackingCloseable closeable) { + super(channel, closeable); } /** @@ -121,7 +112,7 @@ public Seekable(SeekableByteChannel seekable, TrackingCloseable closeable) { @LuaFunction public final Object[] seek(Optional whence, Optional offset) throws LuaException { checkOpen(); - return handleSeek(Nullability.assertNonNull(seekable), whence, offset); + return handleSeek(channel, whence, offset); } } } diff --git a/projects/core/src/main/java/dan200/computercraft/core/apis/handles/HandleGeneric.java b/projects/core/src/main/java/dan200/computercraft/core/apis/handles/HandleGeneric.java index 389c53990..2b328f195 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/apis/handles/HandleGeneric.java +++ b/projects/core/src/main/java/dan200/computercraft/core/apis/handles/HandleGeneric.java @@ -12,7 +12,6 @@ import javax.annotation.Nullable; import java.io.IOException; -import java.nio.channels.Channel; import java.nio.channels.SeekableByteChannel; import java.util.Optional; @@ -75,16 +74,4 @@ protected static Object[] handleSeek(SeekableByteChannel channel, Optional> implements Mount { protected static final String NO_SUCH_FILE = "No such file"; + /** * Limit the entire cache to 64MiB. */ @@ -103,7 +104,7 @@ private long getCachedSize(T file) throws IOException { *

* This should only be called once per file, as the result is cached in {@link #getSize(String)}. * - * @param file The file to compute the size of. + * @param file The file to compute the size of. This will not be a directory. * @return The size of the file. * @throws IOException If the size could not be read. */ @@ -125,7 +126,7 @@ public SeekableByteChannel openForRead(String path) throws IOException { /** * Read the entirety of a file into memory. * - * @param file The file to read into memory. + * @param file The file to read into memory. This will not be a directory. * @return The contents of the file. */ protected abstract byte[] getContents(T file) throws IOException; @@ -141,7 +142,7 @@ public final BasicFileAttributes getAttributes(String path) throws IOException { /** * Get all attributes of the file. * - * @param file The file to compute attributes for. + * @param file The file to compute attributes for. This will not be a directory. * @return The file's attributes. * @throws IOException If the attributes could not be read. */ @@ -150,11 +151,16 @@ protected BasicFileAttributes getAttributes(T file) throws IOException { } protected static class FileEntry> { + public final String path; @Nullable public Map children; long size = -1; + protected FileEntry(String path) { + this.path = path; + } + protected boolean isDirectory() { return children != null; } 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 f72fd7526..288160e76 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,370 +5,141 @@ */ package dan200.computercraft.core.filesystem; -import com.google.common.collect.Sets; +import dan200.computercraft.api.filesystem.FileAttributes; import dan200.computercraft.api.filesystem.FileOperationException; -import dan200.computercraft.api.filesystem.WritableMount; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import dan200.computercraft.api.filesystem.Mount; -import java.io.File; import java.io.IOException; -import java.nio.ByteBuffer; -import java.nio.channels.ClosedChannelException; -import java.nio.channels.NonReadableChannelException; import java.nio.channels.SeekableByteChannel; -import java.nio.channels.WritableByteChannel; +import java.nio.file.FileSystemException; import java.nio.file.*; import java.nio.file.attribute.BasicFileAttributes; import java.util.Collections; import java.util.List; import java.util.Set; -public class FileMount implements WritableMount { - private static final Logger LOG = LoggerFactory.getLogger(FileMount.class); - private static final long MINIMUM_FILE_SIZE = 500; +/** + * A {@link Mount} implementation which provides read-only access to a directory. + */ +public class FileMount implements Mount { private static final Set READ_OPTIONS = Collections.singleton(StandardOpenOption.READ); - private static final Set WRITE_OPTIONS = Sets.newHashSet(StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); - private static final Set APPEND_OPTIONS = Sets.newHashSet(StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.APPEND); - private class WritableCountingChannel implements WritableByteChannel { + protected final Path root; - private final WritableByteChannel inner; - long ignoredBytesLeft; - - WritableCountingChannel(WritableByteChannel inner, long bytesToIgnore) { - this.inner = inner; - ignoredBytesLeft = bytesToIgnore; - } - - @Override - public int write(ByteBuffer b) throws IOException { - count(b.remaining()); - return inner.write(b); - } - - void count(long n) throws IOException { - ignoredBytesLeft -= n; - if (ignoredBytesLeft < 0) { - var newBytes = -ignoredBytesLeft; - ignoredBytesLeft = 0; - - var bytesLeft = capacity - usedSpace; - if (newBytes > bytesLeft) throw new IOException("Out of space"); - usedSpace += newBytes; - } - } - - @Override - public boolean isOpen() { - return inner.isOpen(); - } - - @Override - public void close() throws IOException { - inner.close(); - } + public FileMount(Path root) { + this.root = root; } - private class SeekableCountingChannel extends WritableCountingChannel implements SeekableByteChannel { - private final SeekableByteChannel inner; - - SeekableCountingChannel(SeekableByteChannel inner, long bytesToIgnore) { - super(inner, bytesToIgnore); - this.inner = inner; - } - - @Override - public SeekableByteChannel position(long newPosition) throws IOException { - if (!isOpen()) throw new ClosedChannelException(); - if (newPosition < 0) { - throw new IllegalArgumentException("Cannot seek before the beginning of the stream"); - } - - var delta = newPosition - inner.position(); - if (delta < 0) { - ignoredBytesLeft -= delta; - } else { - count(delta); - } - - return inner.position(newPosition); - } - - @Override - public SeekableByteChannel truncate(long size) throws IOException { - throw new IOException("Not yet implemented"); - } - - @Override - public int read(ByteBuffer dst) throws ClosedChannelException { - if (!inner.isOpen()) throw new ClosedChannelException(); - throw new NonReadableChannelException(); - } - - @Override - public long position() throws IOException { - return inner.position(); - } - - @Override - public long size() throws IOException { - return inner.size(); - } + /** + * Resolve a mount-relative path to one on the file system. + * + * @param path The path to resolve. + * @return The resolved path. + */ + protected Path resolvePath(String path) { + return root.resolve(path); } - private final File rootPath; - private final long capacity; - private long usedSpace; - - public FileMount(File rootPath, long capacity) { - this.rootPath = rootPath; - this.capacity = capacity + MINIMUM_FILE_SIZE; - usedSpace = created() ? measureUsedSpace(this.rootPath) : MINIMUM_FILE_SIZE; + protected boolean created() { + return Files.exists(root); } - // IMount implementation - @Override public boolean exists(String path) { - if (!created()) return path.isEmpty(); - - var file = getRealPath(path); - return file.exists(); + return path.isEmpty() || Files.exists(resolvePath(path)); } @Override public boolean isDirectory(String path) { - if (!created()) return path.isEmpty(); - - var file = getRealPath(path); - return file.exists() && file.isDirectory(); + return path.isEmpty() || Files.isDirectory(resolvePath(path)); } @Override - public boolean isReadOnly(String path) throws IOException { - var file = getRealPath(path); - while (true) { - if (file.exists()) return !file.canWrite(); - if (file.equals(rootPath)) return false; - file = file.getParentFile(); + public void list(String path, List contents) throws FileOperationException { + if (path.isEmpty() && !created()) return; + + try (var stream = Files.newDirectoryStream(resolvePath(path))) { + stream.forEach(x -> contents.add(x.getFileName().toString())); + } catch (IOException e) { + throw remapException(path, e); } } @Override - public void list(String path, List contents) throws IOException { - if (!created()) { - if (!path.isEmpty()) throw new FileOperationException(path, "Not a directory"); - return; - } - - var file = getRealPath(path); - if (!file.exists() || !file.isDirectory()) throw new FileOperationException(path, "Not a directory"); - - var paths = file.list(); - for (var subPath : paths) { - if (new File(file, subPath).exists()) contents.add(subPath); - } + public long getSize(String path) throws FileOperationException { + var attributes = getAttributes(path); + return attributes.isDirectory() ? 0 : attributes.size(); } @Override - public long getSize(String path) throws IOException { - if (!created()) { - if (path.isEmpty()) return 0; - } else { - var file = getRealPath(path); - if (file.exists()) return file.isDirectory() ? 0 : file.length(); - } - - throw new FileOperationException(path, "No such file"); - } - - @Override - public SeekableByteChannel openForRead(String path) throws IOException { - if (created()) { - var file = getRealPath(path); - if (file.exists() && !file.isDirectory()) return Files.newByteChannel(file.toPath(), READ_OPTIONS); - } - - throw new FileOperationException(path, "No such file"); - } - - @Override - public BasicFileAttributes getAttributes(String path) throws IOException { - if (created()) { - var file = getRealPath(path); - if (file.exists()) return Files.readAttributes(file.toPath(), BasicFileAttributes.class); - } - - throw new FileOperationException(path, "No such file"); - } - - // IWritableMount implementation - - @Override - public void makeDirectory(String path) throws IOException { - create(); - var file = getRealPath(path); - if (file.exists()) { - if (!file.isDirectory()) throw new FileOperationException(path, "File exists"); - return; - } - - var dirsToCreate = 1; - var parent = file.getParentFile(); - while (!parent.exists()) { - ++dirsToCreate; - parent = parent.getParentFile(); - } - - if (getRemainingSpace() < dirsToCreate * MINIMUM_FILE_SIZE) { - throw new FileOperationException(path, "Out of space"); - } - - if (file.mkdirs()) { - usedSpace += dirsToCreate * MINIMUM_FILE_SIZE; - } else { - throw new FileOperationException(path, "Access denied"); - } - } - - @Override - public void delete(String path) throws IOException { - if (path.isEmpty()) throw new FileOperationException(path, "Access denied"); - - if (created()) { - var file = getRealPath(path); - if (file.exists()) deleteRecursively(file); - } - } - - private void deleteRecursively(File file) throws IOException { - // Empty directories first - if (file.isDirectory()) { - var children = file.list(); - for (var aChildren : children) { - deleteRecursively(new File(file, aChildren)); - } - } - - // Then delete - var fileSize = file.isDirectory() ? 0 : file.length(); - var success = file.delete(); - if (success) { - usedSpace -= Math.max(MINIMUM_FILE_SIZE, fileSize); - } else { - throw new IOException("Access denied"); - } - } - - @Override - public void rename(String source, String dest) throws IOException { - var sourceFile = getRealPath(source); - var destFile = getRealPath(dest); - if (!sourceFile.exists()) throw new FileOperationException(source, "No such file"); - if (destFile.exists()) throw new FileOperationException(dest, "File exists"); - - var sourcePath = sourceFile.toPath(); - var destPath = destFile.toPath(); - if (destPath.startsWith(sourcePath)) { - throw new FileOperationException(source, "Cannot move a directory inside itself"); - } - - Files.move(sourcePath, destPath); - } - - @Override - public WritableByteChannel openForWrite(String path) throws IOException { - create(); - var file = getRealPath(path); - if (file.exists() && file.isDirectory()) throw new FileOperationException(path, "Cannot write to directory"); - - if (file.exists()) { - usedSpace -= Math.max(file.length(), MINIMUM_FILE_SIZE); - } else if (getRemainingSpace() < MINIMUM_FILE_SIZE) { - throw new FileOperationException(path, "Out of space"); - } - usedSpace += MINIMUM_FILE_SIZE; - - return new SeekableCountingChannel(Files.newByteChannel(file.toPath(), WRITE_OPTIONS), MINIMUM_FILE_SIZE); - } - - @Override - public WritableByteChannel openForAppend(String path) throws IOException { - if (!created()) { - throw new FileOperationException(path, "No such file"); - } - - var file = getRealPath(path); - if (!file.exists()) throw new FileOperationException(path, "No such file"); - if (file.isDirectory()) throw new FileOperationException(path, "Cannot write to directory"); - - // Allowing seeking when appending is not recommended, so we use a separate channel. - return new WritableCountingChannel( - Files.newByteChannel(file.toPath(), APPEND_OPTIONS), - Math.max(MINIMUM_FILE_SIZE - file.length(), 0) - ); - } - - @Override - public long getRemainingSpace() { - return Math.max(capacity - usedSpace, 0); - } - - @Override - public long getCapacity() { - return capacity - MINIMUM_FILE_SIZE; - } - - private File getRealPath(String path) { - return new File(rootPath, path); - } - - private boolean created() { - return rootPath.exists(); - } - - private void create() throws IOException { - if (!rootPath.exists()) { - var success = rootPath.mkdirs(); - if (!success) { - throw new IOException("Access denied"); - } - } - } - - private static class Visitor extends SimpleFileVisitor { - long size; - - @Override - public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) { - size += MINIMUM_FILE_SIZE; - return FileVisitResult.CONTINUE; - } - - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { - size += Math.max(attrs.size(), MINIMUM_FILE_SIZE); - return FileVisitResult.CONTINUE; - } - - @Override - public FileVisitResult visitFileFailed(Path file, IOException exc) { - LOG.error("Error computing file size for {}", file, exc); - return FileVisitResult.CONTINUE; - } - } - - private static long measureUsedSpace(File file) { - if (!file.exists()) return 0; + public BasicFileAttributes getAttributes(String path) throws FileOperationException { + if (path.isEmpty() && !created()) return new FileAttributes(true, 0); try { - var visitor = new Visitor(); - Files.walkFileTree(file.toPath(), visitor); - return visitor.size; + return Files.readAttributes(resolvePath(path), BasicFileAttributes.class); } catch (IOException e) { - LOG.error("Error computing file size for {}", file, e); - return 0; + throw remapException(path, e); } } + + @Override + public SeekableByteChannel openForRead(String path) throws FileOperationException { + var file = resolvePath(path); + if (!Files.isRegularFile(file)) throw new FileOperationException(path, "No such file"); + + try { + return Files.newByteChannel(file, READ_OPTIONS); + } catch (IOException e) { + throw remapException(path, e); + } + } + + /** + * Remap a {@link IOException} to a friendlier {@link FileOperationException}. + * + * @param fallbackPath The path currently being operated on. This is used in errors when we cannot determine a more accurate path. + * @param exn The exception that occurred. + * @return The wrapped exception. + */ + 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()); + } + + /** + * Remap a {@link FileSystemException} to a friendlier {@link FileOperationException}, attempting to remap the path + * provided. + * + * @param fallbackPath The path currently being operated on. This is used in errors when we cannot determine a more accurate path. + * @param exn The exception that occurred. + * @return The wrapped exception. + */ + protected FileOperationException remapException(String fallbackPath, FileSystemException exn) { + var reason = getReason(exn); + + var failedFile = exn.getFile(); + if (failedFile == null) return new FileOperationException(fallbackPath, reason); + + var failedPath = Path.of(failedFile); + return failedPath.startsWith(root) + ? new FileOperationException(root.relativize(failedPath).toString(), 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 c2c6ceb6a..4fa4b9c7c 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 @@ -19,7 +19,6 @@ import java.lang.ref.WeakReference; import java.nio.channels.Channel; import java.nio.channels.SeekableByteChannel; -import java.nio.channels.WritableByteChannel; import java.nio.file.AccessDeniedException; import java.nio.file.attribute.BasicFileAttributes; import java.util.*; @@ -354,7 +353,7 @@ public synchronized FileSystemWrapper openForRead(Strin return openFile(mount, channel, open.apply(channel)); } - public synchronized FileSystemWrapper openForWrite(String path, boolean append, Function open) throws FileSystemException { + public synchronized FileSystemWrapper openForWrite(String path, boolean append, Function open) throws FileSystemException { cleanup(); path = sanitizePath(path); 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 ed4172216..64ddb3b5c 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 @@ -5,6 +5,7 @@ */ package dan200.computercraft.core.filesystem; +import dan200.computercraft.api.filesystem.FileOperationException; import dan200.computercraft.core.util.Nullability; import javax.annotation.Nullable; @@ -42,7 +43,7 @@ public JarMount(File jarFile, String subPath) throws IOException { } // Read in all the entries - root = new FileEntry(); + root = new FileEntry(""); var zipEntries = zip.entries(); while (zipEntries.hasMoreElements()) { var entry = zipEntries.nextElement(); @@ -68,7 +69,7 @@ private void create(ZipEntry entry, String localPath) { var nextEntry = lastEntry.children.get(part); if (nextEntry == null || !nextEntry.isDirectory()) { - lastEntry.children.put(part, nextEntry = new FileEntry()); + lastEntry.children.put(part, nextEntry = new FileEntry(localPath.substring(0, nextIndex))); } lastEntry = nextEntry; @@ -79,26 +80,26 @@ private void create(ZipEntry entry, String localPath) { } @Override - protected long getSize(FileEntry file) throws IOException { - if (file.zipEntry == null) throw new FileNotFoundException(NO_SUCH_FILE); + protected long getSize(FileEntry file) throws FileOperationException { + if (file.zipEntry == null) throw new FileOperationException(file.path, NO_SUCH_FILE); return file.zipEntry.getSize(); } @Override - protected byte[] getContents(FileEntry file) throws IOException { - if (file.zipEntry == null) throw new FileNotFoundException(NO_SUCH_FILE); + protected byte[] getContents(FileEntry file) throws FileOperationException { + if (file.zipEntry == null) throw new FileOperationException(file.path, NO_SUCH_FILE); try (var stream = zip.getInputStream(file.zipEntry)) { return stream.readAllBytes(); } catch (IOException e) { // Mask other IO exceptions as a non-existent file. - throw new FileNotFoundException(NO_SUCH_FILE); + throw new FileOperationException(file.path, NO_SUCH_FILE); } } @Override - public BasicFileAttributes getAttributes(FileEntry file) throws IOException { - if (file.zipEntry == null) throw new FileNotFoundException(NO_SUCH_FILE); + public BasicFileAttributes getAttributes(FileEntry file) throws FileOperationException { + if (file.zipEntry == null) throw new FileOperationException(file.path, NO_SUCH_FILE); return new ZipEntryAttributes(file.zipEntry); } @@ -111,6 +112,10 @@ protected static class FileEntry extends ArchiveMount.FileEntry { @Nullable ZipEntry zipEntry; + protected FileEntry(String path) { + super(path); + } + void setup(ZipEntry entry) { zipEntry = entry; size = entry.getSize(); 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 d37c39de3..f22d87990 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 @@ -12,10 +12,6 @@ import javax.annotation.Nullable; import java.io.IOException; import java.nio.channels.SeekableByteChannel; -import java.nio.channels.WritableByteChannel; -import java.nio.file.AccessDeniedException; -import java.nio.file.FileAlreadyExistsException; -import java.nio.file.NoSuchFileException; import java.nio.file.attribute.BasicFileAttributes; import java.util.List; import java.util.OptionalLong; @@ -158,8 +154,6 @@ public void delete(String path) throws FileSystemException { if (mount.exists(path)) { writableMount.delete(path); } - } catch (AccessDeniedException e) { - throw new FileSystemException("Access denied"); } catch (IOException e) { throw localExceptionOf(path, e); } @@ -177,14 +171,12 @@ public void rename(String source, String dest) throws FileSystemException { } writableMount.rename(source, dest); - } catch (AccessDeniedException e) { - throw new FileSystemException("Access denied"); } catch (IOException e) { throw localExceptionOf(source, e); } } - public WritableByteChannel openForWrite(String path) throws FileSystemException { + public SeekableByteChannel openForWrite(String path) throws FileSystemException { if (writableMount == null) throw exceptionOf(path, "Access denied"); path = toLocal(path); @@ -200,14 +192,12 @@ public WritableByteChannel openForWrite(String path) throws FileSystemException } return writableMount.openForWrite(path); } - } catch (AccessDeniedException e) { - throw new FileSystemException("Access denied"); } catch (IOException e) { throw localExceptionOf(path, e); } } - public WritableByteChannel openForAppend(String path) throws FileSystemException { + public SeekableByteChannel openForAppend(String path) throws FileSystemException { if (writableMount == null) throw exceptionOf(path, "Access denied"); path = toLocal(path); @@ -225,8 +215,6 @@ public WritableByteChannel openForAppend(String path) throws FileSystemException } else { return writableMount.openForAppend(path); } - } catch (AccessDeniedException e) { - throw new FileSystemException("Access denied"); } catch (IOException e) { throw localExceptionOf(path, e); } @@ -244,7 +232,8 @@ private FileSystemException localExceptionOf(@Nullable String localPath, IOExcep if (e instanceof java.nio.file.FileSystemException ex) { // 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 = getReason(ex); + var message = ex.getReason(); + if (message == null) message = "Failed"; return localPath == null ? new FileSystemException(message) : localExceptionOf(localPath, message); } @@ -259,15 +248,4 @@ private FileSystemException localExceptionOf(String path, String message) { private static FileSystemException exceptionOf(String path, String message) { return new FileSystemException("/" + path + ": " + message); } - - private static String getReason(java.nio.file.FileSystemException e) { - var reason = e.getReason(); - if (reason != null) return reason.trim(); - - if (e instanceof FileAlreadyExistsException) return "File exists"; - if (e instanceof NoSuchFileException) return "No such file"; - if (e instanceof AccessDeniedException) return "Access denied"; - - return "Operation failed"; - } } 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 new file mode 100644 index 000000000..07f51e2ea --- /dev/null +++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/WritableFileMount.java @@ -0,0 +1,322 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ +package dan200.computercraft.core.filesystem; + +import com.google.common.collect.Sets; +import dan200.computercraft.api.filesystem.FileOperationException; +import dan200.computercraft.api.filesystem.WritableMount; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nullable; +import java.io.File; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.ClosedChannelException; +import java.nio.channels.NonReadableChannelException; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.*; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.Set; + +/** + * 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); + + private static final long MINIMUM_FILE_SIZE = 500; + private static final Set WRITE_OPTIONS = Sets.newHashSet(StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); + private static final Set APPEND_OPTIONS = Sets.newHashSet(StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.APPEND); + + protected final File rootFile; + private final long capacity; + private long usedSpace; + + public WritableFileMount(File rootFile, long capacity) { + super(rootFile.toPath()); + this.rootFile = rootFile; + this.capacity = capacity + MINIMUM_FILE_SIZE; + usedSpace = created() ? measureUsedSpace(root) : MINIMUM_FILE_SIZE; + } + + protected File resolveFile(String path) { + return new File(rootFile, path); + } + + private void create() throws FileOperationException { + try { + Files.createDirectories(root); + } catch (IOException e) { + throw new FileOperationException("Access denied"); + } + } + + @Override + public long getRemainingSpace() { + return Math.max(capacity - usedSpace, 0); + } + + @Override + public long getCapacity() { + return capacity - MINIMUM_FILE_SIZE; + } + + @Override + public boolean isReadOnly(String path) { + var file = resolveFile(path); + while (true) { + if (file.exists()) return !file.canWrite(); + if (file.equals(rootFile)) return false; + file = file.getParentFile(); + } + } + + @Override + public void makeDirectory(String path) throws IOException { + create(); + var file = resolveFile(path); + if (file.exists()) { + if (!file.isDirectory()) throw new FileOperationException(path, "File exists"); + return; + } + + var dirsToCreate = 1; + var parent = file.getParentFile(); + while (!parent.exists()) { + ++dirsToCreate; + parent = parent.getParentFile(); + } + + if (getRemainingSpace() < dirsToCreate * MINIMUM_FILE_SIZE) { + throw new FileOperationException(path, "Out of space"); + } + + if (file.mkdirs()) { + usedSpace += dirsToCreate * MINIMUM_FILE_SIZE; + } else { + throw new FileOperationException(path, "Access denied"); + } + } + + @Override + public void delete(String path) throws IOException { + if (path.isEmpty()) throw new FileOperationException(path, "Access denied"); + + if (created()) { + var file = resolveFile(path); + if (file.exists()) deleteRecursively(file); + } + } + + private void deleteRecursively(File file) throws IOException { + // Empty directories first + if (file.isDirectory()) { + var children = file.list(); + for (var aChildren : children) { + deleteRecursively(new File(file, aChildren)); + } + } + + // Then delete + var fileSize = file.isDirectory() ? 0 : file.length(); + var success = file.delete(); + if (success) { + usedSpace -= Math.max(MINIMUM_FILE_SIZE, fileSize); + } else { + throw new IOException("Access denied"); + } + } + + @Override + 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 (destFile.startsWith(sourceFile)) { + throw new FileOperationException(source, "Cannot move a directory inside itself"); + } + + try { + Files.move(sourceFile, destFile); + } catch (IOException e) { + throw remapException(source, e); + } + } + + private @Nullable BasicFileAttributes tryGetAttributes(String path, Path resolved) throws FileOperationException { + try { + return Files.readAttributes(resolved, BasicFileAttributes.class); + } catch (NoSuchFileException ignored) { + return null; + } catch (IOException e) { + throw remapException(path, e); + } + } + + @Override + public SeekableByteChannel openForWrite(String path) throws FileOperationException { + create(); + + var file = resolvePath(path); + var attributes = tryGetAttributes(path, file); + if (attributes == null) { + if (getRemainingSpace() < MINIMUM_FILE_SIZE) throw new FileOperationException(path, "Out of space"); + } else if (attributes.isDirectory()) { + throw new FileOperationException(path, "Cannot write to directory"); + } else { + usedSpace -= Math.max(attributes.size(), MINIMUM_FILE_SIZE); + } + + usedSpace += MINIMUM_FILE_SIZE; + + try { + return new CountingChannel(Files.newByteChannel(file, WRITE_OPTIONS), MINIMUM_FILE_SIZE, true); + } catch (IOException e) { + throw remapException(path, e); + } + } + + @Override + public SeekableByteChannel openForAppend(String path) throws IOException { + create(); + + var file = resolvePath(path); + var attributes = tryGetAttributes(path, file); + if (attributes == null) { + if (getRemainingSpace() < MINIMUM_FILE_SIZE) throw new FileOperationException(path, "Out of space"); + } else if (attributes.isDirectory()) { + throw new FileOperationException(path, "Cannot write to directory"); + } + + // Allowing seeking when appending is not recommended, so we use a separate channel. + try { + return new CountingChannel( + Files.newByteChannel(file, APPEND_OPTIONS), + Math.max(MINIMUM_FILE_SIZE - (attributes == null ? 0 : attributes.size()), 0), + false + ); + } catch (IOException e) { + throw remapException(path, e); + } + } + + private class CountingChannel implements SeekableByteChannel { + private final SeekableByteChannel channel; + private long ignoredBytesLeft; + private final boolean canSeek; + + CountingChannel(SeekableByteChannel channel, long bytesToIgnore, boolean canSeek) { + this.channel = channel; + ignoredBytesLeft = bytesToIgnore; + this.canSeek = canSeek; + } + + @Override + public int write(ByteBuffer b) throws IOException { + count(b.remaining()); + return channel.write(b); + } + + void count(long n) throws IOException { + ignoredBytesLeft -= n; + if (ignoredBytesLeft < 0) { + var newBytes = -ignoredBytesLeft; + ignoredBytesLeft = 0; + + var bytesLeft = capacity - usedSpace; + if (newBytes > bytesLeft) throw new IOException("Out of space"); + usedSpace += newBytes; + } + } + + @Override + public boolean isOpen() { + return channel.isOpen(); + } + + @Override + public void close() throws IOException { + channel.close(); + } + + @Override + public SeekableByteChannel position(long newPosition) throws IOException { + if (!isOpen()) throw new ClosedChannelException(); + if (!canSeek) throw new UnsupportedOperationException("File does not support seeking"); + if (newPosition < 0) { + throw new IllegalArgumentException("Cannot seek before the beginning of the stream"); + } + + var delta = newPosition - channel.position(); + if (delta < 0) { + ignoredBytesLeft -= delta; + } else { + count(delta); + } + + return channel.position(newPosition); + } + + @Override + public SeekableByteChannel truncate(long size) throws IOException { + throw new UnsupportedOperationException("File cannot be truncated"); + } + + @Override + public int read(ByteBuffer dst) throws ClosedChannelException { + if (!channel.isOpen()) throw new ClosedChannelException(); + throw new NonReadableChannelException(); + } + + @Override + public long position() throws IOException { + return channel.position(); + } + + @Override + public long size() throws IOException { + return channel.size(); + } + } + + private static long measureUsedSpace(Path path) { + if (!Files.exists(path)) return 0; + + class CountingVisitor extends SimpleFileVisitor { + long size; + + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) { + size += MINIMUM_FILE_SIZE; + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { + size += Math.max(attrs.size(), MINIMUM_FILE_SIZE); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFileFailed(Path file, IOException exc) { + LOG.error("Error computing file size for {}", file, exc); + return FileVisitResult.CONTINUE; + } + } + + try { + var visitor = new CountingVisitor(); + Files.walkFileTree(path, visitor); + return visitor.size; + } catch (IOException e) { + LOG.error("Error computing file size for {}", path, e); + return 0; + } + } +} diff --git a/projects/core/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java b/projects/core/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java index 88101ec25..95d38e535 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java +++ b/projects/core/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java @@ -13,8 +13,8 @@ import dan200.computercraft.core.computer.Computer; import dan200.computercraft.core.computer.ComputerSide; import dan200.computercraft.core.computer.mainthread.NoWorkMainThreadScheduler; -import dan200.computercraft.core.filesystem.FileMount; import dan200.computercraft.core.filesystem.FileSystemException; +import dan200.computercraft.core.filesystem.WritableFileMount; import dan200.computercraft.core.terminal.Terminal; import dan200.computercraft.test.core.computer.BasicEnvironment; import org.junit.jupiter.api.*; @@ -85,7 +85,7 @@ public void before() throws IOException { if (Files.deleteIfExists(REPORT_PATH)) LOG.info("Deleted previous coverage report."); var term = new Terminal(80, 100, true); - WritableMount mount = new FileMount(TestFiles.get("mount").toFile(), 10_000_000); + WritableMount mount = new WritableFileMount(TestFiles.get("mount").toFile(), 10_000_000); // Remove any existing files List children = new ArrayList<>(); diff --git a/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java b/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java index 401b0c9c7..c2134698e 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java +++ b/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java @@ -16,6 +16,7 @@ import dan200.computercraft.core.terminal.Terminal; import dan200.computercraft.test.core.computer.BasicEnvironment; import dan200.computercraft.test.core.filesystem.MemoryMount; +import dan200.computercraft.test.core.filesystem.ReadOnlyWritableMount; import org.junit.jupiter.api.Assertions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,7 +38,7 @@ public static void run(String program, Consumer setup, int maxTimes) { .addFile("test.lua", program) .addFile("startup.lua", "assertion.assert(pcall(loadfile('test.lua', nil, _ENV))) os.shutdown()"); - run(mount, setup, maxTimes); + run(new ReadOnlyWritableMount(mount), setup, maxTimes); } public static void run(String program, int maxTimes) { diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileMountTest.java b/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileMountTest.java index 9b3a1581f..588bca130 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileMountTest.java +++ b/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileMountTest.java @@ -7,9 +7,10 @@ import com.google.common.io.MoreFiles; import com.google.common.io.RecursiveDeleteOption; -import dan200.computercraft.api.filesystem.WritableMount; +import dan200.computercraft.api.filesystem.Mount; +import dan200.computercraft.test.core.filesystem.MountContract; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; import java.io.IOException; import java.nio.file.Files; @@ -17,50 +18,60 @@ import java.util.ArrayList; import java.util.List; -public class FileMountTest implements WritableMountContract { +import static org.junit.jupiter.api.Assertions.*; + +public class FileMountTest implements MountContract { private final List cleanup = new ArrayList<>(); + @Override + public Mount createSkeleton() throws IOException { + var path = Files.createTempDirectory("cctweaked-test"); + cleanup.add(path); + + Files.createDirectories(path.resolve("dir")); + try (var writer = Files.newBufferedWriter(path.resolve("dir/file.lua"))) { + writer.write("print('testing')"); + } + Files.setLastModifiedTime(path.resolve("dir/file.lua"), MODIFY_TIME); + Files.newBufferedWriter(path.resolve("f.lua")).close(); + + return new FileMount(path); + } + + private Mount createEmpty() throws IOException { + var path = Files.createTempDirectory("cctweaked-test"); + cleanup.add(path); + return new FileMount(path.resolve("empty")); + } + @AfterEach public void cleanup() throws IOException { for (var mount : cleanup) MoreFiles.deleteRecursively(mount, RecursiveDeleteOption.ALLOW_INSECURE); } - @Override - public MountAccess createMount(long capacity) throws IOException { - var path = Files.createTempDirectory("cctweaked-test"); - cleanup.add(path); - return new MountAccessImpl(path.resolve("mount"), capacity); + @Test + public void testRootExistsWhenEmpty() throws IOException { + var mount = createEmpty(); + assertTrue(mount.exists(""), "Root always exists"); + assertTrue(mount.isDirectory(""), "Root always is a directory"); } - private static final class MountAccessImpl implements MountAccess { - private final Path root; - private final long capacity; - private final WritableMount mount; + @Test + public void testListWhenEmpty() throws IOException { + var mount = createEmpty(); - private MountAccessImpl(Path root, long capacity) { - this.root = root; - this.capacity = capacity; - mount = new FileMount(root.toFile(), capacity); - } + List list = new ArrayList<>(); + mount.list("", list); + assertEquals(List.of(), list, "Root has no children"); - @Override - public WritableMount mount() { - return mount; - } + assertThrows(IOException.class, () -> mount.list("elsewhere", list)); + } - @Override - public void makeReadOnly(String path) { - Assumptions.assumeTrue(root.resolve(path).toFile().setReadOnly(), "Change file to read-only"); - } + @Test + public void testAttributesWhenEmpty() throws IOException { + var mount = createEmpty(); - @Override - public void ensuresExist() throws IOException { - Files.createDirectories(root); - } - - @Override - public long computeRemainingSpace() { - return new FileMount(root.toFile(), capacity).getRemainingSpace(); - } + assertEquals(0, mount.getSize("")); + assertNotNull(mount.getAttributes("")); } } diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java b/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java index ae96436e3..fc4825a1d 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java +++ b/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java @@ -27,7 +27,7 @@ public class FileSystemTest { private static final long CAPACITY = 1000000; private static FileSystem mkFs() throws FileSystemException { - WritableMount writableMount = new FileMount(ROOT, CAPACITY); + WritableMount writableMount = new WritableFileMount(ROOT, CAPACITY); return new FileSystem("hdd", writableMount); } @@ -65,7 +65,7 @@ public void testWriteTruncates() throws FileSystemException, LuaException, IOExc @Test public void testUnmountCloses() throws FileSystemException { var fs = mkFs(); - WritableMount mount = new FileMount(new File(ROOT, "child"), CAPACITY); + WritableMount mount = new WritableFileMount(new File(ROOT, "child"), CAPACITY); fs.mountWritable("disk", "disk", mount); var writer = fs.openForWrite("disk/out.txt", false, EncodedWritableHandle::openUtf8); diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/JarMountTest.java b/projects/core/src/test/java/dan200/computercraft/core/filesystem/JarMountTest.java index d2917c7ea..b0e7c43e4 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/filesystem/JarMountTest.java +++ b/projects/core/src/test/java/dan200/computercraft/core/filesystem/JarMountTest.java @@ -8,6 +8,9 @@ import com.google.common.io.ByteStreams; import dan200.computercraft.api.filesystem.Mount; import dan200.computercraft.core.TestFiles; +import dan200.computercraft.test.core.CloseScope; +import dan200.computercraft.test.core.filesystem.MountContract; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -16,50 +19,67 @@ import java.io.IOException; import java.nio.channels.Channels; import java.nio.charset.StandardCharsets; -import java.nio.file.attribute.FileTime; -import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; import static org.junit.jupiter.api.Assertions.*; -public class JarMountTest { +public class JarMountTest implements MountContract { private static final File ZIP_FILE = TestFiles.get("jar-mount.zip").toFile(); - private static final FileTime MODIFY_TIME = FileTime.from(Instant.EPOCH.plus(2, ChronoUnit.DAYS)); + private final CloseScope toClose = new CloseScope(); @BeforeAll public static void before() throws IOException { ZIP_FILE.getParentFile().mkdirs(); try (var stream = new ZipOutputStream(new FileOutputStream(ZIP_FILE))) { - stream.putNextEntry(new ZipEntry("dir/")); + stream.putNextEntry(new ZipEntry("root/")); stream.closeEntry(); - stream.putNextEntry(new ZipEntry("dir/file.lua").setLastModifiedTime(MODIFY_TIME)); + stream.putNextEntry(new ZipEntry("root/dir/")); + stream.closeEntry(); + + stream.putNextEntry(new ZipEntry("root/dir/file.lua").setLastModifiedTime(MODIFY_TIME)); stream.write("print('testing')".getBytes(StandardCharsets.UTF_8)); stream.closeEntry(); + + stream.putNextEntry(new ZipEntry("root/f.lua")); + stream.closeEntry(); } } + @AfterEach + public void after() throws Exception { + toClose.close(); + } + + @Override + public Mount createSkeleton() throws IOException { + return toClose.add(new JarMount(ZIP_FILE, "root")); + } + + private Mount createMount(String path) throws IOException { + return toClose.add(new JarMount(ZIP_FILE, "root/" + path)); + } + @Test public void mountsDir() throws IOException { - Mount mount = new JarMount(ZIP_FILE, "dir"); + var mount = createMount("dir"); assertTrue(mount.isDirectory(""), "Root should be directory"); assertTrue(mount.exists("file.lua"), "File should exist"); } @Test public void mountsFile() throws IOException { - Mount mount = new JarMount(ZIP_FILE, "dir/file.lua"); + var mount = createMount("dir/file.lua"); assertTrue(mount.exists(""), "Root should exist"); assertFalse(mount.isDirectory(""), "Root should be a file"); } @Test public void opensFileFromFile() throws IOException { - Mount mount = new JarMount(ZIP_FILE, "dir/file.lua"); + var mount = createMount("dir/file.lua"); byte[] contents; try (var stream = mount.openForRead("")) { contents = ByteStreams.toByteArray(Channels.newInputStream(stream)); @@ -70,7 +90,7 @@ public void opensFileFromFile() throws IOException { @Test public void opensFileFromDir() throws IOException { - Mount mount = new JarMount(ZIP_FILE, "dir"); + var mount = createMount("dir"); byte[] contents; try (var stream = mount.openForRead("file.lua")) { contents = ByteStreams.toByteArray(Channels.newInputStream(stream)); @@ -78,19 +98,4 @@ public void opensFileFromDir() throws IOException { assertEquals(new String(contents, StandardCharsets.UTF_8), "print('testing')"); } - - @Test - public void fileAttributes() throws IOException { - var attributes = new JarMount(ZIP_FILE, "dir").getAttributes("file.lua"); - assertFalse(attributes.isDirectory()); - assertEquals("print('testing')".length(), attributes.size()); - assertEquals(MODIFY_TIME, attributes.lastModifiedTime()); - } - - @Test - public void directoryAttributes() throws IOException { - var attributes = new JarMount(ZIP_FILE, "dir").getAttributes(""); - assertTrue(attributes.isDirectory()); - assertEquals(0, attributes.size()); - } } diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableFileMountTest.java b/projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableFileMountTest.java new file mode 100644 index 000000000..e97c8a691 --- /dev/null +++ b/projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableFileMountTest.java @@ -0,0 +1,67 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ +package dan200.computercraft.core.filesystem; + +import com.google.common.io.MoreFiles; +import com.google.common.io.RecursiveDeleteOption; +import dan200.computercraft.api.filesystem.WritableMount; +import dan200.computercraft.test.core.filesystem.WritableMountContract; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assumptions; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +public class WritableFileMountTest implements WritableMountContract { + private final List cleanup = new ArrayList<>(); + + @Override + public MountAccess createMount(long capacity) throws IOException { + var path = Files.createTempDirectory("cctweaked-test"); + cleanup.add(path); + return new MountAccessImpl(path.resolve("mount"), capacity); + } + + @AfterEach + public void cleanup() throws IOException { + for (var mount : cleanup) MoreFiles.deleteRecursively(mount, RecursiveDeleteOption.ALLOW_INSECURE); + } + + private static final class MountAccessImpl implements MountAccess { + private final Path root; + private final long capacity; + private final WritableMount mount; + + private MountAccessImpl(Path root, long capacity) { + this.root = root; + this.capacity = capacity; + mount = new WritableFileMount(root.toFile(), capacity); + } + + @Override + public WritableMount mount() { + return mount; + } + + @Override + public void makeReadOnly(String path) { + Assumptions.assumeTrue(root.resolve(path).toFile().setReadOnly(), "Change file to read-only"); + } + + @Override + public void ensuresExist() throws IOException { + Files.createDirectories(root); + } + + @Override + public long computeRemainingSpace() { + return new WritableFileMount(root.toFile(), capacity).getRemainingSpace(); + } + } +} diff --git a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/CloseScope.java b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/CloseScope.java new file mode 100644 index 000000000..b387cc2c6 --- /dev/null +++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/CloseScope.java @@ -0,0 +1,61 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ +package dan200.computercraft.test.core; + +import javax.annotation.Nullable; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.Objects; + +/** + * An {@link AutoCloseable} implementation which can be used to combine other [AutoCloseable] instances. + *

+ * Values which implement {@link AutoCloseable} can be dynamically registered with [CloseScope.add]. When the scope is + * closed, each value is closed in the opposite order. + *

+ * This is largely intended for cases where it's not appropriate to nest try-with-resources blocks, for instance when + * nested would be too deep or when objects are dynamically created. + */ +public class CloseScope implements AutoCloseable { + private final Deque toClose = new ArrayDeque<>(); + + /** + * Add a value to be closed when this scope exists. + * + * @param value The value to be closed. + * @param The type of the provided value. + * @return The provided value. + */ + public T add(T value) { + Objects.requireNonNull(value, "Value cannot be null"); + toClose.add(value); + return value; + } + + @Override + public void close() throws Exception { + close(null); + } + + public void close(@Nullable Exception baseException) throws Exception { + while (true) { + var close = toClose.pollLast(); + if (close == null) break; + + try { + close.close(); + } catch (Exception e) { + if (baseException == null) { + baseException = e; + } else { + baseException.addSuppressed(e); + } + } + } + + if (baseException != null) throw baseException; + } +} diff --git a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/computer/BasicEnvironment.java b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/computer/BasicEnvironment.java index 61999477e..7a0da0147 100644 --- a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/computer/BasicEnvironment.java +++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/computer/BasicEnvironment.java @@ -15,6 +15,7 @@ import dan200.computercraft.core.metrics.Metric; import dan200.computercraft.core.metrics.MetricsObserver; import dan200.computercraft.test.core.filesystem.MemoryMount; +import dan200.computercraft.test.core.filesystem.ReadOnlyWritableMount; import java.io.File; import java.io.IOException; @@ -32,7 +33,7 @@ public class BasicEnvironment implements ComputerEnvironment, GlobalEnvironment, private final WritableMount mount; public BasicEnvironment() { - this(new MemoryMount()); + this(new ReadOnlyWritableMount(new MemoryMount())); } public BasicEnvironment(WritableMount mount) { @@ -100,7 +101,7 @@ public static Mount createMount(Class klass, String path, String fallback) { if (!wholeFile.exists()) throw new IllegalStateException("Cannot find ROM mount at " + file); - return new FileMount(wholeFile, 0); + return new FileMount(wholeFile.toPath()); } } diff --git a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MemoryMount.java b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MemoryMount.java index 3484adbdc..67a70a485 100644 --- a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MemoryMount.java +++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MemoryMount.java @@ -6,22 +6,17 @@ package dan200.computercraft.test.core.filesystem; import dan200.computercraft.api.filesystem.FileOperationException; -import dan200.computercraft.api.filesystem.WritableMount; +import dan200.computercraft.api.filesystem.Mount; import dan200.computercraft.core.apis.handles.ArrayByteChannel; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; -import java.nio.channels.Channels; import java.nio.channels.SeekableByteChannel; -import java.nio.channels.WritableByteChannel; import java.nio.charset.StandardCharsets; import java.util.*; /** - * In-memory file mounts. + * A read-only mount {@link Mount} which provides a list of in-memory set of files. */ -public class MemoryMount implements WritableMount { +public class MemoryMount implements Mount { private final Map files = new HashMap<>(); private final Set directories = new HashSet<>(); @@ -29,69 +24,6 @@ public MemoryMount() { directories.add(""); } - @Override - public void makeDirectory(String path) { - var file = new File(path); - while (file != null) { - directories.add(file.getPath()); - file = file.getParentFile(); - } - } - - @Override - public void delete(String path) { - if (files.containsKey(path)) { - files.remove(path); - } else { - directories.remove(path); - for (var file : files.keySet().toArray(new String[0])) { - if (file.startsWith(path)) { - files.remove(file); - } - } - - var parent = new File(path).getParentFile(); - if (parent != null) delete(parent.getPath()); - } - } - - @Override - public void rename(String source, String dest) throws IOException { - throw new IOException("Not supported"); - } - - @Override - public WritableByteChannel openForWrite(final String path) { - return Channels.newChannel(new ByteArrayOutputStream() { - @Override - public void close() throws IOException { - super.close(); - files.put(path, toByteArray()); - } - }); - } - - @Override - public WritableByteChannel openForAppend(final String path) throws IOException { - var stream = new ByteArrayOutputStream() { - @Override - public void close() throws IOException { - super.close(); - files.put(path, toByteArray()); - } - }; - - var current = files.get(path); - if (current != null) stream.write(current); - - return Channels.newChannel(stream); - } - - @Override - public long getRemainingSpace() { - return 1000000L; - } - @Override public boolean exists(String path) { return files.containsKey(path) || directories.contains(path); @@ -114,11 +46,6 @@ public long getSize(String path) { throw new RuntimeException("Not implemented"); } - @Override - public long getCapacity() { - return Long.MAX_VALUE; - } - @Override public SeekableByteChannel openForRead(String path) throws FileOperationException { var file = files.get(path); 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 new file mode 100644 index 000000000..de46d09b2 --- /dev/null +++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MountContract.java @@ -0,0 +1,128 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ +package dan200.computercraft.test.core.filesystem; + +import dan200.computercraft.api.filesystem.FileOperationException; +import dan200.computercraft.api.filesystem.Mount; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.channels.Channels; +import java.nio.charset.StandardCharsets; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * The contract that all {@link Mount} implementations must fulfill. + * + * @see WritableMountContract + */ +public interface MountContract { + FileTime EPOCH = FileTime.from(Instant.EPOCH); + FileTime MODIFY_TIME = FileTime.from(Instant.EPOCH.plus(2, ChronoUnit.DAYS)); + + /** + * Create a skeleton mount. This should contain the following files: + * + *

+ * + * @return The skeleton mount. + */ + Mount createSkeleton() throws IOException; + + /** + * Determine if this attributes provided by this mount support file times. + * + * @return Whether this mount supports {@link BasicFileAttributes#lastModifiedTime()}. + */ + default boolean hasFileTimes() { + return true; + } + + @Test + default void testIsDirectory() throws IOException { + var mount = createSkeleton(); + + assertTrue(mount.isDirectory(""), "Root should be directory"); + assertTrue(mount.isDirectory("dir"), "dir/ should be directory"); + assertFalse(mount.isDirectory("dir/file.lua"), "dir/file.lua should not be a directory"); + assertFalse(mount.isDirectory("doesnt/exist"), "doesnt/exist should not be a directory"); + } + + @Test + default void testExists() throws IOException { + var mount = createSkeleton(); + + assertTrue(mount.exists(""), "Root should exist"); + assertTrue(mount.exists("dir"), "dir/ should exist"); + assertFalse(mount.isDirectory("doesnt/exist"), "doesnt/exist should not exist"); + } + + @Test + default void testList() throws IOException { + var mount = createSkeleton(); + + List list = new ArrayList<>(); + mount.list("", list); + list.sort(Comparator.naturalOrder()); + + assertEquals(List.of("dir", "f.lua"), list); + } + + @Test + default void testOpenFile() throws IOException { + var mount = createSkeleton(); + + byte[] contents; + try (var stream = mount.openForRead("dir/file.lua")) { + contents = Channels.newInputStream(stream).readAllBytes(); + } + + assertEquals(new String(contents, StandardCharsets.UTF_8), "print('testing')"); + } + + @Test + default void testOpenFileFails() throws IOException { + var mount = createSkeleton(); + + var exn = assertThrows(FileOperationException.class, () -> mount.openForRead("doesnt/exist")); + assertEquals("doesnt/exist", exn.getFilename()); + assertEquals("No such file", exn.getMessage()); + } + + @Test + default void testSize() throws IOException { + var mount = createSkeleton(); + + assertEquals(0, mount.getSize("f.lua"), "Empty file has 0 size"); + assertEquals("print('testing')".length(), mount.getSize("dir/file.lua")); + assertEquals(0, mount.getSize("dir"), "Directory has 0 size"); + } + + @Test + default void testFileAttributes() throws IOException { + var attributes = createSkeleton().getAttributes("dir/file.lua"); + assertFalse(attributes.isDirectory()); + assertEquals("print('testing')".length(), attributes.size()); + assertEquals(hasFileTimes() ? MODIFY_TIME : EPOCH, attributes.lastModifiedTime()); + } + + @Test + default void testDirectoryAttributes() throws IOException { + var attributes = createSkeleton().getAttributes(""); + assertTrue(attributes.isDirectory()); + } +} diff --git a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/ReadOnlyWritableMount.java b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/ReadOnlyWritableMount.java new file mode 100644 index 000000000..a656136b6 --- /dev/null +++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/ReadOnlyWritableMount.java @@ -0,0 +1,92 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ +package dan200.computercraft.test.core.filesystem; + +import dan200.computercraft.api.filesystem.FileOperationException; +import dan200.computercraft.api.filesystem.Mount; +import dan200.computercraft.api.filesystem.WritableMount; + +import java.io.IOException; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.List; + +/** + * Wraps a {@link Mount} into a read-only {@link WritableMount}. + * + * @param mount The original read-only mount we're wrapping. + */ +public record ReadOnlyWritableMount(Mount mount) implements WritableMount { + @Override + public boolean exists(String path) throws IOException { + return mount.exists(path); + } + + @Override + public boolean isDirectory(String path) throws IOException { + return mount.isDirectory(path); + } + + @Override + public void list(String path, List contents) throws IOException { + mount.list(path, contents); + } + + @Override + public long getSize(String path) throws IOException { + return mount.getSize(path); + } + + @Override + public SeekableByteChannel openForRead(String path) throws IOException { + return mount.openForRead(path); + } + + @Override + public BasicFileAttributes getAttributes(String path) throws IOException { + return mount.getAttributes(path); + } + + @Override + public void makeDirectory(String path) throws IOException { + throw new FileOperationException(path, "Access denied"); + } + + @Override + public void delete(String path) throws IOException { + throw new FileOperationException(path, "Access denied"); + } + + @Override + public void rename(String source, String dest) throws IOException { + throw new FileOperationException(source, "Access denied"); + } + + @Override + public SeekableByteChannel openForWrite(String path) throws IOException { + throw new FileOperationException(path, "Access denied"); + } + + @Override + public SeekableByteChannel openForAppend(String path) throws IOException { + throw new FileOperationException(path, "Access denied"); + } + + @Override + public long getRemainingSpace() { + return Integer.MAX_VALUE; + } + + @Override + public long getCapacity() { + return Integer.MAX_VALUE; + } + + @Override + public boolean isReadOnly(String path) { + return true; + } +} diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableMountContract.java b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/WritableMountContract.java similarity index 95% rename from projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableMountContract.java rename to projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/WritableMountContract.java index fcdcb506d..dbdc308fd 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableMountContract.java +++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/WritableMountContract.java @@ -3,7 +3,7 @@ * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission. * Send enquiries to dratcliffe@gmail.com */ -package dan200.computercraft.core.filesystem; +package dan200.computercraft.test.core.filesystem; import dan200.computercraft.api.filesystem.WritableMount; import org.junit.jupiter.api.Test; @@ -14,7 +14,9 @@ import static org.junit.jupiter.api.Assertions.*; /** - * The contract that all {@link WritableMount}s must fulfill. + * The contract that all {@link WritableMount} implementations must fulfill. + * + * @see MountContract */ public interface WritableMountContract { long CAPACITY = 1_000_000; diff --git a/projects/fabric/src/main/java/dan200/computercraft/impl/ComputerCraftAPIImpl.java b/projects/fabric/src/main/java/dan200/computercraft/impl/ComputerCraftAPIImpl.java index 2daaf898a..1761533a3 100644 --- a/projects/fabric/src/main/java/dan200/computercraft/impl/ComputerCraftAPIImpl.java +++ b/projects/fabric/src/main/java/dan200/computercraft/impl/ComputerCraftAPIImpl.java @@ -8,14 +8,11 @@ import com.google.auto.service.AutoService; import dan200.computercraft.api.ComputerCraftAPI; import dan200.computercraft.api.detail.DetailRegistry; -import dan200.computercraft.api.filesystem.Mount; import dan200.computercraft.impl.detail.DetailRegistryImpl; -import dan200.computercraft.shared.computer.core.ResourceMount; import dan200.computercraft.shared.details.FluidDetails; import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant; import net.fabricmc.fabric.api.transfer.v1.storage.StorageView; import net.fabricmc.loader.api.FabricLoader; -import net.minecraft.server.MinecraftServer; import javax.annotation.Nullable; @@ -33,13 +30,6 @@ public String getInstalledVersion() { .orElse("unknown"); } - @Nullable - @Override - public Mount createResourceMount(MinecraftServer server, String domain, String subPath) { - var mount = ResourceMount.get(domain, subPath, server.getResourceManager()); - return mount.exists("") ? mount : null; - } - @Override public DetailRegistry> getFluidDetailRegistry() { return fluidDetails; diff --git a/projects/forge/src/main/java/dan200/computercraft/impl/ComputerCraftAPIImpl.java b/projects/forge/src/main/java/dan200/computercraft/impl/ComputerCraftAPIImpl.java index ccebb890e..793f3f6d5 100644 --- a/projects/forge/src/main/java/dan200/computercraft/impl/ComputerCraftAPIImpl.java +++ b/projects/forge/src/main/java/dan200/computercraft/impl/ComputerCraftAPIImpl.java @@ -8,22 +8,18 @@ import com.google.auto.service.AutoService; import dan200.computercraft.api.ComputerCraftAPI; import dan200.computercraft.api.detail.DetailRegistry; -import dan200.computercraft.api.filesystem.Mount; import dan200.computercraft.api.network.wired.WiredElement; import dan200.computercraft.api.peripheral.IPeripheralProvider; import dan200.computercraft.impl.detail.DetailRegistryImpl; -import dan200.computercraft.shared.computer.core.ResourceMount; import dan200.computercraft.shared.details.FluidData; import dan200.computercraft.shared.peripheral.generic.GenericPeripheralProvider; import net.minecraft.core.BlockPos; import net.minecraft.core.Direction; -import net.minecraft.server.MinecraftServer; import net.minecraft.world.level.BlockGetter; import net.minecraftforge.common.capabilities.Capability; import net.minecraftforge.common.util.LazyOptional; import net.minecraftforge.fluids.FluidStack; import net.minecraftforge.fml.ModList; -import net.minecraftforge.server.ServerLifecycleHooks; import javax.annotation.Nullable; @@ -43,13 +39,6 @@ public String getInstalledVersion() { .orElse("unknown"); } - @Override - public @Nullable Mount createResourceMount(MinecraftServer server, String domain, String subPath) { - var manager = ServerLifecycleHooks.getCurrentServer().getResourceManager(); - var mount = ResourceMount.get(domain, subPath, manager); - return mount.exists("") ? mount : null; - } - @Override public void registerPeripheralProvider(IPeripheralProvider provider) { Peripherals.register(provider); diff --git a/tools/parse-reports.py b/tools/parse-reports.py index cba04df08..b733c8ccb 100755 --- a/tools/parse-reports.py +++ b/tools/parse-reports.py @@ -98,7 +98,7 @@ def _parse_junit_file(path: pathlib.Path): print("::group::Full error message") print(full_message) - print("::endgroup") + print("::endgroup::") def parse_junit() -> None: