Setup struct field mapper when constructing matched elements filter dfw.

9 files changed, 134 insertions, 29 deletions

diff --git a/searchsummary/src/tests/docsummary/attribute_combiner/attribute_combiner_test.cpp b/searchsummary/src/tests/docsummary/attribute_combiner/attribute_combiner_test.cpp
index 5d122fa1042..c856c5c805e 100644
--- a/searchsummary/src/tests/docsummary/attribute_combiner/attribute_combiner_test.cpp
+++ b/searchsummary/src/tests/docsummary/attribute_combiner/attribute_combiner_test.cpp
@@ -217,7 +217,7 @@ AttributeCombinerTest::set_field(const vespalib::string &field_name, bool filter
     if (filter_elements) {
         _struct_field_mapper = std::make_shared<search::StructFieldMapper>();
     }
-    writer = AttributeCombinerDFW::create(field_name, attrs.mgr, filter_elements, _struct_field_mapper);
+    writer = AttributeCombinerDFW::create(field_name, *state._attrCtx, filter_elements, _struct_field_mapper);
     EXPECT_TRUE(writer->setFieldWriterStateIndex(0));
     state._fieldWriterStates.resize(1);
 }
diff --git a/searchsummary/src/tests/docsummary/matched_elements_filter/matched_elements_filter_test.cpp b/searchsummary/src/tests/docsummary/matched_elements_filter/matched_elements_filter_test.cpp
index 40d0285b1ec..7c058ee5e1f 100644
--- a/searchsummary/src/tests/docsummary/matched_elements_filter/matched_elements_filter_test.cpp
+++ b/searchsummary/src/tests/docsummary/matched_elements_filter/matched_elements_filter_test.cpp
@@ -3,7 +3,11 @@
 #include <vespa/document/datatype/datatype.h>
 #include <vespa/document/datatype/structdatatype.h>
 #include <vespa/document/document.h>
+#include <vespa/searchcommon/attribute/config.h>
+#include <vespa/searchlib/attribute/attributefactory.h>
+#include <vespa/searchlib/attribute/attributevector.h>
 #include <vespa/searchlib/common/matching_elements.h>
+#include <vespa/searchlib/common/struct_field_mapper.h>
 #include <vespa/searchlib/util/slime_output_raw_buf_adapter.h>
 #include <vespa/searchsummary/docsummary/docsumstate.h>
 #include <vespa/searchsummary/docsummary/idocsumenvironment.h>
@@ -19,8 +23,15 @@
 #include <vespa/log/log.h>
 LOG_SETUP("matched_elements_filter_test");
 
+using search::AttributeFactory;
+using search::AttributeVector;
 using search::MatchingElements;
 using search::StructFieldMapper;
+using search::attribute::BasicType;
+using search::attribute::CollectionType;
+using search::attribute::Config;
+using search::attribute::IAttributeContext;
+using search::attribute::IAttributeVector;
 using vespalib::Slime;
 
 using namespace document;
@@ -93,8 +104,10 @@ public:
         auto* result_class = _config.AddResultClass("test", class_id);
         EXPECT_TRUE(result_class->AddConfigEntry("array", ResType::RES_JSONSTRING));
         EXPECT_TRUE(result_class->AddConfigEntry("map", ResType::RES_JSONSTRING));
+        EXPECT_TRUE(result_class->AddConfigEntry("map2", ResType::RES_JSONSTRING));
         _config.CreateEnumMaps();
     }
+    ~DocsumStore() {}
     const ResultConfig& get_config() const { return _config; }
     const ResultClass* get_class() const { return _config.LookupResultClass(class_id); }
     search::docsummary::DocsumStoreValue getMappedDocsum() {
@@ -113,6 +126,11 @@ public:
             map_value.put(StringFieldValue("c"), *make_elem_value("c", 7));
             write_field_value(map_value);
         }
+        {
+            MapFieldValue map2_value(_map_type);
+            map2_value.put(StringFieldValue("dummy"), *make_elem_value("dummy", 2));
+            write_field_value(map2_value);
+        }
         const char* buf;
         uint32_t buf_len;
         assert(_packer.GetDocsumBlob(&buf, &buf_len));
@@ -120,6 +138,29 @@ public:
     }
 };
 
+class AttributeContext : public IAttributeContext {
+private:
+    AttributeVector::SP _map_value_name;
+    AttributeVector::SP _map2_key;
+    AttributeVector::SP _array_weight;
+public:
+    AttributeContext()
+        : _map_value_name(AttributeFactory::createAttribute("map.value.name", Config(BasicType::STRING, CollectionType::ARRAY))),
+          _map2_key(AttributeFactory::createAttribute("map2.key", Config(BasicType::STRING, CollectionType::ARRAY))),
+          _array_weight(AttributeFactory::createAttribute("array.weight", Config(BasicType::INT32, CollectionType::ARRAY)))
+    {}
+    ~AttributeContext() {}
+    const IAttributeVector* getAttribute(const string&) const override { abort(); }
+    const IAttributeVector* getAttributeStableEnum(const string&) const override { abort(); }
+    void getAttributeList(std::vector<const IAttributeVector*>& list) const override {
+        list.push_back(_map_value_name.get());
+        list.push_back(_map2_key.get());
+        list.push_back(_array_weight.get());
+    }
+    void releaseEnumGuards() override { abort(); }
+    void asyncForAttribute(const vespalib::string&, std::unique_ptr<search::attribute::IAttributeFunctor>) const override { abort(); }
+};
+
 class StateCallback : public GetDocsumsStateCallback {
 private:
     std::string _field_name;
@@ -144,34 +185,43 @@ public:
 
 class MatchedElementsFilterTest : public ::testing::Test {
 private:
-    DocsumStore _store;
+    DocsumStore _doc_store;
+    AttributeContext _attr_ctx;
+    std::shared_ptr<StructFieldMapper> _mapper;
 
     SlimeValue run_filter_field_writer(const std::string& input_field_name, const ElementVector& matching_elements) {
-        int input_field_enum = _store.get_config().GetFieldNameEnum().Lookup(input_field_name.c_str());
-        EXPECT_GE(input_field_enum, 0);
-        MatchedElementsFilterDFW filter(input_field_name, input_field_enum);
-
-        GeneralResult result(_store.get_class());
-        result.inplaceUnpack(_store.getMappedDocsum());
+        auto writer = make_field_writer(input_field_name);
+        GeneralResult result(_doc_store.get_class());
+        result.inplaceUnpack(_doc_store.getMappedDocsum());
         StateCallback callback(input_field_name, matching_elements);
         GetDocsumsState state(callback);
         Slime slime;
         SlimeInserter inserter(slime);
 
-        filter.insertField(doc_id, &result, &state, ResType::RES_JSONSTRING, inserter);
+        writer->insertField(doc_id, &result, &state, ResType::RES_JSONSTRING, inserter);
         return SlimeValue(slime);
     }
 
 public:
     MatchedElementsFilterTest()
-        : _store()
+        : _doc_store(),
+          _attr_ctx(),
+          _mapper(std::make_shared<StructFieldMapper>())
     {
     }
+    ~MatchedElementsFilterTest() {}
+    std::unique_ptr<IDocsumFieldWriter> make_field_writer(const std::string& input_field_name) {
+        int input_field_enum = _doc_store.get_config().GetFieldNameEnum().Lookup(input_field_name.c_str());
+        EXPECT_GE(input_field_enum, 0);
+        return MatchedElementsFilterDFW::create(input_field_name, input_field_enum,
+                                                _attr_ctx, _mapper);
+    }
     void expect_filtered(const std::string& input_field_name, const ElementVector& matching_elements, const std::string& exp_slime_as_json) {
         SlimeValue act = run_filter_field_writer(input_field_name, matching_elements);
         SlimeValue exp(exp_slime_as_json);
         EXPECT_EQ(exp.slime, act.slime);
     }
+    const StructFieldMapper& mapper() const { return *_mapper; }
 };
 
 TEST_F(MatchedElementsFilterTest, filters_elements_in_array_field_value)
@@ -185,6 +235,14 @@ TEST_F(MatchedElementsFilterTest, filters_elements_in_array_field_value)
                                         "{'name':'c','weight':7}]");
 }
 
+TEST_F(MatchedElementsFilterTest, struct_field_mapper_is_setup_for_array_field_value)
+{
+    auto writer = make_field_writer("array");
+    EXPECT_TRUE(mapper().is_struct_field("array"));
+    EXPECT_EQ("", mapper().get_struct_field("array.name"));
+    EXPECT_EQ("array", mapper().get_struct_field("array.weight"));
+}
+
 TEST_F(MatchedElementsFilterTest, filters_elements_in_map_field_value)
 {
     expect_filtered("map", {}, "[]");
@@ -196,10 +254,28 @@ TEST_F(MatchedElementsFilterTest, filters_elements_in_map_field_value)
                                       "{'key':'c','value':{'name':'c','weight':7}}]");
 }
 
+TEST_F(MatchedElementsFilterTest, struct_field_mapper_is_setup_for_map_field_value)
+{
+    {
+        auto writer = make_field_writer("map");
+        EXPECT_TRUE(mapper().is_struct_field("map"));
+        EXPECT_EQ("", mapper().get_struct_field("map.key"));
+        EXPECT_EQ("map", mapper().get_struct_field("map.value.name"));
+        EXPECT_EQ("", mapper().get_struct_field("map.value.weight"));
+    }
+    {
+        auto writer = make_field_writer("map2");
+        EXPECT_TRUE(mapper().is_struct_field("map2"));
+        EXPECT_EQ("map2", mapper().get_struct_field("map2.key"));
+        EXPECT_EQ("", mapper().get_struct_field("map2.value.name"));
+        EXPECT_EQ("", mapper().get_struct_field("map2.value.weight"));
+    }
+}
+
 TEST_F(MatchedElementsFilterTest, field_writer_is_not_generated_as_it_depends_on_data_from_document_store)
 {
-    MatchedElementsFilterDFW filter("foo", 0);
-    EXPECT_FALSE(filter.IsGenerated());
+    auto writer = make_field_writer("array");
+    EXPECT_FALSE(writer->IsGenerated());
 }
 
 GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
index 3f8d4eb8c31..8eb77c0ed9c 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp
@@ -12,6 +12,8 @@
 #include <vespa/log/log.h>
 LOG_SETUP(".searchsummary.docsummary.attribute_combiner_dfw");
 
+using search::attribute::IAttributeContext;
+
 namespace search::docsummary {
 
 AttributeCombinerDFW::AttributeCombinerDFW(const vespalib::string &fieldName, bool filter_elements, std::shared_ptr<StructFieldMapper> struct_field_mapper)
@@ -39,9 +41,9 @@ AttributeCombinerDFW::setFieldWriterStateIndex(uint32_t fieldWriterStateIndex)
 }
 
 std::unique_ptr<IDocsumFieldWriter>
-AttributeCombinerDFW::create(const vespalib::string &fieldName, IAttributeManager &attrMgr, bool filter_elements, std::shared_ptr<StructFieldMapper> struct_field_mapper)
+AttributeCombinerDFW::create(const vespalib::string &fieldName, IAttributeContext &attrCtx, bool filter_elements, std::shared_ptr<StructFieldMapper> struct_field_mapper)
 {
-    StructFieldsResolver structFields(fieldName, attrMgr);
+    StructFieldsResolver structFields(fieldName, attrCtx, true);
     if (structFields.has_error()) {
         return std::unique_ptr<IDocsumFieldWriter>();
     } else if (structFields.is_map_of_struct()) {
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.h b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.h
index 3b7ab911956..a8ab5f2f8f5 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.h
@@ -33,7 +33,7 @@ public:
     ~AttributeCombinerDFW() override;
     bool IsGenerated() const override;
     bool setFieldWriterStateIndex(uint32_t fieldWriterStateIndex) override;
-    static std::unique_ptr<IDocsumFieldWriter> create(const vespalib::string &fieldName, IAttributeManager &attrMgr, bool filter_elements, std::shared_ptr<StructFieldMapper> struct_field_mapper);
+    static std::unique_ptr<IDocsumFieldWriter> create(const vespalib::string &fieldName, search::attribute::IAttributeContext &attrCtx, bool filter_elements, std::shared_ptr<StructFieldMapper> struct_field_mapper);
     void insertField(uint32_t docid, GetDocsumsState *state, ResType type, vespalib::slime::Inserter &target) override;
 };
 
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp
index 59a1a67a890..f5a2f0b5833 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp
@@ -95,13 +95,15 @@ DynamicDocsumConfig::createFieldWriter(const string & fieldName, const string &
         }
     } else if (overrideName == "attributecombiner") {
         if (getEnvironment() && getEnvironment()->getAttributeManager()) {
-            fieldWriter = AttributeCombinerDFW::create(fieldName, *getEnvironment()->getAttributeManager(), false, std::shared_ptr<StructFieldMapper>());
+            auto attr_ctx = getEnvironment()->getAttributeManager()->createContext();
+            fieldWriter = AttributeCombinerDFW::create(fieldName, *attr_ctx, false, std::shared_ptr<StructFieldMapper>());
             rc = static_cast<bool>(fieldWriter);
         }
     } else if (overrideName == "matchedattributeelementsfilter") {
         string source_field = argument.empty() ? fieldName : argument;
         if (getEnvironment() && getEnvironment()->getAttributeManager()) {
-            fieldWriter = AttributeCombinerDFW::create(source_field, *getEnvironment()->getAttributeManager(), true, struct_field_mapper);
+            auto attr_ctx = getEnvironment()->getAttributeManager()->createContext();
+            fieldWriter = AttributeCombinerDFW::create(source_field, *attr_ctx, true, struct_field_mapper);
             rc = static_cast<bool>(fieldWriter);
         }
     } else {
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.cpp
index 1b7533b53e3..631476ea914 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.cpp
@@ -2,6 +2,8 @@
 
 #include "docsumstate.h"
 #include "matched_elements_filter_dfw.h"
+#include "struct_fields_resolver.h"
+#include <vespa/searchcommon/attribute/iattributecontext.h>
 #include <vespa/searchlib/common/matching_elements.h>
 #include <vespa/searchlib/common/struct_field_mapper.h>
 #include <vespa/vespalib/data/slime/binary_format.h>
@@ -19,12 +21,25 @@ using vespalib::slime::inject;
 
 namespace search::docsummary {
 
-MatchedElementsFilterDFW::MatchedElementsFilterDFW(const std::string& input_field_name, uint32_t input_field_enum)
+MatchedElementsFilterDFW::MatchedElementsFilterDFW(const std::string& input_field_name, uint32_t input_field_enum,
+                                                   std::shared_ptr<StructFieldMapper> struct_field_mapper)
     : _input_field_name(input_field_name),
       _input_field_enum(input_field_enum),
-      _struct_field_mapper(std::make_shared<StructFieldMapper>())
+      _struct_field_mapper(std::move(struct_field_mapper))
 {
-    // TODO: Take struct field mapper in constructor and populate based on available attribute vectors.
+}
+
+std::unique_ptr<IDocsumFieldWriter>
+MatchedElementsFilterDFW::create(const std::string& input_field_name, uint32_t input_field_enum,
+                                 search::attribute::IAttributeContext& attr_ctx,
+                                 std::shared_ptr<StructFieldMapper> struct_field_mapper)
+{
+    StructFieldsResolver resolver(input_field_name, attr_ctx, false);
+    if (resolver.has_error()) {
+        return std::unique_ptr<IDocsumFieldWriter>();
+    }
+    resolver.apply_to(*struct_field_mapper);
+    return std::make_unique<MatchedElementsFilterDFW>(input_field_name, input_field_enum, std::move(struct_field_mapper));
 }
 
 MatchedElementsFilterDFW::~MatchedElementsFilterDFW() = default;
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.h b/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.h
index b96d3595b0a..6962accc91d 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.h
@@ -4,6 +4,8 @@
 
 #include "docsumfieldwriter.h"
 
+namespace search::attribute { class IAttributeContext; }
+
 namespace search::docsummary {
 
 /**
@@ -17,7 +19,11 @@ private:
     std::shared_ptr<StructFieldMapper> _struct_field_mapper;
 
 public:
-    MatchedElementsFilterDFW(const std::string& input_field_name, uint32_t input_field_enum);
+    MatchedElementsFilterDFW(const std::string& input_field_name, uint32_t input_field_enum,
+                             std::shared_ptr<StructFieldMapper> struct_field_mapper);
+    static std::unique_ptr<IDocsumFieldWriter> create(const std::string& input_field_name, uint32_t input_field_enum,
+                                                      search::attribute::IAttributeContext& attr_ctx,
+                                                      std::shared_ptr<StructFieldMapper> struct_field_mapper);
     ~MatchedElementsFilterDFW();
     bool IsGenerated() const override { return false; }
     void insertField(uint32_t docid, GeneralResult* result, GetDocsumsState *state,
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.cpp b/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.cpp
index 5a005dfe2b9..c29f0324ce2 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.cpp
@@ -1,7 +1,7 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "struct_fields_resolver.h"
-#include <vespa/searchlib/attribute/iattributemanager.h>
+#include <vespa/searchcommon/attribute/iattributecontext.h>
 #include <vespa/searchlib/common/struct_field_mapper.h>
 #include <algorithm>
 
@@ -9,10 +9,12 @@
 LOG_SETUP(".searchsummary.docsummary.struct_fields_resolver");
 
 using search::attribute::CollectionType;
+using search::attribute::IAttributeContext;
 
 namespace search::docsummary {
 
-StructFieldsResolver::StructFieldsResolver(const vespalib::string& field_name, const IAttributeManager& attr_mgr)
+StructFieldsResolver::StructFieldsResolver(const vespalib::string& field_name, const IAttributeContext& attr_ctx,
+                                           bool require_all_struct_fields_as_attribute)
     : _field_name(field_name),
       _map_key_attribute(),
       _map_value_fields(),
@@ -23,8 +25,7 @@ StructFieldsResolver::StructFieldsResolver(const vespalib::string& field_name, c
       _error(false)
 {
     std::vector<const search::attribute::IAttributeVector *> attrs;
-    auto attr_ctx = attr_mgr.createContext();
-    attr_ctx->getAttributeList(attrs);
+    attr_ctx.getAttributeList(attrs);
     vespalib::string prefix = field_name + ".";
     _map_key_attribute = prefix + "key";
     vespalib::string value_prefix = prefix + "value.";
@@ -58,7 +59,7 @@ StructFieldsResolver::StructFieldsResolver(const vespalib::string& field_name, c
             _array_attributes.emplace_back(prefix + field);
         }
 
-        if (!_map_value_fields.empty()) {
+        if (require_all_struct_fields_as_attribute && !_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;
@@ -76,7 +77,9 @@ void
 StructFieldsResolver::apply_to(StructFieldMapper& mapper) const
 {
     if (is_map_of_struct()) {
-        mapper.add_mapping(_field_name, _map_key_attribute);
+        if (_has_map_key) {
+            mapper.add_mapping(_field_name, _map_key_attribute);
+        }
         for (const auto& sub_field : _map_value_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 f6562353cc0..66d9fd69db4 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/struct_fields_resolver.h
@@ -6,7 +6,7 @@
 #include <vector>
 
 namespace search {
-class IAttributeManager;
+namespace attribute { class IAttributeContext; }
 class StructFieldMapper;
 }
 
@@ -29,7 +29,8 @@ private:
     bool _error;
 
 public:
-    StructFieldsResolver(const vespalib::string& field_name, const IAttributeManager& attr_mgr);
+    StructFieldsResolver(const vespalib::string& field_name, const search::attribute::IAttributeContext& attr_ctx,
+                         bool require_all_struct_fields_as_attributes);
     ~StructFieldsResolver();
     bool is_map_of_struct() const { return !_map_value_fields.empty(); }
     const vespalib::string& get_map_key_attribute() const { return _map_key_attribute; }