Merge pull request #330 from scijava/fix-raf-creation

Fix #329: Empty RAF creation on read

Author: ctrueden

src/main/java/org/scijava/io/handle/FileHandle.java

@@ -60,9 +60,12 @@ public class FileHandle extends AbstractDataHandle<FileLocation> {
 
 	// -- FileHandle methods --
 
-	/** Gets the random access file object backing this FileHandle. */
+	/**
+	 * Gets the random access file object backing this FileHandle. If the
+	 * underlying file does not exist yet, it will be created.
+	 */
 	public RandomAccessFile getRandomAccessFile() throws IOException {
-		return raf();
+		return writer();
 	}
 
 	public String getMode() {
@@ -101,199 +104,199 @@ public Date lastModified() {
 
 	@Override
 	public long offset() throws IOException {
-		return raf().getFilePointer();
+		return exists() ? reader().getFilePointer() : 0;
 	}
 
 	@Override
 	public long length() throws IOException {
-		return exists() ? raf().length() : -1;
+		return exists() ? reader().length() : -1;
 	}
 
 	@Override
 	public void setLength(final long length) throws IOException {
-		raf().setLength(length);
+		writer().setLength(length);
 	}
 
 	@Override
 	public int read() throws IOException {
-		return raf().read();
+		return reader().read();
 	}
 
 	@Override
 	public int read(final byte[] b) throws IOException {
-		return raf().read(b);
+		return reader().read(b);
 	}
 
 	@Override
 	public int read(final byte[] b, final int off, final int len)
 		throws IOException
 	{
-		return raf().read(b, off, len);
+		return reader().read(b, off, len);
 	}
 
 	@Override
 	public void seek(final long pos) throws IOException {
-		raf().seek(pos);
+		reader().seek(pos);
 	}
 
 	// -- DataInput methods --
 
 	@Override
 	public boolean readBoolean() throws IOException {
-		return raf().readBoolean();
+		return reader().readBoolean();
 	}
 
 	@Override
 	public byte readByte() throws IOException {
-		return raf().readByte();
+		return reader().readByte();
 	}
 
 	@Override
 	public char readChar() throws IOException {
-		return raf().readChar();
+		return reader().readChar();
 	}
 
 	@Override
 	public double readDouble() throws IOException {
-		return raf().readDouble();
+		return reader().readDouble();
 	}
 
 	@Override
 	public float readFloat() throws IOException {
-		return raf().readFloat();
+		return reader().readFloat();
 	}
 
 	@Override
 	public void readFully(final byte[] b) throws IOException {
-		raf().readFully(b);
+		reader().readFully(b);
 	}
 
 	@Override
 	public void readFully(final byte[] b, final int off, final int len)
 		throws IOException
 	{
-		raf().readFully(b, off, len);
+		reader().readFully(b, off, len);
 	}
 
 	@Override
 	public int readInt() throws IOException {
-		return raf().readInt();
+		return reader().readInt();
 	}
 
 	@Override
 	public String readLine() throws IOException {
-		return raf().readLine();
+		return reader().readLine();
 	}
 
 	@Override
 	public long readLong() throws IOException {
-		return raf().readLong();
+		return reader().readLong();
 	}
 
 	@Override
 	public short readShort() throws IOException {
-		return raf().readShort();
+		return reader().readShort();
 	}
 
 	@Override
 	public int readUnsignedByte() throws IOException {
-		return raf().readUnsignedByte();
+		return reader().readUnsignedByte();
 	}
 
 	@Override
 	public int readUnsignedShort() throws IOException {
-		return raf().readUnsignedShort();
+		return reader().readUnsignedShort();
 	}
 
 	@Override
 	public String readUTF() throws IOException {
-		return raf().readUTF();
+		return reader().readUTF();
 	}
 
 	@Override
 	public int skipBytes(final int n) throws IOException {
-		return raf().skipBytes(n);
+		return reader().skipBytes(n);
 	}
 
 	// -- DataOutput methods --
 
 	@Override
 	public void write(final byte[] b) throws IOException {
-		raf().write(b);
+		writer().write(b);
 	}
 
 	@Override
 	public void write(final byte[] b, final int off, final int len)
 		throws IOException
 	{
-		raf().write(b, off, len);
+		writer().write(b, off, len);
 	}
 
 	@Override
 	public void write(final int b) throws IOException {
-		raf().write(b);
+		writer().write(b);
 	}
 
 	@Override
 	public void writeBoolean(final boolean v) throws IOException {
-		raf().writeBoolean(v);
+		writer().writeBoolean(v);
 	}
 
 	@Override
 	public void writeByte(final int v) throws IOException {
-		raf().writeByte(v);
+		writer().writeByte(v);
 	}
 
 	@Override
 	public void writeBytes(final String s) throws IOException {
-		raf().writeBytes(s);
+		writer().writeBytes(s);
 	}
 
 	@Override
 	public void writeChar(final int v) throws IOException {
-		raf().writeChar(v);
+		writer().writeChar(v);
 	}
 
 	@Override
 	public void writeChars(final String s) throws IOException {
-		raf().writeChars(s);
+		writer().writeChars(s);
 	}
 
 	@Override
 	public void writeDouble(final double v) throws IOException {
-		raf().writeDouble(v);
+		writer().writeDouble(v);
 	}
 
 	@Override
 	public void writeFloat(final float v) throws IOException {
-		raf().writeFloat(v);
+		writer().writeFloat(v);
 	}
 
 	@Override
 	public void writeInt(final int v) throws IOException {
-		raf().writeInt(v);
+		writer().writeInt(v);
 	}
 
 	@Override
 	public void writeLong(final long v) throws IOException {
-		raf().writeLong(v);
+		writer().writeLong(v);
 	}
 
 	@Override
 	public void writeShort(final int v) throws IOException {
-		raf().writeShort(v);
+		writer().writeShort(v);
 	}
 
 	@Override
 	public void writeUTF(final String str) throws IOException {
-		raf().writeUTF(str);
+		writer().writeUTF(str);
 	}
 
 	// -- Closeable methods --
 
 	@Override
 	public synchronized void close() throws IOException {
-		if (raf != null) raf().close();
+		if (raf != null) raf.close();
 		closed = true;
 	}
 
@@ -306,12 +309,47 @@ public Class<FileLocation> getType() {
 
 	// -- Helper methods --
 
-	private RandomAccessFile raf() throws IOException {
-		if (raf == null) initRAF();
+	/**
+	 * Access method for the internal {@link RandomAccessFile}, that succeeds
+	 * independently of the underlying file existing on disk. This allows us to
+	 * create a new file for writing.
+	 *
+	 * @return the internal {@link RandomAccessFile} creating a new file on disk
+	 *         if needed.
+	 * @throws IOException if the {@link RandomAccessFile} could not be created.
+	 */
+	private RandomAccessFile writer() throws IOException {
+		if (raf == null) initRAF(true);
 		return raf;
 	}
 
-	private synchronized void initRAF() throws IOException {
+	/**
+	 * Access method for the internal {@link RandomAccessFile}, that only succeeds
+	 * if the underlying file exists on disk. This prevents accidental creation of
+	 * an empty file when calling read operations on a non-existent file.
+	 *
+	 * @return the internal {@link RandomAccessFile}.
+	 * @throws IOException if the {@link RandomAccessFile} could not be created,
+	 *           or the backing file does not exists.
+	 */
+	private RandomAccessFile reader() throws IOException {
+		if (raf == null) initRAF(false);
+		return raf;
+	}
+
+	/**
+	 * Initializes the {@link RandomAccessFile}.
+	 *
+	 * @param create whether to create the {@link RandomAccessFile} if the
+	 *          underlying file does not exist yet.
+	 * @throws IOException if the {@link RandomAccessFile} could not be created,
+	 *           or the backing file does not exist and the {@code create}
+	 *           parameter was set to {@code false}.
+	 */
+	private synchronized void initRAF(final boolean create) throws IOException {
+		if (!create && !exists()) {
+			throw new IOException("Trying to read from non-existent file!");
+		}
 		if (closed) throw new IOException("Handle already closed");
 		if (raf != null) return;
 		raf = new RandomAccessFile(get().getFile(), getMode());

src/test/java/org/scijava/io/handle/FileHandleTest.java

@@ -35,10 +35,12 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.net.URI;
 
 import org.junit.Test;
 import org.scijava.Context;
@@ -108,4 +110,28 @@ public void testNotCreatedByClose() throws IOException {
 		handle.close();
 		assertFalse(nonExistentFile.exists());
 	}
+
+	@Test
+	public void testNotCreatedByRead() throws IOException {
+		final Context ctx = new Context();
+		final DataHandleService dhs = ctx.service(DataHandleService.class);
+
+		final File nonExistentFile = //
+			File.createTempFile("FileHandleTest", "none xistent file");
+		final URI uri = nonExistentFile.toURI();
+		assertTrue(nonExistentFile.delete());
+		assertFalse(nonExistentFile.exists());
+
+		final FileLocation loc = new FileLocation(uri);
+		try (final DataHandle<?> handle = dhs.create(loc)) {
+			assertFalse(handle.exists());
+			handle.read(); // this will fail as there is no underlying file!
+			fail("Read successfully from non-existing file!");
+		}
+		catch (final IOException exc) {
+			// should be thrown
+		}
+		assertFalse(nonExistentFile.exists());
+		// reading from the non-existing file should not create it!
+	}
 }