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 RESP2 support #2383

Merged
merged 4 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* Java: Add binary version of `ZRANK WITHSCORE` ([#2896](https://github.com/valkey-io/valkey-glide/pull/2896))
* Go: Add `ZCARD` ([#2838](https://github.com/valkey-io/valkey-glide/pull/2838))
* Go: Add `BZPopMin` ([#2849](https://github.com/valkey-io/valkey-glide/pull/2849))
* Java: Add `RESP2` support ([#2383](https://github.com/valkey-io/valkey-glide/pull/2383))

#### Breaking Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ public abstract class BaseClientConfiguration {
*/
private final ThreadPoolResource threadPoolResource;

/**
* Serialization protocol to be used with the server. If not set, {@link ProtocolVersion#RESP3}
* will be used.
*/
private final ProtocolVersion protocol;

public abstract BaseSubscriptionConfiguration getSubscriptionConfiguration();

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/** Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 */
package glide.api.models.configuration;

/** Represents the communication protocol with the server. */
public enum ProtocolVersion {
/** Use RESP3 to communicate with the server nodes. */
RESP3,
/** Use RESP2 to communicate with the server nodes. */
RESP2
}
15 changes: 13 additions & 2 deletions java/client/src/main/java/glide/managers/ConnectionManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import glide.api.models.configuration.GlideClientConfiguration;
import glide.api.models.configuration.GlideClusterClientConfiguration;
import glide.api.models.configuration.NodeAddress;
import glide.api.models.configuration.ProtocolVersion;
import glide.api.models.configuration.ReadFrom;
import glide.api.models.exceptions.ClosingException;
import glide.api.models.exceptions.ConfigurationError;
Expand Down Expand Up @@ -132,6 +133,10 @@ private ConnectionRequest.Builder setupConnectionRequestBuilderBaseConfiguration
connectionRequestBuilder.setClientAz(configuration.getClientAZ());
}

if (configuration.getProtocol() != null) {
connectionRequestBuilder.setProtocolValue(configuration.getProtocol().ordinal());
}

return connectionRequestBuilder;
}

Expand Down Expand Up @@ -159,7 +164,10 @@ private ConnectionRequest.Builder setupConnectionRequestBuilderGlideClient(
}

if (configuration.getSubscriptionConfiguration() != null) {
// TODO throw ConfigurationError if RESP2
if (configuration.getProtocol() == ProtocolVersion.RESP2) {
throw new ConfigurationError(
"PubSub subscriptions require RESP3 protocol, but RESP2 was configured.");
}
var subscriptionsBuilder = PubSubSubscriptions.newBuilder();
for (var entry : configuration.getSubscriptionConfiguration().getSubscriptions().entrySet()) {
var channelsBuilder = PubSubChannelsOrPatterns.newBuilder();
Expand Down Expand Up @@ -211,7 +219,10 @@ private ConnectionRequest.Builder setupConnectionRequestBuilderGlideClusterClien
connectionRequestBuilder.setClusterModeEnabled(true);

if (configuration.getSubscriptionConfiguration() != null) {
// TODO throw ConfigurationError if RESP2
if (configuration.getProtocol() == ProtocolVersion.RESP2) {
throw new ConfigurationError(
"PubSub subscriptions require RESP3 protocol, but RESP2 was configured.");
}
var subscriptionsBuilder = PubSubSubscriptions.newBuilder();
for (var entry : configuration.getSubscriptionConfiguration().getSubscriptions().entrySet()) {
var channelsBuilder = PubSubChannelsOrPatterns.newBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import glide.api.models.configuration.GlideClientConfiguration;
import glide.api.models.configuration.GlideClusterClientConfiguration;
import glide.api.models.configuration.NodeAddress;
import glide.api.models.configuration.ProtocolVersion;
import glide.api.models.configuration.ReadFrom;
import glide.api.models.configuration.ServerCredentials;
import glide.api.models.configuration.StandaloneSubscriptionConfiguration;
Expand Down Expand Up @@ -146,6 +147,7 @@ public void connection_request_protobuf_generation_with_all_fields_set() {
.build())
.databaseId(DATABASE_ID)
.clientName(CLIENT_NAME)
.protocol(ProtocolVersion.RESP3)
.subscriptionConfiguration(
StandaloneSubscriptionConfiguration.builder()
.subscription(EXACT, gs("channel_1"))
Expand Down Expand Up @@ -180,6 +182,7 @@ public void connection_request_protobuf_generation_with_all_fields_set() {
.build())
.setDatabaseId(DATABASE_ID)
.setClientName(CLIENT_NAME)
.setProtocol(ConnectionRequestOuterClass.ProtocolVersion.RESP3)
.setPubsubSubscriptions(
PubSubSubscriptions.newBuilder()
.putAllChannelsOrPatternsByType(
Expand Down
19 changes: 13 additions & 6 deletions java/integTest/src/test/java/glide/ConnectionTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import glide.api.models.configuration.AdvancedGlideClientConfiguration;
import glide.api.models.configuration.AdvancedGlideClusterClientConfiguration;
import glide.api.models.configuration.BackoffStrategy;
import glide.api.models.configuration.ProtocolVersion;
import glide.api.models.configuration.ReadFrom;
import glide.api.models.configuration.RequestRoutingConfiguration;
import glide.api.models.exceptions.ClosingException;
Expand All @@ -36,22 +37,28 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.ValueSource;

@Timeout(10) // seconds
public class ConnectionTests {

@Test
@ParameterizedTest
@EnumSource(ProtocolVersion.class)
@SneakyThrows
public void basic_client() {
var regularClient = GlideClient.createClient(commonClientConfig().build()).get();
public void basic_client(ProtocolVersion protocol) {
var regularClient =
GlideClient.createClient(commonClientConfig().protocol(protocol).build()).get();
regularClient.close();
}

@Test
@ParameterizedTest
@EnumSource(ProtocolVersion.class)
@SneakyThrows
public void cluster_client() {
var clusterClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get();
public void cluster_client(ProtocolVersion protocol) {
var clusterClient =
GlideClusterClient.createClient(commonClusterClientConfig().protocol(protocol).build())
.get();
clusterClient.close();
}

Expand Down
26 changes: 26 additions & 0 deletions java/integTest/src/test/java/glide/PubSubTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import glide.api.models.configuration.BaseSubscriptionConfiguration.MessageCallback;
import glide.api.models.configuration.ClusterSubscriptionConfiguration;
import glide.api.models.configuration.ClusterSubscriptionConfiguration.PubSubClusterChannelMode;
import glide.api.models.configuration.ProtocolVersion;
import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute;
import glide.api.models.configuration.RequestRoutingConfiguration.SlotType;
import glide.api.models.configuration.StandaloneSubscriptionConfiguration;
Expand Down Expand Up @@ -280,6 +281,31 @@ private void skipTestsOnMac() {
"PubSub doesn't work on mac OS");
}

@SneakyThrows
@ParameterizedTest(name = "standalone = {0}")
@ValueSource(booleans = {true, false})
public void config_error_on_resp2(boolean standalone) {
if (standalone) {
var config =
commonClientConfig()
.subscriptionConfiguration(StandaloneSubscriptionConfiguration.builder().build())
.protocol(ProtocolVersion.RESP2)
.build();
var exception =
assertThrows(ConfigurationError.class, () -> GlideClient.createClient(config));
assertTrue(exception.getMessage().contains("PubSub subscriptions require RESP3 protocol"));
} else {
var config =
commonClusterClientConfig()
.subscriptionConfiguration(ClusterSubscriptionConfiguration.builder().build())
.protocol(ProtocolVersion.RESP2)
.build();
var exception =
assertThrows(ConfigurationError.class, () -> GlideClusterClient.createClient(config));
assertTrue(exception.getMessage().contains("PubSub subscriptions require RESP3 protocol"));
}
}

/** Similar to `test_pubsub_exact_happy_path` in python client tests. */
@SneakyThrows
@ParameterizedTest(name = "standalone = {0}, read messages via {1}")
Expand Down
Loading
Loading