From 80f0ddea83da5bf6e6306db20561c51111d2eb61 Mon Sep 17 00:00:00 2001
From: Tor Brede Vekterli <vekterli@oath.com>
Date: Mon, 11 Jun 2018 13:25:09 +0000
Subject: Fix deserialization of MapValueUpdates for array fields in C++

Must deserialize element with nested data type (since that's what's
on the wire), not with data type of array itself.
---
 document/src/tests/documentupdatetestcase.cpp      | 54 ++++++++++++++++++++++
 .../src/vespa/document/update/mapvalueupdate.cpp   |  5 +-
 2 files changed, 58 insertions(+), 1 deletion(-)

(limited to 'document')

diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp
index 97dd916bc60..67fafadd8cd 100644
--- a/document/src/tests/documentupdatetestcase.cpp
+++ b/document/src/tests/documentupdatetestcase.cpp
@@ -60,6 +60,8 @@ struct DocumentUpdateTest : public CppUnit::TestFixture {
   void testThatDocumentUpdateFlagsIsWorking();
   void testThatCreateIfNonExistentFlagIsSerialized50AndDeserialized50();
   void testThatCreateIfNonExistentFlagIsSerializedAndDeserialized();
+  void array_element_update_can_be_roundtrip_serialized();
+  void array_element_update_applies_to_specified_element();
 
   CPPUNIT_TEST_SUITE(DocumentUpdateTest);
   CPPUNIT_TEST(testSimpleUsage);
@@ -85,6 +87,8 @@ struct DocumentUpdateTest : public CppUnit::TestFixture {
   CPPUNIT_TEST(testThatDocumentUpdateFlagsIsWorking);
   CPPUNIT_TEST(testThatCreateIfNonExistentFlagIsSerialized50AndDeserialized50);
   CPPUNIT_TEST(testThatCreateIfNonExistentFlagIsSerializedAndDeserialized);
+  CPPUNIT_TEST(array_element_update_can_be_roundtrip_serialized);
+  CPPUNIT_TEST(array_element_update_applies_to_specified_element);
   CPPUNIT_TEST_SUITE_END();
 
 };
@@ -1077,4 +1081,54 @@ DocumentUpdateTest::testThatCreateIfNonExistentFlagIsSerializedAndDeserialized()
     CPPUNIT_ASSERT(deserialized->getCreateIfNonExistent());
 }
 
+struct ArrayUpdateFixture {
+    TestDocMan doc_man;
+    std::unique_ptr<Document> doc;
+    const Field& array_field;
+    std::unique_ptr<DocumentUpdate> update;
+
+    ArrayUpdateFixture();
+    ~ArrayUpdateFixture();
+};
+
+ArrayUpdateFixture::ArrayUpdateFixture()
+    : doc_man(),
+      doc(doc_man.createDocument()),
+      array_field(doc->getType().getField("tags")) // of type array<string>
+{
+    update = std::make_unique<DocumentUpdate>(*doc->getDataType(), doc->getId());
+    update->addUpdate(FieldUpdate(array_field)
+                              .addUpdate(MapValueUpdate(IntFieldValue(1),
+                                                        AssignValueUpdate(StringFieldValue("bar")))));
+}
+ArrayUpdateFixture::~ArrayUpdateFixture() = default;
+
+void DocumentUpdateTest::array_element_update_can_be_roundtrip_serialized() {
+    ArrayUpdateFixture f;
+
+    auto buffer = serializeHEAD(*f.update);
+    buffer->flip();
+
+    auto deserialized = DocumentUpdate::createHEAD(f.doc_man.getTypeRepo(), *buffer);
+    CPPUNIT_ASSERT_EQUAL(*f.update, *deserialized);
+}
+
+void DocumentUpdateTest::array_element_update_applies_to_specified_element() {
+    ArrayUpdateFixture f;
+
+    ArrayFieldValue array_value(f.array_field.getDataType());
+    array_value.add("foo");
+    array_value.add("baz");
+    array_value.add("blarg");
+    f.doc->setValue(f.array_field, array_value);
+
+    f.update->applyTo(*f.doc);
+
+    auto result_array = f.doc->getAs<ArrayFieldValue>(f.array_field);
+    CPPUNIT_ASSERT_EQUAL(size_t(3), result_array->size());
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("foo"), (*result_array)[0].getAsString());
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("bar"), (*result_array)[1].getAsString());
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("blarg"), (*result_array)[2].getAsString());
+}
+
 }  // namespace document
diff --git a/document/src/vespa/document/update/mapvalueupdate.cpp b/document/src/vespa/document/update/mapvalueupdate.cpp
index cf7f48c4367..09cec34c388 100644
--- a/document/src/vespa/document/update/mapvalueupdate.cpp
+++ b/document/src/vespa/document/update/mapvalueupdate.cpp
@@ -141,12 +141,15 @@ MapValueUpdate::deserialize(const DocumentTypeRepo& repo,
     VespaDocumentDeserializer deserializer(repo, stream, version);
     switch(type.getClass().id()) {
         case ArrayDataType::classId:
+        {
             _key.reset(new IntFieldValue);
             deserializer.read(*_key);
             buffer.incPos(buffer.getRemaining() - stream.size());
+            const ArrayDataType& arrayType = static_cast<const ArrayDataType&>(type);
             _update.reset(ValueUpdate::createInstance(
-                            repo, type, buffer, version).release());
+                    repo, arrayType.getNestedType(), buffer, version).release());
             break;
+        }
         case WeightedSetDataType::classId:
         {
             const WeightedSetDataType& wset(
-- 
cgit v1.2.3