Merge pull request apache#621 from datastax/java852-fix

Fix checkSchemaAgreement bug introduced in JAVA-852

Author: olim7t

driver-core/pom.xml

@@ -381,6 +381,7 @@
                 <include>**/SSL*Test.java</include>
                 <include>**/UUIDsPID*.java</include>
                 <include>**/ControlConnectionTest.java</include>
+                <include>**/ExtendedPeerCheckDisabledTest.java</include>
               </includes>
             </configuration>
           </plugin>

driver-core/src/main/java/com/datastax/driver/core/ControlConnection.java

@@ -427,10 +427,9 @@ private static InetSocketAddress addressToUseForPeerHost(Row peersRow, InetSocke
         InetAddress peer = peersRow.getInet("peer");
         InetAddress addr = peersRow.getInet("rpc_address");
 
-        // We've already called isValid on the row, which checks this
-        assert addr != null;
-
-        if (peer.equals(connectedHost.getAddress()) || addr.equals(connectedHost.getAddress())) {
+        if (addr == null) {
+            return null;
+        } else if (peer.equals(connectedHost.getAddress()) || addr.equals(connectedHost.getAddress())) {
             // Some DSE versions were inserting a line for the local node in peers (with mostly null values). This has been fixed, but if we
             // detect that's the case, ignore it as it's not really a big deal.
             logger.debug("System.peers on node {} has a line for itself. This is not normal but is a known problem of some DSE version. Ignoring the entry.", connectedHost);
@@ -733,9 +732,6 @@ boolean checkSchemaAgreement() throws ConnectionException, BusyConnectionExcepti
 
         for (Row row : peersFuture.get()) {
 
-            if (!isValidPeer(row, false))
-                continue;
-
             InetSocketAddress addr = addressToUseForPeerHost(row, connection.address, cluster);
             if (addr == null || row.isNull("schema_version"))
                 continue;

driver-core/src/test/java/com/datastax/driver/core/ControlConnectionTest.java

@@ -225,7 +225,7 @@ public Integer apply(InetAddress input) {
     }
 
     @DataProvider
-    public Object[][] disallowedNullColumnsInPeerData() {
+    public static Object[][] disallowedNullColumnsInPeerData() {
         return new Object[][]{
                 {"host_id"},
                 {"data_center"},
@@ -325,20 +325,6 @@ public void should_fetch_whole_peers_table_if_broadcast_address_changed() throws
         }
     }
 
-    /**
-     * Validates that if the com.datastax.driver.EXTENDED_PEER_CHECK system property is set to false that a peer
-     * with null values for host_id, data_center, rack, tokens is not ignored.
-     *
-     * @test_category host:metadata
-     * @jira_ticket JAVA-852
-     * @since 2.1.10
-     */
-    @Test(groups = "isolated", dataProvider = "disallowedNullColumnsInPeerData")
-    @CCMConfig(createCcm = false)
-    public void should_use_peer_if_extended_peer_check_is_disabled(String columns) {
-        System.setProperty("com.datastax.driver.EXTENDED_PEER_CHECK", "false");
-        run_with_null_peer_info(columns, true);
-    }
 
     /**
      * Validates that if the com.datastax.driver.EXTENDED_PEER_CHECK system property is set to true that a peer
@@ -368,7 +354,7 @@ public void should_ignore_and_warn_peers_with_null_entries_by_default(String col
         run_with_null_peer_info(columns, false);
     }
 
-    private void run_with_null_peer_info(String columns, boolean expectPeer2) {
+    static void run_with_null_peer_info(String columns, boolean expectPeer2) {
         // given: A cluster with peer 2 having a null rack.
         ScassandraCluster.ScassandraClusterBuilder builder = ScassandraCluster.builder()
                 .withNodes(3);

driver-core/src/test/java/com/datastax/driver/core/ExtendedPeerCheckDisabledTest.java

@@ -0,0 +1,36 @@
+/*
+ *      Copyright (C) 2012-2015 DataStax Inc.
+ *
+ *   Licensed 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 com.datastax.driver.core;
+
+import org.testng.annotations.Test;
+
+public class ExtendedPeerCheckDisabledTest {
+
+    /**
+     * Validates that if the com.datastax.driver.EXTENDED_PEER_CHECK system property is set to false that a peer
+     * with null values for host_id, data_center, rack, tokens is not ignored.
+     *
+     * @test_category host:metadata
+     * @jira_ticket JAVA-852
+     * @since 2.1.10
+     */
+    @Test(groups = "isolated", dataProvider = "disallowedNullColumnsInPeerData", dataProviderClass = ControlConnectionTest.class)
+    @CCMConfig(createCcm = false)
+    public void should_use_peer_if_extended_peer_check_is_disabled(String columns) {
+        System.setProperty("com.datastax.driver.EXTENDED_PEER_CHECK", "false");
+        ControlConnectionTest.run_with_null_peer_info(columns, true);
+    }
+}

driver-core/src/test/java/com/datastax/driver/core/ScassandraCluster.java

@@ -52,6 +52,8 @@ public class ScassandraCluster {
 
     private final List<Map<String, ?>> keyspaceRows;
 
+    private static final java.util.UUID schemaVersion = UUIDs.random();
+
 
     private final Map<Integer, Map<Integer, Map<String, Object>>> forcedPeerInfos;
 
@@ -297,6 +299,7 @@ private void primeMetadata(Scassandra node) {
                     addPeerInfo(row, dc, n + 1, "rack", getPeerInfo(dc, n + 1, "rack", "rack1"));
                     addPeerInfo(row, dc, n + 1, "release_version", getPeerInfo(dc, n + 1, "release_version", "2.1.8"));
                     addPeerInfo(row, dc, n + 1, "tokens", ImmutableSet.of(tokens.get(n)));
+                    addPeerInfo(row, dc, n + 1, "schema_version", schemaVersion);
                 } else { // prime system.peers.
                     query = "SELECT * FROM system.peers WHERE peer='" + address + "'";
                     metadata = SELECT_PEERS;
@@ -308,6 +311,7 @@ private void primeMetadata(Scassandra node) {
                     addPeerInfo(row, dc, n + 1, "release_version", getPeerInfo(dc, n + 1, "release_version", "2.1.8"));
                     addPeerInfo(row, dc, n + 1, "tokens", ImmutableSet.of(Long.toString(tokens.get(n))));
                     addPeerInfo(row, dc, n + 1, "host_id", UUIDs.random());
+                    addPeerInfo(row, dc, n + 1, "schema_version", schemaVersion);
                     rows.add(row);
                 }
                 client.prime(PrimingRequest.queryBuilder()
@@ -379,7 +383,8 @@ private Object getPeerInfo(int dc, int node, String property, Object defaultValu
             column("rack", TEXT),
             column("release_version", TEXT),
             column("tokens", set(TEXT)),
-            column("host_id", UUID)
+            column("host_id", UUID),
+            column("schema_version", UUID)
     };
 
     public static final org.scassandra.http.client.types.ColumnMetadata[] SELECT_LOCAL = {
@@ -394,6 +399,7 @@ private Object getPeerInfo(int dc, int node, String property, Object defaultValu
             column("rack", TEXT),
             column("release_version", TEXT),
             column("tokens", set(TEXT)),
+            column("schema_version", UUID)
     };
 
     static final org.scassandra.http.client.types.ColumnMetadata[] SELECT_CLUSTER_NAME = {