Merge pull request #6451 from vespa-engine/vekterli/ignore-array-element-update-for-out-of-bounds-index

Ignore array element update with index out of bounds

3 files changed, 39 insertions, 3 deletions

diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp
index eea5dd4decf..b4d98f0dd21 100644
--- a/document/src/tests/documentupdatetestcase.cpp
+++ b/document/src/tests/documentupdatetestcase.cpp
@@ -64,6 +64,7 @@ struct DocumentUpdateTest : public CppUnit::TestFixture {
   void testThatCreateIfNonExistentFlagIsSerializedAndDeserialized();
   void array_element_update_can_be_roundtrip_serialized();
   void array_element_update_applies_to_specified_element();
+  void array_element_update_for_invalid_index_is_ignored();
 
   CPPUNIT_TEST_SUITE(DocumentUpdateTest);
   CPPUNIT_TEST(testSimpleUsage);
@@ -91,6 +92,7 @@ struct DocumentUpdateTest : public CppUnit::TestFixture {
   CPPUNIT_TEST(testThatCreateIfNonExistentFlagIsSerializedAndDeserialized);
   CPPUNIT_TEST(array_element_update_can_be_roundtrip_serialized);
   CPPUNIT_TEST(array_element_update_applies_to_specified_element);
+  CPPUNIT_TEST(array_element_update_for_invalid_index_is_ignored);
   CPPUNIT_TEST_SUITE_END();
 
 };
@@ -1050,4 +1052,17 @@ void DocumentUpdateTest::array_element_update_applies_to_specified_element() {
     CPPUNIT_ASSERT_EQUAL(vespalib::string("blarg"), (*result_array)[2].getAsString());
 }
 
+void DocumentUpdateTest::array_element_update_for_invalid_index_is_ignored() {
+    ArrayUpdateFixture f;
+
+    ArrayFieldValue array_value(f.array_field.getDataType());
+    array_value.add("jerry");
+    f.doc->setValue(f.array_field, array_value);
+
+    f.update->applyTo(*f.doc); // MapValueUpdate for index 1, which does not exist
+
+    auto result_array = f.doc->getAs<ArrayFieldValue>(f.array_field);
+    CPPUNIT_ASSERT_EQUAL(array_value, *result_array);
+}
+
 }  // namespace document
diff --git a/document/src/tests/fieldpathupdatetestcase.cpp b/document/src/tests/fieldpathupdatetestcase.cpp
index 80a63a86b92..fb3ba3f7e40 100644
--- a/document/src/tests/fieldpathupdatetestcase.cpp
+++ b/document/src/tests/fieldpathupdatetestcase.cpp
@@ -69,6 +69,7 @@ struct FieldPathUpdateTestCase : public CppUnit::TestFixture {
     void testSerializeAssignMath();
     void testReadSerializedFile();
     void testGenerateSerializedFile();
+    void array_element_update_for_invalid_index_is_ignored();
 
     CPPUNIT_TEST_SUITE(FieldPathUpdateTestCase);
     CPPUNIT_TEST(testWhereClause);
@@ -108,6 +109,7 @@ struct FieldPathUpdateTestCase : public CppUnit::TestFixture {
     CPPUNIT_TEST(testSerializeAssignMath);
     CPPUNIT_TEST(testReadSerializedFile);
     CPPUNIT_TEST(testGenerateSerializedFile);
+    CPPUNIT_TEST(array_element_update_for_invalid_index_is_ignored);
     CPPUNIT_TEST_SUITE_END();
 private:
     DocumentUpdate::UP
@@ -1183,4 +1185,23 @@ FieldPathUpdateTestCase::testGenerateSerializedFile()
     close(fd);
 }
 
+void FieldPathUpdateTestCase::array_element_update_for_invalid_index_is_ignored() {
+    auto doc = std::make_unique<Document>(_foobar_type, DocumentId("id::foobar::1"));
+    doc->setRepo(*_repo);
+    auto& field = doc->getType().getField("strarray");
+
+    ArrayFieldValue str_array(field.getDataType());
+    str_array.add(StringFieldValue("jerry"));
+    doc->setValue("strarray", str_array);
+
+    DocumentUpdate docUp(*_repo, _foobar_type, DocumentId("id::foobar::1"));
+    docUp.addFieldPathUpdate(FieldPathUpdate::CP(
+            new AssignFieldPathUpdate(*doc->getDataType(), "strarray[1]", "", StringFieldValue("george"))));
+    docUp.applyTo(*doc);
+
+    // Doc is unmodified.
+    auto new_arr = doc->getAs<ArrayFieldValue>(field);
+    CPPUNIT_ASSERT_EQUAL(str_array, *new_arr);
+}
+
 }
diff --git a/document/src/vespa/document/update/mapvalueupdate.cpp b/document/src/vespa/document/update/mapvalueupdate.cpp
index ff1063a3dda..3fc9c8cbea5 100644
--- a/document/src/vespa/document/update/mapvalueupdate.cpp
+++ b/document/src/vespa/document/update/mapvalueupdate.cpp
@@ -71,9 +71,9 @@ MapValueUpdate::applyTo(FieldValue& value) const
         ArrayFieldValue& val(static_cast<ArrayFieldValue&>(value));
         int32_t index = _key->getAsInt();
         if (index < 0 || static_cast<uint32_t>(index) >= val.size()) {
-            throw IllegalStateException(vespalib::make_string(
-                    "Tried to update element %i in an array of %zu elements",
-		            index, val.size()), VESPA_STRLOC);
+            // Silently ignoring updates with index out of bounds matches
+            // behavior of functionally identical fieldpath updates.
+            return true;
         }
         if (!_update->applyTo(val[_key->getAsInt()])) {
             val.remove(_key->getAsInt());