Merge pull request #495 from fyrz/RocksJava-CF-Name-Byte-Handling

[RocksJava] CF Name shall handle bytes correctly

java/rocksjni/rocksjni.cc

@@ -77,8 +77,6 @@ jobject
   const char* db_path = env->GetStringUTFChars(jdb_path, 0);
 
   std::vector<jbyte*> cfnames_to_free;
-  // the zero-terminated version of cfnames_to_free.
-  std::vector<char*> c_cfnames_to_free;
   std::vector<jbyteArray> jcfnames_for_free;
 
   std::vector<rocksdb::ColumnFamilyDescriptor> column_families;
@@ -94,8 +92,8 @@ jobject
       jobject jcf_descriptor = env->CallObjectMethod(iteratorObj,
           rocksdb::ListJni::getNextMethod(env));
       // get ColumnFamilyName
-      jbyteArray byteArray = static_cast<jbyteArray>(env->CallObjectMethod(
-          jcf_descriptor,
+      jbyteArray cf_name_in_byte_array = static_cast<jbyteArray>(
+          env->CallObjectMethod(jcf_descriptor,
           rocksdb::ColumnFamilyDescriptorJni::getColumnFamilyNameMethod(
               env)));
       // get CF Options
@@ -105,18 +103,14 @@ jobject
       rocksdb::ColumnFamilyOptions* cfOptions =
           rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
-      jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-      const int len = env->GetArrayLength(byteArray) + 1;
-      char* c_cfname = new char[len];
-      memcpy(c_cfname, cfname, len - 1);
-      c_cfname[len - 1] = 0;
+      jbyte* cfname = env->GetByteArrayElements(cf_name_in_byte_array, 0);
+      const int len = env->GetArrayLength(cf_name_in_byte_array);
 
       // free allocated cfnames after call to open
       cfnames_to_free.push_back(cfname);
-      c_cfnames_to_free.push_back(c_cfname);
-      jcfnames_for_free.push_back(byteArray);
+      jcfnames_for_free.push_back(cf_name_in_byte_array);
       column_families.push_back(rocksdb::ColumnFamilyDescriptor(
-          c_cfname, *cfOptions));
+          std::string(reinterpret_cast<char *>(cfname), len), *cfOptions));
   }
 
   rocksdb::Status s = rocksdb::DB::OpenForReadOnly(*opt,
@@ -127,8 +121,6 @@ jobject
       i != cfnames_to_free.size(); i++) {
     // free  cfnames
     env->ReleaseByteArrayElements(jcfnames_for_free[i], cfnames_to_free[i], 0);
-    // free c_cfnames
-    delete[] c_cfnames_to_free[i];
   }
 
   // check if open operation was successful
@@ -170,8 +162,6 @@ jobject Java_org_rocksdb_RocksDB_open__JLjava_lang_String_2Ljava_util_List_2I(
   const char* db_path = env->GetStringUTFChars(jdb_path, 0);
 
   std::vector<jbyte*> cfnames_to_free;
-  // the zero-terminated version of cfnames_to_free.
-  std::vector<char*> c_cfnames_to_free;
   std::vector<jbyteArray> jcfnames_for_free;
 
   std::vector<rocksdb::ColumnFamilyDescriptor> column_families;
@@ -187,8 +177,8 @@ jobject Java_org_rocksdb_RocksDB_open__JLjava_lang_String_2Ljava_util_List_2I(
       jobject jcf_descriptor = env->CallObjectMethod(iteratorObj,
           rocksdb::ListJni::getNextMethod(env));
       // get ColumnFamilyName
-      jbyteArray byteArray = static_cast<jbyteArray>(env->CallObjectMethod(
-          jcf_descriptor,
+      jbyteArray cf_name_in_byte_array = static_cast<jbyteArray>(
+          env->CallObjectMethod(jcf_descriptor,
           rocksdb::ColumnFamilyDescriptorJni::getColumnFamilyNameMethod(
               env)));
       // get CF Options
@@ -198,18 +188,14 @@ jobject Java_org_rocksdb_RocksDB_open__JLjava_lang_String_2Ljava_util_List_2I(
       rocksdb::ColumnFamilyOptions* cfOptions =
           rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
-      jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-      const int len = env->GetArrayLength(byteArray) + 1;
-      char* c_cfname = new char[len];
-      memcpy(c_cfname, cfname, len - 1);
-      c_cfname[len - 1] = 0;
+      jbyte* cfname = env->GetByteArrayElements(cf_name_in_byte_array, 0);
+      const int len = env->GetArrayLength(cf_name_in_byte_array);
 
       // free allocated cfnames after call to open
       cfnames_to_free.push_back(cfname);
-      c_cfnames_to_free.push_back(c_cfname);
-      jcfnames_for_free.push_back(byteArray);
+      jcfnames_for_free.push_back(cf_name_in_byte_array);
       column_families.push_back(rocksdb::ColumnFamilyDescriptor(
-          c_cfname, *cfOptions));
+          std::string(reinterpret_cast<char *>(cfname), len), *cfOptions));
   }
 
   rocksdb::Status s = rocksdb::DB::Open(*opt, db_path, column_families,
@@ -220,8 +206,6 @@ jobject Java_org_rocksdb_RocksDB_open__JLjava_lang_String_2Ljava_util_List_2I(
       i != cfnames_to_free.size(); i++) {
     // free  cfnames
     env->ReleaseByteArrayElements(jcfnames_for_free[i], cfnames_to_free[i], 0);
-    // free c_cfnames
-    delete[] c_cfnames_to_free[i];
   }
 
   // check if open operation was successful
@@ -281,7 +265,7 @@ jobject Java_org_rocksdb_RocksDB_listColumnFamilies(
           env->NewByteArray(static_cast<jsize>(column_family_names[i].size()));
       env->SetByteArrayRegion(
           jcf_value, 0, static_cast<jsize>(column_family_names[i].size()),
-          reinterpret_cast<const jbyte*>(column_family_names[i].c_str()));
+          reinterpret_cast<const jbyte*>(column_family_names[i].data()));
       env->CallBooleanMethod(jvalue_list,
           rocksdb::ListJni::getListAddMethodId(env), jcf_value);
     }
@@ -1294,15 +1278,11 @@ jlong Java_org_rocksdb_RocksDB_createColumnFamily(
       rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
   jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-  const int len = env->GetArrayLength(byteArray) + 1;
-  char* c_cfname = new char[len];
-  memcpy(c_cfname, cfname, len - 1);
-  c_cfname[len - 1] = 0;
+  const int len = env->GetArrayLength(byteArray);
 
   rocksdb::Status s = db_handle->CreateColumnFamily(
-      *cfOptions, c_cfname, &handle);
+      *cfOptions, std::string(reinterpret_cast<char *>(cfname), len), &handle);
   env->ReleaseByteArrayElements(byteArray, cfname, 0);
-  delete[] c_cfname;
 
   if (s.ok()) {
     return reinterpret_cast<jlong>(handle);

java/rocksjni/ttl.cc

@@ -57,8 +57,6 @@ jobject
   const char* db_path = env->GetStringUTFChars(jdb_path, 0);
 
   std::vector<jbyte*> cfnames_to_free;
-  // the zero-terminated version of cfnames_to_free.
-  std::vector<char*> c_cfnames_to_free;
   std::vector<jbyteArray> jcfnames_for_free;
 
   std::vector<rocksdb::ColumnFamilyDescriptor> column_families;
@@ -87,17 +85,13 @@ jobject
           rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
       jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-      const int len = env->GetArrayLength(byteArray) + 1;
-      char* c_cfname = new char[len];
-      memcpy(c_cfname, cfname, len - 1);
-      c_cfname[len - 1] = 0;
+      const int len = env->GetArrayLength(byteArray);
 
       // free allocated cfnames after call to open
       cfnames_to_free.push_back(cfname);
-      c_cfnames_to_free.push_back(c_cfname);
       jcfnames_for_free.push_back(byteArray);
       column_families.push_back(rocksdb::ColumnFamilyDescriptor(
-          c_cfname, *cfOptions));
+          std::string(reinterpret_cast<char *>(cfname), len), *cfOptions));
   }
   // get iterator for TTL values
   iteratorObj = env->CallObjectMethod(
@@ -122,8 +116,6 @@ jobject
       i != cfnames_to_free.size(); i++) {
     // free  cfnames
     env->ReleaseByteArrayElements(jcfnames_for_free[i], cfnames_to_free[i], 0);
-    // free c_cfnames
-    delete[] c_cfnames_to_free[i];
   }
 
   // check if open operation was successful
@@ -176,15 +168,12 @@ jlong Java_org_rocksdb_TtlDB_createColumnFamilyWithTtl(
       rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
   jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-  const int len = env->GetArrayLength(byteArray) + 1;
-  char* c_cfname = new char[len];
-  memcpy(c_cfname, cfname, len - 1);
-  c_cfname[len - 1] = 0;
+  const int len = env->GetArrayLength(byteArray);
 
   rocksdb::Status s = db_handle->CreateColumnFamilyWithTtl(
-      *cfOptions, c_cfname, &handle, jttl);
+      *cfOptions, std::string(reinterpret_cast<char *>(cfname),
+          len), &handle, jttl);
   env->ReleaseByteArrayElements(byteArray, cfname, 0);
-  delete[] c_cfname;
 
   if (s.ok()) {
     return reinterpret_cast<jlong>(handle);

java/src/test/java/org/rocksdb/ColumnFamilyTest.java

@@ -5,10 +5,7 @@
 
 package org.rocksdb;
 
-import java.util.HashMap;
-import java.util.List;
-import java.util.ArrayList;
-import java.util.Map;
+import java.util.*;
 
 import org.junit.ClassRule;
 import org.junit.Rule;
@@ -604,4 +601,77 @@ public class ColumnFamilyTest {
     }
   }
 
+  @Test
+  public void testByteCreateFolumnFamily() throws RocksDBException {
+    RocksDB db = null;
+    Options options = null;
+    try {
+      options = new Options().setCreateIfMissing(true);
+      db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath());
+
+      byte[] b0 = new byte[] { (byte)0x00 };
+      byte[] b1 = new byte[] { (byte)0x01 };
+      byte[] b2 = new byte[] { (byte)0x02 };
+      db.createColumnFamily(new ColumnFamilyDescriptor(b0));
+      db.createColumnFamily(new ColumnFamilyDescriptor(b1));
+      List<byte[]> families = RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath());
+      assertThat(families).contains("default".getBytes(), b0, b1);
+      db.createColumnFamily(new ColumnFamilyDescriptor(b2));
+    } finally {
+      if (db != null) {
+        db.close();
+      }
+      if (options != null) {
+        options.dispose();
+      }
+    }
+  }
+
+  @Test
+  public void testCFNamesWithZeroBytes() throws RocksDBException {
+    RocksDB db = null;
+    Options options = null;
+    try {
+      options = new Options().setCreateIfMissing(true);
+      db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath());
+
+      byte[] b0 = new byte[] { 0, 0 };
+      byte[] b1 = new byte[] { 0, 1 };
+      db.createColumnFamily(new ColumnFamilyDescriptor(b0));
+      db.createColumnFamily(new ColumnFamilyDescriptor(b1));
+      List<byte[]> families = RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath());
+      assertThat(families).contains("default".getBytes(), b0, b1);
+    } finally {
+      if (db != null) {
+        db.close();
+      }
+      if (options != null) {
+        options.dispose();
+      }
+    }
+  }
+
+  @Test
+  public void testCFNameSimplifiedChinese() throws RocksDBException {
+    RocksDB db = null;
+    Options options = null;
+    try {
+      options = new Options().setCreateIfMissing(true);
+      db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath());
+      final String simplifiedChinese = "简体字";
+      db.createColumnFamily(new ColumnFamilyDescriptor(simplifiedChinese.getBytes()));
+
+      List<byte[]> families = RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath());
+      assertThat(families).contains("default".getBytes(), simplifiedChinese.getBytes());
+    } finally {
+      if (db != null) {
+        db.close();
+      }
+      if (options != null) {
+        options.dispose();
+      }
+    }
+
+
+  }
 }