[Java] Allow for wrapping a zero length buffer at capacity. Issue #211.

aeron-io · Jun 2, 2020 · 211a863 · 211a863
1 parent 91d3626
commit 211a863
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 52 deletions.
diff --git a/agrona/src/main/java/org/agrona/concurrent/UnsafeBuffer.java b/agrona/src/main/java/org/agrona/concurrent/UnsafeBuffer.java
@@ -161,17 +161,7 @@ public void wrap(final byte[] buffer, final int offset, final int length)
     {
         if (SHOULD_BOUNDS_CHECK)
         {
-            final int bufferLength = buffer.length;
-            if (offset != 0 && (offset < 0 || offset > bufferLength - 1))
-            {
-                throw new IllegalArgumentException("offset=" + offset + " not valid for buffer.length=" + bufferLength);
-            }
-
-            if (length < 0 || length > bufferLength - offset)
-            {
-                throw new IllegalArgumentException(
-                    "offset=" + offset + " length=" + length + " not valid for buffer.length=" + bufferLength);
-            }
+            boundsCheckWrap(offset, length, buffer.length);
         }
 
         addressOffset = ARRAY_BASE_OFFSET + offset;
@@ -209,17 +199,7 @@ public void wrap(final ByteBuffer buffer, final int offset, final int length)
     {
         if (SHOULD_BOUNDS_CHECK)
         {
-            final int bufferCapacity = buffer.capacity();
-            if (offset != 0 && (offset < 0 || offset > bufferCapacity - 1))
-            {
-                throw new IllegalArgumentException("offset=" + offset + " not valid for capacity=" + bufferCapacity);
-            }
-
-            if (length < 0 || length > bufferCapacity - offset)
-            {
-                throw new IllegalArgumentException(
-                    "offset=" + offset + " length=" + length + " not valid for capacity=" + bufferCapacity);
-            }
+            boundsCheckWrap(offset, length, buffer.capacity());
         }
 
         if (buffer != byteBuffer)
@@ -263,17 +243,7 @@ public void wrap(final DirectBuffer buffer, final int offset, final int length)
     {
         if (SHOULD_BOUNDS_CHECK)
         {
-            final int bufferCapacity = buffer.capacity();
-            if (offset != 0 && (offset < 0 || offset > bufferCapacity - 1))
-            {
-                throw new IllegalArgumentException("offset=" + offset + " not valid for capacity=" + bufferCapacity);
-            }
-
-            if (length < 0 || length > bufferCapacity - offset)
-            {
-                throw new IllegalArgumentException(
-                    "offset=" + offset + " length=" + length + " not valid for capacity=" + bufferCapacity);
-            }
+            boundsCheckWrap(offset, length, buffer.capacity());
         }
 
         addressOffset = buffer.addressOffset() + offset;
@@ -1687,6 +1657,25 @@ public int putLongAscii(final int index, final long value)
 
     ///////////////////////////////////////////////////////////////////////////
 
+    private static void boundsCheckWrap(final int offset, final int length, final int capacity)
+    {
+        if (offset < 0)
+        {
+            throw new IllegalArgumentException("invalid offset: " + offset);
+        }
+
+        if (length < 0)
+        {
+            throw new IllegalArgumentException("invalid length: " + length);
+        }
+
+        if ((offset > capacity - length) || (length > capacity - offset))
+        {
+            throw new IllegalArgumentException(
+                "offset=" + offset + " length=" + length + " not valid for capacity=" + capacity);
+        }
+    }
+
     private void boundsCheck(final int index)
     {
         if (index < 0 || index >= capacity)
@@ -1709,7 +1698,7 @@ public void boundsCheck(final int index, final int length)
         boundsCheck0(index, length);
     }
 
-    private void lengthCheck(final int length)
+    private static void lengthCheck(final int length)
     {
         if (length < 0)
         {

diff --git a/agrona/src/test/java/org/agrona/concurrent/UnsafeBufferTest.java b/agrona/src/test/java/org/agrona/concurrent/UnsafeBufferTest.java
@@ -27,6 +27,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class UnsafeBufferTest
 {
@@ -220,6 +221,54 @@ public void shouldWriteMinIntValue()
         assertContainsString(buffer, String.valueOf(Integer.MIN_VALUE), length);
     }
 
+    @ParameterizedTest
+    @MethodSource("valuesAndLengths")
+    public void shouldPutNaturalFromEnd(final int[] valueAndLength)
+    {
+        final MutableDirectBuffer buffer = new UnsafeBuffer(new byte[8 * 1024]);
+        final int value = valueAndLength[0];
+        final int length = valueAndLength[1];
+
+        final int start = buffer.putNaturalIntAsciiFromEnd(value, length);
+        final String message = "for " + Arrays.toString(valueAndLength);
+        assertEquals(0, start, message);
+
+        assertEquals(
+            String.valueOf(value),
+            buffer.getStringWithoutLengthAscii(0, length),
+            message);
+    }
+
+    @Test
+    public void shouldWrapValidRange()
+    {
+        final UnsafeBuffer buffer = new UnsafeBuffer(new byte[8]);
+        final UnsafeBuffer slice = new UnsafeBuffer();
+
+        slice.wrap(buffer);
+        slice.wrap(buffer, 0, 8);
+        slice.wrap(buffer, 1, 7);
+        slice.wrap(buffer, 2, 6);
+        slice.wrap(buffer, 3, 5);
+        slice.wrap(buffer, 4, 4);
+        slice.wrap(buffer, 5, 3);
+        slice.wrap(buffer, 6, 2);
+        slice.wrap(buffer, 7, 1);
+        slice.wrap(buffer, 8, 0);
+    }
+
+    @Test
+    public void shouldNotWrapInValidRange()
+    {
+        final UnsafeBuffer buffer = new UnsafeBuffer(new byte[8]);
+        final UnsafeBuffer slice = new UnsafeBuffer();
+
+        assertThrows(IllegalArgumentException.class, () -> slice.wrap(buffer, -1, 0));
+        assertThrows(IllegalArgumentException.class, () -> slice.wrap(buffer, 0, -1));
+        assertThrows(IllegalArgumentException.class, () -> slice.wrap(buffer, 8, 1));
+        assertThrows(IllegalArgumentException.class, () -> slice.wrap(buffer, 7, 3));
+    }
+
     private void assertContainsString(final UnsafeBuffer buffer, final String value, final int length)
     {
         assertEquals(value, buffer.getStringWithoutLengthAscii(INDEX, length));
@@ -247,22 +296,4 @@ private static int[][] valuesAndLengths()
                 { 9999, 4 },
             };
     }
-
-    @ParameterizedTest
-    @MethodSource("valuesAndLengths")
-    public void shouldPutNaturalFromEnd(final int[] valueAndLength)
-    {
-        final MutableDirectBuffer buffer = new UnsafeBuffer(new byte[8 * 1024]);
-        final int value = valueAndLength[0];
-        final int length = valueAndLength[1];
-
-        final int start = buffer.putNaturalIntAsciiFromEnd(value, length);
-        final String message = "for " + Arrays.toString(valueAndLength);
-        assertEquals(0, start, message);
-
-        assertEquals(
-            String.valueOf(value),
-            buffer.getStringWithoutLengthAscii(0, length),
-            message);
-    }
 }