From 5f915b447ddcd68c9af424ca9bf4b247d85bcca8 Mon Sep 17 00:00:00 2001
From: Brendan MacDonell <bmacdonell@sightmachine.com>
Date: Tue, 25 Oct 2022 19:25:44 -0700
Subject: [PATCH] Fix ChecksumType::kXXH3 in the Java API (#10862)

Summary:
While PR#9749 nominally added support for XXH3 in the Java API, it did not update the `toCppChecksumType` method. As a result, setting the checksum type to XXH3 actually set it to CRC32c instead.

This commit adds the missing entry to portal.h, and also updates the tests so that they verify the options passed to RocksDB, instead of simply checking that the getter returns the value set by the setter.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10862

Reviewed By: pdillinger

Differential Revision: D40665031

Pulled By: ajkr

fbshipit-source-id: 2834419b3361a4bac47db3b858951fb451b5bdc8
---
 java/rocksjni/portal.h                        |  2 +
 .../rocksdb/BlockBasedTableConfigTest.java    | 75 ++++++++++++++++++-
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/java/rocksjni/portal.h b/java/rocksjni/portal.h
index 02a426262..dea119a59 100644
--- a/java/rocksjni/portal.h
+++ b/java/rocksjni/portal.h
@@ -6695,6 +6695,8 @@ class ChecksumTypeJni {
         return ROCKSDB_NAMESPACE::ChecksumType::kxxHash;
       case 0x3:
         return ROCKSDB_NAMESPACE::ChecksumType::kxxHash64;
+      case 0x4:
+        return ROCKSDB_NAMESPACE::ChecksumType::kXXH3;
       default:
         // undefined/default
         return ROCKSDB_NAMESPACE::ChecksumType::kCRC32c;
diff --git a/java/src/test/java/org/rocksdb/BlockBasedTableConfigTest.java b/java/src/test/java/org/rocksdb/BlockBasedTableConfigTest.java
index defffa6c7..330881764 100644
--- a/java/src/test/java/org/rocksdb/BlockBasedTableConfigTest.java
+++ b/java/src/test/java/org/rocksdb/BlockBasedTableConfigTest.java
@@ -9,6 +9,10 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.stream.Stream;
 import org.junit.ClassRule;
 import org.junit.Ignore;
 import org.junit.Rule;
@@ -29,7 +33,6 @@ public class BlockBasedTableConfigTest {
     blockBasedTableConfig.setCacheIndexAndFilterBlocks(true);
     assertThat(blockBasedTableConfig.cacheIndexAndFilterBlocks()).
         isTrue();
-
   }
 
   @Test
@@ -95,6 +98,76 @@ public class BlockBasedTableConfigTest {
     assertThat(blockBasedTableConfig.checksumType()).isEqualTo(ChecksumType.kXXH3);
   }
 
+  @Test
+  public void jniPortal() throws Exception {
+    // Verifies that the JNI layer is correctly translating options.
+    // Since introspecting the options requires creating a database, the checks
+    // cover multiple options at the same time.
+
+    final BlockBasedTableConfig tableConfig = new BlockBasedTableConfig();
+
+    tableConfig.setIndexType(IndexType.kBinarySearch);
+    tableConfig.setDataBlockIndexType(DataBlockIndexType.kDataBlockBinarySearch);
+    tableConfig.setChecksumType(ChecksumType.kNoChecksum);
+    try (final Options options = new Options().setTableFormatConfig(tableConfig)) {
+      String opts = getOptionAsString(options);
+      assertThat(opts).contains("index_type=kBinarySearch");
+      assertThat(opts).contains("data_block_index_type=kDataBlockBinarySearch");
+      assertThat(opts).contains("checksum=kNoChecksum");
+    }
+
+    tableConfig.setIndexType(IndexType.kHashSearch);
+    tableConfig.setDataBlockIndexType(DataBlockIndexType.kDataBlockBinaryAndHash);
+    tableConfig.setChecksumType(ChecksumType.kCRC32c);
+    try (final Options options = new Options().setTableFormatConfig(tableConfig)) {
+      options.useCappedPrefixExtractor(1); // Needed to use kHashSearch
+      String opts = getOptionAsString(options);
+      assertThat(opts).contains("index_type=kHashSearch");
+      assertThat(opts).contains("data_block_index_type=kDataBlockBinaryAndHash");
+      assertThat(opts).contains("checksum=kCRC32c");
+    }
+
+    tableConfig.setIndexType(IndexType.kTwoLevelIndexSearch);
+    tableConfig.setChecksumType(ChecksumType.kxxHash);
+    try (final Options options = new Options().setTableFormatConfig(tableConfig)) {
+      String opts = getOptionAsString(options);
+      assertThat(opts).contains("index_type=kTwoLevelIndexSearch");
+      assertThat(opts).contains("checksum=kxxHash");
+    }
+
+    tableConfig.setIndexType(IndexType.kBinarySearchWithFirstKey);
+    tableConfig.setChecksumType(ChecksumType.kxxHash64);
+    try (final Options options = new Options().setTableFormatConfig(tableConfig)) {
+      String opts = getOptionAsString(options);
+      assertThat(opts).contains("index_type=kBinarySearchWithFirstKey");
+      assertThat(opts).contains("checksum=kxxHash64");
+    }
+
+    tableConfig.setChecksumType(ChecksumType.kXXH3);
+    try (final Options options = new Options().setTableFormatConfig(tableConfig)) {
+      String opts = getOptionAsString(options);
+      assertThat(opts).contains("checksum=kXXH3");
+    }
+  }
+
+  private String getOptionAsString(Options options) throws Exception {
+    options.setCreateIfMissing(true);
+    String dbPath = dbFolder.getRoot().getAbsolutePath();
+    String result;
+    try (final RocksDB db = RocksDB.open(options, dbPath);
+         final Stream<Path> pathStream = Files.walk(Paths.get(dbPath))) {
+      Path optionsPath =
+          pathStream
+              .filter(p -> p.getFileName().toString().startsWith("OPTIONS"))
+              .findAny()
+              .orElseThrow(() -> new AssertionError("Missing options file"));
+      byte[] optionsData = Files.readAllBytes(optionsPath);
+      result = new String(optionsData, StandardCharsets.UTF_8);
+    }
+    RocksDB.destroyDB(dbPath, options);
+    return result;
+  }
+
   @Test
   public void noBlockCache() {
     final BlockBasedTableConfig blockBasedTableConfig = new BlockBasedTableConfig();