Valkey-8: change bitcount end param to optional (#2248)

--------- Signed-off-by: Shoham Elias <[email protected]> Signed-off-by: Shoham Elias <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> Co-authored-by: Yury-Fridlyand <[email protected]>
valkey-io · Sep 9, 2024 · 3874ad0 · 3874ad0
1 parent c8c5b83
commit 3874ad0
Show file tree

Hide file tree

Showing 16 changed files with 252 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -101,12 +101,14 @@
 * Node: Added XGROUP CREATE & XGROUP DESTROY commands ([#2084](https://github.com/valkey-io/valkey-glide/pull/2084))
 * Node: Added BZPOPMAX & BZPOPMIN command ([#2077](https://github.com/valkey-io/valkey-glide/pull/2077))
 * Node: Added XGROUP CREATECONSUMER & XGROUP DELCONSUMER commands ([#2088](https://github.com/valkey-io/valkey-glide/pull/2088))
-* Node: Added GETEX command ([#2107]((https://github.com/valkey-io/valkey-glide/pull/2107))
+* Node: Added GETEX command ([#2107](https://github.com/valkey-io/valkey-glide/pull/2107))
 * Node: Added ZINTER and ZUNION commands ([#2146](https://github.com/valkey-io/valkey-glide/pull/2146))
 * Node: Added XACK commands ([#2112](https://github.com/valkey-io/valkey-glide/pull/2112))
-* Node: Added XGROUP SETID command ([#2135]((https://github.com/valkey-io/valkey-glide/pull/2135))
+* Node: Added XGROUP SETID command ([#2135](https://github.com/valkey-io/valkey-glide/pull/2135))
 * Node: Added binary variant to string commands ([#2183](https://github.com/valkey-io/valkey-glide/pull/2183))
 * Node: Added binary variant to stream commands ([#2200](https://github.com/valkey-io/valkey-glide/pull/2200), [#2222](https://github.com/valkey-io/valkey-glide/pull/2222))
+* Python, Node, Java: change BITCOUNT end param to optional ([#2248](https://github.com/valkey-io/valkey-glide/pull/2248))
+
 
 #### Breaking Changes
 * Node: (Refactor) Convert classes to types ([#2005](https://github.com/valkey-io/valkey-glide/pull/2005))

diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java
@@ -3844,6 +3844,18 @@ public CompletableFuture<Long> bitcount(@NonNull GlideString key) {
                 BitCount, new GlideString[] {key}, this::handleLongResponse);
     }
 
+    @Override
+    public CompletableFuture<Long> bitcount(@NonNull String key, long start) {
+        return commandManager.submitNewCommand(
+                BitCount, new String[] {key, Long.toString(start)}, this::handleLongResponse);
+    }
+
+    @Override
+    public CompletableFuture<Long> bitcount(@NonNull GlideString key, long start) {
+        return commandManager.submitNewCommand(
+                BitCount, new GlideString[] {key, gs(Long.toString(start))}, this::handleLongResponse);
+    }
+
     @Override
     public CompletableFuture<Long> bitcount(@NonNull String key, long start, long end) {
         return commandManager.submitNewCommand(

diff --git a/java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java b/java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
@@ -53,6 +53,48 @@ public interface BitmapBaseCommands {
      */
     CompletableFuture<Long> bitcount(GlideString key);
 
+    /**
+     * Counts the number of set bits (population counting) in a string stored at <code>key</code>. The
+     * offset <code>start</code> is a zero-based index, with <code>0</code> being the first byte of
+     * the list, <code>1</code> being the next byte and so on. This offset can also be a negative
+     * number indicating offsets starting at the end of the list, with <code>-1</code> being the last
+     * byte of the list, <code>-2</code> being the penultimate, and so on.
+     *
+     * @since Valkey 8.0 and above
+     * @see <a href="https://valkey.io/commands/bitcount/">valkey.io</a> for details.
+     * @param key The key for the string to count the set bits of.
+     * @param start The starting offset byte index.
+     * @return The number of set bits in the string byte interval specified by <code>start</code> to
+     *     the last byte. Returns zero if the key is missing as it is treated as an empty string.
+     * @example
+     *     <pre>{@code
+     * Long payload = client.bitcount("myKey1", 1).get();
+     * assert payload == 2L; // From the second to the last bytes of the string stored at "myKey1" are 2 set bits.
+     * }</pre>
+     */
+    CompletableFuture<Long> bitcount(String key, long start);
+
+    /**
+     * Counts the number of set bits (population counting) in a string stored at <code>key</code>. The
+     * offset <code>start</code> is a zero-based index, with <code>0</code> being the first byte of
+     * the list, <code>1</code> being the next byte and so on. This offset can also be a negative
+     * number indicating offsets starting at the end of the list, with <code>-1</code> being the last
+     * byte of the list, <code>-2</code> being the penultimate, and so on.
+     *
+     * @since Valkey 8.0 and above
+     * @see <a href="https://valkey.io/commands/bitcount/">valkey.io</a> for details.
+     * @param key The key for the string to count the set bits of.
+     * @param start The starting offset byte index.
+     * @return The number of set bits in the string byte interval specified by <code>start</code> to
+     *     the last byte. Returns zero if the key is missing as it is treated as an empty string.
+     * @example
+     *     <pre>{@code
+     * Long payload = client.bitcount(gs("myKey1"), 1).get();
+     * assert payload == 2L; // From the second to the last bytes of the string stored at "myKey1" are 2 set bits.
+     * }</pre>
+     */
+    CompletableFuture<Long> bitcount(GlideString key, long start);
+
     /**
      * Counts the number of set bits (population counting) in a string stored at <code>key</code>. The
      * offsets <code>start</code> and <code>end</code> are zero-based indexes, with <code>0</code>

diff --git a/java/client/src/main/java/glide/api/models/BaseTransaction.java b/java/client/src/main/java/glide/api/models/BaseTransaction.java
@@ -5301,6 +5301,26 @@ public <ArgType> T bitcount(@NonNull ArgType key) {
         return getThis();
     }
 
+    /**
+     * Counts the number of set bits (population counting) in a string stored at <code>key</code>. The
+     * offset <code>start</code> is a zero-based index, with <code>0</code> being the first byte of
+     * the list, <code>1</code> being the next byte and so on. This offset can also be a negative
+     * number indicating offsets starting at the end of the list, with <code>-1</code> being the last
+     * byte of the list, <code>-2</code> being the penultimate, and so on.
+     *
+     * @see <a href="https://valkey.io/commands/bitcount/">valkey.io</a> for details.
+     * @param key The key for the string to count the set bits of.
+     * @param start The starting offset byte index.
+     * @return Command Response - The number of set bits in the string byte interval specified by
+     *     <code>start</code> to the last byte. Returns zero if the key is missing as it is treated as
+     *     an empty string.
+     */
+    public <ArgType> T bitcount(@NonNull ArgType key, long start) {
+        checkTypeOrThrow(key);
+        protobufTransaction.addCommands(buildCommand(BitCount, newArgsBuilder().add(key).add(start)));
+        return getThis();
+    }
+
     /**
      * Counts the number of set bits (population counting) in a string stored at <code>key</code>. The
      * offsets <code>start</code> and <code>end</code> are zero-based indexes, with <code>0</code>

diff --git a/java/client/src/test/java/glide/api/GlideClientTest.java b/java/client/src/test/java/glide/api/GlideClientTest.java
@@ -11643,6 +11643,51 @@ public void bitcount_binary_returns_success() {
         assertEquals(bitcount, payload);
     }
 
+    @SneakyThrows
+    @Test
+    public void bitcount_start_returns_success() {
+        // setup
+        String key = "testKey";
+        Long bitcount = 1L;
+        CompletableFuture<Long> testResponse = new CompletableFuture<>();
+        testResponse.complete(bitcount);
+
+        // match on protobuf request
+        when(commandManager.<Long>submitNewCommand(eq(BitCount), eq(new String[] {key, "1"}), any()))
+                .thenReturn(testResponse);
+
+        // exercise
+        CompletableFuture<Long> response = service.bitcount(key, 1);
+        Long payload = response.get();
+
+        // verify
+        assertEquals(testResponse, response);
+        assertEquals(bitcount, payload);
+    }
+
+    @SneakyThrows
+    @Test
+    public void bitcount_start_binary_returns_success() {
+        // setup
+        GlideString key = gs("testKey");
+        Long bitcount = 1L;
+        CompletableFuture<Long> testResponse = new CompletableFuture<>();
+        testResponse.complete(bitcount);
+
+        // match on protobuf request
+        when(commandManager.<Long>submitNewCommand(
+                        eq(BitCount), eq(new GlideString[] {key, gs("1")}), any()))
+                .thenReturn(testResponse);
+
+        // exercise
+        CompletableFuture<Long> response = service.bitcount(key, 1);
+        Long payload = response.get();
+
+        // verify
+        assertEquals(testResponse, response);
+        assertEquals(bitcount, payload);
+    }
+
     @SneakyThrows
     @Test
     public void bitcount_indices_returns_success() {

diff --git a/java/client/src/test/java/glide/api/models/TransactionTests.java b/java/client/src/test/java/glide/api/models/TransactionTests.java
@@ -1172,6 +1172,9 @@ InfScoreBound.NEGATIVE_INFINITY, new ScoreBoundary(3, false), new Limit(1, 2)),
         transaction.bitcount("key");
         results.add(Pair.of(BitCount, buildArgs("key")));
 
+        transaction.bitcount("key", 1);
+        results.add(Pair.of(BitCount, buildArgs("key", "1")));
+
         transaction.bitcount("key", 1, 1);
         results.add(Pair.of(BitCount, buildArgs("key", "1", "1")));
 

diff --git a/java/integTest/src/test/java/glide/SharedCommandTests.java b/java/integTest/src/test/java/glide/SharedCommandTests.java
@@ -10309,6 +10309,24 @@ public void bitcount(BaseClient client) {
                             () -> client.bitcount(key1, 5, 30, BitmapIndexType.BIT).get());
             assertTrue(executionException.getCause() instanceof RequestException);
         }
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            assertEquals(26L, client.bitcount(key1, 0).get());
+            assertEquals(4L, client.bitcount(key1, 5).get());
+            assertEquals(0L, client.bitcount(key1, 80).get());
+            assertEquals(7L, client.bitcount(key1, -2).get());
+            assertEquals(0, client.bitcount(missingKey, 5).get());
+
+            // Exception thrown due to the key holding a value with the wrong type
+            executionException =
+                    assertThrows(ExecutionException.class, () -> client.bitcount(key2, 1).get());
+            assertTrue(executionException.getCause() instanceof RequestException);
+
+        } else {
+            // Exception thrown because optional end was implemented after 8.0.0
+            executionException =
+                    assertThrows(ExecutionException.class, () -> client.bitcount(key1, 5).get());
+            assertTrue(executionException.getCause() instanceof RequestException);
+        }
     }
 
     @SneakyThrows

diff --git a/java/integTest/src/test/java/glide/TransactionTestUtilities.java b/java/integTest/src/test/java/glide/TransactionTestUtilities.java
@@ -1288,6 +1288,10 @@ private static Object[] bitmapCommands(BaseTransaction<?> transaction) {
                     .bitpos(key3, 1, 44, 50, BitmapIndexType.BIT);
         }
 
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            transaction.set(key4, "foobar").bitcount(key4, 0);
+        }
+
         var expectedResults =
                 new Object[] {
                     OK, // set(key1, "foobar")
@@ -1307,13 +1311,24 @@ private static Object[] bitmapCommands(BaseTransaction<?> transaction) {
                 };
 
         if (SERVER_VERSION.isGreaterThanOrEqualTo("7.0.0")) {
-            return concatenateArrays(
-                    expectedResults,
-                    new Object[] {
-                        OK, // set(key1, "foobar")
-                        17L, // bitcount(key, 5, 30, BitmapIndexType.BIT)
-                        46L, // bitpos(key, 1, 44, 50, BitmapIndexType.BIT)
-                    });
+            expectedResults =
+                    concatenateArrays(
+                            expectedResults,
+                            new Object[] {
+                                OK, // set(key3, "foobar")
+                                17L, // bitcount(key3, 5, 30, BitmapIndexType.BIT)
+                                46L, // bitpos(key3, 1, 44, 50, BitmapIndexType.BIT)
+                            });
+        }
+
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            expectedResults =
+                    concatenateArrays(
+                            expectedResults,
+                            new Object[] {
+                                OK, // set(key4, "foobar")
+                                26L, // bitcount(key4, 0)
+                            });
         }
         return expectedResults;
     }

diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts
@@ -6189,6 +6189,7 @@ export class BaseClient {
      * @example
      * ```typescript
      * console.log(await client.bitcount("my_key1")); // Output: 2 - The string stored at "my_key1" contains 2 set bits.
+     * console.log(await client.bitcount("my_key2", { start: 1 })); // Output: 8 - From the second to to the last bytes of the string stored at "my_key2" are contain 8 set bits.
      * console.log(await client.bitcount("my_key2", { start: 1, end: 3 })); // Output: 2 - The second to fourth bytes of the string stored at "my_key2" contain 2 set bits.
      * console.log(await client.bitcount("my_key3", { start: 1, end: 1, indexType: BitmapIndexType.BIT })); // Output: 1 - Indicates that the second bit of the string stored at "my_key3" is set.
      * console.log(await client.bitcount("my_key3", { start: -1, end: -1, indexType: BitmapIndexType.BIT })); // Output: 1 - Indicates that the last bit of the string stored at "my_key3" is set.

diff --git a/node/src/Commands.ts b/node/src/Commands.ts
@@ -2445,8 +2445,10 @@ export function createFunctionRestore(
 export type BitOffsetOptions = {
     /** The starting offset index. */
     start: number;
-    /** The ending offset index. */
-    end: number;
+    /** The ending offset index. Optional since Valkey version 8.0 and above.
+     * If not provided, it will default to the end of the string
+     */
+    end?: number;
     /**
      * The index offset type. This option can only be specified if you are using server version 7.0.0 or above.
      * Could be either {@link BitmapIndexType.BYTE} or {@link BitmapIndexType.BIT}.
@@ -2466,7 +2468,7 @@ export function createBitCount(
 
     if (options) {
         args.push(options.start.toString());
-        args.push(options.end.toString());
+        if (options.end !== undefined) args.push(options.end.toString());
         if (options.indexType) args.push(options.indexType);
     }
 

diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts
@@ -8584,6 +8584,42 @@ export function runBaseTests(config: {
                         }),
                     ).rejects.toThrow(RequestError);
                 }
+
+                if (cluster.checkIfServerVersionLessThan("7.9.0")) {
+                    await expect(
+                        client.bitcount(key1, {
+                            start: 2,
+                        }),
+                    ).rejects.toThrow();
+                } else {
+                    expect(
+                        await client.bitcount(key1, {
+                            start: 0,
+                        }),
+                    ).toEqual(26);
+                    expect(
+                        await client.bitcount(key1, {
+                            start: 5,
+                        }),
+                    ).toEqual(4);
+                    expect(
+                        await client.bitcount(key1, {
+                            start: 80,
+                        }),
+                    ).toEqual(0);
+                    expect(
+                        await client.bitcount(uuidv4(), {
+                            start: 80,
+                        }),
+                    ).toEqual(0);
+
+                    // key exists, but it is not a string
+                    await expect(
+                        client.bitcount(key2, {
+                            start: 1,
+                        }),
+                    ).rejects.toThrow(RequestError);
+                }
             }, protocol);
         },
         config.timeout,

diff --git a/node/tests/TestUtilities.ts b/node/tests/TestUtilities.ts
@@ -1370,6 +1370,15 @@ export async function transactionTest(
         ]);
     }
 
+    if (gte(version, "7.9.0")) {
+        baseTransaction.set(key17, "foobar");
+        responseData.push(['set(key17, "foobar")', "OK"]);
+        baseTransaction.bitcount(key17, {
+            start: 0,
+        });
+        responseData.push(["bitcount(key17, {start:0 }", 26]);
+    }
+
     baseTransaction.bitfield(key17, [
         new BitFieldSet(
             new UnsignedEncoding(10),

diff --git a/python/python/glide/async_commands/bitmap.py b/python/python/glide/async_commands/bitmap.py
@@ -24,7 +24,10 @@ class BitmapIndexType(Enum):
 
 class OffsetOptions:
     def __init__(
-        self, start: int, end: int, index_type: Optional[BitmapIndexType] = None
+        self,
+        start: int,
+        end: Optional[int] = None,
+        index_type: Optional[BitmapIndexType] = None,
     ):
         """
         Represents offsets specifying a string interval to analyze in the `BITCOUNT` command. The offsets are
@@ -34,7 +37,7 @@ def __init__(
 
         Args:
             start (int): The starting offset index.
-            end (int): The ending offset index.
+            end (int): The ending offset index. Optional since Valkey version 8.0.0 and above. If not provided, it will default to the end of the string.
             index_type (Optional[BitmapIndexType]): The index offset type. This option can only be specified if you are
                 using Valkey version 7.0.0 or above. Could be either `BitmapIndexType.BYTE` or `BitmapIndexType.BIT`.
                 If no index type is provided, the indexes will be assumed to be byte indexes.
@@ -44,7 +47,9 @@ def __init__(
         self.index_type = index_type
 
     def to_args(self) -> List[str]:
-        args = [str(self.start), str(self.end)]
+        args = [str(self.start)]
+        if self.end:
+            args.append(str(self.end))
         if self.index_type is not None:
             args.append(self.index_type.value)
 

diff --git a/python/python/glide/async_commands/core.py b/python/python/glide/async_commands/core.py
@@ -5531,6 +5531,8 @@ async def bitcount(
         Examples:
             >>> await client.bitcount("my_key1")
                 2  # The string stored at "my_key1" contains 2 set bits.
+            >>> await client.bitcount("my_key2", OffsetOptions(1))
+                8  # From the second to last bytes of the string stored at "my_key2" there are 8 set bits.
             >>> await client.bitcount("my_key2", OffsetOptions(1, 3))
                 2  # The second to fourth bytes of the string stored at "my_key2" contain 2 set bits.
             >>> await client.bitcount("my_key3", OffsetOptions(1, 1, BitmapIndexType.BIT))

diff --git a/python/python/tests/test_async_client.py b/python/python/tests/test_async_client.py
@@ -7126,6 +7126,25 @@ async def test_bitcount(self, glide_client: TGlideClient):
                     set_key, OffsetOptions(1, 1, BitmapIndexType.BIT)
                 )
 
+        if await check_if_server_version_lt(glide_client, "7.9.0"):
+            # exception thrown optional end was implemented after 8.0.0
+            with pytest.raises(RequestError):
+                await glide_client.bitcount(
+                    key1,
+                    OffsetOptions(
+                        2,
+                    ),
+                )
+        else:
+            assert await glide_client.bitcount(key1, OffsetOptions(0)) == 26
+            assert await glide_client.bitcount(key1, OffsetOptions(5)) == 4
+            assert await glide_client.bitcount(key1, OffsetOptions(80)) == 0
+            assert await glide_client.bitcount(non_existing_key, OffsetOptions(5)) == 0
+
+            # key exists but it is not a string
+            with pytest.raises(RequestError):
+                await glide_client.bitcount(set_key, OffsetOptions(1))
+
     @pytest.mark.parametrize("cluster_mode", [True, False])
     @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3])
     async def test_setbit(self, glide_client: TGlideClient):