Skip to content

Commit

Permalink
refactored Provider to be implementations of HostSupplier
Browse files Browse the repository at this point in the history
  • Loading branch information
aravindanr committed Dec 5, 2020
1 parent 317cfd0 commit 1021e2e
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,11 @@
package com.netflix.conductor.redis.configuration;

import com.netflix.conductor.redis.config.utils.RedisProperties;
import com.netflix.conductor.redis.jedis.ConfigurationHostSupplierProvider;
import com.netflix.dyno.connectionpool.HostSupplier;
import com.netflix.dyno.connectionpool.TokenMapSupplier;
import com.netflix.dyno.connectionpool.impl.ConnectionPoolConfigurationImpl;
import com.netflix.dyno.jedis.DynoJedisClient;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import redis.clients.jedis.commands.JedisCommands;

Expand All @@ -28,11 +26,6 @@
@ConditionalOnProperty(name = "db", havingValue = "dynomite")
public class DynomiteClusterConfiguration extends JedisCommandsConfigurer {

@Bean
public HostSupplier hostSupplier(RedisProperties properties) {
return new ConfigurationHostSupplierProvider(properties).get();
}

protected JedisCommands createJedisCommands(RedisProperties properties, HostSupplier hostSupplier,
TokenMapSupplier tokenMapSupplier) {
ConnectionPoolConfigurationImpl connectionPoolConfiguration =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.netflix.conductor.redis.configuration;

import com.netflix.conductor.redis.config.utils.RedisProperties;
import com.netflix.conductor.redis.jedis.ConfigurationHostSupplier;
import com.netflix.dyno.connectionpool.HostSupplier;
import com.netflix.dyno.connectionpool.TokenMapSupplier;
import org.springframework.context.annotation.Bean;
Expand All @@ -9,7 +10,12 @@
import static com.netflix.conductor.redis.configuration.RedisCommonConfiguration.DEFAULT_CLIENT_INJECTION_NAME;
import static com.netflix.conductor.redis.configuration.RedisCommonConfiguration.READ_CLIENT_INJECTION_NAME;

public abstract class JedisCommandsConfigurer {
abstract class JedisCommandsConfigurer {

@Bean
public HostSupplier hostSupplier(RedisProperties properties) {
return new ConfigurationHostSupplier(properties);
}

@Bean(name = DEFAULT_CLIENT_INJECTION_NAME)
public JedisCommands jedisCommands(RedisProperties properties, HostSupplier hostSupplier,
Expand All @@ -24,5 +30,5 @@ public JedisCommands readJedisCommands(RedisProperties properties, HostSupplier
}

protected abstract JedisCommands createJedisCommands(RedisProperties properties, HostSupplier hostSupplier,
TokenMapSupplier tokenMapSupplier);
TokenMapSupplier tokenMapSupplier);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
*/
package com.netflix.conductor.redis.configuration;

import com.netflix.conductor.redis.configuration.JedisCommandsConfigurer;
import com.netflix.conductor.redis.config.utils.RedisProperties;
import com.netflix.conductor.redis.jedis.ConfigurationHostSupplierProvider;
import com.netflix.conductor.redis.jedis.JedisCluster;
import com.netflix.dyno.connectionpool.Host;
import com.netflix.dyno.connectionpool.HostSupplier;
import com.netflix.dyno.connectionpool.TokenMapSupplier;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import redis.clients.jedis.HostAndPort;
import redis.clients.jedis.commands.JedisCommands;
Expand All @@ -31,11 +28,6 @@
@ConditionalOnProperty(name = "db", havingValue = "redis_cluster")
public class RedisClusterConfiguration extends JedisCommandsConfigurer {

@Bean
public HostSupplier hostSupplier(RedisProperties properties) {
return new ConfigurationHostSupplierProvider(properties).get();
}

@Override
protected JedisCommands createJedisCommands(RedisProperties properties, HostSupplier hostSupplier, TokenMapSupplier tokenMapSupplier) {
GenericObjectPoolConfig genericObjectPoolConfig = new GenericObjectPoolConfig();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
package com.netflix.conductor.redis.configuration;

import com.netflix.conductor.redis.config.utils.RedisProperties;
import com.netflix.conductor.redis.jedis.ConfigurationHostSupplierProvider;
import com.netflix.conductor.redis.jedis.JedisSentinel;
import com.netflix.dyno.connectionpool.Host;
import com.netflix.dyno.connectionpool.HostSupplier;
Expand All @@ -22,7 +21,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import redis.clients.jedis.JedisSentinelPool;
import redis.clients.jedis.commands.JedisCommands;
Expand All @@ -37,11 +35,6 @@ public class RedisSentinelConfiguration extends JedisCommandsConfigurer {

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

@Bean
public HostSupplier hostSupplier(RedisProperties properties) {
return new ConfigurationHostSupplierProvider(properties).get();
}

@Override
protected JedisCommands createJedisCommands(RedisProperties properties, HostSupplier hostSupplier, TokenMapSupplier tokenMapSupplier) {
GenericObjectPoolConfig genericObjectPoolConfig = new GenericObjectPoolConfig();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,27 @@
import java.util.List;
import java.util.stream.Collectors;

public class ConfigurationHostSupplierProvider {
public class ConfigurationHostSupplier implements HostSupplier {

private static final Logger LOGGER = LoggerFactory.getLogger(ConfigurationHostSupplierProvider.class);
private static final Logger log = LoggerFactory.getLogger(ConfigurationHostSupplier.class);

private final RedisProperties properties;

public ConfigurationHostSupplierProvider(RedisProperties properties) {
public ConfigurationHostSupplier(RedisProperties properties) {
this.properties = properties;
}

public HostSupplier get() {
return this::parseHostsFromConfig;
@Override
public List<Host> getHosts() {
return parseHostsFromConfig();
}

private List<Host> parseHostsFromConfig() {
String hosts = properties.getHosts();
if (hosts == null) {
// FIXME This type of validation probably doesn't belong here.
String message = "Missing dynomite/redis hosts. Ensure 'workflow.dynomite.cluster.hosts' has been set in the supplied configuration.";
LOGGER.error(message);
log.error(message);
throw new RuntimeException(message);
}
return parseHostsFrom(hosts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,25 @@
import com.netflix.dyno.connectionpool.HostBuilder;
import com.netflix.dyno.connectionpool.HostSupplier;

import javax.inject.Provider;
import java.util.List;

public class LocalHostSupplierProvider {
public class LocalhostHostSupplier implements HostSupplier {

private final RedisProperties properties;

public LocalHostSupplierProvider(RedisProperties properties) {
public LocalhostHostSupplier(RedisProperties properties) {
this.properties = properties;
}

public HostSupplier get() {
@Override
public List<Host> getHosts() {
Host dynoHost = new HostBuilder()
.setHostname("localhost")
.setIpAddress("0")
.setRack(properties.getAvailabilityZone())
.setStatus(Host.Status.Up)
.createHost();
return () -> Lists.newArrayList(dynoHost);
.setHostname("localhost")
.setIpAddress("0")
.setRack(properties.getAvailabilityZone())
.setStatus(Host.Status.Up)
.createHost();
return Lists.newArrayList(dynoHost);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,23 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class ConfigurationHostSupplierProviderTest {
public class ConfigurationHostSupplierTest {

private RedisProperties properties;

private ConfigurationHostSupplierProvider provider;
private ConfigurationHostSupplier configurationHostSupplier;

@Before
public void setUp() {
properties = mock(RedisProperties.class);
provider = new ConfigurationHostSupplierProvider(properties);
configurationHostSupplier = new ConfigurationHostSupplier(properties);
}

@Test
public void getHost() {
when(properties.getHosts()).thenReturn("dyno1:8102:us-east-1c");

List<Host> hosts = provider.get().getHosts();
List<Host> hosts = configurationHostSupplier.getHosts();
assertEquals(1, hosts.size());

Host firstHost = hosts.get(0);
Expand All @@ -54,7 +54,7 @@ public void getHost() {
public void getMultipleHosts() {
when(properties.getHosts()).thenReturn("dyno1:8102:us-east-1c;dyno2:8103:us-east-1c");

List<Host> hosts = provider.get().getHosts();
List<Host> hosts = configurationHostSupplier.getHosts();
assertEquals(2, hosts.size());

Host firstHost = hosts.get(0);
Expand All @@ -74,7 +74,7 @@ public void getMultipleHosts() {
public void getAuthenticatedHost() {
when(properties.getHosts()).thenReturn("redis1:6432:us-east-1c:password");

List<Host> hosts = provider.get().getHosts();
List<Host> hosts = configurationHostSupplier.getHosts();
assertEquals(1, hosts.size());

Host firstHost = hosts.get(0);
Expand Down

0 comments on commit 1021e2e

Please sign in to comment.