Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java: Add the XREADGROUP command #376

Merged

Conversation

acarbonetto
Copy link

@acarbonetto acarbonetto commented Jun 18, 2024

Dependent on valkey-io#1599

Adds the XREADGROUP command to Java

TODO:

  • Fix IT tests for XREADGROUP
  • Fix IT test for failure/blocking cases

@acarbonetto acarbonetto changed the title Java: Add the XREADGRPUP command Java: Add the XREADGROUP command Jun 18, 2024
@acarbonetto acarbonetto changed the title Java: Add the XREADGROUP command Java: Add the XREADGROUP command Jun 18, 2024
glide-core/src/client/value_conversion.rs Outdated Show resolved Hide resolved
glide-core/src/request_type.rs Outdated Show resolved Hide resolved
@@ -128,7 +129,10 @@ public static <T> Map<String, T[][]> castMapOf2DArray(
return null;
}
return mapOfArrays.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e -> castArrayofArrays(e.getValue(), clazz)));
.collect(
HashMap::new,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change? to accept nulls?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. XREADGROUP returns maps will null values (gross!)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a test for that case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do

@@ -753,6 +771,18 @@ private static Object[] streamCommands(BaseTransaction<?> transaction) {
1L, // xtrim(streamKey1, new MinId(true, "0-2"))
OK, // xgroupCreate(streamKey1, groupName1, "0-0")
OK, // xgroupCreate(streamKey1, groupName1, "0-0", options)
true, // xgroupCreateConsumer(streamKey1, groupName1, consumer1)
Map.of(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you can store this as a variable if it ease reading/debugging

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be a good idea... ;)

Signed-off-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
@acarbonetto acarbonetto force-pushed the java/dev_acarbo_add_xreadgroup_2 branch from b3efde5 to 957e012 Compare June 19, 2024 17:57
@@ -791,6 +791,7 @@ fn convert_to_array_of_pairs(
value_expected_return_type: Option<ExpectedReturnType>,
) -> RedisResult<Value> {
match response {
Value::Nil => Ok(response),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XREADGROUP can/will return null values in the map

Signed-off-by: Andrew Carbonetto <[email protected]>
@acarbonetto acarbonetto merged commit 14e3c24 into java/integ_acarbo_add_xreadgroup Jun 19, 2024
44 checks passed
@acarbonetto acarbonetto deleted the java/dev_acarbo_add_xreadgroup_2 branch June 19, 2024 22:34
acarbonetto added a commit that referenced this pull request Jun 20, 2024
* Java: Add the `XREADGROUP` command (#376)

* Add XGROUP CreateConsumer, DelConsumer

Signed-off-by: Andrew Carbonetto <[email protected]>

* Add XREADGROUP command

Signed-off-by: Andrew Carbonetto <[email protected]>

* Udpate IT tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* Fix IT tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* SPOTLESS & merge conflict fix

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update for review comments

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>

* Remove old test from IT suite

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update xreadgroup docs

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>
acarbonetto added a commit that referenced this pull request Jun 20, 2024
* Add XGROUP CreateConsumer, DelConsumer

Signed-off-by: Andrew Carbonetto <[email protected]>

* Add XREADGROUP command

Signed-off-by: Andrew Carbonetto <[email protected]>

* Udpate IT tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* Fix IT tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* SPOTLESS & merge conflict fix

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update for review comments

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>
acarbonetto added a commit that referenced this pull request Jun 20, 2024
* Java: Add the `XREADGROUP` command (#376)

* Add XGROUP CreateConsumer, DelConsumer

Signed-off-by: Andrew Carbonetto <[email protected]>

* Add XREADGROUP command

Signed-off-by: Andrew Carbonetto <[email protected]>

* Udpate IT tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* Fix IT tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* SPOTLESS & merge conflict fix

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update for review comments

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>

* Added implementation and incomplete transactiontests

* Update test

Signed-off-by: Andrew Carbonetto <[email protected]>

* Add XACK test

Signed-off-by: Andrew Carbonetto <[email protected]>

* Remove extra test

Signed-off-by: Andrew Carbonetto <[email protected]>

* More merge conflict cleanup

Signed-off-by: Andrew Carbonetto <[email protected]>

* Process review comments

Signed-off-by: Andrew Carbonetto <[email protected]>

* XACK add failure tests

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>
Co-authored-by: Guian Gumpac <[email protected]>
cyip10 pushed a commit that referenced this pull request Jun 24, 2024
* Java: Add the `XREADGROUP` command (#376)

* Add XGROUP CreateConsumer, DelConsumer

Signed-off-by: Andrew Carbonetto <[email protected]>

* Add XREADGROUP command

Signed-off-by: Andrew Carbonetto <[email protected]>

* Udpate IT tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* Fix IT tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* SPOTLESS & merge conflict fix

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update for review comments

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>

* Remove old test from IT suite

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update xreadgroup docs

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants