Detect unchanged field in in array/map of struct.

3 files changed, 121 insertions, 45 deletions

diff --git a/searchcore/src/tests/proton/common/document_type_inspector/document_type_inspector_test.cpp b/searchcore/src/tests/proton/common/document_type_inspector/document_type_inspector_test.cpp
index 2dbff7d40dd..167865b5c68 100644
--- a/searchcore/src/tests/proton/common/document_type_inspector/document_type_inspector_test.cpp
+++ b/searchcore/src/tests/proton/common/document_type_inspector/document_type_inspector_test.cpp
@@ -1,78 +1,131 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #include <vespa/searchcore/proton/common/document_type_inspector.h>
 #include <vespa/vespalib/testkit/testapp.h>
-#include <vespa/document/repo/configbuilder.h>
-#include <vespa/document/repo/documenttyperepo.h>
+#include <vespa/document/base/field.h>
+#include <vespa/document/datatype/datatypes.h>
 
 using namespace document;
 using namespace proton;
-using document::config_builder::DocumenttypesConfigBuilderHelper;
-using document::config_builder::Struct;
 
-const int32_t doc_type_id = 787121340;
-const vespalib::string type_name = "test";
-const vespalib::string header_name = type_name + ".header";
-const vespalib::string body_name = type_name + ".body";
-
-std::unique_ptr<const DocumentTypeRepo>
-makeOldDocTypeRepo()
+template <class Type>
+void
+addFields(Type &type, bool fieldF3IsString, bool hasFieldF4, bool hasFieldF5)
 {
-    DocumenttypesConfigBuilderHelper builder;
-    builder.document(doc_type_id, type_name,
-                     Struct(header_name), Struct(body_name).
-                     addField("f1", DataType::T_STRING).
-                     addField("f2", DataType::T_STRING).
-                     addField("f3", DataType::T_STRING).
-                     addField("f4", DataType::T_STRING));
-    return std::unique_ptr<const DocumentTypeRepo>(new DocumentTypeRepo(builder.config()));
+    type.addField(Field("f1", 1, *DataType::STRING, true));
+    type.addField(Field("f2", 2, *DataType::STRING, true));
+    type.addField(Field("f3", 3, fieldF3IsString ? *DataType::STRING : *DataType::INT, true));
+    if (hasFieldF4) {
+        type.addField(Field("f4", 4, *DataType::STRING, true));
+    }
+    if (hasFieldF5) {
+        type.addField(Field("f5", 5, *DataType::STRING, true));
+    }
 }
 
-std::unique_ptr<const DocumentTypeRepo>
-makeNewDocTypeRepo()
+struct DocumentTypeFixture
+{
+    DocumentType _documentType;
+    StructDataType _structFieldType;
+    ArrayDataType _structArrayFieldType;
+    MapDataType _structMapFieldType;
+    MapDataType _mapFieldType;
+
+    DocumentTypeFixture(bool fieldF3IsString, bool hasFieldF4, bool hasFieldF5, bool hasStruct, bool mapKeyIsByte);
+    ~DocumentTypeFixture();
+};
+
+DocumentTypeFixture::DocumentTypeFixture(bool fieldF3IsString, bool hasFieldF4, bool hasFieldF5, bool hasStruct, bool mapKeyIsByte)
+    : _documentType("test"),
+      _structFieldType("struct"),
+      _structArrayFieldType(_structFieldType),
+      _structMapFieldType(mapKeyIsByte ? *DataType::BYTE : *DataType::STRING, _structFieldType),
+      _mapFieldType(mapKeyIsByte ? *DataType::BYTE : *DataType::STRING, *DataType::STRING)
 {
-    DocumenttypesConfigBuilderHelper builder;
-    builder.document(doc_type_id, type_name,
-                     Struct(header_name), Struct(body_name).
-                     addField("f1", DataType::T_STRING).
-                     addField("f2", DataType::T_STRING).
-                     addField("f3", DataType::T_INT).
-                     addField("f5", DataType::T_STRING));
-    return std::unique_ptr<const DocumentTypeRepo>(new DocumentTypeRepo(builder.config()));
+    addFields(_documentType, fieldF3IsString, hasFieldF4, hasFieldF5);
+    if (hasStruct) {
+        addFields(_structFieldType, fieldF3IsString, hasFieldF4, hasFieldF5);
+        _documentType.addField(Field("sarray", 11, _structArrayFieldType, true));
+        _documentType.addField(Field("smap", 12, _structMapFieldType, true));
+        _documentType.addField(Field("map", 13, _mapFieldType, true));
+    }
 }
 
+DocumentTypeFixture::~DocumentTypeFixture() = default;
+
 struct Fixture
 {
-    std::unique_ptr<const DocumentTypeRepo> _oldRepo;
-    std::unique_ptr<const DocumentTypeRepo> _newRepo;
+    DocumentTypeFixture _oldDocType;
+    DocumentTypeFixture _newDocType;
     DocumentTypeInspector _inspector;
-    Fixture()
-        : _oldRepo(makeOldDocTypeRepo()),
-          _newRepo(makeNewDocTypeRepo()),
-          _inspector(*_oldRepo->getDocumentType("test"), *_newRepo->getDocumentType("test"))
+    Fixture(bool hasStruct = true, bool mapKeyIsByte = false)
+        : _oldDocType(true, true, false, hasStruct, mapKeyIsByte),
+          _newDocType(false, false, true, true, false),
+          _inspector(_oldDocType._documentType, _newDocType._documentType)
     {
     }
 };
 
 TEST_F("require that unchanged fields are known", Fixture)
 {
-    EXPECT_TRUE(f._inspector.hasUnchangedField("f1"));
-    EXPECT_TRUE(f._inspector.hasUnchangedField("f2"));
+    const IDocumentTypeInspector &inspector = f._inspector;
+    EXPECT_TRUE(inspector.hasUnchangedField("f1"));
+    EXPECT_TRUE(inspector.hasUnchangedField("f2"));
+    EXPECT_TRUE(inspector.hasUnchangedField("sarray.f1"));
+    EXPECT_TRUE(inspector.hasUnchangedField("sarray.f2"));
+    EXPECT_TRUE(inspector.hasUnchangedField("smap.key"));
+    EXPECT_TRUE(inspector.hasUnchangedField("smap.value.f1"));
+    EXPECT_TRUE(inspector.hasUnchangedField("smap.value.f2"));
+    EXPECT_TRUE(inspector.hasUnchangedField("map.key"));
+    EXPECT_TRUE(inspector.hasUnchangedField("map.value"));
 }
 
 TEST_F("require that changed fields are detected", Fixture)
 {
-    EXPECT_FALSE(f._inspector.hasUnchangedField("f3"));
+    const IDocumentTypeInspector &inspector = f._inspector;
+    EXPECT_FALSE(inspector.hasUnchangedField("f3"));
+    EXPECT_FALSE(inspector.hasUnchangedField("sarray.f3"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.value.f3"));
 }
 
 TEST_F("require that partially missing fields are detected", Fixture)
 {
-    EXPECT_FALSE(f._inspector.hasUnchangedField("f4"));
-    EXPECT_FALSE(f._inspector.hasUnchangedField("f5"));
+    const IDocumentTypeInspector &inspector = f._inspector;
+    EXPECT_FALSE(inspector.hasUnchangedField("f4"));
+    EXPECT_FALSE(inspector.hasUnchangedField("f5"));
+    EXPECT_FALSE(inspector.hasUnchangedField("sarray.f4"));
+    EXPECT_FALSE(inspector.hasUnchangedField("sarray.f5"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.value.f4"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.value.f5"));
 }
 
 TEST_F("require that non-existing fields are NOT known", Fixture)
 {
-    EXPECT_FALSE(f._inspector.hasUnchangedField("not"));
+    const IDocumentTypeInspector &inspector = f._inspector;
+    EXPECT_FALSE(inspector.hasUnchangedField("not"));
+    EXPECT_FALSE(inspector.hasUnchangedField("sarray.not"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.not"));
+}
+
+TEST_F("require that map key type change is detected", Fixture(true, true))
+{
+    const IDocumentTypeInspector &inspector = f._inspector;
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.key"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.value.f1"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.value.f2"));
+    EXPECT_FALSE(inspector.hasUnchangedField("map.key"));
+    EXPECT_FALSE(inspector.hasUnchangedField("map.value"));
+}
+
+TEST_F("require that struct addition is detected", Fixture(false, false))
+{
+    const IDocumentTypeInspector &inspector = f._inspector;
+    EXPECT_FALSE(inspector.hasUnchangedField("sarray.f1"));
+    EXPECT_FALSE(inspector.hasUnchangedField("sarray.f2"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.key"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.value.f1"));
+    EXPECT_FALSE(inspector.hasUnchangedField("smap.value.f2"));
+    EXPECT_FALSE(inspector.hasUnchangedField("map.key"));
+    EXPECT_FALSE(inspector.hasUnchangedField("map.value"));
 }
 
 TEST_MAIN()
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
index 239f91b449f..bc49b300a1a 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
@@ -47,6 +47,8 @@ AttributeWriter::WriteField::buildFieldPath(const DocumentType &docType)
         docType.buildFieldPath(fp, name);
     } catch (document::FieldNotFoundException & e) {
         fp = FieldPath();
+    } catch (vespalib::IllegalArgumentException &e) {
+        fp = FieldPath();
     }
     _fieldPath = std::move(fp);
 }
diff --git a/searchcore/src/vespa/searchcore/proton/common/document_type_inspector.cpp b/searchcore/src/vespa/searchcore/proton/common/document_type_inspector.cpp
index 6cff162ae08..e19fa5351c2 100644
--- a/searchcore/src/vespa/searchcore/proton/common/document_type_inspector.cpp
+++ b/searchcore/src/vespa/searchcore/proton/common/document_type_inspector.cpp
@@ -1,6 +1,11 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "document_type_inspector.h"
+#include <vespa/document/base/exceptions.h>
+#include <vespa/document/base/fieldpath.h>
+
+using document::FieldPath;
+using document::FieldPathEntry;
 
 namespace proton {
 
@@ -14,12 +19,28 @@ DocumentTypeInspector::DocumentTypeInspector(const document::DocumentType &oldDo
 bool
 DocumentTypeInspector::hasUnchangedField(const vespalib::string &name) const
 {
-    if (!_oldDocType.hasField(name) || !_newDocType.hasField(name)) {
+    FieldPath oldPath;
+    FieldPath newPath;
+    try {
+        _oldDocType.buildFieldPath(oldPath, name);
+        _newDocType.buildFieldPath(newPath, name);
+    } catch (document::FieldNotFoundException &e) {
+        return false;
+    } catch (vespalib::IllegalArgumentException &e) {
         return false;
     }
-    const document::Field &oldField = _oldDocType.getField(name);
-    const document::Field &newField = _newDocType.getField(name);
-    return oldField == newField;
+    if (oldPath.size() != newPath.size()) {
+        return false;
+    }
+    for (uint32_t i = 0; i < oldPath.size(); ++i) {
+        const auto &oldEntry = oldPath[i];
+        const auto &newEntry = newPath[i];
+        if (oldEntry.getType() != newEntry.getType() ||
+            oldEntry.getDataType() != newEntry.getDataType()) {
+            return false;
+        }
+    }
+    return true;
 }
 
 } // namespace proton