Use helper class to collect list of nested field names for

array or struct / map of struct.

1 files changed, 73 insertions, 37 deletions

diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
index 9383154979f..b532cfb273a 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
@@ -18,30 +18,30 @@ using search::attribute::CollectionType;
 
 namespace search::docsummary {
 
-AttributeCombinerDFW::AttributeCombinerDFW(const vespalib::string &fieldName)
-    : IDocsumFieldWriter(),
-      _stateIndex(0),
-      _fieldName(fieldName)
-{
-}
-
-AttributeCombinerDFW::~AttributeCombinerDFW() = default;
+namespace {
 
-bool
-AttributeCombinerDFW::IsGenerated() const
+class StructFields
 {
-    return true;
-}
+    std::vector<vespalib::string> _mapFields;
+    std::vector<vespalib::string> _arrayFields;
+    bool _hasMapKey;
+    bool _error;
 
-bool
-AttributeCombinerDFW::setFieldWriterStateIndex(uint32_t fieldWriterStateIndex)
-{
-    _stateIndex = fieldWriterStateIndex;
-    return true;
-}
+public:
+    StructFields(const vespalib::string &fieldName, const IAttributeManager &attrMgr);
+    ~StructFields();
+    const std::vector<vespalib::string> &getMapFields() const { return _mapFields; }
+    const std::vector<vespalib::string> &getArrayFields() const { return _arrayFields; }
+    bool hasMapKey() const { return _hasMapKey; }
+    bool getError() const { return _error; }
+};
 
-std::unique_ptr<IDocsumFieldWriter>
-AttributeCombinerDFW::create(const vespalib::string &fieldName, IAttributeManager &attrMgr)
+
+StructFields::StructFields(const vespalib::string &fieldName, const IAttributeManager &attrMgr)
+    : _mapFields(),
+      _arrayFields(),
+      _hasMapKey(false),
+      _error(false)
 {
     // Note: Doesn't handle imported attributes
     std::vector<AttributeGuard> attrs;
@@ -49,9 +49,6 @@ AttributeCombinerDFW::create(const vespalib::string &fieldName, IAttributeManage
     vespalib::string prefix = fieldName + ".";
     vespalib::string keyName = prefix + "key";
     vespalib::string valuePrefix = prefix + "value.";
-    std::vector<vespalib::string> mapFields;
-    std::vector<vespalib::string> arrayFields;
-    bool foundKey = false;
     for (const auto &guard : attrs) {
         vespalib::string name = guard->getName();
         if (name.substr(0, prefix.size()) != prefix) {
@@ -60,31 +57,70 @@ AttributeCombinerDFW::create(const vespalib::string &fieldName, IAttributeManage
         auto collType = guard->getCollectionType();
         if (collType != CollectionType::Type::ARRAY) {
             LOG(warning, "Attribute %s is not an array attribute", name.c_str());
-            return std::unique_ptr<IDocsumFieldWriter>();
+            _error = true;
+            break;
         }
         if (name.substr(0, valuePrefix.size()) == valuePrefix) {
-            mapFields.emplace_back(name.substr(valuePrefix.size()));
+            _mapFields.emplace_back(name.substr(valuePrefix.size()));
         } else {
-            arrayFields.emplace_back(name.substr(prefix.size()));
+            _arrayFields.emplace_back(name.substr(prefix.size()));
             if (name == keyName) {
-                foundKey = true;
+                _hasMapKey = true;
             }
         }
     }
-    if (!mapFields.empty()) {
-        if (!foundKey) {
-            LOG(warning, "Missing key attribute '%s', have value attributes for map", keyName.c_str());
-            return std::unique_ptr<IDocsumFieldWriter>();
-        }
-        if (arrayFields.size() != 1u) {
-            LOG(warning, "Could not determine if field '%s' is array or map of struct", fieldName.c_str());
-            return std::unique_ptr<IDocsumFieldWriter>();
+    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());
+                _error = true;
+            } else if (_arrayFields.size() != 1u) {
+                LOG(warning, "Could not determine if field '%s' is array or map of struct", fieldName.c_str());
+                _error = true;
+            }
         }
+    }
+}
+
+StructFields::~StructFields() = default;
+
+}
+
+AttributeCombinerDFW::AttributeCombinerDFW(const vespalib::string &fieldName)
+    : IDocsumFieldWriter(),
+      _stateIndex(0),
+      _fieldName(fieldName)
+{
+}
+
+AttributeCombinerDFW::~AttributeCombinerDFW() = default;
+
+bool
+AttributeCombinerDFW::IsGenerated() const
+{
+    return true;
+}
+
+bool
+AttributeCombinerDFW::setFieldWriterStateIndex(uint32_t fieldWriterStateIndex)
+{
+    _stateIndex = fieldWriterStateIndex;
+    return true;
+}
+
+std::unique_ptr<IDocsumFieldWriter>
+AttributeCombinerDFW::create(const vespalib::string &fieldName, IAttributeManager &attrMgr)
+{
+    StructFields structFields(fieldName, attrMgr);
+    if (structFields.getError()) {
+        return std::unique_ptr<IDocsumFieldWriter>();
+    } else if (!structFields.getMapFields().empty()) {
         LOG(warning, "map of struct is not yet supported for field '%s'", fieldName.c_str());
         return std::unique_ptr<IDocsumFieldWriter>();
     }
-    std::sort(arrayFields.begin(), arrayFields.end());
-    return std::make_unique<ArrayAttributeCombinerDFW>(fieldName, arrayFields);
+    return std::make_unique<ArrayAttributeCombinerDFW>(fieldName, structFields.getArrayFields());
 }
 
 void