Skip to content

Commit

Permalink
Address review suggestions
Browse files Browse the repository at this point in the history
- Add support for endpoints without username
- Improve endpoints naming in AutomaticFailoverTest
- Remove more hardcoded settings from Migrate tests
- Fix comments in SSLACL tests
  • Loading branch information
uglide committed May 21, 2024
1 parent 68157e8 commit 079172d
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 50 deletions.
17 changes: 12 additions & 5 deletions src/test/java/redis/clients/jedis/EndpointConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public String getPassword() {
}

public String getUsername() {
return username;
return username == null? "default" : username;
}

public String getHost() {
Expand Down Expand Up @@ -68,8 +68,8 @@ public EndpointURIBuilder() {
}

public EndpointURIBuilder defaultCredentials() {
this.username = EndpointConfig.this.username;
this.password = EndpointConfig.this.password;
this.username = EndpointConfig.this.username == null ? "" : getUsername();
this.password = EndpointConfig.this.getPassword();
return this;
}

Expand All @@ -91,7 +91,7 @@ public EndpointURIBuilder credentials(String u, String p) {

public URI build() {
String userInfo = !(this.username.isEmpty() && this.password.isEmpty()) ?
this.username + ":" + this.password + "@" :
this.username + ':' + this.password + '@' :
"";
return URI.create(
getURISchema(this.tls) + userInfo + getHost() + ":" + getPort() + this.path);
Expand All @@ -103,7 +103,14 @@ public EndpointURIBuilder getURIBuilder() {
}

public DefaultJedisClientConfig.Builder getClientConfigBuilder() {
return DefaultJedisClientConfig.builder().user(username).password(password).ssl(tls);
DefaultJedisClientConfig.Builder builder = DefaultJedisClientConfig.builder()
.password(password).ssl(tls);

if (username != null) {
return builder.user(username);
}

return builder;
}

protected String getURISchema(boolean tls) {
Expand Down
36 changes: 23 additions & 13 deletions src/test/java/redis/clients/jedis/MigratePipeliningTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,17 @@ public class MigratePipeliningTest extends JedisCommandsTestBase {
private static final byte[] bfoo3 = { 0x07, 0x08, 0x03 };
private static final byte[] bbar3 = { 0x09, 0x00, 0x03 };

private static final String host = endpoint.getHost();
private static final int port = 6386;
private static final int portAuth = endpoint.getPort() + 1;
private static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-acl");

private static final EndpointConfig destEndpoint = HostAndPorts.getRedisEndpoint(
"standalone7-with-lfu-policy");

private static final EndpointConfig destEndpointWithAuth = HostAndPorts.getRedisEndpoint(
"standalone1");

private static final String host = destEndpoint.getHost();
private static final int port = destEndpoint.getPort();
private static final int portAuth = destEndpointWithAuth.getPort();
private static final int db = 2;
private static final int dbAuth = 3;
private static final int timeout = Protocol.DEFAULT_TIMEOUT;
Expand All @@ -56,8 +64,8 @@ public void setUp() throws Exception {
dest.flushAll();
dest.select(db);

destAuth = new Jedis(host, portAuth, 500);
destAuth.auth(endpoint.getPassword());
destAuth = new Jedis(destEndpointWithAuth.getHostAndPort(),
destEndpointWithAuth.getClientConfigBuilder().build());
destAuth.flushAll();
destAuth.select(dbAuth);
}
Expand Down Expand Up @@ -258,7 +266,8 @@ public void migrateAuth() {
Pipeline p = jedis.pipelined();

p.set("foo", "bar");
p.migrate(host, portAuth, dbAuth, timeout, new MigrateParams().auth(endpoint.getPassword()), "foo");
p.migrate(host, portAuth, dbAuth, timeout,
new MigrateParams().auth(destEndpointWithAuth.getPassword()), "foo");
p.get("foo");

assertThat(p.syncAndReturnAll(),
Expand All @@ -274,7 +283,8 @@ public void migrateAuthBinary() {
Pipeline p = jedis.pipelined();

p.set(bfoo, bbar);
p.migrate(host, portAuth, dbAuth, timeout, new MigrateParams().auth(endpoint.getPassword()), bfoo);
p.migrate(host, portAuth, dbAuth, timeout,
new MigrateParams().auth(destEndpointWithAuth.getPassword()), bfoo);
p.get(bfoo);

assertThat(p.syncAndReturnAll(),
Expand All @@ -287,13 +297,12 @@ public void migrateAuthBinary() {
public void migrateAuth2() {
assertNull(jedis.get("foo"));



Pipeline p = destAuth.pipelined();

p.set("foo", "bar");
p.migrate(host, endpoint.getPort(), 0, timeout,
new MigrateParams().auth2("acljedis", "fizzbuzz"), "foo");
p.migrate(endpoint.getHost(), endpoint.getPort(), 0, timeout,
new MigrateParams().auth2(endpoint.getUsername(), endpoint.getPassword()),
"foo");
p.get("foo");

assertThat(p.syncAndReturnAll(),
Expand All @@ -309,8 +318,9 @@ public void migrateAuth2Binary() {
Pipeline p = dest.pipelined();

p.set(bfoo, bbar);
p.migrate(host, endpoint.getPort(), 0, timeout,
new MigrateParams().auth2("acljedis", "fizzbuzz"), bfoo);
p.migrate(endpoint.getHost(), endpoint.getPort(), 0, timeout,
new MigrateParams().auth2(endpoint.getUsername(), endpoint.getPassword()),
bfoo);
p.get(bfoo);

assertThat(p.syncAndReturnAll(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class SSLACLJedisClusterTest extends JedisClusterTestBase {

@BeforeClass
public static void prepare() {
// We need to set up certificates first before connecting to the endpoint with enabled TLS
SSLJedisTest.setupTrustStore();

// TODO(imalinovskyi): Remove hardcoded connection settings
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/redis/clients/jedis/SSLACLJedisTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ public class SSLACLJedisTest {

@BeforeClass
public static void prepare() {
// Use to check if the ACL test should be ran. ACL are available only in 6.0 and later
// We need to set up certificates first before connecting to the endpoint with enabled TLS
SSLJedisTest.setupTrustStore();
// Use to check if the ACL test should be ran. ACL are available only in 6.0 and later
org.junit.Assume.assumeTrue("Not running ACL test on this version of Redis",
RedisVersionUtil.checkRedisMajorVersionNumber(6, endpoint));
}
Expand Down
43 changes: 24 additions & 19 deletions src/test/java/redis/clients/jedis/commands/jedis/MigrateTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import redis.clients.jedis.Jedis;
import redis.clients.jedis.Protocol;
import redis.clients.jedis.RedisProtocol;
import redis.clients.jedis.*;
import redis.clients.jedis.exceptions.JedisDataException;
import redis.clients.jedis.params.MigrateParams;

Expand All @@ -32,9 +30,18 @@ public class MigrateTest extends JedisCommandsTestBase {

private Jedis dest;
private Jedis destAuth;
private static final String host = endpoint.getHost();
private static final int port = 6386;
private static final int portAuth = endpoint.getPort() + 1;

private static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-acl");

private static final EndpointConfig destEndpoint = HostAndPorts.getRedisEndpoint(
"standalone7-with-lfu-policy");

private static final EndpointConfig destEndpointWithAuth = HostAndPorts.getRedisEndpoint(
"standalone1");

private static final String host = destEndpoint.getHost();
private static final int port = destEndpoint.getPort();
private static final int portAuth = destEndpointWithAuth.getPort();
private static final int db = 2;
private static final int dbAuth = 3;
private static final int timeout = Protocol.DEFAULT_TIMEOUT;
Expand All @@ -52,8 +59,8 @@ public void setUp() throws Exception {
dest.flushAll();
dest.select(db);

destAuth = new Jedis(host, portAuth, 500);
destAuth.auth(endpoint.getPassword());
destAuth = new Jedis(destEndpointWithAuth.getHostAndPort(),
destEndpointWithAuth.getClientConfigBuilder().build());
destAuth.flushAll();
destAuth.select(dbAuth);
}
Expand Down Expand Up @@ -150,14 +157,14 @@ public void migrateCopyReplace() {
@Test
public void migrateAuth() {
jedis.set("foo", "bar");
assertEquals("OK",
jedis.migrate(host, portAuth, dbAuth, timeout, new MigrateParams().auth(endpoint.getPassword()), "foo"));
assertEquals("OK", jedis.migrate(host, portAuth, dbAuth, timeout,
new MigrateParams().auth(destEndpointWithAuth.getPassword()), "foo"));
assertEquals("bar", destAuth.get("foo"));
assertNull(jedis.get("foo"));

jedis.set(bfoo, bbar);
assertEquals("OK",
jedis.migrate(host, portAuth, dbAuth, timeout, new MigrateParams().auth(endpoint.getPassword()), bfoo));
assertEquals("OK", jedis.migrate(host, portAuth, dbAuth, timeout,
new MigrateParams().auth(destEndpointWithAuth.getPassword()), bfoo));
assertArrayEquals(bbar, destAuth.get(bfoo));
assertNull(jedis.get(bfoo));
}
Expand All @@ -166,14 +173,14 @@ public void migrateAuth() {
public void migrateAuth2() {
destAuth.set("foo", "bar");
assertEquals("OK", destAuth.migrate(host, endpoint.getPort(), 0, timeout,
new MigrateParams().auth2("acljedis", "fizzbuzz"), "foo"));
new MigrateParams().auth2(endpoint.getUsername(), endpoint.getPassword()), "foo"));
assertEquals("bar", jedis.get("foo"));
assertNull(destAuth.get("foo"));

// binary
dest.set(bfoo1, bbar1);
assertEquals("OK", dest.migrate(host, endpoint.getPort(), 0, timeout,
new MigrateParams().auth2("acljedis", "fizzbuzz"), bfoo1));
new MigrateParams().auth2(endpoint.getUsername(), endpoint.getPassword()), bfoo1));
assertArrayEquals(bbar1, jedis.get(bfoo1));
assertNull(dest.get(bfoo1));
}
Expand All @@ -182,10 +189,8 @@ public void migrateAuth2() {
public void migrateCopyReplaceAuth() {
jedis.set("foo", "bar1");
destAuth.set("foo", "bar2");
assertEquals(
"OK",
jedis.migrate(host, portAuth, dbAuth, timeout,
new MigrateParams().copy().replace().auth(endpoint.getPassword()), "foo"));
assertEquals("OK", jedis.migrate(host, portAuth, dbAuth, timeout,
new MigrateParams().copy().replace().auth(destEndpointWithAuth.getPassword()), "foo"));
assertEquals("bar1", destAuth.get("foo"));
assertEquals("bar1", jedis.get("foo"));

Expand All @@ -194,7 +199,7 @@ public void migrateCopyReplaceAuth() {
assertEquals(
"OK",
jedis.migrate(host, portAuth, dbAuth, timeout,
new MigrateParams().copy().replace().auth(endpoint.getPassword()), bfoo));
new MigrateParams().copy().replace().auth(destEndpointWithAuth.getPassword()), bfoo));
assertArrayEquals(bbar1, destAuth.get(bfoo));
assertArrayEquals(bbar1, jedis.get(bfoo));
}
Expand Down
21 changes: 10 additions & 11 deletions src/test/java/redis/clients/jedis/misc/AutomaticFailoverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ public class AutomaticFailoverTest {

private static final Logger log = LoggerFactory.getLogger(AutomaticFailoverTest.class);

// TODO(imalinovskyi): Figure out how we deploy this endpoint
private final HostAndPort hostPort_0 = new HostAndPort(HostAndPorts.getRedisEndpoint("standalone0").getHost(), 6378);
private final EndpointConfig endpointStandalone0 = HostAndPorts.getRedisEndpoint("standalone0");
private final EndpointConfig endpointStandalone7 = HostAndPorts.getRedisEndpoint("standalone7-with-lfu-policy");
private final HostAndPort hostPortWithFailure = new HostAndPort(HostAndPorts.getRedisEndpoint("standalone0").getHost(), 6378);
private final EndpointConfig workingEndpointWithPriority1 = HostAndPorts.getRedisEndpoint("standalone0");
private final EndpointConfig workingEndpointWithPriority2 = HostAndPorts.getRedisEndpoint("standalone7-with-lfu-policy");

private final JedisClientConfig clientConfig = DefaultJedisClientConfig.builder().build();

Expand All @@ -44,8 +43,8 @@ private List<MultiClusterClientConfig.ClusterConfig> getClusterConfigs(

@Before
public void setUp() {
jedis2 = new Jedis(endpointStandalone7.getHostAndPort(),
endpointStandalone7.getClientConfigBuilder().build());
jedis2 = new Jedis(workingEndpointWithPriority2.getHostAndPort(),
workingEndpointWithPriority2.getClientConfigBuilder().build());
jedis2.flushAll();
}

Expand All @@ -57,7 +56,7 @@ public void cleanUp() {
@Test
public void pipelineWithSwitch() {
MultiClusterPooledConnectionProvider provider = new MultiClusterPooledConnectionProvider(
new MultiClusterClientConfig.Builder(getClusterConfigs(clientConfig, hostPort_0, endpointStandalone7.getHostAndPort())).build());
new MultiClusterClientConfig.Builder(getClusterConfigs(clientConfig, hostPortWithFailure, workingEndpointWithPriority2.getHostAndPort())).build());

try (UnifiedJedis client = new UnifiedJedis(provider)) {
AbstractPipeline pipe = client.pipelined();
Expand All @@ -74,7 +73,7 @@ public void pipelineWithSwitch() {
@Test
public void transactionWithSwitch() {
MultiClusterPooledConnectionProvider provider = new MultiClusterPooledConnectionProvider(
new MultiClusterClientConfig.Builder(getClusterConfigs(clientConfig, hostPort_0, endpointStandalone7.getHostAndPort())).build());
new MultiClusterClientConfig.Builder(getClusterConfigs(clientConfig, hostPortWithFailure, workingEndpointWithPriority2.getHostAndPort())).build());

try (UnifiedJedis client = new UnifiedJedis(provider)) {
AbstractTransaction tx = client.multi();
Expand All @@ -94,7 +93,7 @@ public void commandFailover() {
int slidingWindowSize = 10;

MultiClusterClientConfig.Builder builder = new MultiClusterClientConfig.Builder(
getClusterConfigs(clientConfig, hostPort_0, endpointStandalone7.getHostAndPort()))
getClusterConfigs(clientConfig, hostPortWithFailure, workingEndpointWithPriority2.getHostAndPort()))
.circuitBreakerSlidingWindowMinCalls(slidingWindowMinCalls)
.circuitBreakerSlidingWindowSize(slidingWindowSize);

Expand Down Expand Up @@ -132,7 +131,7 @@ public void pipelineFailover() {
int slidingWindowSize = 10;

MultiClusterClientConfig.Builder builder = new MultiClusterClientConfig.Builder(
getClusterConfigs(clientConfig, hostPort_0, endpointStandalone7.getHostAndPort()))
getClusterConfigs(clientConfig, hostPortWithFailure, workingEndpointWithPriority2.getHostAndPort()))
.circuitBreakerSlidingWindowMinCalls(slidingWindowMinCalls)
.circuitBreakerSlidingWindowSize(slidingWindowSize)
.fallbackExceptionList(Arrays.asList(JedisConnectionException.class));
Expand Down Expand Up @@ -165,7 +164,7 @@ public void failoverFromAuthError() {
int slidingWindowSize = 10;

MultiClusterClientConfig.Builder builder = new MultiClusterClientConfig.Builder(
getClusterConfigs(clientConfig, endpointStandalone0.getHostAndPort(), endpointStandalone7.getHostAndPort()))
getClusterConfigs(clientConfig, workingEndpointWithPriority1.getHostAndPort(), workingEndpointWithPriority2.getHostAndPort()))
.circuitBreakerSlidingWindowMinCalls(slidingWindowMinCalls)
.circuitBreakerSlidingWindowSize(slidingWindowSize)
.fallbackExceptionList(Arrays.asList(JedisAccessControlException.class));
Expand Down
1 change: 0 additions & 1 deletion src/test/resources/endpoints.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"standalone0": {
"username": "default",
"password": "foobared",
"tls": false,
"endpoints": [
Expand Down

0 comments on commit 079172d

Please sign in to comment.