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.

projects/common/src/main/java/dan200/computercraft/impl/AbstractComputerCraftAPI.java

@@ -7,6 +7,7 @@ package dan200.computercraft.impl;
 
 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.redstone.BundledRedstoneProvider;
 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 abstract class AbstractComputerCraftAPI implements ComputerCraftAPIServic
     }
 
     @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

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 net.minecraft.util.profiling.ProfilerFiller;
 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 final class ResourceMount extends ArchiveMount<ResourceMount.FileEntry> {
         }
     }
 
-    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 @@ public final class ResourceMount extends ArchiveMount<ResourceMount.FileEntry> {
         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 @@ public final class ResourceMount extends ArchiveMount<ResourceMount.FileEntry> {
                     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 final class ResourceMount extends ArchiveMount<ResourceMount.FileEntry> {
     @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 final class ResourceMount extends ArchiveMount<ResourceMount.FileEntry> {
     protected static class FileEntry extends ArchiveMount.FileEntry<FileEntry> {
         final ResourceLocation identifier;
 
-        FileEntry(ResourceLocation identifier) {
+        FileEntry(String path, ResourceLocation identifier) {
+            super(path);
             this.identifier = identifier;
         }
     }

projects/common/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java

@@ -25,6 +25,7 @@ import static dan200.computercraft.core.util.Nullability.assertNonNull;
 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 class UploadFileMessage extends ComputerServerMessage {
     public static void send(AbstractContainerMenu container, List<FileUpload> files, Consumer<UploadFileMessage> 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<FileSlice> slices = new ArrayList<>(files.size());
@@ -137,7 +138,7 @@ public class UploadFileMessage extends ComputerServerMessage {
                         ? 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;
                 }
 

projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java

@@ -202,6 +202,7 @@ public final class DiskDriveBlockEntity extends AbstractContainerBlockEntity {
         ejectQueued.set(true);
     }
 
+    @GuardedBy("this")
     private void mountDisk(IComputerAccess computer, MountInfo info, MediaStack disk) {
         var mount = disk.getMount((ServerLevel) getLevel());
         if (mount != null) {

projects/common/src/test/java/dan200/computercraft/TestPlatformHelper.java

@@ -8,7 +8,6 @@ package dan200.computercraft;
 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 class TestPlatformHelper extends AbstractComputerCraftAPI implements Plat
         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> T tryGetRegistryObject(ResourceKey<Registry<T>> registry, ResourceLocation id) {

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<Unit>();
-        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<String> 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();
     }
 }