Simplify API of SlimeFillerFilter to make it easier to use.

5 files changed, 89 insertions, 34 deletions

diff --git a/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp b/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp
index fb902eda080..20d3af65458 100644
--- a/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp
+++ b/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp
@@ -306,7 +306,7 @@ SlimeFillerTest::expect_insert(const vespalib::string& exp, const FieldValue& fv
 {
     Slime slime;
     SlimeInserter inserter(slime);
-    SlimeFiller filler(inserter, nullptr, &filter);
+    SlimeFiller filler(inserter, nullptr, filter.begin());
     fv.accept(filler);
     auto act = slime_to_string(slime);
     EXPECT_EQ(exp, act);
@@ -318,7 +318,7 @@ SlimeFillerTest::expect_insert_callback(const std::vector<vespalib::string>& exp
     Slime slime;
     SlimeInserter inserter(slime);
     MockStringFieldConverter converter;
-    SlimeFiller filler(inserter, &converter, nullptr);
+    SlimeFiller filler(inserter, &converter, SlimeFillerFilter::all());
     fv.accept(filler);
     auto act_null = slime_to_string(slime);
     EXPECT_EQ("null", act_null);
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp
index 1baa6bca189..87cc5f4fe6a 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp
@@ -67,14 +67,14 @@ private:
     Cursor& _array;
     Symbol _key_sym;
     Symbol _val_sym;
-    std::optional<const SlimeFillerFilter*> _filter;
+    SlimeFillerFilter::Iterator _filter;
 
 public:
-    MapFieldValueInserter(Inserter& parent_inserter, std::optional<const SlimeFillerFilter*> filter)
+    MapFieldValueInserter(Inserter& parent_inserter, SlimeFillerFilter::Iterator filter)
         : _array(parent_inserter.insertArray()),
           _key_sym(_array.resolve("key")),
           _val_sym(_array.resolve("value")),
-          _filter(std::move(filter))
+          _filter(filter)
     {
     }
     void insert_entry(const FieldValue& key, const FieldValue& value) {
@@ -83,9 +83,9 @@ public:
         SlimeFiller key_conv(ki);
 
         key.accept(key_conv);
-        if (_filter.has_value()) {
+        if (_filter.should_render()) {
             ObjectSymbolInserter vi(c, _val_sym);
-            SlimeFiller val_conv(vi, nullptr, _filter.value());
+            SlimeFiller val_conv(vi, nullptr, _filter);
             value.accept(val_conv);
         }
     }
@@ -97,7 +97,7 @@ SlimeFiller::SlimeFiller(Inserter& inserter)
     : _inserter(inserter),
       _matching_elems(nullptr),
       _string_converter(nullptr),
-      _filter(nullptr)
+      _filter(SlimeFillerFilter::all())
 {
 }
 
@@ -105,11 +105,11 @@ SlimeFiller::SlimeFiller(Inserter& inserter, const std::vector<uint32_t>* matchi
     : _inserter(inserter),
       _matching_elems(matching_elems),
       _string_converter(nullptr),
-      _filter(nullptr)
+      _filter(SlimeFillerFilter::all())
 {
 }
 
-SlimeFiller::SlimeFiller(Inserter& inserter, IStringFieldConverter* string_converter, const SlimeFillerFilter* filter)
+SlimeFiller::SlimeFiller(Inserter& inserter, IStringFieldConverter* string_converter, SlimeFillerFilter::Iterator filter)
     : _inserter(inserter),
       _matching_elems(nullptr),
       _string_converter(string_converter),
@@ -145,7 +145,7 @@ SlimeFiller::visit(const MapFieldValue& v)
     if (empty_or_empty_after_filtering(v)) {
         return;
     }
-    MapFieldValueInserter map_inserter(_inserter, SlimeFillerFilter::get_filter(_filter, "value"));
+    MapFieldValueInserter map_inserter(_inserter, _filter.check_field("value"));
     if (filter_matching_elements()) {
         assert(v.has_no_erased_keys());
         for (uint32_t id_to_keep : (*_matching_elems)) {
@@ -276,11 +276,11 @@ SlimeFiller::visit(const StructFieldValue& value)
     Cursor& c = _inserter.insertObject();
     for (StructFieldValue::const_iterator itr = value.begin(); itr != value.end(); ++itr) {
         auto& name = itr.field().getName();
-        auto sub_filter = SlimeFillerFilter::get_filter(_filter, name);
-        if (sub_filter.has_value()) {
+        auto sub_filter = _filter.check_field(name);
+        if (sub_filter.should_render()) {
             Memory keymem(name);
             ObjectInserter vi(c, keymem);
-            SlimeFiller conv(vi, nullptr, sub_filter.value());
+            SlimeFiller conv(vi, nullptr, sub_filter);
             FieldValue::UP nextValue(value.getValue(itr.field()));
             (*nextValue).accept(conv);
         }
@@ -370,7 +370,7 @@ SlimeFiller::insert_summary_field_with_field_filter(const document::FieldValue&
     CheckUndefinedValueVisitor check_undefined;
     value.accept(check_undefined);
     if (!check_undefined.is_undefined()) {
-        SlimeFiller visitor(inserter, nullptr, filter);
+        SlimeFiller visitor(inserter, nullptr, (filter != nullptr) ? filter->begin() : SlimeFillerFilter::all());
         value.accept(visitor);
     }
 }
@@ -381,7 +381,7 @@ SlimeFiller::insert_juniper_field(const document::FieldValue& value, vespalib::s
     CheckUndefinedValueVisitor check_undefined;
     value.accept(check_undefined);
     if (!check_undefined.is_undefined()) {
-        SlimeFiller visitor(inserter, &converter, nullptr);
+        SlimeFiller visitor(inserter, &converter, SlimeFillerFilter::all());
         value.accept(visitor);
     }
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h
index 620610d782d..f28d628349d 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h
@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include "slime_filler_filter.h"
 #include <vespa/document/fieldvalue/fieldvaluevisitor.h>
 #include <cstdint>
 #include <vector>
@@ -13,7 +14,6 @@ namespace vespalib::slime { struct Inserter; }
 namespace search::docsummary {
 
 class IStringFieldConverter;
-class SlimeFillerFilter;
 
 /*
  * Class inserting a field value into a slime object.
@@ -23,7 +23,7 @@ class SlimeFiller : public document::ConstFieldValueVisitor {
     vespalib::slime::Inserter&   _inserter;
     const std::vector<uint32_t>* _matching_elems;
     IStringFieldConverter*       _string_converter;
-    const SlimeFillerFilter*     _filter;
+    SlimeFillerFilter::Iterator  _filter;
 
     bool filter_matching_elements() const {
         return _matching_elems != nullptr;
@@ -55,7 +55,8 @@ class SlimeFiller : public document::ConstFieldValueVisitor {
 public:
     SlimeFiller(vespalib::slime::Inserter& inserter);
     SlimeFiller(vespalib::slime::Inserter& inserter, const std::vector<uint32_t>* matching_elems);
-    SlimeFiller(vespalib::slime::Inserter& inserter, IStringFieldConverter* string_converter, const SlimeFillerFilter* filter);
+    SlimeFiller(vespalib::slime::Inserter& inserter, IStringFieldConverter* string_converter,
+                SlimeFillerFilter::Iterator filter);
     ~SlimeFiller() override;
 
     static void insert_summary_field(const document::FieldValue& value, vespalib::slime::Inserter& inserter);
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.cpp
index 80e14188f5f..8981af0362f 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.cpp
@@ -6,6 +6,25 @@
 
 namespace search::docsummary {
 
+SlimeFillerFilter::Iterator::Iterator(bool should_render_in) noexcept
+    : _should_render(should_render_in),
+      _next()
+{
+}
+
+SlimeFillerFilter::Iterator::Iterator(const SlimeFillerFilter* next) noexcept
+    : _should_render(true),
+      _next(next)
+{
+}
+
+SlimeFillerFilter::Iterator
+SlimeFillerFilter::Iterator::check_field(vespalib::stringref field_name) const
+{
+    assert(_should_render);
+    return (_next != nullptr) ? _next->check_field(field_name) : SlimeFillerFilter::Iterator(true);
+}
+
 SlimeFillerFilter::SlimeFillerFilter()
     : _filter()
 {
@@ -13,20 +32,23 @@ SlimeFillerFilter::SlimeFillerFilter()
 
 SlimeFillerFilter::~SlimeFillerFilter() = default;
 
-std::optional<const SlimeFillerFilter*>
-SlimeFillerFilter::get_filter(vespalib::stringref field_name) const
+SlimeFillerFilter::Iterator
+SlimeFillerFilter::check_field(vespalib::stringref field_name) const
 {
     auto itr = _filter.find(field_name);
     if (itr == _filter.end()) {
-        return std::nullopt;
+        // This field does not pass the filter -> should NOT be rendered.
+        return SlimeFillerFilter::Iterator(false);
     }
-    return itr->second.get();
+    // This field passes the filter -> should be rendered.
+    // We also keep track of the next filter in the hierarchy.
+    return SlimeFillerFilter::Iterator(itr->second.get());
 }
 
-std::optional<const SlimeFillerFilter*>
-SlimeFillerFilter::get_filter(const SlimeFillerFilter* filter, vespalib::stringref field_name)
+SlimeFillerFilter::Iterator
+SlimeFillerFilter::begin() const
 {
-    return (filter != nullptr) ? filter->get_filter(field_name) : nullptr;
+    return SlimeFillerFilter::Iterator(this);
 }
 
 bool
@@ -82,4 +104,10 @@ SlimeFillerFilter::add_remaining(std::unique_ptr<SlimeFillerFilter>& filter, ves
     }
 }
 
+SlimeFillerFilter::Iterator
+SlimeFillerFilter::all()
+{
+    return SlimeFillerFilter::Iterator(true);
+}
+
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.h b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.h
index a9f441dc505..8bdc683fbb8 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.h
@@ -8,28 +8,54 @@
 
 namespace search::docsummary {
 
-/*
+/**
  * Class filtering which fields to render in a struct field.
  */
 class SlimeFillerFilter {
+public:
+    /**
+     * Iterator used to step through the sub fields of a struct field
+     * to find out which parts to render.
+     */
+    class Iterator {
+    private:
+        friend class SlimeFillerFilter;
+        bool _should_render;
+        const SlimeFillerFilter* _next;
+        explicit Iterator(bool should_render_in) noexcept;
+        explicit Iterator(const SlimeFillerFilter* next) noexcept;
+    public:
+        Iterator check_field(vespalib::stringref field_name) const;
+        bool should_render() const noexcept { return _should_render; }
+    };
+
+private:
     vespalib::hash_map<vespalib::string, std::unique_ptr<SlimeFillerFilter>> _filter;
-    std::optional<const SlimeFillerFilter*> get_filter(vespalib::stringref field_name) const;
+    Iterator check_field(vespalib::stringref field_name) const;
+
 public:
     SlimeFillerFilter();
     ~SlimeFillerFilter();
-    /*
-     * If field is blocked by the filter then the return value is not set,
-     * otherwise it is set to the filter for the next level.
-     */
-    static std::optional<const SlimeFillerFilter*> get_filter(const SlimeFillerFilter* filter, vespalib::stringref field_name);
+
+    Iterator begin() const;
+
     bool empty() const;
+
+    /**
+     * Add a field path (e.g 'my_field.my_subfield') that should be rendered.
+     */
     SlimeFillerFilter& add(vespalib::stringref field_path);
 
-    /*
+    /**
      * Called by DocsumFilter::prepareFieldSpec() with each input field name as field_path. First component
      * is assumed to be the same as the output field name.
      */
     static void add_remaining(std::unique_ptr<SlimeFillerFilter>& filter, vespalib::stringref field_path);
+
+    /**
+     * Returns a pass-through filter iterator that renders all parts of a struct field.
+     */
+    static Iterator all();
 };
 
 }