Java: Pub & Sub = <3

[Yury-Fridlyand]

Issue #, if available:
PubSub support for Java client
Ref: #1602, #1616, #1650, #1643 and #1741

Description of changes:

• Configuration for pubsub
• Receiving pubsub messages
• Sending messages
• UT
• IT

Configuration with callback:

    @Test
    @SneakyThrows
    public void basic_client() {
        MessageCallback callback =
            (msg, context) -> System.out.printf("Received %s, context %s\n", msg, context);

        var regularClient =
                RedisClient.CreateClient(
                                RedisClientConfiguration.builder()
                                        .address(NodeAddress.builder().port(6379).build())
                                        .requestTimeout(3000)
                                        .subscriptionConfiguration( // subscribe happens here
                                                StandaloneSubscriptionConfiguration.builder()
                                                        .subscription(EXACT, "ch1")
                                                        .subscription(PATTERN, "chat*")
                                                        .callback(callback)
                                                        .build())
                                        .build())
                        .get();

        Thread.sleep(100500);
        regularClient.close(); // unsubscribe happens here
    }

To send messages use

client.publish(channel, message).get();
clusterClient.spublish(channel, message).get();

Configuration without callback:

    @Test
    @SneakyThrows
    public void basic_client() {
        var regularClient =
                RedisClient.CreateClient(
                                RedisClientConfiguration.builder()
                                        .address(NodeAddress.builder().port(6379).build())
                                        .requestTimeout(3000)
                                        .subscriptionConfiguration(
                                                StandaloneSubscriptionConfiguration.builder()
                                                        .subscription(EXACT, "ch1")
                                                        .subscription(PATTERN, "chat*")
                                                        .build())
                                        .build())
                        .get();

        Thread.sleep(100500);

        Message msg = null;
        while ((msg = regularClient.tryGetPubSubMessage()) != null) {
            System.out.printf("Received from queue %s\n", msg);
        }
    }

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

[ikolomi]

reviewed and posted comments from the beginning to java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java

[ikolomi]

what is this?

[Yury-Fridlyand]

Not implemented and won't be in the foreseeable future.

[alexey-temnikov]

Can we have a more meaningful error message here, along with the TODO link to documentation (perhaps explaining the limitation?)

[Yury-Fridlyand]

Sure, WIP

[Yury-Fridlyand]

Updated in 5a11ff4

[ikolomi]

how its applies to pubsub?

[Yury-Fridlyand]

MessageHandler uses responseResolver to get the message from received push.

[jduo]

Does this answer your question @ikolomi ?

[ikolomi]

how it is related to the pubsub

[Yury-Fridlyand]

I had to rework it since BaseClient got more private fields to store.

[jduo]

Does this answer your question @ikolomi ?

[ikolomi]

we have a validation that context cannot be set w/o the callback and the subscriptions cannot be used with resp2, do we have it here?
https://github.com/aws/glide-for-redis/blob/e2a804c2f1d6355a797a60f28e95c2d16826a856/python/python/glide/config.py#L451

[Yury-Fridlyand]

java client does not support RESP2 yet. I put a todo.

[Yury-Fridlyand]

It is prohibited https://github.com/aws/glide-for-redis/blob/905a17b4fa9dec0ed3f9ad08b26437b97ba39323/java/client/src/main/java/glide/api/models/configuration/BaseSubscriptionConfiguration.java#L89-L104

[jduo]

@ikolomi , does this cover your concern?

[jduo]

Even though this takes in both callback and context in one method, the caller could supply null to either of these. I've added an explict check for this in #1773.

[ikolomi]

sharded?

[Yury-Fridlyand]

very sharded

[ikolomi]

align with the reference https://github.com/aws/glide-for-redis/blob/e2a804c2f1d6355a797a60f28e95c2d16826a856/python/python/glide/async_commands/cluster_commands.py#L461

[jduo]

This is done in #1773

[jduo]

This client needs to close() right? Need a try-with-resources?

[Yury-Fridlyand]

all instances are mocks here, so I think we can omit that.

[jduo]

RedisClient itself isn't a mock. Adding try-with-resources.

[jduo]

Nevermind, the client is now mocked.

[jonathanl-bq]

Signing off on the Rust changes with some minor comments.

[jonathanl-bq]

If we only have 2 keys, why not just return an array of 2 elements? Not a big deal, but I'm curious why we went with a Map.

[Yury-Fridlyand]

Good idea, but I'm following python client.

[ikolomi]

1. The feature is not done until all the comments are resolved and explicitly approved by myself.
2. Do reach out actively to me to in case of any questions arise.

[ikolomi]

This has to be implemented to be on a par with python implementation

[Yury-Fridlyand]

Please see #1770

[jduo]

Does this cover the ask @ikolomi ?

[ikolomi]

align with the reference https://github.com/aws/glide-for-redis/blob/e2a804c2f1d6355a797a60f28e95c2d16826a856/python/python/glide/async_commands/cluster_commands.py#L461

[ikolomi]

1. implement getPubSubMessage() flavors
2. implement the missing tests per reference ( https://github.com/aws/glide-for-redis/blob/main/python/python/tests/test_pubsub.py) :

test_pubsub_exact_happy_path_coexistence
test_pubsub_exact_happy_path_many_channels_co_existence
test_sharded_pubsub_co_existence
test_pubsub_pattern_co_existence
test_pubsub_pattern_many_channels
test_pubsub_combined_exact_and_pattern_one_client
test_pubsub_combined_exact_and_pattern_multiple_clients
test_pubsub_combined_exact_pattern_and_sharded_multi_client
test_pubsub_combined_different_channels_with_same_name
test_pubsub_two_publishing_clients_same_name
test_pubsub_exact_max_size_message
test_pubsub_sharded_max_size_message
test_pubsub_exact_max_size_message_callback
test_pubsub_sharded_max_size_message_callback
test_pubsub_resp2_raise_an_error
test_pubsub_context_with_no_callback_raise_error

[Yury-Fridlyand]

1. All "coexistence" tests depend on getPubSubMessage and wait for it.
2. All "max_size_message" tests disabled on python side due to a bug and there is no reason to implement them now until bug fixed.
3. RESP2 is not supported by java client - no test there.
4. All rest are implemented.

[eifrah-aws]

why are we still using Strings?

[eifrah-aws]

Please add a GlideString variant for this function

[jduo]

@eifrah-aws , I've removed spublish and made two overloads of publish(). They both use ArgType now. Please see: #1773.

[eifrah-aws]

GlideString variant

[jduo]

See #1773

[eifrah-aws]

GlideString variant

[jduo]

See #1773