Incoporate PR review feedback & fix test failure

Signed-off-by: Ashish Singh <[email protected]>
opensearch-project · Apr 15, 2024 · d1ed1ad · d1ed1ad
1 parent 122bcd3
commit d1ed1ad
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 15 deletions.
diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java
@@ -237,7 +237,7 @@ String hash(PathInput pathInput) {
                 String input = pathInput.indexUUID() + pathInput.shardId() + pathInput.dataCategory().getName() + pathInput.dataType()
                     .getName();
                 long hash = FNV1a.hash64(input);
-                return longToCompositeBase64AndBinaryEncoding(hash);
+                return longToCompositeBase64AndBinaryEncoding(hash, 20);
             }
         };
 

diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java
@@ -130,15 +130,6 @@ static long urlBase64ToLong(String base64Str) {
         return ByteBuffer.wrap(hashBytes).getLong();
     }
 
-    /**
-     * Converts an input hash which occupies 64 bits of memory into a composite encoded string. The string will have 2 parts -
-     * 1. Base 64 string and 2. Binary String. We will use the first 6 bits for creating the base 64 string.
-     * For the second part, we will use the next 14 bits. For eg - A010001010100010.
-     */
-    static String longToCompositeBase64AndBinaryEncoding(long value) {
-        return longToCompositeBase64AndBinaryEncoding(value, 20);
-    }
-
     /**
      * Converts an input hash which occupies 64 bits of memory into a composite encoded string. The string will have 2 parts -
      * 1. Base 64 string and 2. Binary String. We will use the first 6 bits for creating the base 64 string.

diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java
@@ -259,7 +259,7 @@ public void testLongToCompositeUrlBase64AndBinaryEncodingUsing20Bits() {
             "610010010111111"
         );
         for (Map.Entry<Long, String> entry : longToExpectedBase64String.entrySet()) {
-            String base64Str = RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(entry.getKey());
+            String base64Str = RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(entry.getKey(), 20);
             assertEquals(entry.getValue(), base64Str);
             assertEquals(15, entry.getValue().length());
             assertEquals(longToUrlBase64(entry.getKey()).charAt(0), base64Str.charAt(0));
@@ -268,7 +268,7 @@ public void testLongToCompositeUrlBase64AndBinaryEncodingUsing20Bits() {
         int iters = randomInt(1000);
         for (int i = 0; i < iters; i++) {
             long value = randomLong();
-            assertEquals(RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(value).charAt(0), longToUrlBase64(value).charAt(0));
+            assertEquals(RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(value, 20).charAt(0), longToUrlBase64(value).charAt(0));
         }
     }
 
@@ -302,7 +302,7 @@ public void testLongToCompositeUrlBase64AndBinaryEncoding() {
             assertEquals(expectedCompositeEncoding, actualCompositeEncoding);
             assertEquals(59, expectedCompositeEncoding.length());
             assertEquals(longToUrlBase64(entry.getKey()).charAt(0), actualCompositeEncoding.charAt(0));
-            assertEquals(RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(hashValue), actualCompositeEncoding.substring(0, 15));
+            assertEquals(RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(hashValue, 20), actualCompositeEncoding.substring(0, 15));
 
             Long computedHashValue = compositeUrlBase64BinaryEncodingToLong(actualCompositeEncoding);
             assertEquals(hashValue, computedHashValue);

diff --git a/...va/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshotTests.java b/...va/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshotTests.java
@@ -482,13 +482,13 @@ public void testFromXContentInvalid() throws IOException {
                 case 12:
                     version = "1";
                     pathHashAlgorithm = PathHashAlgorithm.FNV_1A_COMPOSITE_1;
-                    failure = "Invalid combination of pathType=null pathHashAlgorithm=FNV_1A_COMPOSITE for version=1";
+                    failure = "Invalid combination of pathType=null pathHashAlgorithm=FNV_1A_COMPOSITE_1 for version=1";
                     break;
                 case 13:
                     version = "2";
                     pathType = PathType.FIXED;
                     pathHashAlgorithm = PathHashAlgorithm.FNV_1A_COMPOSITE_1;
-                    failure = "Invalid combination of pathType=FIXED pathHashAlgorithm=FNV_1A_COMPOSITE for version=2";
+                    failure = "Invalid combination of pathType=FIXED pathHashAlgorithm=FNV_1A_COMPOSITE_1 for version=2";
                     break;
                 case 14:
                     version = "2";