Fix a couple of issues with FileSystemMounts

- Only generate resource pack mounts if the desired directory exists.
 - Allow mounting files, as well as directories (fixes #90).

As always, also a wee bit of cleanup to some of the surrounding code.

src/main/java/dan200/computercraft/ComputerCraft.java

@@ -100,6 +100,7 @@ import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
+import java.nio.file.Files;
 import java.util.*;
 import java.util.function.Function;
 import java.util.zip.ZipEntry;
@@ -955,7 +956,7 @@ public class ComputerCraft
                     {
                         // Mount a resource pack from a jar
                         FileSystem fs = FileSystems.newFileSystem( resourcePack.toPath(), ComputerCraft.class.getClassLoader() );
-                        mounts.add( new FileSystemMount( fs, subPath ) );
+                        if( Files.exists( fs.getPath( subPath ) ) ) mounts.add( new FileSystemMount( fs, subPath ) );
                     }
                     else
                     {

src/main/java/dan200/computercraft/api/peripheral/IComputerAccess.java

@@ -39,7 +39,10 @@ public interface IComputerAccess
      * @see IMount
      */
     @Nullable
-    String mount( @Nonnull String desiredLocation, @Nonnull IMount mount );
+    default String mount( @Nonnull String desiredLocation, @Nonnull IMount mount )
+    {
+        return mount( desiredLocation, mount, getAttachmentName() );
+    }
 
     /**
      * Mount a mount onto the computer's file system in a read only mode.
@@ -75,7 +78,10 @@ public interface IComputerAccess
      * @see IMount
      */
     @Nullable
-    String mountWritable( @Nonnull String desiredLocation, @Nonnull IWritableMount mount );
+    default String mountWritable( @Nonnull String desiredLocation, @Nonnull IWritableMount mount )
+    {
+        return mountWritable( desiredLocation, mount, getAttachmentName() );
+    }
 
     /**
      * Mount a mount onto the computer's file system in a writable mode.

src/main/java/dan200/computercraft/core/apis/ComputerAccess.java

@@ -1,5 +1,6 @@
 package dan200.computercraft.core.apis;
 
+import com.google.common.base.Preconditions;
 import dan200.computercraft.api.filesystem.IMount;
 import dan200.computercraft.api.filesystem.IWritableMount;
 import dan200.computercraft.api.peripheral.IComputerAccess;
@@ -11,6 +12,7 @@ import dan200.computercraft.core.filesystem.FileSystemException;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.util.HashSet;
+import java.util.Objects;
 import java.util.Set;
 
 public abstract class ComputerAccess implements IComputerAccess, IComputerOwned
@@ -33,22 +35,17 @@ public abstract class ComputerAccess implements IComputerAccess, IComputerOwned
         m_mounts.clear();
     }
 
-    @Override
-    public String mount( @Nonnull String desiredLoc, @Nonnull IMount mount )
-    {
-        return mount( desiredLoc, mount, getAttachmentName() );
-    }
-
     @Override
     public synchronized String mount( @Nonnull String desiredLoc, @Nonnull IMount mount, @Nonnull String driveName )
     {
+        Objects.requireNonNull( desiredLoc, "desiredLocation cannot be null" );
+        Objects.requireNonNull( mount, "mount cannot be null" );
+        Objects.requireNonNull( driveName, "driveName cannot be null" );
+
         // Mount the location
         String location;
         FileSystem fileSystem = m_environment.getFileSystem();
-        if( fileSystem == null )
+        if( fileSystem == null ) throw new IllegalStateException( "File system has not been created" );
-        {
-            throw new IllegalStateException( "File system has not been created" );
-        }
 
         synchronized( fileSystem )
         {
@@ -64,29 +61,22 @@ public abstract class ComputerAccess implements IComputerAccess, IComputerOwned
                 }
             }
         }
-        if( location != null )
-        {
-            m_mounts.add( location );
-        }
-        return location;
-    }
 
-    @Override
+        if( location != null ) m_mounts.add( location );
-    public String mountWritable( @Nonnull String desiredLoc, @Nonnull IWritableMount mount )
+        return location;
-    {
-        return mountWritable( desiredLoc, mount, getAttachmentName() );
     }
 
     @Override
     public synchronized String mountWritable( @Nonnull String desiredLoc, @Nonnull IWritableMount mount, @Nonnull String driveName )
     {
+        Objects.requireNonNull( desiredLoc, "desiredLocation cannot be null" );
+        Objects.requireNonNull( mount, "mount cannot be null" );
+        Objects.requireNonNull( driveName, "driveName cannot be null" );
+
         // Mount the location
         String location;
         FileSystem fileSystem = m_environment.getFileSystem();
-        if( fileSystem == null )
+        if( fileSystem == null ) throw new IllegalStateException( "File system has not been created" );
-        {
-            throw new IllegalStateException( "File system has not been created" );
-        }
 
         synchronized( fileSystem )
         {
@@ -102,26 +92,19 @@ public abstract class ComputerAccess implements IComputerAccess, IComputerOwned
                 }
             }
         }
-        if( location != null )
+
-        {
+        if( location != null ) m_mounts.add( location );
-            m_mounts.add( location );
-        }
         return location;
     }
 
     @Override
     public void unmount( String location )
     {
-        if( location != null )
+        if( location == null ) return;
-        {
+        if( !m_mounts.contains( location ) ) throw new IllegalStateException( "You didn't mount this location" );
-            if( !m_mounts.contains( location ) )
-            {
-                throw new RuntimeException( "You didn't mount this location" );
-            }
 
-            m_environment.getFileSystem().unmount( location );
+        m_environment.getFileSystem().unmount( location );
-            m_mounts.remove( location );
+        m_mounts.remove( location );
-        }
     }
 
     @Override
@@ -133,6 +116,7 @@ public abstract class ComputerAccess implements IComputerAccess, IComputerOwned
     @Override
     public void queueEvent( @Nonnull final String event, final Object[] arguments )
     {
+        Preconditions.checkNotNull( event, "event cannot be null" );
         m_environment.queueEvent( event, arguments );
     }
 
@@ -148,13 +132,9 @@ public abstract class ComputerAccess implements IComputerAccess, IComputerOwned
         try
         {
             FileSystem fileSystem = m_environment.getFileSystem();
-            if( !fileSystem.exists( desiredLoc ) )
+            if( !fileSystem.exists( desiredLoc ) ) return desiredLoc;
-            {
-                return desiredLoc;
-            }
 
-            // We used to check foo2,foo3,foo4,etc here
+            // We used to check foo2, foo3, foo4, etc here but the disk drive does this itself now
-            // but the disk drive does this itself now
             return null;
         }
         catch( FileSystemException e )

src/main/java/dan200/computercraft/core/filesystem/FileSystemMount.java

@@ -20,9 +20,15 @@ public class FileSystemMount implements IMount
     {
         Path rootPath = fileSystem.getPath( root );
         rootEntry = new Entry( "", rootPath );
+        populate( rootEntry );
+    }
+
+    private void populate( Entry root ) throws IOException
+    {
+        if( !root.directory ) return;
 
         Queue<Entry> entries = new ArrayDeque<>();
-        entries.add( rootEntry );
+        entries.add( root );
         while( !entries.isEmpty() )
         {
             Entry entry = entries.remove();

src/main/java/dan200/computercraft/core/filesystem/JarMount.java

@@ -149,10 +149,7 @@ public class JarMount implements IMount
                     {
                         m_root = new FileInZip( entryName, entry.isDirectory(), entry.getSize() );
                         m_rootPath = subPath;
-                        if( !m_root.isDirectory() )
+                        if( !m_root.isDirectory() ) break;
-                        {
-                            break;
-                        }
                     }
                     else
                     {

src/main/java/dan200/computercraft/shared/media/items/ItemTreasureDisk.java

@@ -7,6 +7,7 @@
 package dan200.computercraft.shared.media.items;
 
 import dan200.computercraft.ComputerCraft;
+import dan200.computercraft.api.ComputerCraftAPI;
 import dan200.computercraft.api.filesystem.IMount;
 import dan200.computercraft.api.media.IMedia;
 import dan200.computercraft.core.filesystem.SubMount;
@@ -116,7 +117,7 @@ public class ItemTreasureDisk extends Item
 
     private static IMount getTreasureMount()
     {
-        return ComputerCraft.createResourceMount( ComputerCraft.class, "computercraft", "lua/treasure" );
+        return ComputerCraftAPI.createResourceMount( ComputerCraft.class, "computercraft", "lua/treasure" );
     }
 
     // private stuff

src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java

@@ -4,17 +4,13 @@
  * Send enquiries to dratcliffe@gmail.com
  */
 
-package dan200.computercraft.core;
+package dan200.computercraft.core.filesystem;
 
 import com.google.common.io.Files;
 import dan200.computercraft.api.filesystem.IWritableMount;
 import dan200.computercraft.api.lua.LuaException;
 import dan200.computercraft.core.apis.ObjectWrapper;
 import dan200.computercraft.core.apis.handles.EncodedWritableHandle;
-import dan200.computercraft.core.filesystem.FileMount;
-import dan200.computercraft.core.filesystem.FileSystem;
-import dan200.computercraft.core.filesystem.FileSystemException;
-import dan200.computercraft.core.filesystem.FileSystemWrapper;
 import org.junit.Test;
 
 import java.io.BufferedWriter;

src/test/java/dan200/computercraft/core/filesystem/FilesystemMountTest.java

@@ -0,0 +1,62 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2018. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+
+package dan200.computercraft.core.filesystem;
+
+import dan200.computercraft.api.filesystem.IMount;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class FilesystemMountTest
+{
+    private static final File ZIP_FILE = new File( "test-files/filesystem-mount.zip" );
+
+    @BeforeClass
+    public static void before() throws IOException
+    {
+        if( ZIP_FILE.exists() ) return;
+
+        try( ZipOutputStream stream = new ZipOutputStream( new FileOutputStream( ZIP_FILE ) ) )
+        {
+            stream.putNextEntry( new ZipEntry( "dir/" ) );
+            stream.closeEntry();
+
+            stream.putNextEntry( new ZipEntry( "dir/file.lua" ) );
+            stream.write( "print('testing')".getBytes( StandardCharsets.UTF_8 ) );
+            stream.closeEntry();
+        }
+    }
+
+    @Test
+    public void mountsDir() throws IOException
+    {
+        FileSystem fs = FileSystems.newFileSystem( ZIP_FILE.toPath(), getClass().getClassLoader() );
+        IMount mount = new FileSystemMount( fs, "dir" );
+        assertTrue( "Root should be directory", mount.isDirectory( "" ) );
+        assertTrue( "File should exist", mount.exists( "file.lua" ) );
+    }
+
+    @Test
+    public void mountsFile() throws IOException
+    {
+        FileSystem fs = FileSystems.newFileSystem( ZIP_FILE.toPath(), getClass().getClassLoader() );
+        IMount mount = new FileSystemMount( fs, "dir/file.lua" );
+        assertTrue( "Root should exist", mount.exists( "" ) );
+        assertFalse( "Root should be a file", mount.isDirectory( "" ) );
+    }
+}

src/test/java/dan200/computercraft/core/filesystem/JarMountTest.java

@@ -0,0 +1,59 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2018. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+
+package dan200.computercraft.core.filesystem;
+
+import dan200.computercraft.api.filesystem.IMount;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+@SuppressWarnings( "deprecation" )
+public class JarMountTest
+{
+    private static final File ZIP_FILE = new File( "test-files/jar-mount.zip" );
+
+    @BeforeClass
+    public static void before() throws IOException
+    {
+        if( ZIP_FILE.exists() ) return;
+
+        try( ZipOutputStream stream = new ZipOutputStream( new FileOutputStream( ZIP_FILE ) ) )
+        {
+            stream.putNextEntry( new ZipEntry( "dir/" ) );
+            stream.closeEntry();
+
+            stream.putNextEntry( new ZipEntry( "dir/file.lua" ) );
+            stream.write( "print('testing')".getBytes( StandardCharsets.UTF_8 ) );
+            stream.closeEntry();
+        }
+    }
+
+    @Test
+    public void mountsDir() throws IOException
+    {
+        IMount mount = new JarMount( ZIP_FILE, "dir" );
+        assertTrue( "Root should be directory", mount.isDirectory( "" ) );
+        assertTrue( "File should exist", mount.exists( "file.lua" ) );
+    }
+
+    @Test
+    public void mountsFile() throws IOException
+    {
+        IMount mount = new JarMount( ZIP_FILE, "dir/file.lua" );
+        assertTrue( "Root should exist", mount.exists( "" ) );
+        assertFalse( "Root should be a file", mount.isDirectory( "" ) );
+    }
+}