Java: Add `BITCOUNT` command #1384

GumpacG · 2024-05-06T21:17:04Z

Issue #, if available:
N/A

Description of changes:
Implements BITCOUNT command in the java client.
Failures in GHA are due to redis 6's lack of support for BITCOUNT fix depends on #1284.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

java/client/src/main/java/glide/api/models/commands/BitmapOptions.java

java/integTest/src/test/java/glide/SharedCommandTests.java

java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java

java/client/src/main/java/glide/api/models/BaseTransaction.java

acarbonetto · 2024-05-06T22:02:57Z

java/integTest/src/test/java/glide/SharedCommandTests.java

+        assertEquals(0, client.bitcount(missingKey).get());
+
+        assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"));
+        assertEquals(16, client.bitcount(key, 2, 5, BitmapOptions.BYTE).get());


Suggested change

assertEquals(16, client.bitcount(key, 2, 5, BitmapOptions.BYTE).get());

assertEquals(16L, client.bitcount(key, 2, 5, BitmapOptions.BYTE).get());

Both should work

java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java

java/client/src/main/java/glide/api/models/BaseTransaction.java

java/client/src/test/java/glide/api/models/TransactionTests.java

java/integTest/src/test/java/glide/SharedCommandTests.java

aaron-congo · 2024-05-07T19:00:12Z

We usually add unit tests for the command in RedisClientTest as well, can we add them here as well? See this PR for an example

GumpacG · 2024-05-08T20:24:39Z

Currently, there are conflicts and will be addressed once #1284 is merged to reduce resolving conflicts.

java/client/src/main/java/glide/api/models/BaseTransaction.java

java/integTest/src/test/java/glide/SharedCommandTests.java

acarbonetto

nit: minor comments to be addressed

java/client/src/main/java/glide/api/models/commands/bitmap/BitmapIndexType.java

java/integTest/src/test/java/glide/SharedCommandTests.java

java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java

Yury-Fridlyand

Couple small comments

java/client/src/main/java/glide/api/models/BaseTransaction.java

java/client/src/test/java/glide/api/RedisClientTest.java

java/client/src/test/java/glide/api/models/TransactionTests.java

java/integTest/src/test/java/glide/SharedCommandTests.java

* Implemented Bitcount command for java client Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Removed unrelated change Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Addressed PR comments Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Addressed PR comments Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Removed unnecessary constructor Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> --------- Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

…s.java Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com>

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

* Java: Add command `BITCOUNT` (#263) * Implemented Bitcount command for java client Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Removed unrelated change Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Addressed PR comments Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Addressed PR comments Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Removed unnecessary constructor Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> --------- Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Resolved conflicts Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Renamed interface for consistency Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Address PR comments Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Added missed javadoc Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Addressed comments Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Update java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com> * Update java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com> * addressed PR comments Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Addressed PR comments Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Removed wrong version check Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Rebased and resolved conflicts Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> --------- Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com>

GumpacG requested review from aaron-congo, Yury-Fridlyand and acarbonetto May 6, 2024 21:17

GumpacG requested a review from a team as a code owner May 6, 2024 21:17

acarbonetto reviewed May 6, 2024

View reviewed changes

aaron-congo reviewed May 7, 2024

View reviewed changes

Yury-Fridlyand added the java issues and fixes related to the java client label May 8, 2024

Yury-Fridlyand requested review from aaron-congo, jonathanl-bq, barshaul, tjzhang-BQ and shohamazon May 8, 2024 02:56

GumpacG requested a review from acarbonetto May 8, 2024 22:25

tjzhang-BQ approved these changes May 8, 2024

View reviewed changes

GumpacG commented May 9, 2024

View reviewed changes

java/client/src/main/java/glide/api/models/BaseTransaction.java Outdated Show resolved Hide resolved

java/integTest/src/test/java/glide/SharedCommandTests.java Outdated Show resolved Hide resolved

acarbonetto approved these changes May 9, 2024

View reviewed changes

java/client/src/main/java/glide/api/models/commands/bitmap/BitmapIndexType.java Outdated Show resolved Hide resolved

java/integTest/src/test/java/glide/SharedCommandTests.java Show resolved Hide resolved

aaron-congo reviewed May 9, 2024

View reviewed changes

java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java Outdated Show resolved Hide resolved

java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java Outdated Show resolved Hide resolved

Yury-Fridlyand approved these changes May 9, 2024

View reviewed changes

Yury-Fridlyand reviewed May 9, 2024

View reviewed changes

java/integTest/src/test/java/glide/SharedCommandTests.java Outdated Show resolved Hide resolved

aaron-congo approved these changes May 9, 2024

View reviewed changes

GumpacG and others added 8 commits May 10, 2024 10:58

Resolved conflicts

4e9a193

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Renamed interface for consistency

67d6ffa

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Address PR comments

2d61bae

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Added missed javadoc

ea40f28

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Addressed comments

01e7673

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Update java/client/src/main/java/glide/api/commands/BitmapBaseCommand…

8ea6bb3

…s.java Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com>

Update java/client/src/main/java/glide/api/commands/BitmapBaseCommand…

0c494da

…s.java Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com>

GumpacG added 4 commits May 10, 2024 10:59

addressed PR comments

73b9579

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Addressed PR comments

93f68ec

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Removed wrong version check

5573a46

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

Rebased and resolved conflicts

a01e094

Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>

GumpacG force-pushed the java/integ_guiang_bitcount branch from f147fb2 to a01e094 Compare May 10, 2024 18:35

Elen-Ghulam approved these changes May 10, 2024

View reviewed changes

acarbonetto approved these changes May 10, 2024

View reviewed changes

acarbonetto merged commit 01c78b1 into aws:main May 10, 2024
45 checks passed

acarbonetto deleted the java/integ_guiang_bitcount branch May 10, 2024 19:41

GumpacG mentioned this pull request May 14, 2024

Java: Add GETBIT command #1400

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java: Add `BITCOUNT` command #1384

Java: Add `BITCOUNT` command #1384

GumpacG commented May 6, 2024

acarbonetto May 6, 2024

Yury-Fridlyand May 6, 2024

aaron-congo commented May 7, 2024

GumpacG commented May 8, 2024

acarbonetto left a comment

Yury-Fridlyand left a comment

	assertEquals(16, client.bitcount(key, 2, 5, BitmapOptions.BYTE).get());
	assertEquals(16L, client.bitcount(key, 2, 5, BitmapOptions.BYTE).get());

Java: Add BITCOUNT command #1384

Java: Add BITCOUNT command #1384

Conversation

GumpacG commented May 6, 2024

acarbonetto May 6, 2024

Choose a reason for hiding this comment

Yury-Fridlyand May 6, 2024

Choose a reason for hiding this comment

aaron-congo commented May 7, 2024

GumpacG commented May 8, 2024

acarbonetto left a comment

Choose a reason for hiding this comment

Yury-Fridlyand left a comment

Choose a reason for hiding this comment

Java: Add `BITCOUNT` command #1384

Java: Add `BITCOUNT` command #1384