Change StructFieldsResolver to remember attribute names and add function to populate mapper.

7 files changed, 98 insertions, 69 deletions

diff --git a/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.cpp
index 88be8aac8c5..a1f3ce4d392 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.cpp
@@ -1,8 +1,9 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "array_attribute_combiner_dfw.h"
-#include "docsum_field_writer_state.h"
 #include "attribute_field_writer.h"
+#include "docsum_field_writer_state.h"
+#include "struct_fields_resolver.h"
 #include <vespa/searchcommon/attribute/iattributecontext.h>
 #include <vespa/searchcommon/attribute/iattributevector.h>
 #include <vespa/searchlib/common/matching_elements.h>
@@ -100,22 +101,15 @@ ArrayAttributeFieldWriterState::insertField(uint32_t docId, vespalib::slime::Ins
 }
 
 ArrayAttributeCombinerDFW::ArrayAttributeCombinerDFW(const vespalib::string &fieldName,
-                                                     const std::vector<vespalib::string> &fields,
+                                                     const StructFieldsResolver& fields_resolver,
                                                      bool filter_elements,
                                                      std::shared_ptr<StructFieldMapper> struct_field_mapper)
     : AttributeCombinerDFW(fieldName, filter_elements, std::move(struct_field_mapper)),
-      _fields(fields),
-      _attributeNames()
+      _fields(fields_resolver.get_array_fields()),
+      _attributeNames(fields_resolver.get_array_attributes())
 {
-    _attributeNames.reserve(_fields.size());
-    vespalib::string prefix = fieldName + ".";
-    for (const auto &field : _fields) {
-        _attributeNames.emplace_back(prefix + field);
-    }
     if (filter_elements && _struct_field_mapper && !_struct_field_mapper->is_struct_field(fieldName)) {
-        for (const auto &sub_field : _attributeNames) {
-            _struct_field_mapper->add_mapping(fieldName, sub_field);
-        }
+        fields_resolver.apply_to(*_struct_field_mapper);
     }
 }
 
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.h b/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.h
index 473bdbbc4e7..c3e686965cf 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.h
@@ -9,10 +9,12 @@ namespace search::attribute { class IAttributeContext; }
 namespace search::docsummary {
 
 class DocsumFieldWriterState;
+class StructFieldsResolver;
 
-/*
- * This class reads values from multiple struct field attributes and
- * inserts them as an array of struct.
+/**
+ * This class reads values from multiple struct field attributes and inserts them as an array of struct.
+ *
+ * Used to write both array of struct fields and map of primitives fields.
  */
 class ArrayAttributeCombinerDFW : public AttributeCombinerDFW
 {
@@ -22,7 +24,7 @@ class ArrayAttributeCombinerDFW : public AttributeCombinerDFW
     std::unique_ptr<DocsumFieldWriterState> allocFieldWriterState(search::attribute::IAttributeContext &context, const MatchingElements* matching_elements) override;
 public:
     ArrayAttributeCombinerDFW(const vespalib::string &fieldName,
-                              const std::vector<vespalib::string> &fields,
+                              const StructFieldsResolver& fields_resolver,
                               bool filter_elements,
                               std::shared_ptr<StructFieldMapper> struct_field_mapper);
     ~ArrayAttributeCombinerDFW() override;
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
index db6ffc0f80d..3f8d4eb8c31 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
@@ -42,12 +42,12 @@ std::unique_ptr<IDocsumFieldWriter>
 AttributeCombinerDFW::create(const vespalib::string &fieldName, IAttributeManager &attrMgr, bool filter_elements, std::shared_ptr<StructFieldMapper> struct_field_mapper)
 {
     StructFieldsResolver structFields(fieldName, attrMgr);
-    if (structFields.getError()) {
+    if (structFields.has_error()) {
         return std::unique_ptr<IDocsumFieldWriter>();
-    } else if (!structFields.getMapFields().empty()) {
-        return std::make_unique<StructMapAttributeCombinerDFW>(fieldName, structFields.getMapFields(), filter_elements, std::move(struct_field_mapper));
+    } else if (structFields.is_map_of_struct()) {
+        return std::make_unique<StructMapAttributeCombinerDFW>(fieldName, structFields, filter_elements, std::move(struct_field_mapper));
     }
-    return std::make_unique<ArrayAttributeCombinerDFW>(fieldName, structFields.getArrayFields(), filter_elements, std::move(struct_field_mapper));
+    return std::make_unique<ArrayAttributeCombinerDFW>(fieldName, structFields, filter_elements, std::move(struct_field_mapper));
 }
 
 void
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.cpp b/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.cpp
index 4333861f0b2..5a005dfe2b9 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.cpp
@@ -2,6 +2,7 @@
 
 #include "struct_fields_resolver.h"
 #include <vespa/searchlib/attribute/iattributemanager.h>
+#include <vespa/searchlib/common/struct_field_mapper.h>
 #include <algorithm>
 
 #include <vespa/log/log.h>
@@ -11,47 +12,58 @@ using search::attribute::CollectionType;
 
 namespace search::docsummary {
 
-StructFieldsResolver::StructFieldsResolver(const vespalib::string& fieldName, const IAttributeManager& attrMgr)
-    : _mapFields(),
-      _arrayFields(),
-      _hasMapKey(false),
+StructFieldsResolver::StructFieldsResolver(const vespalib::string& field_name, const IAttributeManager& attr_mgr)
+    : _field_name(field_name),
+      _map_key_attribute(),
+      _map_value_fields(),
+      _map_value_attributes(),
+      _array_fields(),
+      _array_attributes(),
+      _has_map_key(false),
       _error(false)
 {
     std::vector<const search::attribute::IAttributeVector *> attrs;
-    auto attrCtx = attrMgr.createContext();
-    attrCtx->getAttributeList(attrs);
-    vespalib::string prefix = fieldName + ".";
-    vespalib::string keyName = prefix + "key";
-    vespalib::string valuePrefix = prefix + "value.";
+    auto attr_ctx = attr_mgr.createContext();
+    attr_ctx->getAttributeList(attrs);
+    vespalib::string prefix = field_name + ".";
+    _map_key_attribute = prefix + "key";
+    vespalib::string value_prefix = prefix + "value.";
     for (const auto attr : attrs) {
         vespalib::string name = attr->getName();
         if (name.substr(0, prefix.size()) != prefix) {
             continue;
         }
-        auto collType = attr->getCollectionType();
-        if (collType != CollectionType::Type::ARRAY) {
-            LOG(warning, "Attribute %s is not an array attribute", name.c_str());
+        if (attr->getCollectionType() != CollectionType::Type::ARRAY) {
+            LOG(warning, "Attribute '%s' is not an array attribute", name.c_str());
             _error = true;
             break;
         }
-        if (name.substr(0, valuePrefix.size()) == valuePrefix) {
-            _mapFields.emplace_back(name.substr(valuePrefix.size()));
+        if (name.substr(0, value_prefix.size()) == value_prefix) {
+            _map_value_fields.emplace_back(name.substr(value_prefix.size()));
         } else {
-            _arrayFields.emplace_back(name.substr(prefix.size()));
-            if (name == keyName) {
-                _hasMapKey = true;
+            _array_fields.emplace_back(name.substr(prefix.size()));
+            if (name == _map_key_attribute) {
+                _has_map_key = true;
             }
         }
     }
     if (!_error) {
-        std::sort(_arrayFields.begin(), _arrayFields.end());
-        std::sort(_mapFields.begin(), _mapFields.end());
-        if (!_mapFields.empty()) {
-            if (!_hasMapKey) {
-                LOG(warning, "Missing key attribute '%s', have value attributes for map", keyName.c_str());
+        std::sort(_map_value_fields.begin(), _map_value_fields.end());
+        for (const auto& field : _map_value_fields) {
+            _map_value_attributes.emplace_back(value_prefix + field);
+        }
+
+        std::sort(_array_fields.begin(), _array_fields.end());
+        for (const auto& field : _array_fields) {
+            _array_attributes.emplace_back(prefix + field);
+        }
+
+        if (!_map_value_fields.empty()) {
+            if (!_has_map_key) {
+                LOG(warning, "Missing key attribute '%s', have value attributes for map", _map_key_attribute.c_str());
                 _error = true;
-            } else if (_arrayFields.size() != 1u) {
-                LOG(warning, "Could not determine if field '%s' is array or map of struct", fieldName.c_str());
+            } else if (_array_fields.size() != 1u) {
+                LOG(warning, "Could not determine if field '%s' is array or map of struct", field_name.c_str());
                 _error = true;
             }
         }
@@ -60,5 +72,20 @@ StructFieldsResolver::StructFieldsResolver(const vespalib::string& fieldName, co
 
 StructFieldsResolver::~StructFieldsResolver() = default;
 
+void
+StructFieldsResolver::apply_to(StructFieldMapper& mapper) const
+{
+    if (is_map_of_struct()) {
+        mapper.add_mapping(_field_name, _map_key_attribute);
+        for (const auto& sub_field : _map_value_attributes) {
+            mapper.add_mapping(_field_name, sub_field);
+        }
+    } else {
+        for (const auto& sub_field : _array_attributes) {
+            mapper.add_mapping(_field_name, sub_field);
+        }
+    }
+}
+
 }
 
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.h b/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.h
index 5708f4f940c..f6562353cc0 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.h
@@ -5,7 +5,10 @@
 #include <vespa/vespalib/stllike/string.h>
 #include <vector>
 
-namespace search { class IAttributeManager; }
+namespace search {
+class IAttributeManager;
+class StructFieldMapper;
+}
 
 namespace search::docsummary {
 
@@ -15,17 +18,27 @@ namespace search::docsummary {
  */
 class StructFieldsResolver {
 private:
-    std::vector<vespalib::string> _mapFields;
-    std::vector<vespalib::string> _arrayFields;
-    bool _hasMapKey;
+    using StringVector = std::vector<vespalib::string>;
+    vespalib::string _field_name;
+    vespalib::string _map_key_attribute;
+    StringVector _map_value_fields;
+    StringVector _map_value_attributes;
+    StringVector _array_fields;
+    StringVector _array_attributes;
+    bool _has_map_key;
     bool _error;
 
 public:
-    StructFieldsResolver(const vespalib::string& fieldName, const IAttributeManager& attrMgr);
+    StructFieldsResolver(const vespalib::string& field_name, const IAttributeManager& attr_mgr);
     ~StructFieldsResolver();
-    const std::vector<vespalib::string>& getMapFields() const { return _mapFields; }
-    const std::vector<vespalib::string>& getArrayFields() const { return _arrayFields; }
-    bool getError() const { return _error; }
+    bool is_map_of_struct() const { return !_map_value_fields.empty(); }
+    const vespalib::string& get_map_key_attribute() const { return _map_key_attribute; }
+    const StringVector& get_map_value_fields() const { return _map_value_fields; }
+    const StringVector& get_map_value_attributes() const { return _map_value_attributes; }
+    const StringVector& get_array_fields() const { return _array_fields; }
+    const StringVector& get_array_attributes() const { return _array_attributes; }
+    bool has_error() const { return _error; }
+    void apply_to(StructFieldMapper& mapper) const;
 };
 
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.cpp
index 1b43acc7231..d5922ddf46b 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.cpp
@@ -1,8 +1,9 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include "struct_map_attribute_combiner_dfw.h"
-#include "docsum_field_writer_state.h"
 #include "attribute_field_writer.h"
+#include "docsum_field_writer_state.h"
+#include "struct_fields_resolver.h"
+#include "struct_map_attribute_combiner_dfw.h"
 #include <vespa/searchcommon/attribute/iattributecontext.h>
 #include <vespa/searchcommon/attribute/iattributevector.h>
 #include <vespa/searchlib/common/matching_elements.h>
@@ -121,25 +122,16 @@ StructMapAttributeFieldWriterState::insertField(uint32_t docId, vespalib::slime:
 }
 
 StructMapAttributeCombinerDFW::StructMapAttributeCombinerDFW(const vespalib::string &fieldName,
-                                                             const std::vector<vespalib::string> &valueFields,
+                                                             const StructFieldsResolver& fields_resolver,
                                                              bool filter_elements,
                                                              std::shared_ptr<StructFieldMapper> struct_field_mapper)
     : AttributeCombinerDFW(fieldName, filter_elements, std::move(struct_field_mapper)),
-      _keyAttributeName(),
-      _valueFields(valueFields),
-      _valueAttributeNames()
+      _keyAttributeName(fields_resolver.get_map_key_attribute()),
+      _valueFields(fields_resolver.get_map_value_fields()),
+      _valueAttributeNames(fields_resolver.get_map_value_attributes())
 {
-    _keyAttributeName = fieldName + ".key";
-    _valueAttributeNames.reserve(_valueFields.size());
-    vespalib::string prefix = fieldName + ".value.";
-    for (const auto &field : _valueFields) {
-        _valueAttributeNames.emplace_back(prefix + field);
-    }
     if (filter_elements && _struct_field_mapper && !_struct_field_mapper->is_struct_field(fieldName)) {
-        _struct_field_mapper->add_mapping(fieldName, _keyAttributeName);
-        for (const auto &sub_field : _valueAttributeNames) {
-            _struct_field_mapper->add_mapping(fieldName, sub_field);
-        }
+        fields_resolver.apply_to(*_struct_field_mapper);
     }
 }
 
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.h b/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.h
index 1f92172d68b..a28e487fb1c 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.h
@@ -9,6 +9,7 @@ namespace search::attribute { class IAttributeContext; }
 namespace search::docsummary {
 
 class DocsumFieldWriterState;
+class StructFieldsResolver;
 
 /*
  * This class reads values from multiple struct field attributes and
@@ -23,7 +24,7 @@ class StructMapAttributeCombinerDFW : public AttributeCombinerDFW
     std::unique_ptr<DocsumFieldWriterState> allocFieldWriterState(search::attribute::IAttributeContext &context, const MatchingElements* matching_elements) override;
 public:
     StructMapAttributeCombinerDFW(const vespalib::string &fieldName,
-                                  const std::vector<vespalib::string> &valueFields,
+                                  const StructFieldsResolver& fields_resolver,
                                   bool filter_elements,
                                   std::shared_ptr<StructFieldMapper> struct_field_mapper);
     ~StructMapAttributeCombinerDFW() override;