Merge pull request #280 from scijava/bytebank-tests

Bytebank tests and bugfixes

Author: ctrueden

src/main/java/org/scijava/io/ByteArrayByteBank.java

@@ -68,7 +68,7 @@ public ByteArrayByteBank(final int initialCapacity) {
 	 */
 	public ByteArrayByteBank(final ByteArray bytes) {
 		buffer = bytes;
-		bytes.size();
+		maxBufferedPos = bytes.size();
 	}
 
 	/**
@@ -98,12 +98,17 @@ public void setBytes(final long startpos, final byte[] bytes,
 		// copy the data
 		System.arraycopy(bytes, offset, buffer.getArray(), (int) startpos, length);
 		buffer.setSize(neededCapacity);
-		updateMaxPos(startpos + length);
+		updateMaxPos(startpos + length - 1);
 	}
 
 	@Override
 	public void setByte(final long pos, final byte b) {
 		checkWritePos(pos, pos);
+		buffer.ensureCapacity((int) pos);
+		// NB: update the size of the underlying buffer before appending to it
+		if (pos == buffer.size()) {
+			buffer.setSize((int) (pos + 1));
+		}
 		buffer.setValue((int) pos, b);
 		updateMaxPos(pos);
 	}
@@ -131,7 +136,8 @@ public int getBytes(final long startPos, final byte[] b, final int offset,
 		final int length)
 	{
 		checkReadPos(startPos, startPos + length);
-		final int readLength = (int) Math.min(getMaxPos() - startPos, length);
+		// ensure we don't try to read data which is not in the buffer
+		final int readLength = (int) Math.min(getMaxPos() - startPos + 1, length);
 		System.arraycopy(buffer.getArray(), (int) startPos, b, offset, readLength);
 		return readLength;
 	}

src/main/java/org/scijava/io/ByteBank.java

@@ -80,7 +80,18 @@ default int getBytes(long startPos, byte[] bytes) {
 	 * @param length the number of elements to append from the bytes array
 	 */
 	default void appendBytes(byte[] bytes, int length) {
-		setBytes(getMaxPos() + 1, bytes, 0, length);
+		appendBytes(bytes, 0, length);
+	}
+
+	/**
+	 * Appends the given bytes to the buffer
+	 *
+	 * @param bytes the array containing the bytes to append to the buffer
+	 * @param offset the offset in the bytes array
+	 * @param length the number of elements to append from the bytes array
+	 */
+	default void appendBytes(byte[] bytes, int offset, int length) {
+		setBytes(getMaxPos() + 1, bytes, offset, length);
 	}
 
 	/**

src/main/java/org/scijava/io/nio/ByteBufferByteBank.java

@@ -93,12 +93,15 @@ public void setBytes(final long startpos, final byte[] bytes,
 		buffer.put(bytes, offset, length);
 
 		// update the maxpos
-		updateMaxPos(startpos + length);
+		updateMaxPos(startpos + length - 1);
 	}
 
 	@Override
 	public void setByte(final long pos, final byte b) {
 		checkWritePos(pos, pos);
+		if (pos == buffer.capacity()) {
+			ensureCapacity((int) pos + 1);
+		}
 		buffer.put((int) pos, b);
 		updateMaxPos(pos);
 	}
@@ -127,10 +130,12 @@ public int getBytes(final long startPos, final byte[] b, final int offset,
 		final int length)
 	{
 		checkReadPos(startPos, startPos + length);
+		// ensure we don't try to read data which is not in the buffer
+		final int readLength = (int) Math.min(getMaxPos() - startPos + 1, length);
 		buffer.position((int) startPos);
-		buffer.get(b, offset, length);
+		buffer.get(b, offset, readLength);
 
-		return length;
+		return readLength;
 	}
 
 	@Override

src/test/java/org/scijava/io/ByteArrayByteBankTest.java

@@ -0,0 +1,46 @@
+/*
+ * #%L
+ * SciJava Common shared library for SciJava software.
+ * %%
+ * Copyright (C) 2009 - 2016 Board of Regents of the University of
+ * Wisconsin-Madison, Broad Institute of MIT and Harvard, and Max Planck
+ * Institute of Molecular Cell Biology and Genetics.
+ * %%
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ * #L%
+ */
+
+package org.scijava.io;
+
+/**
+ * Tests {@link ByteArrayByteBank}
+ *
+ * @author Gabriel Einsdorf
+ * @see ByteBankTest
+ */
+public class ByteArrayByteBankTest extends ByteBankTest {
+
+	@Override
+	public ByteBank createByteBank() {
+		return new ByteArrayByteBank();
+	}
+}

src/test/java/org/scijava/io/ByteBankTest.java

@@ -0,0 +1,149 @@
+/*
+ * #%L
+ * SciJava Common shared library for SciJava software.
+ * %%
+ * Copyright (C) 2009 - 2016 Board of Regents of the University of
+ * Wisconsin-Madison, Broad Institute of MIT and Harvard, and Max Planck
+ * Institute of Molecular Cell Biology and Genetics.
+ * %%
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ * #L%
+ */
+
+package org.scijava.io;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Abstract superclass for testing {@link ByteBank} implementations.
+ *
+ * @author Gabriel Einsdorf
+ */
+public abstract class ByteBankTest {
+
+	private static byte[] testBytes = { 0, -1, 2, -3, 4, 120, -128, 127, 32, 42 };
+	private ByteBank bank;
+
+	/**
+	 * @return the ByteBank implementation to test
+	 */
+	public abstract ByteBank createByteBank();
+
+	@Before
+	public void setup() {
+		bank = createByteBank();
+	}
+
+	@Test
+	public void testSetGetBytesArray() {
+		// read in full array
+		bank.setBytes(0l, testBytes.clone(), 0, testBytes.length);
+
+		// read out full array
+		assertEqualRead(testBytes.length, 0);
+		assertEqualRead(testBytes.length - 4, 2);
+	}
+
+	@Test
+	public void testSetGetBytesPartialArray() {
+		// read in the partial array
+		bank.setBytes(0l, testBytes, 0, testBytes.length - 4);
+
+		// read out the partial array
+		assertEqualRead(testBytes.length - 4, 0);
+		assertEqualRead(testBytes.length - 4, 2);
+	}
+
+	@Test
+	public void testSetGetByte() {
+		final int numElements = 200_000;
+		for (int i = 0; i < numElements; i++) {
+			bank.setByte(i, (byte) i);
+		}
+
+		for (int i = 0; i < numElements; i++) {
+			assertEquals((byte) i, bank.getByte(i));
+		}
+	}
+
+	@Test
+	public void testClear() {
+		bank.setBytes(0, testBytes, 0, testBytes.length);
+		assertEquals(testBytes.length - 1, bank.getMaxPos());
+
+		bank.clear();
+		assertEquals(0, bank.getMaxPos());
+	}
+
+	@Test
+	public void testAppendBytes() {
+		// simple append
+		bank.appendBytes(testBytes, testBytes.length);
+		assertEqualRead(testBytes.length, 0);
+
+		// append to buffer that already contains data
+		bank.clear();
+		bank.setByte(0l, (byte) 42);
+		bank.setByte(1l, (byte) 43);
+		bank.appendBytes(testBytes, testBytes.length);
+
+		final byte[] expected = new byte[testBytes.length + 2];
+		expected[0] = 42;
+		expected[1] = 43;
+		System.arraycopy(testBytes, 0, expected, 2, testBytes.length);
+
+		final byte[] actuals = new byte[expected.length];
+		bank.getBytes(0, actuals);
+
+		assertArrayEquals(expected, actuals);
+	}
+
+	/**
+	 * Asserts that {@link #bank} contains the same bytes as {@link #testBytes},
+	 * allows to specify an offset.
+	 *
+	 * @param length how many bytes (starting from the offset) of
+	 *          {@link #testBytes} are tested, this allows to test partial reads.
+	 * @param offset the offset position
+	 */
+	private void assertEqualRead(final int length, final int offset) {
+		// read from offset up to the length of the given array
+		final byte[] bytes = new byte[length];
+		final int read = bank.getBytes(offset, bytes);
+
+		final byte[] expected = new byte[length];
+		System.arraycopy(testBytes, offset, expected, 0, read);
+		assertArrayEquals(expected, bytes);
+
+		// read from offset to offset
+		final byte[] offsetBytes = new byte[testBytes.length];
+		final int readOffset = bank.getBytes(offset, offsetBytes, offset, length);
+
+		final byte[] offsetExpected = new byte[testBytes.length];
+		System.arraycopy(testBytes, offset, offsetExpected, offset, readOffset);
+		assertArrayEquals(expected, bytes);
+	}
+}

src/test/java/org/scijava/io/location/BytesLocationTest.java

@@ -64,7 +64,7 @@ public void testBytesOffsetLength() {
 
 		final byte[] testDigits = new byte[digits.length];
 		loc.getByteBank().getBytes(0, testDigits);
-		assertEquals(length, loc.getByteBank().getMaxPos());
+		assertEquals(length - 1, loc.getByteBank().getMaxPos());
 
 		final byte[] expectedDigits = new byte[digits.length];
 		System.arraycopy(digits, offset, expectedDigits, 0, length);

src/test/java/org/scijava/io/nio/ByteBufferByteBankTest.java

@@ -35,15 +35,41 @@
 import static org.junit.Assert.assertTrue;
 
 import java.nio.ByteBuffer;
+import java.util.function.Function;
 
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+import org.scijava.io.ByteBank;
+import org.scijava.io.ByteBankTest;
 
 /**
  * Tests {@link ByteBufferByteBank}.
  *
  * @author Curtis Rueden
+ * @author Gabriel Einsdorf
+ * @see ByteBankTest
  */
-public class ByteBufferByteBankTest {
+@RunWith(Parameterized.class)
+public class ByteBufferByteBankTest extends ByteBankTest {
+
+	@Parameter
+	public Function<Integer, ByteBuffer> supplier;
+
+	@Parameters
+	public static Object[] params() {
+		final Function<Integer, ByteBuffer> alloc = ByteBuffer::allocate;
+		final Function<Integer, ByteBuffer> allocDirect =
+			ByteBuffer::allocateDirect;
+		return new Function[] { alloc, allocDirect };
+	}
+
+	@Override
+	public ByteBank createByteBank() {
+		return new ByteBufferByteBank(supplier);
+	}
 
 	@Test
 	public void testReadOnlyDefault() {