Propagate configured imported fields to DocumentTypeRepo (C++)

5 files changed, 139 insertions, 57 deletions

diff --git a/document/src/tests/repo/documenttyperepo_test.cpp b/document/src/tests/repo/documenttyperepo_test.cpp
index b263ad75930..0bc80ebcd16 100644
--- a/document/src/tests/repo/documenttyperepo_test.cpp
+++ b/document/src/tests/repo/documenttyperepo_test.cpp
@@ -532,6 +532,47 @@ TEST("Reference fields are resolved to correct reference type") {
     EXPECT_EQUAL(*ref1_type, type->getFieldsType().getField("ref3").getDataType());
 }
 
+TEST("Config with no imported fields has empty imported fields set in DocumentType") {
+    DocumenttypesConfigBuilderHelper builder;
+    builder.document(doc_type_id, type_name,
+                     Struct(header_name), Struct(body_name));
+    DocumentTypeRepo repo(builder.config());
+    const auto *type = repo.getDocumentType(doc_type_id);
+    ASSERT_TRUE(type != nullptr);
+    EXPECT_TRUE(type->imported_field_names().empty());
+    EXPECT_FALSE(type->has_imported_field_name("foo"));
+}
+
+TEST("Configured imported field names are available in the DocumentType") {
+    const int type_2_id = doc_type_id + 1;
+    // Note: we cheat a bit by specifying imported field names in types that have no
+    // reference fields. Add to test if we add config read-time validation of this. :)
+    DocumenttypesConfigBuilderHelper builder;
+    // Type with one imported field
+    builder.document(doc_type_id, type_name,
+                     Struct(header_name), Struct(body_name))
+                     .imported_field("my_cool_field");
+    // Type with two imported fields
+    builder.document(type_2_id, type_name_2,
+                     Struct(header_name_2), Struct(body_name_2))
+                     .imported_field("my_awesome_field")
+                     .imported_field("my_swag_field");
+
+    DocumentTypeRepo repo(builder.config());
+    const auto* type = repo.getDocumentType(doc_type_id);
+    ASSERT_TRUE(type != nullptr);
+    EXPECT_EQUAL(1u, type->imported_field_names().size());
+    EXPECT_TRUE(type->has_imported_field_name("my_cool_field"));
+    EXPECT_FALSE(type->has_imported_field_name("my_awesome_field"));
+
+    type = repo.getDocumentType(type_2_id);
+    ASSERT_TRUE(type != nullptr);
+    EXPECT_EQUAL(2u, type->imported_field_names().size());
+    EXPECT_TRUE(type->has_imported_field_name("my_awesome_field"));
+    EXPECT_TRUE(type->has_imported_field_name("my_swag_field"));
+    EXPECT_FALSE(type->has_imported_field_name("my_cool_field"));
+}
+
 namespace {
 
 const TensorDataType &
diff --git a/document/src/vespa/document/datatype/documenttype.cpp b/document/src/vespa/document/datatype/documenttype.cpp
index ffc250eacba..c2739dc1303 100644
--- a/document/src/vespa/document/datatype/documenttype.cpp
+++ b/document/src/vespa/document/datatype/documenttype.cpp
@@ -23,9 +23,10 @@ DocumentType::DocumentType() = default;
 DocumentType::DocumentType(stringref name, int32_t id)
     : StructuredDataType(name, id),
       _inheritedTypes(),
-      _ownedFields(new StructDataType(name + ".header")),
+      _ownedFields(std::make_shared<StructDataType>(name + ".header")),
       _fields(_ownedFields.get()),
-      _fieldSets()
+      _fieldSets(),
+      _imported_field_names()
 {
     if (name != "document") {
         _inheritedTypes.push_back(DataType::DOCUMENT);
@@ -36,7 +37,8 @@ DocumentType::DocumentType(stringref name, int32_t id, const StructDataType& fie
     : StructuredDataType(name, id),
       _inheritedTypes(),
       _fields(&fields),
-      _fieldSets()
+      _fieldSets(),
+      _imported_field_names()
 {
     if (name != "document") {
         _inheritedTypes.push_back(DataType::DOCUMENT);
@@ -46,12 +48,13 @@ DocumentType::DocumentType(stringref name, int32_t id, const StructDataType& fie
 DocumentType::DocumentType(stringref name)
     : StructuredDataType(name),
       _inheritedTypes(),
-      _ownedFields(new StructDataType(name + ".header")),
+      _ownedFields(std::make_shared<StructDataType>(name + ".header")),
       _fields(_ownedFields.get()),
-      _fieldSets()
+      _fieldSets(),
+      _imported_field_names()
 {
     if (name != "document") {
-        _inheritedTypes.push_back(DataType::DOCUMENT);
+        _inheritedTypes.emplace_back(DataType::DOCUMENT);
     }
 }
 
@@ -59,27 +62,28 @@ DocumentType::DocumentType(stringref name, const StructDataType& fields)
     : StructuredDataType(name),
       _inheritedTypes(),
       _fields(&fields),
-      _fieldSets()
+      _fieldSets(),
+      _imported_field_names()
 {
     if (name != "document") {
-        _inheritedTypes.push_back(DataType::DOCUMENT);
+        _inheritedTypes.emplace_back(DataType::DOCUMENT);
     }
 }
 
 DocumentType::~DocumentType() = default;
 
 DocumentType &
-DocumentType::addFieldSet(const vespalib::string & name, const FieldSet::Fields & fields)
+DocumentType::addFieldSet(const vespalib::string & name, FieldSet::Fields fields)
 {
-    _fieldSets[name] = FieldSet(name, fields);
+    _fieldSets[name] = FieldSet(name, std::move(fields));
     return *this;
 }
 
 const DocumentType::FieldSet *
 DocumentType::getFieldSet(const vespalib::string & name) const
 {
-    FieldSetMap::const_iterator it(_fieldSets.find(name));
-    return (it != _fieldSets.end()) ? & it->second : NULL;
+    auto it = _fieldSets.find(name);
+    return (it != _fieldSets.end()) ? & it->second : nullptr;
 }
 
 void
@@ -107,37 +111,34 @@ DocumentType::inherit(const DocumentType &docType) {
                 "Document type " + docType.toString() + " already inherits type "
                 + toString() + ". Cannot add cyclic dependencies.", VESPA_STRLOC);
     }
-        // If we already inherits this type, there is no point in adding it
-        // again.
+    // If we already inherits this type, there is no point in adding it again.
     if (isA(docType)) {
-            // If we already directly inherits it, complain
-        for (std::vector<const DocumentType *>::const_iterator
-                it = _inheritedTypes.begin(); it != _inheritedTypes.end(); ++it)
-        {
-            if (**it == docType) {
+        // If we already directly inherits it, complain
+        for (const auto* inherited : _inheritedTypes) {
+            if (*inherited == docType) {
                 throw IllegalArgumentException(
                         "DocumentType " + getName() + " already inherits "
                         "document type " + docType.getName(), VESPA_STRLOC);
             }
         }
-            // Indirectly already inheriting it is oki, as this can happen
-            // due to inherited documents inheriting the same type.
+        // Indirectly already inheriting it is oki, as this can happen
+        // due to inherited documents inheriting the same type.
         LOG(info, "Document type %s inherits document type %s from multiple "
                   "types.", getName().c_str(), docType.getName().c_str());
         return;
     }
-        // Add non-conflicting types.
+    // Add non-conflicting types.
     Field::Set fs = docType._fields->getFieldSet();
-    for (Field::Set::const_iterator it = fs.begin(); it != fs.end(); ++it) {
+    for (const auto* field : fs) {
         if (!_ownedFields.get()) {
             _ownedFields.reset(_fields->clone());
             _fields = _ownedFields.get();
         }
-        _ownedFields->addInheritedField(**it);
+        _ownedFields->addInheritedField(*field);
     }
     // If we inherit default document type Document.0, remove that if adding
     // another parent, as that has to also inherit Document
-    if (_inheritedTypes.size() == 1 && *_inheritedTypes[0] == *DataType::DOCUMENT) {
+    if ((_inheritedTypes.size() == 1) && (*_inheritedTypes[0] == *DataType::DOCUMENT)) {
         _inheritedTypes.clear();
     }
     _inheritedTypes.push_back(&docType);
@@ -168,8 +169,7 @@ DocumentType::print(std::ostream& out, bool verbose, const std::string& indent)
     out << ")";
     if (verbose) {
         if (!_inheritedTypes.empty()) {
-            std::vector<const DocumentType *>::const_iterator it(
-                    _inheritedTypes.begin());
+            auto it = _inheritedTypes.begin();
             out << "\n" << indent << "    : ";
             (*it)->print(out, false, "");
             while (++it != _inheritedTypes.end()) {
@@ -188,17 +188,18 @@ DocumentType::operator==(const DataType& other) const
 {
     if (&other == this) return true;
     if (!DataType::operator==(other)) return false;
-    const DocumentType* o(dynamic_cast<const DocumentType*>(&other));
-    if (o == 0) return false;
+    const auto* o(dynamic_cast<const DocumentType*>(&other));
+    if (o == nullptr) return false;
     if (*_fields != *o->_fields) return false;
     if (_inheritedTypes.size() != o->_inheritedTypes.size()) return false;
-    std::vector<const DocumentType *>::const_iterator it1(_inheritedTypes.begin());
-    std::vector<const DocumentType *>::const_iterator it2(o->_inheritedTypes.begin());
+    auto it1 = _inheritedTypes.begin();
+    auto it2 = o->_inheritedTypes.begin();
     while (it1 != _inheritedTypes.end()) {
         if (**it1 != **it2) return false;
         ++it1;
         ++it2;
     }
+    // TODO imported fields? like in the Java impl, field sets are not considered either... :I
     return true;
 }
 
@@ -228,6 +229,14 @@ DocumentType::getFieldSet() const
     return _fields->getFieldSet();
 }
 
+bool DocumentType::has_imported_field_name(const vespalib::string& name) const noexcept {
+    return (_imported_field_names.find(name) != _imported_field_names.end());
+}
+
+void DocumentType::add_imported_field_name(const vespalib::string& name) {
+    _imported_field_names.insert(name);
+}
+
 DocumentType *
 DocumentType::clone() const {
     return new DocumentType(*this);
diff --git a/document/src/vespa/document/datatype/documenttype.h b/document/src/vespa/document/datatype/documenttype.h
index 7fcf5dfa237..ed6e9e66ab5 100644
--- a/document/src/vespa/document/datatype/documenttype.h
+++ b/document/src/vespa/document/datatype/documenttype.h
@@ -12,9 +12,11 @@
 #pragma once
 
 #include <vespa/document/datatype/structdatatype.h>
-
+#include <vespa/vespalib/stllike/hash_set.h>
+#include <vespa/vespalib/stllike/string.h>
 #include <vector>
 #include <map>
+#include <set>
 
 namespace document {
 
@@ -25,12 +27,17 @@ class DocumentType : public StructuredDataType {
 public:
     class FieldSet {
     public:
-        typedef std::set<vespalib::string> Fields;
-        FieldSet() : _name(), _fields() {}
-        FieldSet(const vespalib::string & name) : _name(name), _fields() {}
-        FieldSet(const vespalib::string & name, const Fields & fields) : _name(name), _fields(fields) {}
-        const vespalib::string & getName() const { return _name; }
-        const Fields & getFields() const { return _fields; }
+        using Fields = std::set<vespalib::string>;
+        FieldSet() = default;
+        explicit FieldSet(const vespalib::string & name) : _name(name), _fields() {}
+        FieldSet(const vespalib::string & name, Fields fields) : _name(name), _fields(std::move(fields)) {}
+        FieldSet(const FieldSet&) = default;
+        FieldSet& operator=(const FieldSet&) = default;
+        FieldSet(FieldSet&&) noexcept = default;
+        FieldSet& operator=(FieldSet&&) noexcept = default;
+
+        const vespalib::string & getName() const noexcept { return _name; }
+        const Fields & getFields() const noexcept { return _fields; }
         FieldSet & add(vespalib::string & field) {
             _fields.insert(field);
             return *this;
@@ -39,28 +46,31 @@ public:
         vespalib::string _name;
         Fields           _fields;
     };
-    typedef std::map<vespalib::string, FieldSet> FieldSetMap;
+    using FieldSetMap = std::map<vespalib::string, FieldSet>;
+    using ImportedFieldNames = vespalib::hash_set<vespalib::string>;
+
     std::vector<const DocumentType *> _inheritedTypes;
-    StructDataType::SP _ownedFields;
-    const StructDataType* _fields;
-    FieldSetMap _fieldSets;
+    StructDataType::SP                _ownedFields;
+    const StructDataType*             _fields;
+    FieldSetMap                       _fieldSets;
+    ImportedFieldNames                _imported_field_names;
 
 public:
-    typedef std::unique_ptr<DocumentType> UP;
-    typedef std::shared_ptr<DocumentType> SP;
+    using UP = std::unique_ptr<DocumentType>;
+    using SP = std::shared_ptr<DocumentType>;
 
     DocumentType();
     DocumentType(vespalib::stringref name, int32_t id);
     DocumentType(vespalib::stringref name, int32_t id,
                  const StructDataType& fields);
 
-    DocumentType(vespalib::stringref name);
+    explicit DocumentType(vespalib::stringref name);
     DocumentType(vespalib::stringref name,
                  const StructDataType& fields);
 
-    ~DocumentType();
+    ~DocumentType() override;
 
-    const StructDataType& getFieldsType() const { return *_fields; }
+    const StructDataType& getFieldsType() const noexcept { return *_fields; }
 
     void addField(const Field&);
 
@@ -89,9 +99,16 @@ public:
     Field::Set getFieldSet() const override;
     DocumentType* clone() const override;
 
-    DocumentType & addFieldSet(const vespalib::string & name, const FieldSet::Fields & fields);
+    DocumentType & addFieldSet(const vespalib::string & name, FieldSet::Fields fields);
     const FieldSet * getFieldSet(const vespalib::string & name) const;
 
+    const ImportedFieldNames& imported_field_names() const noexcept {
+        return _imported_field_names;
+    }
+    bool has_imported_field_name(const vespalib::string& name) const noexcept;
+    // Ideally the type would be immutable, but this is how it's built today.
+    void add_imported_field_name(const vespalib::string& name);
+
     DECLARE_IDENTIFIABLE(DocumentType);
 };
 
diff --git a/document/src/vespa/document/repo/configbuilder.h b/document/src/vespa/document/repo/configbuilder.h
index 15ee0da0c79..52ed8d878a2 100644
--- a/document/src/vespa/document/repo/configbuilder.h
+++ b/document/src/vespa/document/repo/configbuilder.h
@@ -38,9 +38,9 @@ struct TypeOrId {
 };
 
 struct Struct : DatatypeConfig {
-    Struct(const vespalib::string &name) {
+    explicit Struct(vespalib::string name) {
         type = Type::STRUCT;
-        sstruct.name = name;
+        sstruct.name = std::move(name);
     }
     Struct &setCompression(Sstruct::Compression::Type t, int32_t level,
                            int32_t threshold, int32_t min_size) {
@@ -63,7 +63,7 @@ struct Struct : DatatypeConfig {
 };
 
 struct Array : DatatypeConfig {
-    Array(TypeOrId nested_type) {
+    explicit Array(TypeOrId nested_type) {
         addNestedType(nested_type);
         type = Type::ARRAY;
         array.element.id = nested_type.id;
@@ -71,7 +71,7 @@ struct Array : DatatypeConfig {
 };
 
 struct Wset : DatatypeConfig {
-    Wset(TypeOrId nested_type) {
+    explicit Wset(TypeOrId nested_type) {
         addNestedType(nested_type);
         type = Type::WSET;
         wset.key.id = nested_type.id;
@@ -94,7 +94,7 @@ struct Map : DatatypeConfig {
 };
 
 struct AnnotationRef : DatatypeConfig {
-    AnnotationRef(int32_t annotation_type_id) {
+    explicit AnnotationRef(int32_t annotation_type_id) {
         type = Type::ANNOTATIONREF;
         annotationref.annotation.id = annotation_type_id;
     }
@@ -103,7 +103,7 @@ struct AnnotationRef : DatatypeConfig {
 struct DocTypeRep {
     DocumenttypesConfig::Documenttype &doc_type;
 
-    DocTypeRep(DocumenttypesConfig::Documenttype &type)
+    explicit DocTypeRep(DocumenttypesConfig::Documenttype &type)
         : doc_type(type) {}
     DocTypeRep &inherit(int32_t id) {
         doc_type.inherits.resize(doc_type.inherits.size() + 1);
@@ -127,6 +127,12 @@ struct DocTypeRep {
         doc_type.referencetype.back().targetTypeId = target_type_id;
         return *this;
     }
+
+    DocTypeRep& imported_field(vespalib::string field_name) {
+        doc_type.importedfield.resize(doc_type.importedfield.size() + 1);
+        doc_type.importedfield.back().name = std::move(field_name);
+        return *this;
+    }
 };
 
 class DocumenttypesConfigBuilderHelper {
diff --git a/document/src/vespa/document/repo/documenttyperepo.cpp b/document/src/vespa/document/repo/documenttyperepo.cpp
index da59f527115..58dbd16beb6 100644
--- a/document/src/vespa/document/repo/documenttyperepo.cpp
+++ b/document/src/vespa/document/repo/documenttyperepo.cpp
@@ -441,10 +441,10 @@ void addFieldSet(const DocumenttypesConfig::Documenttype::FieldsetsMap & fsv, Do
     for (const auto & entry : fsv) {
         const DocumenttypesConfig::Documenttype::Fieldsets & fs(entry.second);
         DocumentType::FieldSet::Fields fields;
-        for (size_t j(0); j < fs.fields.size(); j++) {
-            fields.insert(fs.fields[j]);
+        for (const auto& f : fs.fields) {
+            fields.insert(f);
         }
-        doc_type.addFieldSet(entry.first, fields);
+        doc_type.addFieldSet(entry.first, std::move(fields));
     }
 }
 
@@ -457,6 +457,14 @@ void addReferenceTypes(const vector<DocumenttypesConfig::Documenttype::Reference
     }
 }
 
+void add_imported_fields(const DocumenttypesConfig::Documenttype::ImportedfieldVector& imported_fields,
+                         DocumentType& doc_type)
+{
+    for (const auto& imported : imported_fields) {
+        doc_type.add_imported_field_name(imported.name);
+    }
+}
+
 void configureDataTypeRepo(const DocumenttypesConfig::Documenttype &doc_type, DocumentTypeMap &type_map) {
     DataTypeRepo *data_types = type_map[doc_type.id];
     inheritAnnotationTypes(doc_type.inherits, type_map, data_types->annotations);
@@ -467,6 +475,7 @@ void configureDataTypeRepo(const DocumenttypesConfig::Documenttype &doc_type, Do
     setAnnotationDataTypes(doc_type.annotationtype, data_types->annotations, data_types->repo);
     inheritDocumentTypes(doc_type.inherits, type_map, *data_types->doc_type);
     addFieldSet(doc_type.fieldsets, *data_types->doc_type);
+    add_imported_fields(doc_type.importedfield, *data_types->doc_type);
 }
 
 void addDataTypeRepo(DataTypeRepo::UP data_types, DocumentTypeMap &doc_types) {