Skip to content

Commit

Permalink
DATAREDIS-1046 - Polishing.
Browse files Browse the repository at this point in the history
Guard tests to allow execution against single node instance for quick dev turnaround and replace usage of Optional with null and @nullable annotations.

Original Pull Request: #558
  • Loading branch information
christophstrobl committed Sep 11, 2020
1 parent 717a196 commit bdbba08
Show file tree
Hide file tree
Showing 13 changed files with 168 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import org.springframework.core.env.MapPropertySource;
Expand All @@ -50,7 +49,7 @@ public class RedisClusterConfiguration implements RedisConfiguration, ClusterCon

private Set<RedisNode> clusterNodes;
private @Nullable Integer maxRedirects;
private Optional<String> username = Optional.empty();
private @Nullable String username = null;
private RedisPassword password = RedisPassword.none();

/**
Expand Down Expand Up @@ -189,16 +188,17 @@ private void appendClusterNodes(Set<String> hostAndPorts) {
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#setUsername(String)
*/
@Override
public void setUsername(String username) {
this.username = Optional.of(username);
public void setUsername(@Nullable String username) {
this.username = username;
}

/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#getUsername()
*/
@Nullable
@Override
public Optional<String> getUsername() {
public String getUsername() {
return this.username;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.IntSupplier;
import java.util.function.Supplier;
Expand Down Expand Up @@ -138,11 +137,11 @@ static Integer getDatabaseOrElse(@Nullable RedisConfiguration configuration, Sup
* @param configuration can be {@literal null}.
* @param other a {@code Supplier} whose result is returned if given {@link RedisConfiguration} is not
* {@link #isAuthenticationAware(RedisConfiguration) password aware}.
* @return never {@literal null}.
* @return can be {@literal null}.
* @throws IllegalArgumentException if {@code other} is {@literal null}.
*/
static Optional<String> getUsernameOrElse(@Nullable RedisConfiguration configuration,
Supplier<Optional<String>> other) {
@Nullable
static String getUsernameOrElse(@Nullable RedisConfiguration configuration, Supplier<String> other) {

Assert.notNull(other, "Other must not be null!");
return isAuthenticationAware(configuration) ? ((WithAuthentication) configuration).getUsername() : other.get();
Expand Down Expand Up @@ -203,7 +202,7 @@ interface WithAuthentication {
*
* @param username the username.
*/
void setUsername(String username);
void setUsername(@Nullable String username);

/**
* Create and set a {@link RedisPassword} for given {@link String}.
Expand Down Expand Up @@ -233,9 +232,10 @@ default void setPassword(@Nullable char[] password) {
/**
* Get the username to use when connecting.
*
* @return {@link Optional#empty()} if none set.
* @return {@literal null} if none set.
*/
Optional<String> getUsername();
@Nullable
String getUsername();

/**
* Get the RedisPassword to use when connecting.
Expand Down Expand Up @@ -381,10 +381,11 @@ default void setMaster(final String name) {
/**
* Get the username used when authenticating with a Redis Server.
*
* @return never {@literal null}.
* @return can be {@literal null} if not set.
* @since 2.4
*/
default Optional<String> getDataNodeUsername() {
@Nullable
default String getDataNodeUsername() {
return getUsername();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import org.springframework.core.env.MapPropertySource;
Expand Down Expand Up @@ -51,7 +50,7 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
private Set<RedisNode> sentinels;
private int database;

private Optional<String> dataNodeUsername = Optional.empty();
private @Nullable String dataNodeUsername = null;
private RedisPassword dataNodePassword = RedisPassword.none();
private RedisPassword sentinelPassword = RedisPassword.none();

Expand Down Expand Up @@ -236,16 +235,17 @@ public void setDatabase(int index) {
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#setUsername(String)
*/
@Override
public void setUsername(String username) {
this.dataNodeUsername = Optional.of(username);
public void setUsername(@Nullable String username) {
this.dataNodeUsername = username;
}

/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#getUsername()
*/
@Nullable
@Override
public Optional<String> getUsername() {
public String getUsername() {
return this.dataNodeUsername;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
*/
package org.springframework.data.redis.connection;

import java.util.Optional;

import org.springframework.data.redis.connection.RedisConfiguration.DomainSocketConfiguration;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

/**
Expand All @@ -34,7 +33,7 @@ public class RedisSocketConfiguration implements RedisConfiguration, DomainSocke

private String socket = DEFAULT_SOCKET;
private int database;
private Optional<String> username = Optional.empty();
private @Nullable String username = null;
private RedisPassword password = RedisPassword.none();

/**
Expand Down Expand Up @@ -100,16 +99,17 @@ public void setDatabase(int index) {
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#setUsername(String)
*/
@Override
public void setUsername(String username) {
this.username = Optional.of(username);
public void setUsername(@Nullable String username) {
this.username = username;
}

/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#getUsername()
*/
@Nullable
@Override
public Optional<String> getUsername() {
public String getUsername() {
return this.username;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
*/
package org.springframework.data.redis.connection;

import java.util.Optional;

import org.springframework.data.redis.connection.RedisConfiguration.WithDatabaseIndex;
import org.springframework.data.redis.connection.RedisConfiguration.WithHostAndPort;
import org.springframework.data.redis.connection.RedisConfiguration.WithPassword;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

/**
Expand All @@ -39,7 +38,7 @@ public class RedisStandaloneConfiguration
private String hostName = DEFAULT_HOST;
private int port = DEFAULT_PORT;
private int database;
private Optional<String> username = Optional.empty();
private @Nullable String username = null;
private RedisPassword password = RedisPassword.none();

/**
Expand Down Expand Up @@ -133,16 +132,17 @@ public void setDatabase(int index) {
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#setUsername(String)
*/
@Override
public void setUsername(String username) {
this.username = Optional.of(username);
public void setUsername(@Nullable String username) {
this.username = username;
}

/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#getUsername()
*/
@Nullable
@Override
public Optional<String> getUsername() {
public String getUsername() {
return this.username;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

import org.springframework.data.redis.connection.RedisConfiguration.StaticMasterReplicaConfiguration;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

/**
* Configuration class used for setting up {@link RedisConnection} via {@link RedisConnectionFactory} using the provided
* Master / Replica configuration to nodes know to not change address. Eg. when connecting to
* <a href="https://aws.amazon.com/documentation/elasticache/">AWS ElastiCache with Read Replicas</a>. <br/>
* Note: Redis is undergoing a nomenclature change where the term replica is used synonymously to slave.
* Please also note that a Master/Replica connection cannot be used for Pub/Sub operations.
* Note: Redis is undergoing a nomenclature change where the term replica is used synonymously to slave. Please also
* note that a Master/Replica connection cannot be used for Pub/Sub operations.
*
* @author Mark Paluch
* @author Christoph Strobl
Expand All @@ -41,7 +41,7 @@ public class RedisStaticMasterReplicaConfiguration implements RedisConfiguration

private List<RedisStandaloneConfiguration> nodes = new ArrayList<>();
private int database;
private Optional<String> username = Optional.empty();
private @Nullable String username = null;
private RedisPassword password = RedisPassword.none();

/**
Expand Down Expand Up @@ -137,16 +137,17 @@ public void setDatabase(int index) {
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#setUsername(String)
*/
@Override
public void setUsername(String username) {
this.username = Optional.of(username);
public void setUsername(@Nullable String username) {
this.username = username;
}

/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisConfiguration.WithAuthentication#getUsername()
*/
@Nullable
@Override
public Optional<String> getUsername() {
public String getUsername() {
return this.username;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;

/**
* Connection factory creating <a href="https://github.com/xetorthio/jedis">Jedis</a> based connections.
Expand Down Expand Up @@ -328,7 +329,10 @@ public void afterPropertiesSet() {
clientConfiguration.getHostnameVerifier().orElse(null));

getRedisPassword().map(String::new).ifPresent(shardInfo::setPassword);
getRedisUsername().ifPresent(shardInfo::setUser);
String username = getRedisUsername();
if (StringUtils.hasText(username)) {
shardInfo.setUser(username);
}

int readTimeout = getReadTimeout();

Expand Down Expand Up @@ -375,8 +379,8 @@ protected Pool<Jedis> createRedisSentinelPool(RedisSentinelConfiguration config)
String sentinelPassword = config.getSentinelPassword().toOptional().map(String::new).orElse(null);

return new JedisSentinelPool(config.getMaster().getName(), convertToJedisSentinelSet(config.getSentinels()),
poolConfig, getConnectTimeout(), getReadTimeout(), getUsername(), getPassword(), getDatabase(),
getClientName(), getConnectTimeout(), getReadTimeout(), sentinelUser, sentinelPassword, getClientName());
poolConfig, getConnectTimeout(), getReadTimeout(), getUsername(), getPassword(), getDatabase(), getClientName(),
getConnectTimeout(), getReadTimeout(), sentinelUser, sentinelPassword, getClientName());
}

/**
Expand Down Expand Up @@ -556,7 +560,7 @@ public void setUseSsl(boolean useSsl) {
*/
@Nullable
private String getUsername() {
return getRedisUsername().orElse(null);
return getRedisUsername();
}

/**
Expand All @@ -569,7 +573,8 @@ public String getPassword() {
return getRedisPassword().map(String::new).orElse(null);
}

private Optional<String> getRedisUsername() {
@Nullable
private String getRedisUsername() {
return RedisConfiguration.getUsernameOrElse(this.configuration, standaloneConfig::getUsername);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.beans.factory.DisposableBean;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.dao.DataAccessException;
Expand All @@ -62,6 +61,7 @@
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;

/**
* Connection factory creating <a href="https://github.com/mp911de/lettuce">Lettuce</a>-based connections.
Expand Down Expand Up @@ -788,9 +788,11 @@ public void setClientName(@Nullable String clientName) {
this.getMutableConfiguration().setClientName(clientName);
}

private Optional<String> getRedisUsername() {
@Nullable
private String getRedisUsername() {
return RedisConfiguration.getUsernameOrElse(configuration, standaloneConfig::getUsername);
}

/**
* Returns the password used for authenticating with the Redis server.
*
Expand Down Expand Up @@ -1164,11 +1166,10 @@ private RedisURI createRedisSocketURIAndApplySettings(String socketPath) {

private void applyAuthentication(RedisURI.Builder builder) {

Optional<String> username = getRedisUsername();
if (username.isPresent()) {
String username = getRedisUsername();
if (StringUtils.hasText(username)) {
// See https://github.com/lettuce-io/lettuce-core/issues/1404
username.ifPresent(
it -> builder.withAuthentication(it, new String(getRedisPassword().toOptional().orElse(new char[0]))));
builder.withAuthentication(username, new String(getRedisPassword().toOptional().orElse(new char[0])));
} else {
getRedisPassword().toOptional().ifPresent(builder::withPassword);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ private Set<Flag> parseFlags(Set<NodeFlag> source) {

TRANSACTION_RESULT_UNWRAPPER = transactionResult -> transactionResult.stream().collect(Collectors.toList());


}

public static List<Tuple> toTuple(List<byte[]> list) {
Expand Down Expand Up @@ -653,12 +652,12 @@ public static RedisURI sentinelConfigurationToRedisURI(RedisSentinelConfiguratio
builder.withSentinel(sentinelBuilder.build());
}

Optional<String> username = sentinelConfiguration.getUsername();
String username = sentinelConfiguration.getUsername();
RedisPassword password = sentinelConfiguration.getPassword();

if (username.isPresent()) {
if (StringUtils.hasText(username)) {
// See https://github.com/lettuce-io/lettuce-core/issues/1404
builder.withAuthentication(username.get(), new String(password.toOptional().orElse(new char[0])));
builder.withAuthentication(username, new String(password.toOptional().orElse(new char[0])));
} else {
password.toOptional().ifPresent(builder::withPassword);
}
Expand Down
Loading

0 comments on commit bdbba08

Please sign in to comment.