From 2adef79c465f286b6ee0bf64d5f9718af70fa1d3 Mon Sep 17 00:00:00 2001 From: Patricio Echague Date: Sat, 22 Jun 2013 14:38:12 -0700 Subject: [PATCH] Check for not null on cluster when running autodiscovery it fails during the initialization and it is never able to set the cluster object properly remaining as null for future runs. --- .../cassandra/connection/NodeDiscovery.java | 24 +++++++---------- .../hector/api/factory/HFactory.java | 4 +++ .../connection/NodeDiscoveryTest.java | 27 +++++++++++++------ 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/me/prettyprint/cassandra/connection/NodeDiscovery.java b/core/src/main/java/me/prettyprint/cassandra/connection/NodeDiscovery.java index f48f7f2ff..8a2a46142 100644 --- a/core/src/main/java/me/prettyprint/cassandra/connection/NodeDiscovery.java +++ b/core/src/main/java/me/prettyprint/cassandra/connection/NodeDiscovery.java @@ -7,6 +7,7 @@ import me.prettyprint.cassandra.service.CassandraHost; import me.prettyprint.cassandra.service.CassandraHostConfigurator; import me.prettyprint.cassandra.service.ThriftCluster; +import me.prettyprint.hector.api.Cluster; import me.prettyprint.hector.api.Keyspace; import me.prettyprint.hector.api.ddl.KeyspaceDefinition; import me.prettyprint.hector.api.factory.HFactory; @@ -24,23 +25,13 @@ public class NodeDiscovery { private CassandraHostConfigurator cassandraHostConfigurator; private HConnectionManager connectionManager; private DataCenterValidator dataCenterValidator; - private ThriftCluster cluster; - + public NodeDiscovery(CassandraHostConfigurator cassandraHostConfigurator, HConnectionManager connectionManager) { - this((ThriftCluster) HFactory.getCluster(connectionManager.getClusterName()), cassandraHostConfigurator, connectionManager); + this.cassandraHostConfigurator = cassandraHostConfigurator; + this.connectionManager = connectionManager; + this.dataCenterValidator = new DataCenterValidator(cassandraHostConfigurator.getAutoDiscoveryDataCenters()); } - /** - * Package scoped constructor that allows cluster to be overriden for tests - */ - @VisibleForTesting - NodeDiscovery(ThriftCluster cluster, CassandraHostConfigurator cassandraHostConfigurator, HConnectionManager connectionManager) { - this.cassandraHostConfigurator = cassandraHostConfigurator; - this.connectionManager = connectionManager; - this.dataCenterValidator = new DataCenterValidator(cassandraHostConfigurator.getAutoDiscoveryDataCenters()); - this.cluster = cluster; - } - /** * Find any nodes that are not already in the connection manager but are in * the cassandra ring and add them @@ -67,6 +58,11 @@ public void doAddNodes() { * Find any unknown nodes in the cassandra ring */ public Set discoverNodes() { + Cluster cluster = HFactory.getCluster(connectionManager.getClusterName()); + if (cluster == null) { + return null; + } + Set existingHosts = connectionManager.getHosts(); Set foundHosts = new HashSet(); diff --git a/core/src/main/java/me/prettyprint/hector/api/factory/HFactory.java b/core/src/main/java/me/prettyprint/hector/api/factory/HFactory.java index 47e12a9e6..79af7ab18 100644 --- a/core/src/main/java/me/prettyprint/hector/api/factory/HFactory.java +++ b/core/src/main/java/me/prettyprint/hector/api/factory/HFactory.java @@ -202,6 +202,10 @@ public static Cluster createCluster(String clusterName, return cluster; } } + + public static void setClusterForTest(Cluster cluster) { + clusters.put(cluster.getName(), cluster); + } /** * Shutdown this cluster, removing it from the Map. This operation is diff --git a/core/src/test/java/me/prettyprint/cassandra/connection/NodeDiscoveryTest.java b/core/src/test/java/me/prettyprint/cassandra/connection/NodeDiscoveryTest.java index 129fcff37..dce24cc18 100644 --- a/core/src/test/java/me/prettyprint/cassandra/connection/NodeDiscoveryTest.java +++ b/core/src/test/java/me/prettyprint/cassandra/connection/NodeDiscoveryTest.java @@ -9,6 +9,7 @@ import me.prettyprint.cassandra.service.ThriftCluster; import me.prettyprint.hector.api.ddl.KeyspaceDefinition; +import me.prettyprint.hector.api.factory.HFactory; import org.apache.cassandra.thrift.EndpointDetails; import org.apache.cassandra.thrift.TokenRange; import org.junit.Before; @@ -17,8 +18,9 @@ import com.google.common.collect.Sets; -public class NodeDiscoveryTest { - +public class NodeDiscoveryTest { + + public static final String TEST_CLUSTER_NAME = "TestCluster"; CassandraHostConfigurator cassandraHostConfigurator; HConnectionManager connectionManager; ThriftCluster cluster; @@ -41,11 +43,14 @@ public void setup() { public void testDoAdd() { List tokens = createRange("localhost", "google.com"); Mockito.when(cluster.describeRing("TestKeyspace")).thenReturn(tokens); + Mockito.when(cluster.getName()).thenReturn(TEST_CLUSTER_NAME); + Mockito.when(connectionManager.getClusterName()).thenReturn(TEST_CLUSTER_NAME); Mockito.when(connectionManager.getHosts()).thenReturn(Sets.newHashSet(new CassandraHost("localhost",9160))); - - NodeDiscovery q = new NodeDiscovery(cluster, cassandraHostConfigurator, connectionManager); + HFactory.setClusterForTest(cluster); + + NodeDiscovery q = new NodeDiscovery(cassandraHostConfigurator, connectionManager); q.doAddNodes(); - + Mockito.verify(connectionManager).addCassandraHost(new CassandraHost("google.com",9160)); } @@ -53,9 +58,12 @@ public void testDoAdd() { public void testMultipleAdded() { List tokens = createRange("localhost", "google.com", "yahoo.com", "datastax.com"); Mockito.when(cluster.describeRing("TestKeyspace")).thenReturn(tokens); + Mockito.when(cluster.getName()).thenReturn(TEST_CLUSTER_NAME); + Mockito.when(connectionManager.getClusterName()).thenReturn(TEST_CLUSTER_NAME); Mockito.when(connectionManager.getHosts()).thenReturn(Sets.newHashSet(new CassandraHost("localhost",9160))); - - NodeDiscovery q = new NodeDiscovery(cluster, cassandraHostConfigurator, connectionManager); + HFactory.setClusterForTest(cluster); + + NodeDiscovery q = new NodeDiscovery(cassandraHostConfigurator, connectionManager); q.doAddNodes(); Mockito.verify(connectionManager).addCassandraHost(new CassandraHost("google.com",9160)); @@ -67,9 +75,12 @@ public void testMultipleAdded() { public void testNoneAdded() { List tokens = createRange("localhost"); Mockito.when(cluster.describeRing("TestKeyspace")).thenReturn(tokens); + Mockito.when(cluster.getName()).thenReturn(TEST_CLUSTER_NAME); + Mockito.when(connectionManager.getClusterName()).thenReturn(TEST_CLUSTER_NAME); Mockito.when(connectionManager.getHosts()).thenReturn(Sets.newHashSet(new CassandraHost("localhost",9160))); + HFactory.setClusterForTest(cluster); - NodeDiscovery q = new NodeDiscovery(cluster, cassandraHostConfigurator, connectionManager); + NodeDiscovery q = new NodeDiscovery(cassandraHostConfigurator, connectionManager); Assert.assertEquals(0, q.discoverNodes().size()); }