MessageBuffer: exception and validation fixes with docs

Author: frsyuki

msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java

@@ -19,6 +19,7 @@
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
 import java.nio.BufferOverflowException;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
@@ -182,24 +183,76 @@ public class MessageBuffer
      */
     protected final ByteBuffer reference;
 
-    public static MessageBuffer allocate(int length)
+    /**
+     * Allocates a new MessageBuffer backed by a byte array.
+     *
+     * @throws IllegalArgumentException If the capacity is a negative integer
+     *
+     */
+    public static MessageBuffer allocate(int size)
     {
-        return wrap(new byte[length]);
+        if (size < 0) {
+            throw new IllegalArgumentException("size must not be negative");
+        }
+        return wrap(new byte[size]);
     }
 
+    /**
+     * Wraps a ByteBuffer into a MessageBuffer.
+     *
+     * The new MessageBuffer will be backed by the given byte array. Modifications to the new MessageBuffer will cause the byte array to be modified and vice versa.
+     *
+     * The new buffer's size will be array.length. hasArray() will return true.
+     *
+     * @param array the byte array that will gack this MessageBuffer
+     * @return
+     *
+     */
     public static MessageBuffer wrap(byte[] array)
     {
         return newMessageBuffer(array, 0, array.length);
     }
 
+    /**
+     * Wraps a ByteBuffer into a MessageBuffer.
+     *
+     * The new MessageBuffer will be backed by the given byte array. Modifications to the new MessageBuffer will cause the byte array to be modified and vice versa.
+     *
+     * The new buffer's size will be length. hasArray() will return true.
+     *
+     * @param array the byte array that will gack this MessageBuffer
+     * @param offset The offset of the subarray to be used; must be non-negative and no larger than array.length
+     * @param length The length of the subarray to be used; must be non-negative and no larger than array.length - offset
+     * @return
+     *
+     */
     public static MessageBuffer wrap(byte[] array, int offset, int length)
     {
         return newMessageBuffer(array, offset, length);
     }
 
+    /**
+     * Wraps a ByteBuffer into a MessageBuffer.
+     *
+     * The new MessageBuffer will be backed by the given byte buffer. Modifications to the new MessageBuffer will cause the byte buffer to be modified and vice versa. However, change of position, limit, or mark of given byte buffer doesn't affect MessageBuffer.
+     *
+     * The new buffer's size will be bb.remaining(). hasArray() will return the same result with bb.hasArray().
+     *
+     * @param bb the byte buffer that will gack this MessageBuffer
+     * @throws IllegalArgumentException given byte buffer returns false both from hasArray() and isDirect()
+     * @throws UnsupportedOperationException given byte buffer is a direct buffer and this platform doesn't support Unsafe API
+     * @return
+     *
+     */
     public static MessageBuffer wrap(ByteBuffer bb)
     {
-        return newMessageBuffer(bb).slice(bb.position(), bb.remaining());
+        MessageBuffer b = newMessageBuffer(bb);
+        if (bb.position() > 0 || bb.limit() != bb.capacity()) {
+            return b.slice(bb.position(), bb.remaining());
+        }
+        else {
+            return b;
+        }
     }
 
     /**
@@ -214,8 +267,24 @@ private static MessageBuffer newMessageBuffer(byte[] arr, int off, int len)
         try {
             return (MessageBuffer) mbArrConstructor.newInstance(arr, off, len);
         }
-        catch (Throwable e) {
-            throw new RuntimeException(e);
+        catch (InstantiationException e) {
+            // should never happen
+            throw new IllegalStateException(e);
+        }
+        catch (IllegalAccessException e) {
+            // should never happen unless security manager restricts this reflection
+            throw new IllegalStateException(e);
+        }
+        catch (InvocationTargetException e) {
+            if (e.getCause() instanceof RuntimeException) {
+                // underlaying constructor may throw RuntimeException
+                throw (RuntimeException) e.getCause();
+            }
+            else if (e.getCause() instanceof Error) {
+                throw (Error) e.getCause();
+            }
+            // should never happen
+            throw new IllegalStateException(e.getCause());
         }
     }
 
@@ -232,18 +301,35 @@ private static MessageBuffer newMessageBuffer(ByteBuffer bb)
             // We need to use reflection to create MessageBuffer instances in order to prevent TypeProfile generation for getInt method. TypeProfile will be
             // generated to resolve one of the method references when two or more classes overrides the method.
             return (MessageBuffer) mbBBConstructor.newInstance(bb);
-        } catch (Exception e) {
-            throw new RuntimeException(e);
+        }
+        catch (InstantiationException e) {
+            // should never happen
+            throw new IllegalStateException(e);
+        }
+        catch (IllegalAccessException e) {
+            // should never happen unless security manager restricts this reflection
+            throw new IllegalStateException(e);
+        }
+        catch (InvocationTargetException e) {
+            if (e.getCause() instanceof RuntimeException) {
+                // underlaying constructor may throw RuntimeException
+                throw (RuntimeException) e.getCause();
+            }
+            else if (e.getCause() instanceof Error) {
+                throw (Error) e.getCause();
+            }
+            // should never happen
+            throw new IllegalStateException(e.getCause());
         }
     }
 
     public static void releaseBuffer(MessageBuffer buffer)
     {
-        if (isUniversalBuffer || buffer.base instanceof byte[]) {
+        if (isUniversalBuffer || buffer.base != null) {
             // We have nothing to do. Wait until the garbage-collector collects this array object
         }
-        else if (DirectBufferAccess.isDirectByteBufferInstance(buffer.base)) {
-            DirectBufferAccess.clean(buffer.base);
+        else if (DirectBufferAccess.isDirectByteBufferInstance(buffer.reference)) {
+            DirectBufferAccess.clean(buffer.reference);
         }
         else {
             // Maybe cannot reach here
@@ -260,7 +346,7 @@ else if (DirectBufferAccess.isDirectByteBufferInstance(buffer.base)) {
      */
     MessageBuffer(byte[] arr, int offset, int length)
     {
-        this.base = arr;
+        this.base = arr;  // non-null is already checked at newMessageBuffer
         this.address = ARRAY_BYTE_BASE_OFFSET + offset;
         this.size = length;
         this.reference = null;
@@ -275,7 +361,7 @@ else if (DirectBufferAccess.isDirectByteBufferInstance(buffer.base)) {
     {
         if (bb.isDirect()) {
             if (isUniversalBuffer) {
-                throw new IllegalStateException("Cannot create MessageBuffer from DirectBuffer");
+                throw new UnsupportedOperationException("Cannot create MessageBuffer from a DirectBuffer on this platform");
             }
             // Direct buffer or off-heap memory
             this.base = null;
@@ -290,7 +376,7 @@ else if (bb.hasArray()) {
             this.reference = null;
         }
         else {
-            throw new IllegalArgumentException("Only the array-backed ByteBuffer or DirectBuffer are supported");
+            throw new IllegalArgumentException("Only the array-backed ByteBuffer or DirectBuffer is supported");
         }
     }
 
@@ -475,7 +561,7 @@ public void putMessageBuffer(int index, MessageBuffer src, int srcOffset, int le
      */
     public ByteBuffer sliceAsByteBuffer(int index, int length)
     {
-        if (hasArray()) {
+        if (base != null) {
             return ByteBuffer.wrap((byte[]) base, (int) ((address - ARRAY_BYTE_BASE_OFFSET) + index), length);
         }
         else {
@@ -496,7 +582,7 @@ public ByteBuffer sliceAsByteBuffer()
 
     public boolean hasArray()
     {
-        return base instanceof byte[];
+        return base != null;
     }
 
     /**