Address PR comments.

facebook · Apr 9, 2024 · 5cc529a · 5cc529a
1 parent 2c0df97
commit 5cc529a
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 31 deletions.
diff --git a/java/CMakeLists.txt b/java/CMakeLists.txt
@@ -21,7 +21,7 @@ set(JNI_NATIVE_SOURCES
         rocksjni/cassandra_value_operator.cc
         rocksjni/checkpoint.cc
         rocksjni/clock_cache.cc
-        rocksjni/columncamilydescriptor.cc
+        rocksjni/columnfamilydescriptor.cc
         rocksjni/columnfamilyhandle.cc
         rocksjni/compact_range_options.cc
         rocksjni/compaction_filter.cc

diff --git a/java/rocksjni/columncamilydescriptor.cc → java/rocksjni/columnfamilydescriptor.cc b/java/rocksjni/columncamilydescriptor.cc → java/rocksjni/columnfamilydescriptor.cc
@@ -49,23 +49,6 @@ jbyteArray Java_org_rocksdb_ColumnFamilyDescriptor_getName(
   return ROCKSDB_NAMESPACE::JniUtil::copyBytes(env, cf_options->name);
 }
 
-/*
- * Class:     org_rocksdb_ColumnFamilyDescriptor
- * Method:    getOption
- * Signature: (J)J
- */
-jlong Java_org_rocksdb_ColumnFamilyDescriptor_getOption(
-    JNIEnv*, jclass, jlong jcf_descriptor_handle) {
-  auto cf_options =
-      reinterpret_cast<ROCKSDB_NAMESPACE::ColumnFamilyDescriptor*>(
-          jcf_descriptor_handle);
-
-  return GET_CPLUSPLUS_POINTER(
-      &cf_options->options);  // TODO -> Should we copy options and leave it on
-                              // Java developer to delete?
-                              //  At the moment we setup owningHandle_=false
-}
-
 /*
  * Class:     org_rocksdb_ColumnFamilyDescriptor
  * Method:    disposeJni

diff --git a/java/rocksjni/rocksjni.cc b/java/rocksjni/rocksjni.cc
@@ -404,6 +404,9 @@ jlongArray Java_org_rocksdb_RocksDB_createColumnFamilies__J_3J(
 
   env->GetLongArrayRegion(jcf_descriptor_handles, 0, jlen,
                           cf_descriptor_handles.get());
+  if (env->ExceptionCheck()) {
+    return nullptr;
+  }
 
   for (jsize i = 0; i < jlen; i++) {
     auto cf_descriptor_handle =

diff --git a/java/src/main/java/org/rocksdb/ColumnFamilyDescriptor.java b/java/src/main/java/org/rocksdb/ColumnFamilyDescriptor.java
@@ -38,8 +38,8 @@ public ColumnFamilyDescriptor(final byte[] columnFamilyName,
   }
 
   private ColumnFamilyDescriptor(final byte[] columnFamilyName,
-      final ColumnFamilyOptions columnFamilyOptions, boolean implicitlyCreatedColumnFamilyOptions)
-      throws RocksDBException {
+      final ColumnFamilyOptions columnFamilyOptions,
+      final boolean implicitlyCreatedColumnFamilyOptions) throws RocksDBException {
     super(createNativeInstance(columnFamilyName, columnFamilyOptions));
     this.columnFamilyOptions = columnFamilyOptions;
     this.implicitlyCreatedColumnFamilyOptions = implicitlyCreatedColumnFamilyOptions;
@@ -92,9 +92,12 @@ public int hashCode() {
   }
   @Override
   protected void disposeInternal(final long handle) {
-    disposeJni(nativeHandle_);
-    if (implicitlyCreatedColumnFamilyOptions) {
-      columnFamilyOptions.close();
+    try {
+      if (implicitlyCreatedColumnFamilyOptions) {
+        columnFamilyOptions.close();
+      }
+    } finally {
+      disposeJni(handle);
     }
   }
   private static native void disposeJni(final long nativeHandle);

diff --git a/java/src/test/java/org/rocksdb/ColumnFamilyDescriptorTest.java b/java/src/test/java/org/rocksdb/ColumnFamilyDescriptorTest.java
@@ -12,7 +12,7 @@ public class ColumnFamilyDescriptorTest {
 
   @Test
   public void newInstance() throws RocksDBException {
-    try (ColumnFamilyDescriptor cf_descriptor =
+    try (final ColumnFamilyDescriptor cf_descriptor =
              new ColumnFamilyDescriptor("new_CF".getBytes(StandardCharsets.UTF_8))) {
       assertThat(cf_descriptor.getName()).isEqualTo("new_CF".getBytes(StandardCharsets.UTF_8));
       assertThat(cf_descriptor.getOptions()).isNotNull();
@@ -23,19 +23,19 @@ public void newInstance() throws RocksDBException {
   @Test
   public void fromHandle2Descriptor() throws RocksDBException {
     try (final RocksDB db = RocksDB.open(dbFolder.getRoot().getAbsolutePath());
-         ColumnFamilyDescriptor cfDescriptor =
+         final ColumnFamilyDescriptor cfDescriptor =
              new ColumnFamilyDescriptor("myCf".getBytes(StandardCharsets.UTF_8))) {
-      ColumnFamilyHandle cfHandle = db.createColumnFamily(cfDescriptor);
+      final ColumnFamilyHandle cfHandle = db.createColumnFamily(cfDescriptor);
 
-      ColumnFamilyDescriptor result =
+      final ColumnFamilyDescriptor result =
           cfHandle.getDescriptor(); // This creates new ColumnFamilyDescriptor in JNI code.
                                     //   instance of ColumnFamilyOptions is created with underlying
                                     //   C++ allocation and ColumnFamilyDescriptor must free this
                                     //   memory.
 
       assertThat(result).isNotNull();
 
-      ColumnFamilyOptions cfOptions = result.getOptions();
+      final ColumnFamilyOptions cfOptions = result.getOptions();
       assertThat(cfOptions.isOwningHandle()).isTrue();
 
       result.close(); // Closing  ColumnFamilyDescriptor must
@@ -46,11 +46,11 @@ public void fromHandle2Descriptor() throws RocksDBException {
 
   @Test
   public void closeTransitiveResources() throws RocksDBException {
-    ColumnFamilyDescriptor cfDescriptor =
+    final ColumnFamilyDescriptor cfDescriptor =
         new ColumnFamilyDescriptor("myCf".getBytes(StandardCharsets.UTF_8));
     assertThat(cfDescriptor.isOwningHandle()).isTrue();
 
-    ColumnFamilyOptions cfOptions = cfDescriptor.getOptions();
+    final ColumnFamilyOptions cfOptions = cfDescriptor.getOptions();
 
     assertThat(cfOptions.isOwningHandle()).isTrue();
 

diff --git a/src.mk b/src.mk
@@ -651,7 +651,7 @@ JNI_NATIVE_SOURCES =                                          \
   java/rocksjni/checkpoint.cc                                 \
   java/rocksjni/clock_cache.cc                                \
   java/rocksjni/cache.cc                                      \
-  java/rocksjni/columncamilydescriptor.cc                     \
+  java/rocksjni/columnfamilydescriptor.cc                     \
   java/rocksjni/columnfamilyhandle.cc                         \
   java/rocksjni/compact_range_options.cc                      \
   java/rocksjni/compaction_filter.cc                          \