[RocksJava] Slice / DirectSlice improvements

Summary:
- AssertionError when initialized with Non-Direct Buffer
- Tests + coverage for DirectSlice
- Slice sigsegv fixes when initializing from String and byte arrays
- Slice Tests

Test Plan: Run tests without source modifications.

Reviewers: yhchiang, adamretter, ankgup87

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D30081

java/Makefile

@@ -58,6 +58,7 @@ JAVA_TESTS = org.rocksdb.test.BackupableDBOptionsTest\
 		org.rocksdb.test.ComparatorTest\
 		org.rocksdb.test.DBOptionsTest\
 		org.rocksdb.test.DirectComparatorTest\
+		org.rocksdb.test.DirectSliceTest\
 		org.rocksdb.test.EnvironmentTest\
 		org.rocksdb.test.FilterTest\
 		org.rocksdb.test.FlushTest\
@@ -74,6 +75,7 @@ JAVA_TESTS = org.rocksdb.test.BackupableDBOptionsTest\
 		org.rocksdb.test.RocksEnvTest\
 		org.rocksdb.test.RocksIteratorTest\
 		org.rocksdb.test.SizeUnitTest\
+		org.rocksdb.test.SliceTest\
 		org.rocksdb.test.SnapshotTest\
 		org.rocksdb.test.StatisticsCollectorTest\
 		org.rocksdb.test.WriteBatchHandlerTest\

java/org/rocksdb/DirectSlice.java

@@ -56,6 +56,7 @@ public class DirectSlice extends AbstractSlice<ByteBuffer> {
    */
   public DirectSlice(final ByteBuffer data, final int length) {
     super();
+    assert(data.isDirect());
     createNewDirectSlice0(data, length);
   }
 
@@ -68,6 +69,7 @@ public class DirectSlice extends AbstractSlice<ByteBuffer> {
    */
   public DirectSlice(final ByteBuffer data) {
     super();
+    assert(data.isDirect());
     createNewDirectSlice1(data);
   }
 

java/org/rocksdb/Slice.java

@@ -77,8 +77,8 @@ public class Slice extends AbstractSlice<byte[]> {
    */
   @Override
   protected void disposeInternal() {
-    super.disposeInternal();
     disposeInternalBuf(nativeHandle_);
+    super.disposeInternal();
   }
 
   @Override protected final native byte[] data0(long handle);

java/org/rocksdb/test/DirectSliceTest.java

@@ -0,0 +1,105 @@
+// Copyright (c) 2014, Facebook, Inc.  All rights reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
+package org.rocksdb.test;
+
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.rocksdb.DirectSlice;
+
+import java.nio.ByteBuffer;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class DirectSliceTest {
+  @ClassRule
+  public static final RocksMemoryResource rocksMemoryResource =
+      new RocksMemoryResource();
+
+  @Test
+  public void directSlice() {
+    DirectSlice directSlice = null;
+    DirectSlice otherSlice = null;
+    try {
+      directSlice = new DirectSlice("abc");
+      otherSlice = new DirectSlice("abc");
+      assertThat(directSlice.toString()).isEqualTo("abc");
+      // clear first slice
+      directSlice.clear();
+      assertThat(directSlice.toString()).isEmpty();
+      // get first char in otherslice
+      assertThat(otherSlice.get(0)).isEqualTo("a".getBytes()[0]);
+      // remove prefix
+      otherSlice.removePrefix(1);
+      assertThat(otherSlice.toString()).isEqualTo("bc");
+    } finally {
+      if (directSlice != null) {
+        directSlice.dispose();
+      }
+      if (otherSlice != null) {
+        otherSlice.dispose();
+      }
+    }
+  }
+
+  @Test
+  public void directSliceWithByteBuffer() {
+    DirectSlice directSlice = null;
+    try {
+      byte[] data = "Some text".getBytes();
+      ByteBuffer buffer = ByteBuffer.allocateDirect(data.length);
+      buffer.put(data);
+      directSlice = new DirectSlice(buffer);
+      assertThat(directSlice.toString()).isEqualTo("Some text");
+    } finally {
+      if (directSlice != null) {
+        directSlice.dispose();
+      }
+    }
+  }
+
+  @Test
+  public void directSliceWithByteBufferAndLength() {
+    DirectSlice directSlice = null;
+    try {
+      byte[] data = "Some text".getBytes();
+      ByteBuffer buffer = ByteBuffer.allocateDirect(data.length);
+      buffer.put(data);
+      directSlice = new DirectSlice(buffer, 4);
+      assertThat(directSlice.toString()).isEqualTo("Some");
+    } finally {
+      if (directSlice != null) {
+        directSlice.dispose();
+      }
+    }
+  }
+
+  @Test(expected = AssertionError.class)
+  public void directSliceInitWithoutDirectAllocation() {
+    DirectSlice directSlice = null;
+    try {
+      byte[] data = "Some text".getBytes();
+      ByteBuffer buffer = ByteBuffer.wrap(data);
+      directSlice = new DirectSlice(buffer);
+    } finally {
+      if (directSlice != null) {
+        directSlice.dispose();
+      }
+    }
+  }
+
+  @Test(expected = AssertionError.class)
+  public void directSlicePrefixInitWithoutDirectAllocation() {
+    DirectSlice directSlice = null;
+    try {
+      byte[] data = "Some text".getBytes();
+      ByteBuffer buffer = ByteBuffer.wrap(data);
+      directSlice = new DirectSlice(buffer, 4);
+    } finally {
+      if (directSlice != null) {
+        directSlice.dispose();
+      }
+    }
+  }
+}

java/org/rocksdb/test/SliceTest.java

@@ -0,0 +1,105 @@
+// Copyright (c) 2014, Facebook, Inc.  All rights reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
+package org.rocksdb.test;
+
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.rocksdb.Slice;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class SliceTest {
+
+  @ClassRule
+  public static final RocksMemoryResource rocksMemoryResource =
+      new RocksMemoryResource();
+
+  @Test
+  public void slice() {
+    Slice slice = null;
+    Slice otherSlice = null;
+    Slice thirdSlice = null;
+    try {
+      slice = new Slice("testSlice");
+      assertThat(slice.empty()).isFalse();
+      assertThat(slice.size()).isEqualTo(9);
+      assertThat(slice.data()).isEqualTo("testSlice".getBytes());
+
+      otherSlice = new Slice("otherSlice".getBytes());
+      assertThat(otherSlice.data()).isEqualTo("otherSlice".getBytes());
+
+      thirdSlice = new Slice("otherSlice".getBytes(), 5);
+      assertThat(thirdSlice.data()).isEqualTo("Slice".getBytes());
+    } finally {
+      if (slice != null) {
+        slice.dispose();
+      }
+      if (otherSlice != null) {
+        otherSlice.dispose();
+      }
+      if (thirdSlice != null) {
+        thirdSlice.dispose();
+      }
+    }
+  }
+
+  @Test
+  public void sliceEquals() {
+    Slice slice = null;
+    Slice slice2 = null;
+    try {
+      slice = new Slice("abc");
+      slice2 = new Slice("abc");
+      assertThat(slice.equals(slice2)).isTrue();
+    } finally {
+      if (slice != null) {
+        slice.dispose();
+      }
+      if (slice2 != null) {
+        slice2.dispose();
+      }
+    }
+  }
+
+
+  @Test
+  public void sliceStartWith() {
+    Slice slice = null;
+    Slice match = null;
+    Slice noMatch = null;
+    try {
+      slice = new Slice("matchpoint");
+      match = new Slice("mat");
+      noMatch = new Slice("nomatch");
+
+      //assertThat(slice.startsWith(match)).isTrue();
+      assertThat(slice.startsWith(noMatch)).isFalse();
+    } finally {
+      if (slice != null) {
+        slice.dispose();
+      }
+      if (match != null) {
+        match.dispose();
+      }
+      if (noMatch != null) {
+        noMatch.dispose();
+      }
+    }
+  }
+
+  @Test
+  public void sliceToString() {
+    Slice slice = null;
+    try {
+      slice = new Slice("stringTest");
+      assertThat(slice.toString()).isEqualTo("stringTest");
+      assertThat(slice.toString(true)).isNotEqualTo("");
+    } finally {
+      if (slice != null) {
+        slice.dispose();
+      }
+    }
+  }
+}

java/rocksjni/slice.cc

@@ -25,9 +25,15 @@
  * Signature: (Ljava/lang/String;)V
  */
 void Java_org_rocksdb_AbstractSlice_createNewSliceFromString(
-    JNIEnv* env, jobject jobj, jstring str) {
+    JNIEnv* env, jobject jobj, jstring jstr) {
-  const std::string s = rocksdb::JniUtil::copyString(env, str);
+
-  const rocksdb::Slice* slice = new rocksdb::Slice(s);
+  const char* str = env->GetStringUTFChars(jstr, 0);
+  const int len = strlen(str);
+  char* buf = new char[len];
+  memcpy(buf, str, len);
+  env->ReleaseStringUTFChars(jstr, str);
+
+  rocksdb::Slice* slice = new rocksdb::Slice(buf);
   rocksdb::AbstractSliceJni::setHandle(env, jobj, slice);
 }
 
@@ -85,8 +91,8 @@ jint Java_org_rocksdb_AbstractSlice_compare0(
  */
 jboolean Java_org_rocksdb_AbstractSlice_startsWith0(
     JNIEnv* env, jobject jobj, jlong handle, jlong otherHandle) {
-  const rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  auto slice = reinterpret_cast<rocksdb::Slice*>(handle);
-  const rocksdb::Slice* otherSlice =
+  auto otherSlice =
     reinterpret_cast<rocksdb::Slice*>(otherHandle);
   return slice->starts_with(*otherSlice);
 }
@@ -130,19 +136,20 @@ void Java_org_rocksdb_Slice_createNewSlice0(
 void Java_org_rocksdb_Slice_createNewSlice1(
     JNIEnv * env, jobject jobj, jbyteArray data) {
 
-  const int len = env->GetArrayLength(data);
+  const int len = env->GetArrayLength(data) + 1;
 
   jboolean isCopy;
   jbyte* ptrData = env->GetByteArrayElements(data, &isCopy);
-  const char* buf = new char[len];
+  char* buf = new char[len];
-  memcpy(const_cast<char*>(buf), ptrData, len);
+
+  memcpy(buf, ptrData, len - 1);
+  buf[len-1]='\0';
 
   const rocksdb::Slice* slice =
-    new rocksdb::Slice(buf, env->GetArrayLength(data));
+      new rocksdb::Slice(buf, len - 1);
-  rocksdb::AbstractSliceJni::setHandle(env, jobj, slice);
 
+  rocksdb::AbstractSliceJni::setHandle(env, jobj, slice);
   env->ReleaseByteArrayElements(data, ptrData, JNI_ABORT);
-
   // NOTE: buf will be deleted in the org.rocksdb.Slice#dispose method
 }
 
@@ -153,11 +160,11 @@ void Java_org_rocksdb_Slice_createNewSlice1(
  */
 jbyteArray Java_org_rocksdb_Slice_data0(
     JNIEnv* env, jobject jobj, jlong handle) {
-  const rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  auto slice = reinterpret_cast<rocksdb::Slice*>(handle);
   const int len = static_cast<int>(slice->size());
   const jbyteArray data = env->NewByteArray(len);
   env->SetByteArrayRegion(data, 0, len,
-    reinterpret_cast<jbyte*>(const_cast<char*>(slice->data())));
+    reinterpret_cast<const jbyte*>(slice->data()));
   return data;
 }
 
@@ -172,10 +179,6 @@ void Java_org_rocksdb_Slice_disposeInternalBuf(
     delete [] slice->data_;
 }
 
-// </editor-fold>
-
-// <editor-fold desc="org.rocksdb.DirectSlice>
-
 /*
  * Class:     org_rocksdb_DirectSlice
  * Method:    createNewDirectSlice0