JAVA-1584: Validate that no bound values are unset in protocol v3

changelog/README.md

@@ -4,6 +4,8 @@
 
 ### 4.0.0-alpha3 (in progress)
 
+- [bug] JAVA-1584: Validate that no bound values are unset in protocol v3
+
 ### 4.0.0-alpha2
 
 - [new feature] JAVA-1525: Handle token metadata

core/src/main/java/com/datastax/oss/driver/internal/core/CassandraProtocolVersionRegistry.java

@@ -165,6 +165,16 @@ public ProtocolVersion highestCommon(Collection<Node> nodes) {
     }
   }
 
+  @Override
+  public boolean supports(ProtocolVersion version, ProtocolFeature feature) {
+    switch (feature) {
+      case UNSET_BOUND_VALUES:
+        return version.getCode() >= 4;
+      default:
+        throw new IllegalArgumentException("Unhandled protocol feature: " + feature);
+    }
+  }
+
   private NavigableMap<Integer, ProtocolVersion> byCode(ProtocolVersion[][] versionRanges) {
     NavigableMap<Integer, ProtocolVersion> map = new TreeMap<>();
     for (ProtocolVersion[] versionRange : versionRanges) {

core/src/main/java/com/datastax/oss/driver/internal/core/ProtocolFeature.java

@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2017-2017 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.oss.driver.internal.core;
+
+/** Features of the native protocol that are only supported in specific versions. */
+public enum ProtocolFeature {
+
+  /** The ability to leave variables unset in prepared statements. */
+  UNSET_BOUND_VALUES,
+  ;
+}

core/src/main/java/com/datastax/oss/driver/internal/core/ProtocolVersionRegistry.java

@@ -66,4 +66,7 @@ public interface ProtocolVersionRegistry {
    *     the driver initialization to fail.
    */
   ProtocolVersion highestCommon(Collection<Node> nodes);
+
+  /** Whether a given version supports a given feature. */
+  boolean supports(ProtocolVersion version, ProtocolFeature feature);
 }

core/src/main/java/com/datastax/oss/driver/internal/core/cql/Conversions.java

@@ -26,6 +26,7 @@
 import com.datastax.oss.driver.api.core.cql.BoundStatement;
 import com.datastax.oss.driver.api.core.cql.ColumnDefinition;
 import com.datastax.oss.driver.api.core.cql.ColumnDefinitions;
+import com.datastax.oss.driver.api.core.cql.CqlSession;
 import com.datastax.oss.driver.api.core.cql.ExecutionInfo;
 import com.datastax.oss.driver.api.core.cql.PrepareRequest;
 import com.datastax.oss.driver.api.core.cql.SimpleStatement;
@@ -50,9 +51,10 @@
 import com.datastax.oss.driver.api.core.servererrors.UnavailableException;
 import com.datastax.oss.driver.api.core.servererrors.WriteFailureException;
 import com.datastax.oss.driver.api.core.servererrors.WriteTimeoutException;
-import com.datastax.oss.driver.api.core.cql.CqlSession;
 import com.datastax.oss.driver.api.core.type.codec.TypeCodecs;
 import com.datastax.oss.driver.api.core.type.codec.registry.CodecRegistry;
+import com.datastax.oss.driver.internal.core.ProtocolFeature;
+import com.datastax.oss.driver.internal.core.ProtocolVersionRegistry;
 import com.datastax.oss.driver.internal.core.context.InternalDriverContext;
 import com.datastax.oss.driver.internal.core.metadata.token.ByteOrderedToken;
 import com.datastax.oss.driver.internal.core.metadata.token.Murmur3Token;
@@ -104,6 +106,7 @@ static Message toMessage(
     }
     CodecRegistry codecRegistry = context.codecRegistry();
     ProtocolVersion protocolVersion = context.protocolVersion();
+    ProtocolVersionRegistry registry = context.protocolVersionRegistry();
     if (statement instanceof SimpleStatement) {
       SimpleStatement simpleStatement = (SimpleStatement) statement;
 
@@ -126,6 +129,9 @@ static Message toMessage(
       return new Query(simpleStatement.getQuery(), queryOptions);
     } else if (statement instanceof BoundStatement) {
       BoundStatement boundStatement = (BoundStatement) statement;
+      if (!registry.supports(protocolVersion, ProtocolFeature.UNSET_BOUND_VALUES)) {
+        ensureAllSet(boundStatement);
+      }
       QueryOptions queryOptions =
           new QueryOptions(
               consistency,
@@ -141,6 +147,9 @@ static Message toMessage(
       return new Execute(Bytes.getArray(id), queryOptions);
     } else if (statement instanceof BatchStatement) {
       BatchStatement batchStatement = (BatchStatement) statement;
+      if (!registry.supports(protocolVersion, ProtocolFeature.UNSET_BOUND_VALUES)) {
+        ensureAllSet(batchStatement);
+      }
       List<Object> queriesOrIds = new ArrayList<>(batchStatement.size());
       List<List<ByteBuffer>> values = new ArrayList<>(batchStatement.size());
       for (BatchableStatement child : batchStatement) {
@@ -221,6 +230,26 @@ private static ByteBuffer encode(
     }
   }
 
+  private static void ensureAllSet(BoundStatement boundStatement) {
+    for (int i = 0; i < boundStatement.size(); i++) {
+      if (!boundStatement.isSet(i)) {
+        throw new IllegalStateException(
+            "Unset value at index "
+                + i
+                + ". "
+                + "If you want this value to be null, please set it to null explicitly.");
+      }
+    }
+  }
+
+  private static void ensureAllSet(BatchStatement batchStatement) {
+    for (BatchableStatement<?> batchableStatement : batchStatement) {
+      if (batchableStatement instanceof BoundStatement) {
+        ensureAllSet(((BoundStatement) batchableStatement));
+      }
+    }
+  }
+
   static AsyncResultSet toResultSet(
       Result result,
       ExecutionInfo executionInfo,

core/src/test/java/com/datastax/oss/driver/internal/core/cql/RequestHandlerTestHarness.java

@@ -17,6 +17,7 @@
 
 import com.datastax.oss.driver.api.core.ConsistencyLevel;
 import com.datastax.oss.driver.api.core.CqlIdentifier;
+import com.datastax.oss.driver.api.core.ProtocolVersion;
 import com.datastax.oss.driver.api.core.config.CoreDriverOption;
 import com.datastax.oss.driver.api.core.config.DriverConfig;
 import com.datastax.oss.driver.api.core.config.DriverConfigProfile;
@@ -25,6 +26,8 @@
 import com.datastax.oss.driver.api.core.session.Request;
 import com.datastax.oss.driver.api.core.specex.SpeculativeExecutionPolicy;
 import com.datastax.oss.driver.api.core.time.TimestampGenerator;
+import com.datastax.oss.driver.internal.core.ProtocolFeature;
+import com.datastax.oss.driver.internal.core.ProtocolVersionRegistry;
 import com.datastax.oss.driver.internal.core.channel.DriverChannel;
 import com.datastax.oss.driver.internal.core.context.InternalDriverContext;
 import com.datastax.oss.driver.internal.core.context.NettyOptions;
@@ -75,6 +78,7 @@ public static Builder builder() {
   @Mock private RetryPolicy retryPolicy;
   @Mock private SpeculativeExecutionPolicy speculativeExecutionPolicy;
   @Mock private TimestampGenerator timestampGenerator;
+  @Mock private ProtocolVersionRegistry protocolVersionRegistry;
 
   private RequestHandlerTestHarness(Builder builder) {
     MockitoAnnotations.initMocks(this);
@@ -129,6 +133,12 @@ private RequestHandlerTestHarness(Builder builder) {
 
     Mockito.when(session.setKeyspace(any(CqlIdentifier.class)))
         .thenReturn(CompletableFuture.completedFuture(null));
+
+    Mockito.when(context.protocolVersionRegistry()).thenReturn(protocolVersionRegistry);
+    Mockito.when(
+            protocolVersionRegistry.supports(
+                any(ProtocolVersion.class), any(ProtocolFeature.class)))
+        .thenReturn(true);
   }
 
   public DefaultSession getSession() {

integration-tests/src/test/java/com/datastax/oss/driver/api/core/cql/BatchStatementIT.java

@@ -15,10 +15,13 @@
  */
 package com.datastax.oss.driver.api.core.cql;
 
+import com.datastax.oss.driver.api.core.Cluster;
+import com.datastax.oss.driver.api.core.CqlIdentifier;
 import com.datastax.oss.driver.api.core.servererrors.InvalidQueryException;
 import com.datastax.oss.driver.api.testinfra.CassandraRequirement;
 import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
 import com.datastax.oss.driver.api.testinfra.cluster.ClusterRule;
+import com.datastax.oss.driver.api.testinfra.cluster.ClusterUtils;
 import com.datastax.oss.driver.categories.ParallelizableTests;
 import java.util.Iterator;
 import org.junit.Before;
@@ -314,6 +317,30 @@ public void should_fail_counter_batch_with_non_counter_increment() {
     cluster.session().execute(batchStatement);
   }
 
+  @Test(expected = IllegalStateException.class)
+  public void should_not_allow_unset_value_when_protocol_less_than_v4() {
+    //    CREATE TABLE test (k0 text, k1 int, v int, PRIMARY KEY (k0, k1))
+    try (Cluster<CqlSession> v3Cluster = ClusterUtils.newCluster(ccm, "protocol.version = V3")) {
+      CqlIdentifier keyspace = cluster.keyspace();
+      CqlSession session = v3Cluster.connect(keyspace);
+      PreparedStatement prepared = session.prepare("INSERT INTO test (k0, k1, v) values (?, ?, ?)");
+
+      BatchStatementBuilder builder = BatchStatement.builder(BatchType.LOGGED);
+      builder.addStatements(
+          // All set => OK
+          prepared.bind(name.getMethodName(), 1, 1),
+          // One variable unset => should fail
+          prepared
+              .boundStatementBuilder()
+              .setString(0, name.getMethodName())
+              .setInt(1, 2)
+              .unset(2)
+              .build());
+
+      session.execute(builder.build());
+    }
+  }
+
   private void verifyBatchInsert() {
     // validate data inserted by the batch.
     Statement<?> select =

integration-tests/src/test/java/com/datastax/oss/driver/api/core/cql/BoundStatementIT.java

@@ -17,14 +17,12 @@
 
 import com.datastax.oss.driver.api.core.Cluster;
 import com.datastax.oss.driver.api.core.CqlIdentifier;
-import com.datastax.oss.driver.api.core.config.DriverConfigProfile;
 import com.datastax.oss.driver.api.testinfra.CassandraRequirement;
 import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
 import com.datastax.oss.driver.api.testinfra.cluster.ClusterRule;
 import com.datastax.oss.driver.api.testinfra.cluster.ClusterUtils;
 import com.datastax.oss.driver.categories.ParallelizableTests;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -55,27 +53,16 @@ public void setupSchema() {
   }
 
   @Test(expected = IllegalStateException.class)
-  @Ignore
-  public void should_not_allow_unset_value_on_bound_statement_when_protocol_less_than_v4() {
-    // TODO reenable this if JAVA-1584 is fixed.
+  public void should_not_allow_unset_value_when_protocol_less_than_v4() {
     try (Cluster<CqlSession> v3Cluster = ClusterUtils.newCluster(ccm, "protocol.version = V3")) {
-      CqlIdentifier keyspace = ClusterUtils.uniqueKeyspaceId();
-      DriverConfigProfile slowProfile = ClusterUtils.slowProfile(v3Cluster);
-      ClusterUtils.createKeyspace(v3Cluster, keyspace, slowProfile);
+      CqlIdentifier keyspace = cluster.keyspace();
       CqlSession session = v3Cluster.connect(keyspace);
-      PreparedStatement prepared =
-          session.prepare("INSERT INTO test2 (k, v0, v1) values (?, ?, ?)");
+      PreparedStatement prepared = session.prepare("INSERT INTO test2 (k, v0) values (?, ?)");
 
       BoundStatement boundStatement =
-          prepared
-              .boundStatementBuilder()
-              .setString(0, name.getMethodName())
-              .unset(1)
-              .setString(2, name.getMethodName())
-              .build();
+          prepared.boundStatementBuilder().setString(0, name.getMethodName()).unset(1).build();
 
       session.execute(boundStatement);
-      ClusterUtils.dropKeyspace(v3Cluster, keyspace, slowProfile);
     }
   }