This is an automated email from the ASF dual-hosted git repository. bdeggleston pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push: new 47d4971 Fix SimpleStrategy option validation 47d4971 is described below commit 47d4971b56d97ba8a528f7c17bfd6b11f1ababa3 Author: Dinesh A. Joshi <dinesh.jo...@yahoo.com> AuthorDate: Tue Feb 12 23:37:06 2019 -0800 Fix SimpleStrategy option validation Patch by Dinesh Joshi; Reviewed by Blake Eggleston for CASSANDRA-15007 --- CHANGES.txt | 1 + .../apache/cassandra/locator/SimpleStrategy.java | 18 ++++++++---- .../cassandra/locator/SimpleStrategyTest.java | 32 ++++++++++++++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index ffa251b..09d1733 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0 + * Fix SimpleStrategy option validation (CASSANDRA-15007) * Don't try to cancel 2i compactions when starting anticompaction (CASSANDRA-15024) * Avoid NPE in RepairRunnable.recordFailure (CASSANDRA-15025) * SSL Cert Hot Reloading should check for sanity of the new keystore/truststore before loading it (CASSANDRA-14991) diff --git a/src/java/org/apache/cassandra/locator/SimpleStrategy.java b/src/java/org/apache/cassandra/locator/SimpleStrategy.java index 748d2d3..610ffe1 100644 --- a/src/java/org/apache/cassandra/locator/SimpleStrategy.java +++ b/src/java/org/apache/cassandra/locator/SimpleStrategy.java @@ -38,12 +38,14 @@ import org.apache.cassandra.dht.Token; */ public class SimpleStrategy extends AbstractReplicationStrategy { + private static final String REPLICATION_FACTOR = "replication_factor"; private final ReplicationFactor rf; public SimpleStrategy(String keyspaceName, TokenMetadata tokenMetadata, IEndpointSnitch snitch, Map<String, String> configOptions) { super(keyspaceName, tokenMetadata, snitch, configOptions); - this.rf = ReplicationFactor.fromString(this.configOptions.get("replication_factor")); + validateOptionsInternal(configOptions); + this.rf = ReplicationFactor.fromString(this.configOptions.get(REPLICATION_FACTOR)); } public EndpointsForRange calculateNaturalReplicas(Token token, TokenMetadata metadata) @@ -76,16 +78,20 @@ public class SimpleStrategy extends AbstractReplicationStrategy return rf; } - public void validateOptions() throws ConfigurationException + private final static void validateOptionsInternal(Map<String, String> configOptions) throws ConfigurationException { - String rf = configOptions.get("replication_factor"); - if (rf == null) + if (configOptions.get(REPLICATION_FACTOR) == null) throw new ConfigurationException("SimpleStrategy requires a replication_factor strategy option."); - validateReplicationFactor(rf); + } + + public void validateOptions() throws ConfigurationException + { + validateOptionsInternal(configOptions); + validateReplicationFactor(configOptions.get(REPLICATION_FACTOR)); } public Collection<String> recognizedOptions() { - return Collections.<String>singleton("replication_factor"); + return Collections.singleton(REPLICATION_FACTOR); } } diff --git a/test/unit/org/apache/cassandra/locator/SimpleStrategyTest.java b/test/unit/org/apache/cassandra/locator/SimpleStrategyTest.java index 507cc1f..9e24de7 100644 --- a/test/unit/org/apache/cassandra/locator/SimpleStrategyTest.java +++ b/test/unit/org/apache/cassandra/locator/SimpleStrategyTest.java @@ -29,13 +29,16 @@ import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.apache.cassandra.SchemaLoader; import org.apache.cassandra.Util; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.dht.Murmur3Partitioner; import org.apache.cassandra.dht.Range; +import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.schema.Schema; import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.dht.IPartitioner; @@ -287,6 +290,35 @@ public class SimpleStrategyTest strategy.getNaturalReplicasForToken(tk(101))); } + @Rule + public ExpectedException expectedEx = ExpectedException.none(); + + @Test + public void testSimpleStrategyThrowsConfigurationException() throws ConfigurationException, UnknownHostException + { + expectedEx.expect(ConfigurationException.class); + expectedEx.expectMessage("SimpleStrategy requires a replication_factor strategy option."); + + IEndpointSnitch snitch = new SimpleSnitch(); + DatabaseDescriptor.setEndpointSnitch(snitch); + + List<InetAddressAndPort> endpoints = Lists.newArrayList(InetAddressAndPort.getByName("127.0.0.1"), + InetAddressAndPort.getByName("127.0.0.2"), + InetAddressAndPort.getByName("127.0.0.3")); + + Multimap<InetAddressAndPort, Token> tokens = HashMultimap.create(); + tokens.put(endpoints.get(0), tk(100)); + tokens.put(endpoints.get(1), tk(200)); + tokens.put(endpoints.get(2), tk(300)); + + TokenMetadata metadata = new TokenMetadata(); + metadata.updateNormalTokens(tokens); + + Map<String, String> configOptions = new HashMap<>(); + + SimpleStrategy strategy = new SimpleStrategy("ks", metadata, snitch, configOptions); + } + private AbstractReplicationStrategy getStrategy(String keyspaceName, TokenMetadata tmd, IEndpointSnitch snitch) { KeyspaceMetadata ksmd = Schema.instance.getKeyspaceMetadata(keyspaceName); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org