Merge pull request #437 from fyrz/RocksJava-SliceTests-Fixes

[RocksJava] Slice / DirectSlice improvements

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/CompressionType.java

@@ -29,6 +29,8 @@ public enum CompressionType {
    * <p>If library cannot be found the enumeration
    * value {@code NO_COMPRESSION} will be returned.</p>
    *
+   * @param libraryName compression library name.
+   *
    * @return CompressionType instance.
    */
   public static CompressionType getCompressionType(String libraryName) {

java/org/rocksdb/DBOptionsInterface.java

@@ -15,8 +15,8 @@ public interface DBOptionsInterface {
    * <p>You almost definitely want to call this function if your system is
    * bottlenecked by RocksDB.</p>
    *
-   * @param The total number of threads to be used by RocksDB. A good value
+   * @param totalThreads The total number of threads to be used by RocksDB.
-   *            is the number of cores.
+   *     A good value is the number of cores.
    *
    * @return the instance of the current Options
    */

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 auto* str = env->GetStringUTFChars(jstr, 0);
+  const int len = strlen(str);
+  char* buf = new char[len];
+  memcpy(buf, str, len);
+  env->ReleaseStringUTFChars(jstr, str);
+
+  const auto* slice = new rocksdb::Slice(buf);
   rocksdb::AbstractSliceJni::setHandle(env, jobj, slice);
 }
 
@@ -38,7 +44,7 @@ void Java_org_rocksdb_AbstractSlice_createNewSliceFromString(
  */
 jint Java_org_rocksdb_AbstractSlice_size0(
     JNIEnv* env, jobject jobj, jlong handle) {
-  const rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  const auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
   return static_cast<jint>(slice->size());
 }
 
@@ -49,7 +55,7 @@ jint Java_org_rocksdb_AbstractSlice_size0(
  */
 jboolean Java_org_rocksdb_AbstractSlice_empty0(
     JNIEnv* env, jobject jobj, jlong handle) {
-  const rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  const auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
   return slice->empty();
 }
 
@@ -60,7 +66,7 @@ jboolean Java_org_rocksdb_AbstractSlice_empty0(
  */
 jstring Java_org_rocksdb_AbstractSlice_toString0(
     JNIEnv* env, jobject jobj, jlong handle, jboolean hex) {
-  const rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  const auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
   const std::string s = slice->ToString(hex);
   return env->NewStringUTF(s.c_str());
 }
@@ -72,8 +78,8 @@ jstring Java_org_rocksdb_AbstractSlice_toString0(
  */
 jint Java_org_rocksdb_AbstractSlice_compare0(
     JNIEnv* env, jobject jobj, jlong handle, jlong otherHandle) {
-  const rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  const auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
-  const rocksdb::Slice* otherSlice =
+  const auto* otherSlice =
     reinterpret_cast<rocksdb::Slice*>(otherHandle);
   return slice->compare(*otherSlice);
 }
@@ -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);
+  const auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
-  const rocksdb::Slice* otherSlice =
+  const auto* otherSlice =
     reinterpret_cast<rocksdb::Slice*>(otherHandle);
   return slice->starts_with(*otherSlice);
 }
@@ -118,7 +124,7 @@ void Java_org_rocksdb_Slice_createNewSlice0(
   jbyte* ptrData = new jbyte[len];
   env->GetByteArrayRegion(data, offset, len, ptrData);
 
-  const rocksdb::Slice* slice = new rocksdb::Slice((const char*)ptrData, len);
+  const auto* slice = new rocksdb::Slice((const char*)ptrData, len);
   rocksdb::AbstractSliceJni::setHandle(env, jobj, slice);
 }
 
@@ -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);
 
-  const rocksdb::Slice* slice =
+  memcpy(buf, ptrData, len - 1);
-    new rocksdb::Slice(buf, env->GetArrayLength(data));
+  buf[len-1]='\0';
-  rocksdb::AbstractSliceJni::setHandle(env, jobj, slice);
 
-  env->ReleaseByteArrayElements(data, ptrData, JNI_ABORT);
+  const auto* slice =
+      new rocksdb::Slice(buf, len - 1);
 
+  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);
+  const 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;
 }
 
@@ -168,8 +175,8 @@ jbyteArray Java_org_rocksdb_Slice_data0(
  */
 void Java_org_rocksdb_Slice_disposeInternalBuf(
     JNIEnv * env, jobject jobj, jlong handle) {
-    const rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  const auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
-    delete [] slice->data_;
+  delete [] slice->data_;
 }
 
 // </editor-fold>
@@ -183,9 +190,9 @@ void Java_org_rocksdb_Slice_disposeInternalBuf(
  */
 void Java_org_rocksdb_DirectSlice_createNewDirectSlice0(
     JNIEnv* env, jobject jobj, jobject data, jint length) {
-  const char* ptrData =
+  const auto* ptrData =
-    reinterpret_cast<char*>(env->GetDirectBufferAddress(data));
+     reinterpret_cast<char*>(env->GetDirectBufferAddress(data));
-  const rocksdb::Slice* slice = new rocksdb::Slice(ptrData, length);
+  const auto* slice = new rocksdb::Slice(ptrData, length);
   rocksdb::AbstractSliceJni::setHandle(env, jobj, slice);
 }
 
@@ -196,9 +203,9 @@ void Java_org_rocksdb_DirectSlice_createNewDirectSlice0(
  */
 void Java_org_rocksdb_DirectSlice_createNewDirectSlice1(
     JNIEnv* env, jobject jobj, jobject data) {
-  const char* ptrData =
+  const auto* ptrData =
     reinterpret_cast<char*>(env->GetDirectBufferAddress(data));
-  const rocksdb::Slice* slice = new rocksdb::Slice(ptrData);
+  const auto* slice = new rocksdb::Slice(ptrData);
   rocksdb::AbstractSliceJni::setHandle(env, jobj, slice);
 }
 
@@ -209,7 +216,7 @@ void Java_org_rocksdb_DirectSlice_createNewDirectSlice1(
  */
 jobject Java_org_rocksdb_DirectSlice_data0(
     JNIEnv* env, jobject jobj, jlong handle) {
-  const rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  const auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
   return env->NewDirectByteBuffer(const_cast<char*>(slice->data()),
     slice->size());
 }
@@ -221,7 +228,7 @@ jobject Java_org_rocksdb_DirectSlice_data0(
  */
 jbyte Java_org_rocksdb_DirectSlice_get0(
     JNIEnv* env, jobject jobj, jlong handle, jint offset) {
-  rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  const auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
   return (*slice)[offset];
 }
 
@@ -232,7 +239,7 @@ jbyte Java_org_rocksdb_DirectSlice_get0(
  */
 void Java_org_rocksdb_DirectSlice_clear0(
     JNIEnv* env, jobject jobj, jlong handle) {
-  rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
   delete [] slice->data_;
   slice->clear();
 }
@@ -244,7 +251,7 @@ void Java_org_rocksdb_DirectSlice_clear0(
  */
 void Java_org_rocksdb_DirectSlice_removePrefix0(
     JNIEnv* env, jobject jobj, jlong handle, jint length) {
-  rocksdb::Slice* slice = reinterpret_cast<rocksdb::Slice*>(handle);
+  auto* slice = reinterpret_cast<rocksdb::Slice*>(handle);
   slice->remove_prefix(length);
 }