JAVA-3057 Allow decoding a UDT that has more fields than expected

patch by Ammar Khaku; reviewed by Andy Tolbert and Bret McGuire
reference: #1635

Author: akhaku

core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/UdtCodec.java

@@ -30,10 +30,14 @@
 import java.nio.BufferUnderflowException;
 import java.nio.ByteBuffer;
 import net.jcip.annotations.ThreadSafe;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @ThreadSafe
 public class UdtCodec implements TypeCodec<UdtValue> {
 
+  private static final Logger LOG = LoggerFactory.getLogger(UdtCodec.class);
+
   private final UserDefinedType cqlType;
 
   public UdtCodec(@NonNull UserDefinedType cqlType) {
@@ -107,10 +111,8 @@ public UdtValue decode(@Nullable ByteBuffer bytes, @NonNull ProtocolVersion prot
       int i = 0;
       while (input.hasRemaining()) {
         if (i == cqlType.getFieldTypes().size()) {
-          throw new IllegalArgumentException(
-              String.format(
-                  "Too many fields in encoded UDT value, expected %d",
-                  cqlType.getFieldTypes().size()));
+          LOG.debug("Encountered unexpected fields when parsing codec {}", cqlType);
+          break;
         }
         int elementSize = input.getInt();
         ByteBuffer element;

core/src/test/java/com/datastax/oss/driver/internal/core/type/codec/UdtCodecTest.java

@@ -136,18 +136,18 @@ public void should_decode_udt() {
   }
 
   @Test
-  public void should_fail_to_decode_udt_when_too_many_fields() {
-    assertThatThrownBy(
-            () ->
-                decode(
-                    "0x"
-                        + ("00000004" + "00000001")
-                        + "ffffffff"
-                        + ("00000001" + "61")
-                        // extra contents
-                        + "ffffffff"))
-        .isInstanceOf(IllegalArgumentException.class)
-        .hasMessage("Too many fields in encoded UDT value, expected 3");
+  public void should_decode_udt_when_too_many_fields() {
+    UdtValue udt =
+        decode(
+            "0x"
+                + ("00000004" + "00000001")
+                + "ffffffff"
+                + ("00000001" + "61")
+                // extra contents
+                + "ffffffff");
+    assertThat(udt.getInt(0)).isEqualTo(1);
+    assertThat(udt.isNull(1)).isTrue();
+    assertThat(udt.getString(2)).isEqualTo("a");
   }
 
   /** Test for JAVA-2557. Ensures that the codec can decode null fields with any negative length. */

integration-tests/src/test/java/com/datastax/oss/driver/internal/core/type/codec/UdtCodecIT.java

@@ -0,0 +1,77 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.type.codec;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import com.datastax.oss.driver.api.core.CqlSession;
+import com.datastax.oss.driver.api.core.cql.Row;
+import com.datastax.oss.driver.api.core.data.UdtValue;
+import com.datastax.oss.driver.api.core.type.UserDefinedType;
+import com.datastax.oss.driver.api.core.type.codec.TypeCodec;
+import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
+import com.datastax.oss.driver.api.testinfra.session.SessionRule;
+import com.datastax.oss.driver.categories.ParallelizableTests;
+import java.util.Objects;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TestRule;
+
+@Category(ParallelizableTests.class)
+public class UdtCodecIT {
+
+  private CcmRule ccmRule = CcmRule.getInstance();
+
+  private SessionRule<CqlSession> sessionRule = SessionRule.builder(ccmRule).build();
+
+  @Rule public TestRule chain = RuleChain.outerRule(ccmRule).around(sessionRule);
+
+  @Test
+  public void should_decoding_udt_be_backward_compatible() {
+    CqlSession session = sessionRule.session();
+    session.execute("CREATE TYPE test_type_1 (a text, b int)");
+    session.execute("CREATE TABLE test_table_1 (e int primary key, f frozen<test_type_1>)");
+    // insert a row using version 1 of the UDT schema
+    session.execute("INSERT INTO test_table_1(e, f) VALUES(1, {a: 'a', b: 1})");
+    UserDefinedType udt =
+        session
+            .getMetadata()
+            .getKeyspace(sessionRule.keyspace())
+            .flatMap(ks -> ks.getUserDefinedType("test_type_1"))
+            .orElseThrow(IllegalStateException::new);
+    TypeCodec<?> oldCodec = session.getContext().getCodecRegistry().codecFor(udt);
+    // update UDT schema
+    session.execute("ALTER TYPE test_type_1 add i text");
+    // insert a row using version 2 of the UDT schema
+    session.execute("INSERT INTO test_table_1(e, f) VALUES(2, {a: 'b', b: 2, i: 'b'})");
+    Row row =
+        Objects.requireNonNull(session.execute("SELECT f FROM test_table_1 WHERE e = ?", 2).one());
+    // Try to read new row with old codec. Using row.getUdtValue() would not cause any issues,
+    // because new codec will be automatically registered (using all 3 attributes).
+    // If application leverages generic row.get(String, Codec) method, data reading with old codec
+    // should
+    // be backward-compatible.
+    UdtValue value = Objects.requireNonNull((UdtValue) row.get("f", oldCodec));
+    assertThat(value.getString("a")).isEqualTo("b");
+    assertThat(value.getInt("b")).isEqualTo(2);
+    assertThatThrownBy(() -> value.getString("i")).hasMessage("i is not a field in this UDT");
+  }
+}