This is an automated email from the ASF dual-hosted git repository. jensdeppe pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push: new 934f517e92 Revert "GEODE-10126: Replace Geode for Redis properties with system properties (#7478)" (#7576) 934f517e92 is described below commit 934f517e92854cbfeb09d46f8ed9de806ef3eccb Author: Jens Deppe <jde...@vmware.com> AuthorDate: Fri Apr 8 16:53:23 2022 -0700 Revert "GEODE-10126: Replace Geode for Redis properties with system properties (#7478)" (#7576) This reverts commit ee1c412d8af3e6311f569948c3248eddc1fe8009. --- .../geode/distributed/ConfigurationProperties.java | 77 ++++++++++ .../internal/AbstractDistributionConfig.java | 26 ++++ .../distributed/internal/DistributionConfig.java | 94 ++++++++++++ .../internal/DistributionConfigImpl.java | 80 +++++++++- .../geode/internal/cache/GemFireCacheImpl.java | 1 - .../internal/DistributionConfigImplTest.java | 8 +- .../GeodeRedisServerStartupAcceptanceTest.java | 139 +++++++---------- ...eRedisServerStartupUsingGfshAcceptanceTest.java | 58 +++---- .../apache/geode/redis/GeodeRedisServerRule.java | 14 +- .../apache/geode/redis/TestRedisConfiguration.java | 97 ------------ .../test/dunit/rules/RedisClusterStartupRule.java | 18 +-- .../executor/hash/HashesAndCrashesDUnitTest.java | 6 +- .../commands/executor/key/RenameDUnitTest.java | 6 +- .../commands/executor/list/RPushDUnitTest.java | 4 +- .../commands/executor/sortedset/ZRemDUnitTest.java | 4 +- .../sortedset/ZRemRangeByRankDUnitTest.java | 8 +- .../commands/executor/string/MSetDUnitTest.java | 6 +- .../commands/executor/string/MSetNXDUnitTest.java | 6 +- .../auth/AuthWhileServersRestartDUnitTest.java | 4 +- .../redis/session/SessionsAndCrashesDUnitTest.java | 4 +- .../executor/connection/AuthIntegrationTest.java | 26 ++-- .../geode/redis/internal/GeodeRedisServer.java | 23 ++- .../geode/redis/internal/GeodeRedisService.java | 25 +-- .../geode/redis/internal/RedisConfiguration.java | 59 ------- .../SystemPropertyBasedRedisConfiguration.java | 170 --------------------- .../redis/internal/netty/NettyRedisServer.java | 15 +- .../redis/internal/services/RegionProvider.java | 10 +- 27 files changed, 448 insertions(+), 540 deletions(-) diff --git a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java index 1a4e205e2e..d8d32565a6 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java @@ -1920,6 +1920,83 @@ public interface ConfigurationProperties { * <U>Since</U>: Geode 1.0 */ String OFF_HEAP_MEMORY_SIZE = "off-heap-memory-size"; + /** + * The static String definition of the <i>"geode-for-redis-bind-address"</i> property <a + * name="geode-for-redis-bind-address"/a> + * </p> + * <U>Description</U>: Specifies the address on which Geode for Redis is + * listening. If set to the empty string or this property is not specified, the server listens on + * all local addresses. + * </p> + * <U>Default</U>: "" + * </p> + * <U>Since</U>: Geode 1.15 + */ + String GEODE_FOR_REDIS_BIND_ADDRESS = "geode-for-redis-bind-address"; + /** + * The static String definition of the <i>"geode-for-redis-enabled"</i> property <a + * name="geode-for-redis-enabled"/a> + * </p> + * <U>Description</U>: When the default value of false, Geode for Redis is + * not available. + * Set to true to enable Geode for Redis.</td> + * </p> + * <U>Default</U>: false + * When the default value of false, Geode for Redis is not available. + * Set to true to enable Geode for Redis. + * </p> + * <U>Since</U>: Geode 1.15 + */ + String GEODE_FOR_REDIS_ENABLED = "geode-for-redis-enabled"; + /** + * The static String definition of the <i>"geode-for-redis-username"</i> property <a + * name="geode-for-redis-username"/a> + * </p> + * <U>Description</U>: Specifies the default username that the server uses when a client attempts + * to authenticate using only a password. This requires a SecurityManager to be configured. + * </p> + * <U>Default</U>: default + * </p> + * <U>Since</U>: Geode 1.15 + */ + String GEODE_FOR_REDIS_USERNAME = "geode-for-redis-username"; + /** + * The static String definition of the <i>"geode-for-redis-port"</i> property <a + * name="geode-for-redis-port"/a> + * </p> + * <U>Description</U>: Specifies the port on which the Geode for Redis server listens for clients. + * A value of 0 selects a random port.</td> + * </p> + * <U>Default</U>: 6379 + * </p> + * <U>Allowed values</U>: 0..65535 + * </p> + * <U>Since</U>: Geode 1.15 + */ + String GEODE_FOR_REDIS_PORT = "geode-for-redis-port"; + /** + * The static String definition of the <i>"geode-for-redis-redundant-copies"</i> property <a + * name="geode-for-redis-redundant-copies"/a> + * </p> + * <U>Description</U>: Specifies the number of redundant copies Geode for Redis will attempt to + * keep in the cluster. A value of 0 means no extra copies of data will be stored in + * the cluster. Note that extra servers need to be running for redundant copies to be made. For + * example if the cluster only has one server then no redundant copies will exist no matter what + * the value of this property is. Also note that Geode for Redis uses a Geode partitioned region + * to implement redundant copies and this property corresponds to the partitioned region's + * "redundant-copies" attribute. + * Because of this, geode-for-redis-redundant-copies can also be customized by setting + * {@link #ENFORCE_UNIQUE_HOST} or {@link #REDUNDANCY_ZONE}. + * This property must be set the same on every server in the cluster that is running a + * Geode for Redis server.</td> + * </p> + * <U>Default</U>: 1 + * </p> + * <U>Allowed values</U>: 0..3 + * </p> + * <U>Since</U>: Geode 1.15 + */ + String GEODE_FOR_REDIS_REDUNDANT_COPIES = "geode-for-redis-redundant-copies"; /** * The static String definition of the <i>"lock-memory"</i> property <a name="lock-memory"/a> * </p> diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java index 0253d55285..5a90772afa 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java @@ -58,6 +58,11 @@ import static org.apache.geode.distributed.ConfigurationProperties.GATEWAY_SSL_P import static org.apache.geode.distributed.ConfigurationProperties.GATEWAY_SSL_REQUIRE_AUTHENTICATION; import static org.apache.geode.distributed.ConfigurationProperties.GATEWAY_SSL_TRUSTSTORE; import static org.apache.geode.distributed.ConfigurationProperties.GATEWAY_SSL_TRUSTSTORE_PASSWORD; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_ENABLED; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_REDUNDANT_COPIES; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_USERNAME; import static org.apache.geode.distributed.ConfigurationProperties.GROUPS; import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS; import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT; @@ -667,6 +672,17 @@ public abstract class AbstractDistributionConfig extends AbstractConfig return value; } + @ConfigAttributeChecker(name = GEODE_FOR_REDIS_BIND_ADDRESS) + protected String checkRedisBindAddress(String value) { + if (value != null && value.length() > 0 && !LocalHostUtil.isLocalHost(value)) { + throw new IllegalArgumentException( + String.format( + "The geode-for-redis-bind-address %s is not a valid address for this machine. These are the valid addresses for this machine: %s", + value, LocalHostUtil.getMyAddresses())); + } + return value; + } + /** * First check if sslComponents are in the list of valid components. If so, check that no other * *-ssl-* properties other than cluster-ssl-* are set. This would mean one is mixing the "old" @@ -1300,6 +1316,16 @@ public abstract class AbstractDistributionConfig extends AbstractConfig "The protocol that GemFireMemcachedServer understands. Default is ASCII. Values may be ASCII or BINARY"); m.put(MEMCACHED_BIND_ADDRESS, "The address the GemFireMemcachedServer will listen on for remote connections. Default is \"\" which causes the GemFireMemcachedServer to listen on the host's default address. This property is ignored if memcached-port is \"0\"."); + m.put(GEODE_FOR_REDIS_BIND_ADDRESS, + "Specifies the address on which the Geode for Redis server is listening. If set to the empty string or this property is not specified, then all local addresses are listened to. Default is an empty string."); + m.put(GEODE_FOR_REDIS_ENABLED, + "When false Geode for Redis is not available. Set to true to enable Geode for Redis. Default is false."); + m.put(GEODE_FOR_REDIS_USERNAME, + "Specifies the username that the Geode for Redis server uses when a client attempts to authenticate using only a password. The default is 'default'."); + m.put(GEODE_FOR_REDIS_PORT, + "Specifies the port on which the Geode for Redis server listens for client connections. A value of 0 selects a random port. Default is 6379."); + m.put(GEODE_FOR_REDIS_REDUNDANT_COPIES, + "Specifies how many extra copies of data will be stored in the cluster by Geode for Redis. Default is 1."); m.put(ENABLE_CLUSTER_CONFIGURATION, "Enables cluster configuration support in dedicated locators. This allows the locator to share configuration information amongst members and save configuration changes made using GFSH."); m.put(ENABLE_MANAGEMENT_REST_SERVICE, diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java index 138fb1d89c..a7cbb37e73 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java @@ -58,6 +58,11 @@ import static org.apache.geode.distributed.ConfigurationProperties.GATEWAY_SSL_P import static org.apache.geode.distributed.ConfigurationProperties.GATEWAY_SSL_REQUIRE_AUTHENTICATION; import static org.apache.geode.distributed.ConfigurationProperties.GATEWAY_SSL_TRUSTSTORE; import static org.apache.geode.distributed.ConfigurationProperties.GATEWAY_SSL_TRUSTSTORE_PASSWORD; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_ENABLED; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_REDUNDANT_COPIES; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_USERNAME; import static org.apache.geode.distributed.ConfigurationProperties.GROUPS; import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS; import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT; @@ -3487,6 +3492,95 @@ public interface DistributionConfig String MEMCACHED_BIND_ADDRESS_NAME = MEMCACHED_BIND_ADDRESS; String DEFAULT_MEMCACHED_BIND_ADDRESS = ""; + /** + * Returns the value of the {@link ConfigurationProperties#GEODE_FOR_REDIS_BIND_ADDRESS} property + * <p> + * Returns the value of the + * {@link ConfigurationProperties#GEODE_FOR_REDIS_BIND_ADDRESS} property + * + * @return the bind address for GeodeRedisServer + * + * @since GemFire 8.0 + */ + @ConfigAttributeGetter(name = GEODE_FOR_REDIS_BIND_ADDRESS) + String getRedisBindAddress(); + + @ConfigAttributeSetter(name = GEODE_FOR_REDIS_BIND_ADDRESS) + void setRedisBindAddress(String bindAddress); + + @ConfigAttribute(type = String.class) + String REDIS_BIND_ADDRESS_NAME = GEODE_FOR_REDIS_BIND_ADDRESS; + String DEFAULT_REDIS_BIND_ADDRESS = ""; + + /** + * Returns the value of the {@link ConfigurationProperties#GEODE_FOR_REDIS_ENABLED} property + * <p> + * Returns the value of the + * {@link ConfigurationProperties#GEODE_FOR_REDIS_ENABLED} property + * + * @return boolean value indicating whether or not a Redis API for Geode Server should be started + */ + @ConfigAttributeGetter(name = GEODE_FOR_REDIS_ENABLED) + boolean getRedisEnabled(); + + @ConfigAttributeSetter(name = GEODE_FOR_REDIS_ENABLED) + void setRedisEnabled(boolean redisEnabled); + + + @ConfigAttribute(type = Boolean.class) + String REDIS_ENABLED_NAME = GEODE_FOR_REDIS_ENABLED; + boolean DEFAULT_REDIS_ENABLED = false; + + /** + * Returns the value of the {@link ConfigurationProperties#GEODE_FOR_REDIS_USERNAME} property + * <p> + * Returns the value of the + * {@link ConfigurationProperties#GEODE_FOR_REDIS_USERNAME} property + * + * @return the authentication username for GeodeRedisServer + */ + @ConfigAttributeGetter(name = GEODE_FOR_REDIS_USERNAME) + String getRedisUsername(); + + @ConfigAttributeSetter(name = GEODE_FOR_REDIS_USERNAME) + void setRedisUsername(String username); + + @ConfigAttribute(type = String.class) + String REDIS_USERNAME_NAME = GEODE_FOR_REDIS_USERNAME; + String DEFAULT_REDIS_USERNAME = "default"; + + /** + * Returns the value of the {@link ConfigurationProperties#GEODE_FOR_REDIS_PORT} property + * + * @return the port on which GeodeRedisServer should be started + */ + @ConfigAttributeGetter(name = GEODE_FOR_REDIS_PORT) + int getRedisPort(); + + @ConfigAttributeSetter(name = GEODE_FOR_REDIS_PORT) + void setRedisPort(int value); + + @ConfigAttribute(type = Integer.class, min = 0, max = 65535) + String REDIS_PORT_NAME = GEODE_FOR_REDIS_PORT; + int DEFAULT_REDIS_PORT = 6379; + + /** + * Returns the value of the {@link ConfigurationProperties#GEODE_FOR_REDIS_REDUNDANT_COPIES} + * property + * + * @return the Geode for Redis redundant copies + * + */ + @ConfigAttributeGetter(name = GEODE_FOR_REDIS_REDUNDANT_COPIES) + int getRedisRedundantCopies(); + + @ConfigAttributeSetter(name = GEODE_FOR_REDIS_REDUNDANT_COPIES) + void setRedisRedundantCopies(int value); + + @ConfigAttribute(type = Integer.class, min = 0, max = 3) + String REDIS_REDUNDANT_COPIES_NAME = GEODE_FOR_REDIS_REDUNDANT_COPIES; + int DEFAULT_REDIS_REDUNDANT_COPIES = 1; + // Added for the HTTP service /** diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java index a0cc225db6..50dace8f04 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java @@ -489,6 +489,22 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement */ private boolean distributedTransactions = DEFAULT_DISTRIBUTED_TRANSACTIONS; + /** + * Bind address for GeodeRedisServer + */ + private String redisBindAddress = DEFAULT_REDIS_BIND_ADDRESS; + + private Boolean redisEnabled = DEFAULT_REDIS_ENABLED; + + private String redisUsername = DEFAULT_REDIS_USERNAME; + + /** + * port on which GeodeRedisServer is started + */ + private int redisPort = DEFAULT_REDIS_PORT; + private int redisRedundantCopies = DEFAULT_REDIS_REDUNDANT_COPIES; + + private boolean jmxManager = Boolean.getBoolean(InternalLocator.FORCE_LOCATOR_DM_TYPE) || DEFAULT_JMX_MANAGER; private boolean jmxManagerStart = DEFAULT_JMX_MANAGER_START; @@ -778,6 +794,11 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement memcachedPort = other.getMemcachedPort(); memcachedProtocol = other.getMemcachedProtocol(); memcachedBindAddress = other.getMemcachedBindAddress(); + redisPort = other.getRedisPort(); + redisBindAddress = other.getRedisBindAddress(); + redisUsername = other.getRedisUsername(); + redisEnabled = other.getRedisEnabled(); + redisRedundantCopies = other.getRedisRedundantCopies(); userCommandPackages = other.getUserCommandPackages(); // following added for 8.0 @@ -3305,6 +3326,11 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement .append(httpServiceBindAddress, that.httpServiceBindAddress) .append(memcachedProtocol, that.memcachedProtocol) .append(memcachedBindAddress, that.memcachedBindAddress) + .append(redisBindAddress, that.redisBindAddress) + .append(redisUsername, that.redisUsername) + .append(redisPort, that.redisPort) + .append(redisRedundantCopies, that.redisRedundantCopies) + .append(redisEnabled, that.redisEnabled) .append(jmxManagerBindAddress, that.jmxManagerBindAddress) .append(jmxManagerHostnameForClients, that.jmxManagerHostnameForClients) .append(jmxManagerPasswordFile, that.jmxManagerPasswordFile) @@ -3400,7 +3426,9 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement .append(loadSharedConfigurationFromDir).append(clusterConfigDir).append(httpServicePort) .append(httpServiceBindAddress).append(startDevRestApi).append(memcachedPort) .append(memcachedProtocol).append(memcachedBindAddress).append(distributedTransactions) - .append(jmxManager) + .append(redisPort).append(redisBindAddress).append(redisUsername).append( + redisRedundantCopies) + .append(redisEnabled).append(jmxManager) .append(jmxManagerStart).append(jmxManagerPort).append(jmxManagerBindAddress) .append(jmxManagerHostnameForClients).append(jmxManagerPasswordFile) .append(jmxManagerAccessFile).append(jmxManagerHttpPort).append(jmxManagerUpdateRate) @@ -3501,6 +3529,56 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement memcachedProtocol = protocol; } + @Override + public int getRedisPort() { + return redisPort; + } + + @Override + public void setRedisPort(int value) { + redisPort = value; + } + + @Override + public int getRedisRedundantCopies() { + return redisRedundantCopies; + } + + @Override + public void setRedisRedundantCopies(int value) { + redisRedundantCopies = value; + } + + @Override + public String getRedisBindAddress() { + return redisBindAddress; + } + + @Override + public void setRedisBindAddress(String bindAddress) { + redisBindAddress = bindAddress; + } + + @Override + public String getRedisUsername() { + return redisUsername; + } + + @Override + public void setRedisUsername(String username) { + redisUsername = username; + } + + @Override + public boolean getRedisEnabled() { + return redisEnabled; + } + + @Override + public void setRedisEnabled(boolean redisServiceEnabled) { + redisEnabled = redisServiceEnabled; + } + @Override public String getOffHeapMemorySize() { return offHeapMemorySize; diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java index e385581fbf..afc041ffed 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java @@ -1505,7 +1505,6 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has } } catch (Exception ex) { logger.warn("Cache service " + service.getClass().getName() + " failed to initialize", ex); - throw ex; } } } diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigImplTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigImplTest.java index a875695813..c50147d292 100644 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigImplTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigImplTest.java @@ -108,7 +108,7 @@ public class DistributionConfigImplTest { @Test public void testGetAttributeNames() { String[] attNames = AbstractDistributionConfig._getAttNames(); - assertThat(attNames).hasSize(167); + assertThat(attNames).hasSize(172); List<String> boolList = new ArrayList<>(); List<String> intList = new ArrayList<>(); @@ -142,9 +142,9 @@ public class DistributionConfigImplTest { // TODO - This makes no sense. One has no idea what the correct expected number of attributes // are. - assertThat(boolList).hasSize(35); - assertThat(intList).hasSize(34); - assertThat(stringList).hasSize(88); + assertThat(boolList).hasSize(36); + assertThat(intList).hasSize(36); + assertThat(stringList).hasSize(90); assertThat(fileList).hasSize(5); assertThat(otherList).hasSize(5); } diff --git a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/GeodeRedisServerStartupAcceptanceTest.java b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/GeodeRedisServerStartupAcceptanceTest.java index c518b5c661..523e63c335 100644 --- a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/GeodeRedisServerStartupAcceptanceTest.java +++ b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/GeodeRedisServerStartupAcceptanceTest.java @@ -16,10 +16,10 @@ package org.apache.geode.redis; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_BIND_ADDRESS; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_ENABLED; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_REDUNDANT_COPIES; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_ENABLED; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_REDUNDANT_COPIES; import static org.apache.geode.test.dunit.IgnoredException.addIgnoredException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -28,20 +28,16 @@ import java.net.BindException; import java.net.InetSocketAddress; import java.net.ServerSocket; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; -import redis.clients.jedis.Jedis; -import org.apache.geode.distributed.internal.DistributionConfig; import org.apache.geode.internal.AvailablePortHelper; import org.apache.geode.internal.cache.PartitionedRegion; import org.apache.geode.internal.inet.LocalHostUtil; import org.apache.geode.redis.internal.GeodeRedisServer; import org.apache.geode.redis.internal.GeodeRedisService; import org.apache.geode.redis.internal.services.RegionProvider; -import org.apache.geode.test.dunit.VM; import org.apache.geode.test.dunit.rules.ClusterStartupRule; import org.apache.geode.test.dunit.rules.MemberVM; import org.apache.geode.test.dunit.rules.RedisClusterStartupRule; @@ -52,18 +48,13 @@ public class GeodeRedisServerStartupAcceptanceTest { @Rule public RedisClusterStartupRule cluster = new RedisClusterStartupRule(); - @After - public void cleanup() { - VM.getVM(0).bounce(); - } - @Category(IgnoreInRepeatTestTasks.class) @Test public void startupOnDefaultPort() { MemberVM server = cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "6379") - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); + .withProperty(GEODE_FOR_REDIS_PORT, "6379") + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true")); assertThat(cluster.getRedisPort(server)).isEqualTo(GeodeRedisServer.DEFAULT_REDIS_SERVER_PORT); } @@ -71,8 +62,8 @@ public class GeodeRedisServerStartupAcceptanceTest { @Test public void startupOnDefaultPort_whenPortIsNotSpecified() { MemberVM server = cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true")); assertThat(cluster.getRedisPort(server)).isEqualTo(GeodeRedisServer.DEFAULT_REDIS_SERVER_PORT); } @@ -80,9 +71,9 @@ public class GeodeRedisServerStartupAcceptanceTest { @Test public void startupOnRandomPort_whenPortIsZero() { MemberVM server = cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "0") - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); + .withProperty(GEODE_FOR_REDIS_PORT, "0") + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true")); assertThat(cluster.getRedisPort(server)) .isNotEqualTo(GeodeRedisServer.DEFAULT_REDIS_SERVER_PORT); @@ -90,7 +81,9 @@ public class GeodeRedisServerStartupAcceptanceTest { @Test public void doNotStartup_whenRedisServiceIsNotEnabled() { - MemberVM server = cluster.startServerVM(0); + MemberVM server = cluster.startServerVM(0, s -> s + .withProperty(GEODE_FOR_REDIS_PORT, "0") + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost")); server.invoke(() -> { assertThat(ClusterStartupRule.getCache().getService(GeodeRedisService.class)) @@ -102,10 +95,10 @@ public class GeodeRedisServerStartupAcceptanceTest { @Test public void startupFailsGivenIllegalPort() { assertThatThrownBy(() -> cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "-1") - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true"))) - .hasStackTraceContaining(GEODE_FOR_REDIS_PORT + " is out of range"); + .withProperty(GEODE_FOR_REDIS_PORT, "-1") + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true"))).hasRootCauseMessage( + "Could not set \"geode-for-redis-port\" to \"-1\" because its value can not be less than \"0\"."); } @Test @@ -117,9 +110,9 @@ public class GeodeRedisServerStartupAcceptanceTest { interferingSocket.bind(new InetSocketAddress("localhost", port)); assertThatThrownBy(() -> cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "" + port) - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true"))) + .withProperty(GEODE_FOR_REDIS_PORT, "" + port) + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true"))) .hasRootCauseInstanceOf(BindException.class); } } @@ -128,11 +121,11 @@ public class GeodeRedisServerStartupAcceptanceTest { public void startupFailsGivenInvalidBindAddress() { int port = AvailablePortHelper.getRandomAvailableTCPPort(); - addIgnoredException("1.1.1.1 is not a valid address for this machine"); + addIgnoredException("Could not start server compatible with Redis"); assertThatThrownBy(() -> cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "" + port) - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "1.1.1.1") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true"))) + .withProperty(GEODE_FOR_REDIS_PORT, "" + port) + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "1.1.1.1") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true"))) .hasStackTraceContaining( "The geode-for-redis-bind-address 1.1.1.1 is not a valid address for this machine"); } @@ -141,34 +134,31 @@ public class GeodeRedisServerStartupAcceptanceTest { public void startupFailsGivenInvalidRedundantCopies() { int port = AvailablePortHelper.getRandomAvailableTCPPort(); - addIgnoredException("Redundant copies is out of range"); + addIgnoredException("Could not start server compatible with Redis"); assertThatThrownBy(() -> cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "" + port) - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true") - .withSystemProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES, "4"))) - .hasStackTraceContaining(GEODE_FOR_REDIS_REDUNDANT_COPIES + " is out of range"); - } - - @Test - public void startupFailsGivenNegativeRedundantCopies() { - int port = AvailablePortHelper.getRandomAvailableTCPPort(); - - addIgnoredException("Redundant copies is out of range"); + .withProperty(GEODE_FOR_REDIS_PORT, "" + port) + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true") + .withProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES, "4"))) + .hasStackTraceContaining( + "Could not set \"" + GEODE_FOR_REDIS_REDUNDANT_COPIES + + "\" to \"4\" because its value can not be greater than \"3\"."); assertThatThrownBy(() -> cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "" + port) - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true") - .withSystemProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES, "-1"))) - .hasStackTraceContaining(GEODE_FOR_REDIS_REDUNDANT_COPIES + " is out of range"); + .withProperty(GEODE_FOR_REDIS_PORT, "" + port) + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true") + .withProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES, "-1"))) + .hasStackTraceContaining( + "Could not set \"" + GEODE_FOR_REDIS_REDUNDANT_COPIES + + "\" to \"-1\" because its value can not be less than \"0\"."); } @Test public void startupOnSpecifiedPort() { MemberVM server = cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "4242") - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); + .withProperty(GEODE_FOR_REDIS_PORT, "4242") + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true")); assertThat(cluster.getRedisPort(server)).isEqualTo(4242); } @@ -177,9 +167,9 @@ public class GeodeRedisServerStartupAcceptanceTest { public void startupWorksGivenAnyLocalAddress() { String anyLocal = LocalHostUtil.getAnyLocalAddress().getHostAddress(); MemberVM server = cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "0") - .withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, anyLocal) - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); + .withProperty(GEODE_FOR_REDIS_PORT, "0") + .withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, anyLocal) + .withProperty(GEODE_FOR_REDIS_ENABLED, "true")); assertThat(cluster.getRedisPort(server)) .isNotEqualTo(GeodeRedisServer.DEFAULT_REDIS_SERVER_PORT); @@ -188,38 +178,19 @@ public class GeodeRedisServerStartupAcceptanceTest { @Test public void startupWorksGivenNoBindAddress() { MemberVM server = cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, "0") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); + .withProperty(GEODE_FOR_REDIS_PORT, "0") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true")); assertThat(cluster.getRedisPort(server)) .isNotEqualTo(GeodeRedisServer.DEFAULT_REDIS_SERVER_PORT); } - @Test - public void bindAddressDefaultsToServerBindAddress() throws Exception { - MemberVM server = cluster.startServerVM(0, s -> s - .withProperty(DistributionConfig.SERVER_BIND_ADDRESS_NAME, "127.0.0.1") - .withSystemProperty(GEODE_FOR_REDIS_PORT, "0") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); - - int port = cluster.getRedisPort(server); - try (Jedis jedis = new Jedis("127.0.0.1", port)) { - assertThat(jedis.ping()).isEqualTo("PONG"); - } - - String hostname = LocalHostUtil.getLocalHostName(); - try (Jedis jedis = new Jedis(hostname, port)) { - assertThatThrownBy(jedis::ping) - .hasStackTraceContaining("Connection refused "); - } - } - @Test public void startupWorksGivenRedundantCopiesOfZero() { MemberVM server = cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES, "0") - .withSystemProperty(GEODE_FOR_REDIS_PORT, "0") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); + .withProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES, "0") + .withProperty(GEODE_FOR_REDIS_PORT, "0") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true")); int RedundantCopies = cluster.getMember(0).invoke("getRedundantCopies", () -> { PartitionedRegion pr = (PartitionedRegion) RedisClusterStartupRule.getCache() @@ -232,9 +203,9 @@ public class GeodeRedisServerStartupAcceptanceTest { @Test public void startupWorksGivenRedundantCopiesOfThree() { MemberVM server = cluster.startServerVM(0, s -> s - .withSystemProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES, "3") - .withSystemProperty(GEODE_FOR_REDIS_PORT, "0") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true")); + .withProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES, "3") + .withProperty(GEODE_FOR_REDIS_PORT, "0") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true")); int RedundantCopies = cluster.getMember(0).invoke("getRedundantCopies", () -> { PartitionedRegion pr = (PartitionedRegion) RedisClusterStartupRule.getCache() diff --git a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/GeodeRedisServerStartupUsingGfshAcceptanceTest.java b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/GeodeRedisServerStartupUsingGfshAcceptanceTest.java index 1784ce5a7a..62dd84deee 100644 --- a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/GeodeRedisServerStartupUsingGfshAcceptanceTest.java +++ b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/GeodeRedisServerStartupUsingGfshAcceptanceTest.java @@ -15,25 +15,20 @@ package org.apache.geode.redis.internal.commands.executor; -import static org.apache.geode.redis.internal.RedisConfiguration.DEFAULT_REDIS_PORT; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_BIND_ADDRESS; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_ENABLED; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; -import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.ServerSocket; -import java.util.Set; import org.junit.Rule; import org.junit.Test; import redis.clients.jedis.Jedis; import redis.clients.jedis.exceptions.JedisConnectionException; +import org.apache.geode.distributed.ConfigurationProperties; import org.apache.geode.internal.AvailablePortHelper; import org.apache.geode.internal.inet.LocalHostUtil; import org.apache.geode.test.dunit.rules.RequiresRedisHome; @@ -58,8 +53,9 @@ public class GeodeRedisServerStartupUsingGfshAcceptanceTest { "start server", "--server-port", "0", "--name", "same-port-and-address-server", - "--J=-D" + GEODE_FOR_REDIS_BIND_ADDRESS + "=localhost", - "--J=-D" + GEODE_FOR_REDIS_PORT + "=" + port, + "--J=-Dgemfire.geode-for-redis-enabled=true", + "--J=-Dgemfire.geode-for-redis-bind-address=localhost", + "--J=-Dgemfire.geode-for-redis-port=" + port, "--classpath=" + redisHome.getGeodeForRedisHome() + "/lib/*"); GfshExecution execution; @@ -82,8 +78,8 @@ public class GeodeRedisServerStartupUsingGfshAcceptanceTest { "start server", "--server-port", "0", "--name", "same-port-all-addresses-server", - "--J=-D" + GEODE_FOR_REDIS_BIND_ADDRESS + "=0.0.0.0", - "--J=-D" + GEODE_FOR_REDIS_PORT + "=" + port, + "--J=-Dgemfire.geode-for-redis-enabled=true", + "--J=-Dgemfire.geode-for-redis-port=" + port, "--classpath=" + redisHome.getGeodeForRedisHome() + "/lib/*"); GfshExecution execution; @@ -104,7 +100,8 @@ public class GeodeRedisServerStartupUsingGfshAcceptanceTest { "start server", "--server-port", "0", "--name", "invalid-bind-server", - "--J=-D" + GEODE_FOR_REDIS_BIND_ADDRESS + "=1.1.1.1", + "--J=-Dgemfire.geode-for-redis-enabled=true", + "--J=-Dgemfire.geode-for-redis-bind-address=1.1.1.1", "--classpath=" + redisHome.getGeodeForRedisHome() + "/lib/*"); GfshExecution execution; @@ -119,11 +116,11 @@ public class GeodeRedisServerStartupUsingGfshAcceptanceTest { @Test public void gfshStartsRedisServer_whenRedisEnabled() { String command = "start server --server-port=0 " - + "--J=-D" + GEODE_FOR_REDIS_ENABLED + "=true" + + "--J=-Dgemfire." + ConfigurationProperties.GEODE_FOR_REDIS_ENABLED + "=true" + " --classpath=" + redisHome.getGeodeForRedisHome() + "/lib/*"; gfshRule.execute(command); - try (Jedis jedis = new Jedis(BIND_ADDRESS, DEFAULT_REDIS_PORT)) { + try (Jedis jedis = new Jedis(BIND_ADDRESS, 6379)) { assertThat(jedis.ping()).isEqualTo("PONG"); } } @@ -132,7 +129,8 @@ public class GeodeRedisServerStartupUsingGfshAcceptanceTest { public void gfshStartsRedisServer_whenCustomPort() { int port = AvailablePortHelper.getRandomAvailableTCPPort(); String command = "start server --server-port=0 " - + " --J=-D" + GEODE_FOR_REDIS_PORT + "=" + port + + "--J=-Dgemfire." + ConfigurationProperties.GEODE_FOR_REDIS_ENABLED + "=true" + + " --J=-Dgemfire." + ConfigurationProperties.GEODE_FOR_REDIS_PORT + "=" + port + " --classpath=" + redisHome.getGeodeForRedisHome() + "/lib/*"; gfshRule.execute(command); @@ -145,12 +143,12 @@ public class GeodeRedisServerStartupUsingGfshAcceptanceTest { @Test public void gfshStartsRedisServer_whenCustomPortAndBindAddress() { int port = AvailablePortHelper.getRandomAvailableTCPPort(); - Set<InetAddress> myAddresses = LocalHostUtil.getMyAddresses(); - String anyLocal = myAddresses.stream().findFirst().get().getHostAddress(); - + String anyLocal = LocalHostUtil.getAnyLocalAddress().getHostAddress(); String command = "start server --server-port=0 " - + " --J=-D" + GEODE_FOR_REDIS_PORT + "=" + port - + " --J=-D" + GEODE_FOR_REDIS_BIND_ADDRESS + "=" + anyLocal + + "--J=-Dgemfire." + ConfigurationProperties.GEODE_FOR_REDIS_ENABLED + "=true" + + " --J=-Dgemfire." + ConfigurationProperties.GEODE_FOR_REDIS_PORT + "=" + port + + " --J=-Dgemfire." + ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS + "=" + + anyLocal + " --classpath=" + redisHome.getGeodeForRedisHome() + "/lib/*"; gfshRule.execute(command); @@ -161,27 +159,13 @@ public class GeodeRedisServerStartupUsingGfshAcceptanceTest { } @Test - public void gfshStartsRedisServer_withAnyLocalBindAddress() { + public void gfshDoesNotStartRedisServer_whenNotRedisEnabled() { int port = AvailablePortHelper.getRandomAvailableTCPPort(); String anyLocal = LocalHostUtil.getAnyLocalAddress().getHostAddress(); - - String command = "start server --server-port=0 " - + " --J=-D" + GEODE_FOR_REDIS_PORT + "=" + port - + " --J=-D" + GEODE_FOR_REDIS_BIND_ADDRESS + "=" + anyLocal - + " --classpath=" + redisHome.getGeodeForRedisHome() + "/lib/*"; - - gfshRule.execute(command); - - try (Jedis jedis = new Jedis(BIND_ADDRESS, port)) { - assertThat(jedis.ping()).isEqualTo("PONG"); - } - } - - @Test - public void gfshDoesNotStartRedisServer_whenRedisDisabled() { - int port = AvailablePortHelper.getRandomAvailableTCPPort(); String command = "start server --server-port=0 " - + "--J=-D" + GEODE_FOR_REDIS_ENABLED + "=false" + + "--J=-Dgemfire." + ConfigurationProperties.GEODE_FOR_REDIS_PORT + "=" + port + + " --J=-Dgemfire." + ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS + "=" + + anyLocal + " --classpath=" + redisHome.getGeodeForRedisHome() + "/lib/*"; gfshRule.execute(command); diff --git a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java b/geode-for-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java index ab91ab4aee..265584d008 100644 --- a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java +++ b/geode-for-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java @@ -16,6 +16,8 @@ package org.apache.geode.redis; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_REDUNDANT_COPIES; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_USERNAME; import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS; import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL; import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT; @@ -24,7 +26,6 @@ import org.apache.geode.cache.CacheFactory; import org.apache.geode.cache.GemFireCache; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.redis.internal.GeodeRedisServer; -import org.apache.geode.redis.internal.RedisConfiguration; import org.apache.geode.test.junit.rules.serializable.SerializableExternalResource; public class GeodeRedisServerRule extends SerializableExternalResource { @@ -38,6 +39,7 @@ public class GeodeRedisServerRule extends SerializableExternalResource { cacheFactory.set(LOG_LEVEL, "warn"); cacheFactory.set(MCAST_PORT, "0"); cacheFactory.set(LOCATORS, ""); + cacheFactory.set(GEODE_FOR_REDIS_REDUNDANT_COPIES, "0"); } public void setEnableUnsupportedCommands(boolean allow) { @@ -46,10 +48,8 @@ public class GeodeRedisServerRule extends SerializableExternalResource { @Override protected void before() { - RedisConfiguration config = TestRedisConfiguration.builder() - .withAddress("127.0.0.1").withPort(0).build(); cache = cacheFactory.create(); - server = new GeodeRedisServer(config, (InternalCache) cache); + server = new GeodeRedisServer("localhost", 0, (InternalCache) cache); server.setAllowUnsupportedCommands(enableUnsupportedCommands); // Ensure that buckets are created up front @@ -74,6 +74,12 @@ public class GeodeRedisServerRule extends SerializableExternalResource { return server; } + public GeodeRedisServerRule withUsername(String username) { + cacheFactory.set(GEODE_FOR_REDIS_USERNAME, username); + + return this; + } + public int getPort() { return server.getPort(); } diff --git a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/TestRedisConfiguration.java b/geode-for-redis/src/commonTest/java/org/apache/geode/redis/TestRedisConfiguration.java deleted file mode 100644 index 95e035c7a8..0000000000 --- a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/TestRedisConfiguration.java +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more contributor license - * agreements. See the NOTICE file distributed with this work for additional information regarding - * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance with the License. You may obtain a - * copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ - -package org.apache.geode.redis; - -import org.apache.geode.redis.internal.RedisConfiguration; -import org.apache.geode.test.dunit.rules.RedisClusterStartupRule; - -public class TestRedisConfiguration implements RedisConfiguration { - - private final String bindAddress; - private final int port; - private final int redundantCopies; - private final String username; - - public static class Builder { - private String address = RedisClusterStartupRule.BIND_ADDRESS; - private int port = RedisConfiguration.DEFAULT_REDIS_PORT; - private int redundantCopies = RedisConfiguration.DEFAULT_REDIS_REDUNDANT_COPIES; - private String username = RedisConfiguration.DEFAULT_REDIS_USERNAME; - - private Builder() {} - - public RedisConfiguration build() { - return new TestRedisConfiguration(address, port, redundantCopies, username); - } - - public Builder withAddress(String address) { - this.address = address; - return this; - } - - public Builder withPort(int port) { - this.port = port; - return this; - } - - public Builder withRedundantCopies(int redundantCopies) { - this.redundantCopies = redundantCopies; - return this; - } - - public Builder withUsername(String username) { - this.username = username; - return this; - } - } - - private TestRedisConfiguration(String bindAddress, int port, int redundantCopies, - String username) { - this.bindAddress = bindAddress; - this.port = port; - this.redundantCopies = redundantCopies; - this.username = username; - } - - public static Builder builder() { - return new Builder(); - } - - @Override - public boolean isEnabled() { - return true; - } - - @Override - public int getPort() { - return port; - } - - @Override - public String getBindAddress() { - return bindAddress; - } - - @Override - public int getRedundantCopies() { - return redundantCopies; - } - - @Override - public String getUsername() { - return username; - } -} diff --git a/geode-for-redis/src/commonTest/java/org/apache/geode/test/dunit/rules/RedisClusterStartupRule.java b/geode-for-redis/src/commonTest/java/org/apache/geode/test/dunit/rules/RedisClusterStartupRule.java index ce57091dfa..b675daa487 100644 --- a/geode-for-redis/src/commonTest/java/org/apache/geode/test/dunit/rules/RedisClusterStartupRule.java +++ b/geode-for-redis/src/commonTest/java/org/apache/geode/test/dunit/rules/RedisClusterStartupRule.java @@ -16,9 +16,9 @@ package org.apache.geode.test.dunit.rules; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_BIND_ADDRESS; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_ENABLED; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_ENABLED; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import java.util.Properties; import java.util.Set; @@ -89,17 +89,17 @@ public class RedisClusterStartupRule extends ClusterStartupRule { } private ServerStarterRule withRedis(ServerStarterRule rule) { - return rule.withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, BIND_ADDRESS) - .withSystemProperty(GEODE_FOR_REDIS_PORT, "0") - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true") + return rule.withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, BIND_ADDRESS) + .withProperty(GEODE_FOR_REDIS_PORT, "0") + .withProperty(GEODE_FOR_REDIS_ENABLED, "true") .withSystemProperty(GeodeRedisServer.ENABLE_UNSUPPORTED_COMMANDS_PARAM, "true"); } private ServerStarterRule withRedis(ServerStarterRule rule, String redisPort) { - return rule.withSystemProperty(GEODE_FOR_REDIS_BIND_ADDRESS, BIND_ADDRESS) - .withSystemProperty(GEODE_FOR_REDIS_PORT, redisPort) - .withSystemProperty(GEODE_FOR_REDIS_ENABLED, "true") + return rule.withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, BIND_ADDRESS) + .withProperty(GEODE_FOR_REDIS_PORT, redisPort) + .withProperty(GEODE_FOR_REDIS_ENABLED, "true") .withSystemProperty(GeodeRedisServer.ENABLE_UNSUPPORTED_COMMANDS_PARAM, "true"); } diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/hash/HashesAndCrashesDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/hash/HashesAndCrashesDUnitTest.java index 5e4ea52909..ebc34c6a0c 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/hash/HashesAndCrashesDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/hash/HashesAndCrashesDUnitTest.java @@ -16,7 +16,7 @@ package org.apache.geode.redis.internal.commands.executor.hash; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.assertj.core.api.Assertions.assertThat; import java.time.Duration; @@ -72,13 +72,13 @@ public class HashesAndCrashesDUnitTest { int redisPort2 = AvailablePortHelper.getRandomAvailableTCPPort(); serverOperator2 = s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, redisPort2 + "") + .withProperty(GEODE_FOR_REDIS_PORT, redisPort2 + "") .withConnectionToLocator(locatorPort); clusterStartUp.startRedisVM(2, serverOperator2); int redisPort3 = AvailablePortHelper.getRandomAvailableTCPPort(); serverOperator3 = s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, redisPort3 + "") + .withProperty(GEODE_FOR_REDIS_PORT, redisPort3 + "") .withConnectionToLocator(locatorPort); clusterStartUp.startRedisVM(3, serverOperator3); diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/key/RenameDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/key/RenameDUnitTest.java index c1c27e7bfc..68c1ce52bb 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/key/RenameDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/key/RenameDUnitTest.java @@ -15,7 +15,7 @@ package org.apache.geode.redis.internal.commands.executor.key; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT; import static org.assertj.core.api.Assertions.assertThat; @@ -81,7 +81,7 @@ public class RenameDUnitTest { final String finalRedisPort = Integer.toString(server3Port); final int finalLocatorPort = locatorPort; clusterStartUp.startRedisVM(3, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) + .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) .withConnectionToLocator(finalLocatorPort)); int redisServerPort1 = clusterStartUp.getRedisPort(1); @@ -251,7 +251,7 @@ public class RenameDUnitTest { GeodeAwaitility.await().during(Duration.ofMillis(2000)).until(() -> true); clusterStartUp.crashVM(3); clusterStartUp.startRedisVM(3, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) + .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) .withConnectionToLocator(finalLocatorPort)); clusterStartUp.rebalanceAllRegions(); } diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/list/RPushDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/list/RPushDUnitTest.java index ad8a296928..1a9c57237b 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/list/RPushDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/list/RPushDUnitTest.java @@ -15,7 +15,7 @@ package org.apache.geode.redis.internal.commands.executor.list; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS; import static org.assertj.core.api.Assertions.assertThat; @@ -161,7 +161,7 @@ public class RPushDUnitTest { Thread.sleep(1000); clusterStartUp.crashVM(3); clusterStartUp.startRedisVM(3, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, Integer.toString(finalRedisPort)) + .withProperty(GEODE_FOR_REDIS_PORT, Integer.toString(finalRedisPort)) .withConnectionToLocator(finalLocatorPort)); clusterStartUp.rebalanceAllRegions(); } diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemDUnitTest.java index dc21a389ed..31bf3b61bb 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemDUnitTest.java @@ -14,7 +14,7 @@ */ package org.apache.geode.redis.internal.commands.executor.sortedset; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS; import static org.assertj.core.api.Assertions.assertThat; @@ -88,7 +88,7 @@ public class ZRemDUnitTest { int locatorPort) { int redisPort = AvailablePortHelper.getRandomAvailableTCPPort(); SerializableFunction<ServerStarterRule> serverOperator = s -> s - .withSystemProperty(GEODE_FOR_REDIS_PORT, redisPort + "") + .withProperty(GEODE_FOR_REDIS_PORT, redisPort + "") .withConnectionToLocator(locatorPort); MemberVM server = clusterStartUp.startRedisVM(vmId, serverOperator); diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByRankDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByRankDUnitTest.java index 4160c9c03d..03567d5237 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByRankDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByRankDUnitTest.java @@ -14,7 +14,7 @@ */ package org.apache.geode.redis.internal.commands.executor.sortedset; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.apache.geode.test.awaitility.GeodeAwaitility.await; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT; @@ -70,13 +70,13 @@ public class ZRemRangeByRankDUnitTest { int[] serverPorts = AvailablePortHelper.getRandomAvailableTCPPorts(3); MemberVM server1 = clusterStartUp.startRedisVM(1, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[0])) + .withProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[0])) .withConnectionToLocator(locatorPort)); MemberVM server2 = clusterStartUp.startRedisVM(2, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[1])) + .withProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[1])) .withConnectionToLocator(locatorPort)); MemberVM server3 = clusterStartUp.startRedisVM(3, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[2])) + .withProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[2])) .withConnectionToLocator(locatorPort)); servers = new ArrayList<>(); diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/MSetDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/MSetDUnitTest.java index 588b29149b..1293811be8 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/MSetDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/MSetDUnitTest.java @@ -15,8 +15,8 @@ package org.apache.geode.redis.internal.commands.executor.string; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.apache.geode.redis.internal.RedisConstants.SERVER_ERROR_MESSAGE; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; import static org.apache.geode.redis.internal.services.RegionProvider.DEFAULT_REDIS_REGION_NAME; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT; @@ -85,7 +85,7 @@ public class MSetDUnitTest { String finalRedisPort = Integer.toString(server3Port); int finalLocatorPort = locatorPort; clusterStartUp.startRedisVM(3, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) + .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) .withConnectionToLocator(finalLocatorPort)); clusterStartUp.enableDebugLogging(1); @@ -149,7 +149,7 @@ public class MSetDUnitTest { Thread.sleep(2000); clusterStartUp.crashVM(3); clusterStartUp.startRedisVM(3, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) + .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) .withConnectionToLocator(finalLocatorPort)); clusterStartUp.enableDebugLogging(3); rebalanceAllRegions(server1); diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/MSetNXDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/MSetNXDUnitTest.java index 9ec60164ab..e9134ef962 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/MSetNXDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/MSetNXDUnitTest.java @@ -15,7 +15,7 @@ package org.apache.geode.redis.internal.commands.executor.string; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT; import static org.assertj.core.api.Assertions.assertThat; @@ -66,7 +66,7 @@ public class MSetNXDUnitTest { final String finalRedisPort = Integer.toString(server3Port); final int finalLocatorPort = locatorPort; clusterStartUp.startRedisVM(3, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) + .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) .withConnectionToLocator(finalLocatorPort)); final int redisServerPort1 = clusterStartUp.getRedisPort(1); @@ -129,7 +129,7 @@ public class MSetNXDUnitTest { Thread.sleep(2000); clusterStartUp.crashVM(3); clusterStartUp.startRedisVM(3, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) + .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) .withConnectionToLocator(finalLocatorPort)); rebalanceAllRegions(server1); } diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/auth/AuthWhileServersRestartDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/auth/AuthWhileServersRestartDUnitTest.java index e6d8ad0e19..6c625d530f 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/auth/AuthWhileServersRestartDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/auth/AuthWhileServersRestartDUnitTest.java @@ -15,7 +15,7 @@ package org.apache.geode.redis.internal.executor.auth; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS; import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT; import static redis.clients.jedis.JedisCluster.DEFAULT_MAX_ATTEMPTS; @@ -72,7 +72,7 @@ public class AuthWhileServersRestartDUnitTest { String finalRedisPort = Integer.toString(server3Port); operatorForVM3 = serverOperator.compose(o -> o - .withSystemProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) + .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort) .withConnectionToLocator(locatorPort)); clusterStartUp.startRedisVM(3, operatorForVM3); diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionsAndCrashesDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionsAndCrashesDUnitTest.java index a07a83ca07..5c6726607f 100644 --- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionsAndCrashesDUnitTest.java +++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionsAndCrashesDUnitTest.java @@ -16,7 +16,7 @@ package org.apache.geode.redis.session; import static java.util.Collections.singletonMap; -import static org.apache.geode.redis.internal.SystemPropertyBasedRedisConfiguration.GEODE_FOR_REDIS_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT; import static org.assertj.core.api.Assertions.assertThat; import java.util.ArrayList; @@ -86,7 +86,7 @@ public class SessionsAndCrashesDUnitTest { int locatorPort = locator.getPort(); return cluster.startRedisVM(vmId, x -> x - .withSystemProperty(GEODE_FOR_REDIS_PORT, redisPort.toString()) + .withProperty(GEODE_FOR_REDIS_PORT, redisPort.toString()) .withConnectionToLocator(locatorPort)); } diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/connection/AuthIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/connection/AuthIntegrationTest.java index 71a3a5f967..971ec23324 100644 --- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/connection/AuthIntegrationTest.java +++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/connection/AuthIntegrationTest.java @@ -45,12 +45,11 @@ import redis.clients.jedis.Jedis; import org.apache.geode.cache.CacheFactory; import org.apache.geode.cache.GemFireCache; import org.apache.geode.distributed.ConfigurationProperties; +import org.apache.geode.distributed.internal.InternalDistributedSystem; import org.apache.geode.examples.SimpleSecurityManager; import org.apache.geode.internal.AvailablePortHelper; import org.apache.geode.internal.cache.InternalCache; -import org.apache.geode.redis.TestRedisConfiguration; import org.apache.geode.redis.internal.GeodeRedisServer; -import org.apache.geode.redis.internal.RedisConfiguration; import org.apache.geode.security.AuthenticationExpiredException; import org.apache.geode.security.AuthenticationFailedException; import org.apache.geode.security.ResourcePermission; @@ -118,19 +117,15 @@ public class AuthIntegrationTest extends AbstractAuthIntegrationTest { cf.set(LOG_LEVEL, "error"); cf.set(MCAST_PORT, "0"); cf.set(LOCATORS, ""); + if (username != null) { + cf.set(ConfigurationProperties.GEODE_FOR_REDIS_USERNAME, username); + } if (withSecurityManager) { cf.set(ConfigurationProperties.SECURITY_MANAGER, SimpleSecurityManager.class.getName()); } cache = cf.create(); port = AvailablePortHelper.getRandomAvailableTCPPort(); - - TestRedisConfiguration.Builder redisConfigBuilder = TestRedisConfiguration.builder(); - redisConfigBuilder.withAddress("localhost").withPort(port); - if (username != null) { - redisConfigBuilder.withUsername(username); - } - - server = new GeodeRedisServer(redisConfigBuilder.build(), (InternalCache) cache); + server = new GeodeRedisServer("localhost", port, (InternalCache) cache); server.getRegionProvider().getSlotAdvisor().getBucketSlots(); jedis = new Jedis("localhost", port, 100000); } @@ -141,6 +136,13 @@ public class AuthIntegrationTest extends AbstractAuthIntegrationTest { setupCache(username, withSecurityManager); } + @Test + public void testAuthConfig() throws Exception { + setupCacheWithSecurity(false); + InternalDistributedSystem iD = (InternalDistributedSystem) cache.getDistributedSystem(); + assertThat(iD.getConfig().getRedisUsername()).isEqualTo(getUsername()); + } + @Test public void givenNoSecurity_accessWithAuthAndOnlyPassword_fails() throws Exception { setupCacheWithoutSecurity(); @@ -276,9 +278,7 @@ public class AuthIntegrationTest extends AbstractAuthIntegrationTest { .set(ConfigurationProperties.SECURITY_MANAGER, securityManager.getName()) .create(); port = AvailablePortHelper.getRandomAvailableTCPPort(); - RedisConfiguration config = TestRedisConfiguration.builder() - .withAddress("localhost").withPort(port).build(); - server = new GeodeRedisServer(config, (InternalCache) cache); + server = new GeodeRedisServer("localhost", port, (InternalCache) cache); jedis = new Jedis("localhost", port, 100000); } diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java index 87d74b0ea0..29a0ed5f15 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java @@ -67,15 +67,15 @@ public class GeodeRedisServer { private boolean shutdown; /** - * Constructor for {@code GeodeRedisServer} that will configure the server to bind to the - * address and port provided in the configuration. + * Constructor for {@code GeodeRedisServer} that will configure the server to bind to the given + * address and port. + * + * @param bindAddress The address to which the server will attempt to bind to; null + * causes it to bind to all local addresses. + * @param port The port the server will bind to, will throw an IllegalArgumentException if + * argument is less than 0. If the port is 0 a random port is assigned. */ - public GeodeRedisServer(RedisConfiguration configuration, InternalCache cache) { - int port = configuration.getPort(); - String bindAddress = configuration.getBindAddress(); - - logger.info(String.format("Starting GeodeRedisServer on bind address %s on port %s", - bindAddress, port)); + public GeodeRedisServer(String bindAddress, int port, InternalCache cache) { unsupportedCommandsEnabled = Boolean.getBoolean(ENABLE_UNSUPPORTED_COMMANDS_PARAM); @@ -84,8 +84,7 @@ public class GeodeRedisServer { RedisMemberInfoRetrievalFunction infoFunction = RedisMemberInfoRetrievalFunction.register(); eventDistributor = new EventDistributor(); - regionProvider = new RegionProvider(cache, configuration, stripedCoordinator, redisStats, - eventDistributor); + regionProvider = new RegionProvider(cache, stripedCoordinator, redisStats, eventDistributor); pubSub = new PubSubImpl(new Subscriptions(redisStats), regionProvider, redisStats); activeExpirationManager = new ActiveExpirationManager(regionProvider); @@ -93,8 +92,8 @@ public class GeodeRedisServer { DistributedMember member = cache.getDistributedSystem().getDistributedMember(); RedisSecurityService securityService = new RedisSecurityService(cache.getSecurityService()); - nettyRedisServer = new NettyRedisServer(cache.getInternalDistributedSystem().getConfig(), - configuration, regionProvider, pubSub, + nettyRedisServer = new NettyRedisServer(() -> cache.getInternalDistributedSystem().getConfig(), + regionProvider, pubSub, this::allowUnsupportedCommands, port, bindAddress, redisStats, member, securityService, eventDistributor); diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java index 7765d882db..ba87db7076 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java @@ -18,26 +18,23 @@ package org.apache.geode.redis.internal; import org.apache.logging.log4j.Logger; import org.apache.geode.cache.Cache; +import org.apache.geode.distributed.internal.InternalDistributedSystem; import org.apache.geode.distributed.internal.ResourceEvent; import org.apache.geode.distributed.internal.ResourceEventsListener; import org.apache.geode.internal.cache.CacheService; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.logging.internal.log4j.api.LogService; -import org.apache.geode.management.ManagementException; import org.apache.geode.management.internal.beans.CacheServiceMBeanBase; public class GeodeRedisService implements CacheService, ResourceEventsListener { private static final Logger logger = LogService.getLogger(); private GeodeRedisServer redisServer; private InternalCache cache; - private SystemPropertyBasedRedisConfiguration configuration; @Override public boolean init(Cache cache) { this.cache = (InternalCache) cache; - configuration = new SystemPropertyBasedRedisConfiguration( - ((InternalCache) cache).getInternalDistributedSystem().getConfig()); - if (!configuration.isEnabled()) { + if (!this.cache.getInternalDistributedSystem().getConfig().getRedisEnabled()) { return false; } @@ -59,12 +56,18 @@ public class GeodeRedisService implements CacheService, ResourceEventsListener { } private void startRedisServer(InternalCache cache) { - if (configuration.isEnabled()) { - try { - redisServer = new GeodeRedisServer(configuration, cache); - } catch (IllegalArgumentException ex) { - throw new ManagementException(ex); - } + InternalDistributedSystem system = cache.getInternalDistributedSystem(); + + if (system.getConfig().getRedisEnabled()) { + int port = system.getConfig().getRedisPort(); + String bindAddress = system.getConfig().getRedisBindAddress(); + assert bindAddress != null; + + logger.info( + String.format("Starting GeodeRedisServer on bind address %s on port %s", + bindAddress, port)); + + redisServer = new GeodeRedisServer(bindAddress, port, cache); } } diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConfiguration.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConfiguration.java deleted file mode 100644 index 2cc2d5ba82..0000000000 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConfiguration.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more contributor license - * agreements. See the NOTICE file distributed with this work for additional information regarding - * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance with the License. You may obtain a - * copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ - -package org.apache.geode.redis.internal; - -/** - * Interface that defines access to 'official' properties for Geode for Redis - */ -public interface RedisConfiguration { - - /** - * The default Geode for Redis port. Can be set using - * {@link SystemPropertyBasedRedisConfiguration#GEODE_FOR_REDIS_PORT}. - */ - int DEFAULT_REDIS_PORT = 6379; - - /** - * The default Geode for Redis bind address. Can be set using - * {@link SystemPropertyBasedRedisConfiguration#GEODE_FOR_REDIS_BIND_ADDRESS}. If this property - * is not set, then the Geode property {@code server-bind-address} is considered and used as the - * bind address if it is set. Otherwise, the default is to bind to all local addresses. - */ - String DEFAULT_REDIS_BIND_ADDRESS = "0.0.0.0"; - - /** - * The default Geode for Redis redundant copies. Must be in the range (0..3). Can be set using - * {@link SystemPropertyBasedRedisConfiguration#GEODE_FOR_REDIS_REDUNDANT_COPIES}. - */ - int DEFAULT_REDIS_REDUNDANT_COPIES = 1; - - /** - * The default Geode for Redis username. Can be set using - * {@link SystemPropertyBasedRedisConfiguration#GEODE_FOR_REDIS_USERNAME}. - */ - String DEFAULT_REDIS_USERNAME = "default"; - - boolean isEnabled(); - - int getPort(); - - String getBindAddress(); - - int getRedundantCopies(); - - String getUsername(); - -} diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/SystemPropertyBasedRedisConfiguration.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/SystemPropertyBasedRedisConfiguration.java deleted file mode 100644 index 1d76b11af6..0000000000 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/SystemPropertyBasedRedisConfiguration.java +++ /dev/null @@ -1,170 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more contributor license - * agreements. See the NOTICE file distributed with this work for additional information regarding - * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance with the License. You may obtain a - * copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ - -package org.apache.geode.redis.internal; - -import org.apache.commons.lang3.StringUtils; - -import org.apache.geode.distributed.internal.DistributionConfig; -import org.apache.geode.internal.inet.LocalHostUtil; - -/** - * Concrete implementation of {@link RedisConfiguration} used during runtime. Note that the - * presence of any of the system properties will automatically enable Geode for Redis. In order to - * use all defaults one can also set {@link #GEODE_FOR_REDIS_ENABLED} to {@code true}. - */ -public class SystemPropertyBasedRedisConfiguration implements RedisConfiguration { - - /** - * Default is {@link RedisConfiguration#DEFAULT_REDIS_PORT}. - */ - public static final String GEODE_FOR_REDIS_PORT = "gemfire.geode-for-redis-port"; - - /** - * Default is {@link RedisConfiguration#DEFAULT_REDIS_BIND_ADDRESS}. - */ - public static final String GEODE_FOR_REDIS_BIND_ADDRESS = "gemfire.geode-for-redis-bind-address"; - - /** - * Default is {@link RedisConfiguration#DEFAULT_REDIS_REDUNDANT_COPIES}. - */ - public static final String GEODE_FOR_REDIS_REDUNDANT_COPIES = - "gemfire.geode-for-redis-redundant-copies"; - - /** - * Default is {@link RedisConfiguration#DEFAULT_REDIS_USERNAME}. - */ - public static final String GEODE_FOR_REDIS_USERNAME = "gemfire.geode-for-redis-username"; - - /** - * Default is {@code false}. - */ - public static final String GEODE_FOR_REDIS_ENABLED = "gemfire.geode-for-redis-enabled"; - - private int port; - private String bindAddress; - private int redundantCopies; - private String username; - private final boolean enabled; - - public SystemPropertyBasedRedisConfiguration(DistributionConfig distributionConfig) - throws IllegalArgumentException { - String geodeBindAddress = distributionConfig.getServerBindAddress(); - - boolean tmpEnabled = validateAndSetPort(); - tmpEnabled |= validateAndSetBindAddress(geodeBindAddress); - tmpEnabled |= validateAndSetRedundantCopies(); - tmpEnabled |= validateAndSetUsername(); - tmpEnabled |= Boolean.getBoolean(GEODE_FOR_REDIS_ENABLED); - - enabled = tmpEnabled; - } - - public boolean isEnabled() { - return enabled; - } - - public int getPort() { - return port; - } - - public String getBindAddress() { - return bindAddress; - } - - public int getRedundantCopies() { - return redundantCopies; - } - - public String getUsername() { - return username; - } - - private boolean validateAndSetPort() { - String portString = System.getProperty(GEODE_FOR_REDIS_PORT); - if (StringUtils.isEmpty(portString)) { - port = DEFAULT_REDIS_PORT; - return false; - } - - try { - port = Integer.parseInt(portString); - } catch (NumberFormatException e) { - throw new IllegalArgumentException(GEODE_FOR_REDIS_PORT + " is invalid: " + portString); - } - - if (port < 0 || port > 65535) { - throw new IllegalArgumentException(GEODE_FOR_REDIS_PORT + " is out of range (0..65535): " - + port); - } - - return true; - } - - private boolean validateAndSetRedundantCopies() { - String copiesString = System.getProperty(GEODE_FOR_REDIS_REDUNDANT_COPIES); - if (StringUtils.isEmpty(copiesString)) { - redundantCopies = DEFAULT_REDIS_REDUNDANT_COPIES; - return false; - } - - try { - redundantCopies = Integer.parseInt(copiesString); - } catch (NumberFormatException e) { - throw new IllegalArgumentException( - GEODE_FOR_REDIS_REDUNDANT_COPIES + " is invalid: " + copiesString); - } - - if (redundantCopies < 0 || redundantCopies > 3) { - throw new IllegalArgumentException(GEODE_FOR_REDIS_REDUNDANT_COPIES + - " is out of range (0..3): " + redundantCopies); - } - - return true; - } - - private boolean validateAndSetBindAddress(String geodeBindAddress) { - String value = System.getProperty(GEODE_FOR_REDIS_BIND_ADDRESS); - if (StringUtils.isEmpty(value)) { - if (StringUtils.isEmpty(geodeBindAddress)) { - bindAddress = DEFAULT_REDIS_BIND_ADDRESS; - } else { - bindAddress = geodeBindAddress; - } - return false; - } - - if (!LocalHostUtil.isLocalHost(value)) { - throw new IllegalArgumentException( - String.format( - "The geode-for-redis-bind-address %s is not a valid address for this machine. These are the valid addresses for this machine: %s", - value, LocalHostUtil.getMyAddresses())); - } - - bindAddress = value; - return true; - } - - private boolean validateAndSetUsername() { - String value = System.getProperty(GEODE_FOR_REDIS_USERNAME); - if (StringUtils.isEmpty(value)) { - username = DEFAULT_REDIS_USERNAME; - return false; - } - - username = value; - return true; - } -} diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/NettyRedisServer.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/NettyRedisServer.java index 74bf042193..0d85e19b67 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/NettyRedisServer.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/NettyRedisServer.java @@ -63,7 +63,6 @@ import org.apache.geode.internal.security.SecurableCommunicationChannel; import org.apache.geode.logging.internal.executors.LoggingThreadFactory; import org.apache.geode.logging.internal.log4j.api.LogService; import org.apache.geode.management.ManagementException; -import org.apache.geode.redis.internal.RedisConfiguration; import org.apache.geode.redis.internal.eventing.EventDistributor; import org.apache.geode.redis.internal.pubsub.PubSub; import org.apache.geode.redis.internal.services.RegionProvider; @@ -77,8 +76,7 @@ public class NettyRedisServer { private static final Logger logger = LogService.getLogger(); - private final DistributionConfig gemfireConfig; - private final RedisConfiguration redisConfig; + private final Supplier<DistributionConfig> configSupplier; private final RegionProvider regionProvider; private final PubSub pubsub; private final Supplier<Boolean> allowUnsupportedSupplier; @@ -93,13 +91,12 @@ public class NettyRedisServer { private final EventDistributor eventDistributor; private final int writeTimeoutSeconds; - public NettyRedisServer(DistributionConfig gemfireConfig, RedisConfiguration redisConfig, + public NettyRedisServer(Supplier<DistributionConfig> configSupplier, RegionProvider regionProvider, PubSub pubsub, Supplier<Boolean> allowUnsupportedSupplier, int port, String requestedAddress, RedisStats redisStats, DistributedMember member, RedisSecurityService securityService, EventDistributor eventDistributor) { - this.gemfireConfig = gemfireConfig; - this.redisConfig = redisConfig; + this.configSupplier = configSupplier; this.regionProvider = regionProvider; this.pubsub = pubsub; this.allowUnsupportedSupplier = allowUnsupportedSupplier; @@ -160,7 +157,7 @@ public class NettyRedisServer { } private ChannelInitializer<SocketChannel> createChannelInitializer() { - String redisUsername = redisConfig.getUsername(); + String redisUsername = configSupplier.get().getRedisUsername(); return new ChannelInitializer<SocketChannel>() { @Override @@ -185,7 +182,7 @@ public class NettyRedisServer { private void addSSLIfEnabled(SocketChannel ch, ChannelPipeline p) { SSLConfig sslConfigForServer = - SSLConfigurationFactory.getSSLConfigForComponent(gemfireConfig, + SSLConfigurationFactory.getSSLConfigForComponent(configSupplier.get(), SecurableCommunicationChannel.SERVER); if (!sslConfigForServer.isEnabled()) { @@ -261,7 +258,7 @@ public class NettyRedisServer { * @return Buffer size to use for server */ private int getBufferSize() { - return gemfireConfig.getSocketBufferSize(); + return configSupplier.get().getSocketBufferSize(); } /** diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/RegionProvider.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/RegionProvider.java index 0edd2399cd..3fd6278b79 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/RegionProvider.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/RegionProvider.java @@ -36,6 +36,7 @@ import org.apache.geode.cache.RegionShortcut; import org.apache.geode.cache.partition.PartitionListener; import org.apache.geode.cache.partition.PartitionRegionHelper; import org.apache.geode.distributed.DistributedMember; +import org.apache.geode.distributed.internal.DistributionConfig; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.cache.InternalRegionFactory; import org.apache.geode.internal.cache.PartitionedRegion; @@ -44,7 +45,6 @@ import org.apache.geode.internal.cache.control.HeapMemoryMonitor; import org.apache.geode.internal.cache.control.InternalResourceManager; import org.apache.geode.internal.cache.control.MemoryThresholds; import org.apache.geode.internal.cache.execute.BucketMovedException; -import org.apache.geode.redis.internal.RedisConfiguration; import org.apache.geode.redis.internal.RedisException; import org.apache.geode.redis.internal.commands.executor.cluster.RedisPartitionResolver; import org.apache.geode.redis.internal.data.RedisCrossSlotException; @@ -82,9 +82,8 @@ public class RegionProvider { private final CacheTransactionManager txManager; private final String redisRegionName; - public RegionProvider(InternalCache cache, RedisConfiguration configuration, - StripedCoordinator stripedCoordinator, RedisStats redisStats, - PartitionListener partitionListener) { + public RegionProvider(InternalCache cache, StripedCoordinator stripedCoordinator, + RedisStats redisStats, PartitionListener partitionListener) { this.stripedCoordinator = stripedCoordinator; this.redisStats = redisStats; @@ -93,7 +92,8 @@ public class RegionProvider { PartitionAttributesFactory<RedisKey, RedisData> attributesFactory = new PartitionAttributesFactory<>(); - attributesFactory.setRedundantCopies(configuration.getRedundantCopies()); + DistributionConfig config = cache.getInternalDistributedSystem().getConfig(); + attributesFactory.setRedundantCopies(config.getRedisRedundantCopies()); attributesFactory.setPartitionResolver(new RedisPartitionResolver()); attributesFactory.setTotalNumBuckets(REDIS_REGION_BUCKETS); attributesFactory.addPartitionListener(partitionListener);