Factor out IStringFieldConverter from IJuniperConverter.

12 files changed, 98 insertions, 86 deletions

diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp
index 17fb15b39cd..72f32ec8a29 100644
--- a/searchcore/src/tests/proton/docsummary/docsummary.cpp
+++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp
@@ -311,12 +311,9 @@ class MockJuniperConverter : public IJuniperConverter
 {
     vespalib::string _result;
 public:
-    void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter&) override {
+    void convert(vespalib::stringref input, vespalib::slime::Inserter&) override {
         _result = input;
     }
-    void insert_juniper_field(const document::StringFieldValue& input, vespalib::slime::Inserter&) override {
-        _result = input.getValueRef();
-    }
     const vespalib::string& get_result() const noexcept { return _result; }
 };
 
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 777e42198f7..4ebdfb10cb7 100644
--- a/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp
+++ b/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp
@@ -36,6 +36,7 @@
 #include <vespa/searchsummary/docsummary/docsum_field_writer.h>
 #include <vespa/searchsummary/docsummary/i_docsum_field_writer_factory.h>
 #include <vespa/searchsummary/docsummary/i_juniper_converter.h>
+#include <vespa/searchsummary/docsummary/i_string_field_converter.h>
 #include <vespa/searchsummary/docsummary/linguisticsannotation.h>
 #include <vespa/searchsummary/docsummary/resultconfig.h>
 #include <vespa/searchsummary/docsummary/slime_filler.h>
@@ -83,6 +84,7 @@ using document::WeightedSetFieldValue;
 using search::docsummary::AnnotationConverter;
 using search::docsummary::IDocsumFieldWriterFactory;
 using search::docsummary::IJuniperConverter;
+using search::docsummary::IStringFieldConverter;
 using search::docsummary::DocsumFieldWriter;
 using search::docsummary::ResultConfig;
 using search::docsummary::SlimeFiller;
@@ -184,15 +186,27 @@ class MockJuniperConverter : public IJuniperConverter
 {
     vespalib::string _result;
 public:
-    void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter&) override {
+    void convert(vespalib::stringref input, vespalib::slime::Inserter&) override {
         _result = input;
     }
-    void insert_juniper_field(const document::StringFieldValue& input, vespalib::slime::Inserter&) override {
-        _result = input.getValueRef();
-    }
     const vespalib::string& get_result() const noexcept { return _result; }
 };
 
+class PassThroughStringFieldConverter : public IStringFieldConverter
+{
+    IJuniperConverter& _juniper_converter;
+public:
+    PassThroughStringFieldConverter(IJuniperConverter& juniper_converter)
+        : IStringFieldConverter(),
+          _juniper_converter(juniper_converter)
+    {
+    }
+    ~PassThroughStringFieldConverter() override = default;
+    void convert(const document::StringFieldValue& input, vespalib::slime::Inserter& inserter) override {
+        _juniper_converter.convert(input.getValueRef(), inserter);
+    }
+};
+
 }
 
 class SlimeFillerTest : public testing::Test
@@ -362,8 +376,9 @@ SlimeFillerTest::expect_insert_callback(const vespalib::string& exp, const Field
     Slime slime;
     SlimeInserter inserter(slime);
     MockJuniperConverter converter;
-    AnnotationConverter stacked_converter(converter);
-    SlimeFiller filler(inserter, tokenize ? (IJuniperConverter*)&stacked_converter : (IJuniperConverter *) &converter);
+    AnnotationConverter annotation_converter(converter);
+    PassThroughStringFieldConverter passthrough_converter(converter);
+    SlimeFiller filler(inserter, tokenize ? (IStringFieldConverter*) &annotation_converter : (IStringFieldConverter*) &passthrough_converter);
     fv.accept(filler);
     auto act_null = slime_to_string(slime);
     EXPECT_EQ("null", act_null);
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.cpp
index 0682f39c602..ec7740b371d 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.cpp
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "annotation_converter.h"
+#include "i_juniper_converter.h"
 #include "linguisticsannotation.h"
 #include <vespa/document/annotation/alternatespanlist.h>
 #include <vespa/document/annotation/annotation.h>
@@ -78,9 +79,9 @@ const StringFieldValue &ensureStringFieldValue(const FieldValue &value) {
 
 }
 
-AnnotationConverter::AnnotationConverter(IJuniperConverter& orig_converter)
-    : IJuniperConverter(),
-      _orig_converter(orig_converter),
+AnnotationConverter::AnnotationConverter(IJuniperConverter& juniper_converter)
+    : IStringFieldConverter(),
+      _juniper_converter(juniper_converter),
       _text(),
       _out()
 {
@@ -166,19 +167,12 @@ AnnotationConverter::handleIndexingTerms(const StringFieldValue& value)
 }
 
 void
-AnnotationConverter::insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter)
-{
-    StringFieldValue value(input);
-    insert_juniper_field(value, inserter);
-}
-
-void
-AnnotationConverter::insert_juniper_field(const StringFieldValue &input, vespalib::slime::Inserter& inserter)
+AnnotationConverter::convert(const StringFieldValue &input, vespalib::slime::Inserter& inserter)
 {
     _out.clear();
     _text = input.getValueRef();
     handleIndexingTerms(input);
-    _orig_converter.insert_juniper_field(_out.str(), inserter);
+    _juniper_converter.convert(_out.str(), inserter);
 }
 
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.h b/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.h
index 67c5a17aeaf..59b03c64540 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.h
@@ -2,7 +2,7 @@
 
 #pragma once
 
-#include "i_juniper_converter.h"
+#include "i_string_field_converter.h"
 #include <vespa/vespalib/stllike/asciistream.h>
 
 namespace document { class Span; }
@@ -11,14 +11,16 @@ namespace vespalib { class asciistream; }
 
 namespace search::docsummary {
 
+class IJuniperConverter;
+
 /*
  * Class converting a string field value with annotations into a string
  * with interlinear annotations used by juniper before passing it to
- * the original juniper converter.
+ * the juniper converter.
  */
-class AnnotationConverter : public IJuniperConverter
+class AnnotationConverter : public IStringFieldConverter
 {
-    IJuniperConverter&     _orig_converter;
+    IJuniperConverter&     _juniper_converter;
     vespalib::stringref    _text;
     vespalib::asciistream  _out;
 
@@ -28,10 +30,9 @@ class AnnotationConverter : public IJuniperConverter
     void annotateSpans(const document::Span& span, ForwardIt it, ForwardIt last);
     void handleIndexingTerms(const document::StringFieldValue& value);
 public:
-    AnnotationConverter(IJuniperConverter& orig_converter);
+    AnnotationConverter(IJuniperConverter& juniper_converter);
     ~AnnotationConverter() override;
-    void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter) override;
-    void insert_juniper_field(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) override;
+    void convert(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) override;
 };
 
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp
index 982a52f8479..0ea5bc9a604 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp
@@ -114,8 +114,7 @@ class JuniperConverter : public IJuniperConverter
 public:
     JuniperConverter(const DynamicTeaserDFW& writer, uint32_t doc_id, GetDocsumsState& state);
     ~JuniperConverter() override;
-    void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter) override;
-    void insert_juniper_field(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) override;
+    void convert(vespalib::stringref input, vespalib::slime::Inserter& inserter) override;
 };
 
 JuniperConverter::JuniperConverter(const DynamicTeaserDFW& writer, uint32_t doc_id, GetDocsumsState& state)
@@ -129,17 +128,11 @@ JuniperConverter::JuniperConverter(const DynamicTeaserDFW& writer, uint32_t doc_
 JuniperConverter::~JuniperConverter() = default;
 
 void
-JuniperConverter::insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter)
+JuniperConverter::convert(vespalib::stringref input, vespalib::slime::Inserter& inserter)
 {
     _writer.insert_juniper_field(_doc_id, input, _state, inserter);
 }
 
-void
-JuniperConverter::insert_juniper_field(const document::StringFieldValue& input, vespalib::slime::Inserter& inserter)
-{
-    _writer.insert_juniper_field(_doc_id, input.getValueRef(), _state, inserter);
-}
-
 }
 
 void
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/i_juniper_converter.h b/searchsummary/src/vespa/searchsummary/docsummary/i_juniper_converter.h
index 00751082567..a52002d37f5 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/i_juniper_converter.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/i_juniper_converter.h
@@ -12,17 +12,12 @@ namespace search::docsummary {
 /**
  * Interface class for inserting a dynamic string based on an
  * annotated full string and query context.
- *
- * For streaming search we use the same interface in an adapter that
- * calls a snippet modifier (vsm::SnippetModifier) to add the annotation
- * needed by juniper.
  */
 class IJuniperConverter
 {
 public:
     virtual ~IJuniperConverter() = default;
-    virtual void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter) = 0;
-    virtual void insert_juniper_field(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) = 0;
+    virtual void convert(vespalib::stringref input, vespalib::slime::Inserter& inserter) = 0;
 };
 
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/i_string_field_converter.h b/searchsummary/src/vespa/searchsummary/docsummary/i_string_field_converter.h
new file mode 100644
index 00000000000..0e80fc28ded
--- /dev/null
+++ b/searchsummary/src/vespa/searchsummary/docsummary/i_string_field_converter.h
@@ -0,0 +1,22 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include <vespa/vespalib/stllike/string.h>
+
+namespace document { class StringFieldValue; }
+namespace vespalib::slime { struct Inserter; }
+
+namespace search::docsummary {
+
+/**
+ * Interface class for inserting a dynamic string.
+ */
+class IStringFieldConverter
+{
+public:
+    virtual ~IStringFieldConverter() = default;
+    virtual void convert(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) = 0;
+};
+
+}
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp
index f78a2c12691..230c03d6644 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp
@@ -1,8 +1,8 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "slime_filler.h"
-#include "annotation_converter.h"
 #include "i_juniper_converter.h"
+#include "i_string_field_converter.h"
 #include "resultconfig.h"
 #include "searchdatatype.h"
 #include <vespa/document/datatype/positiondatatype.h>
@@ -90,21 +90,21 @@ public:
 SlimeFiller::SlimeFiller(Inserter& inserter)
     : _inserter(inserter),
       _matching_elems(nullptr),
-      _juniper_converter(nullptr)
+      _string_converter(nullptr)
 {
 }
 
 SlimeFiller::SlimeFiller(Inserter& inserter, const std::vector<uint32_t>* matching_elems)
     : _inserter(inserter),
       _matching_elems(matching_elems),
-      _juniper_converter(nullptr)
+      _string_converter(nullptr)
 {
 }
 
-SlimeFiller::SlimeFiller(Inserter& inserter, IJuniperConverter* juniper_converter)
+SlimeFiller::SlimeFiller(Inserter& inserter, IStringFieldConverter* string_converter)
     : _inserter(inserter),
       _matching_elems(nullptr),
-      _juniper_converter(juniper_converter)
+      _string_converter(string_converter)
 {
 }
 
@@ -158,7 +158,7 @@ SlimeFiller::visit(const ArrayFieldValue& value)
     }
     Cursor& a = _inserter.insertArray();
     ArrayInserter ai(a);
-    SlimeFiller conv(ai, _juniper_converter);
+    SlimeFiller conv(ai, _string_converter);
     if (filter_matching_elements()) {
         for (uint32_t id_to_keep : (*_matching_elems)) {
             value[id_to_keep].accept(conv);
@@ -173,8 +173,8 @@ SlimeFiller::visit(const ArrayFieldValue& value)
 void
 SlimeFiller::visit(const StringFieldValue& value)
 {
-    if (_juniper_converter != nullptr) {
-        _juniper_converter->insert_juniper_field(value, _inserter);
+    if (_string_converter != nullptr) {
+        _string_converter->convert(value, _inserter);
     } else {
         _inserter.insertString(Memory(value.getValueRef()));
     }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h
index 11b62c95a4d..329dd3c6bb2 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h
@@ -10,7 +10,7 @@ namespace vespalib::slime { struct Inserter; }
 
 namespace search::docsummary {
 
-class IJuniperConverter;
+class IStringFieldConverter;
 
 /*
  * Class inserting a field value into a slime object.
@@ -19,7 +19,7 @@ class SlimeFiller : public document::ConstFieldValueVisitor {
 
     vespalib::slime::Inserter&   _inserter;
     const std::vector<uint32_t>* _matching_elems;
-    IJuniperConverter*           _juniper_converter;
+    IStringFieldConverter*       _string_converter;
 
     bool filter_matching_elements() const {
         return _matching_elems != nullptr;
@@ -51,7 +51,7 @@ 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, IJuniperConverter* juniper_converter);
+    SlimeFiller(vespalib::slime::Inserter& inserter, IStringFieldConverter* string_converter);
     ~SlimeFiller() override;
 };
 
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp
index 9031a0299aa..5c6a87664e5 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp
@@ -32,7 +32,7 @@ SummaryFieldConverter::insert_summary_field_with_filter(const FieldValue& value,
 }
 
 void
-SummaryFieldConverter::insert_juniper_field(const document::FieldValue& value, vespalib::slime::Inserter& inserter, IJuniperConverter& converter)
+SummaryFieldConverter::insert_juniper_field(const document::FieldValue& value, vespalib::slime::Inserter& inserter, IStringFieldConverter& converter)
 {
     CheckUndefinedValueVisitor check_undefined;
     value.accept(check_undefined);
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.h b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.h
index 3524a1c0e02..ce3bf80b365 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.h
@@ -11,7 +11,7 @@ namespace vespalib::slime { struct Inserter; }
 
 namespace search::docsummary {
 
-class IJuniperConverter;
+class IStringFieldConverter;
 
 /**
  * This class converts a summary field for docsum fetching.
@@ -24,7 +24,7 @@ public:
      * Insert the given field value, but only the elements that are contained in the matching_elems vector.
      */
     static void insert_summary_field_with_filter(const document::FieldValue& value, vespalib::slime::Inserter& inserter, const std::vector<uint32_t>& matching_elems);
-    static void insert_juniper_field(const document::FieldValue& value, vespalib::slime::Inserter& inserter, IJuniperConverter& converter);
+    static void insert_juniper_field(const document::FieldValue& value, vespalib::slime::Inserter& inserter, IStringFieldConverter& converter);
 };
 
 }
diff --git a/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp b/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp
index 253ac33c909..ca1de0082f0 100644
--- a/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp
+++ b/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp
@@ -6,6 +6,7 @@
 #include <vespa/searchsummary/docsummary/check_undefined_value_visitor.h>
 #include <vespa/searchsummary/docsummary/i_docsum_store_document.h>
 #include <vespa/searchsummary/docsummary/i_juniper_converter.h>
+#include <vespa/searchsummary/docsummary/i_string_field_converter.h>
 #include <vespa/searchsummary/docsummary/summaryfieldconverter.h>
 #include <vespa/document/base/exceptions.h>
 #include <vespa/document/fieldvalue/iteratorhandler.h>
@@ -30,38 +31,34 @@ bool is_struct_or_multivalue_field_type(const document::DataType& data_type)
  * This class creates a modified field value which is then passed to
  * the original juniper converter.
  */
-class SnippetModifierJuniperConverter : public IJuniperConverter
+class SnippetModifierJuniperConverter : public IStringFieldConverter
 {
-    IJuniperConverter& _orig_converter;
-    FieldModifier&     _modifier;
+    IJuniperConverter& _juniper_converter;
+    FieldModifier*     _modifier;
     FieldPath          _empty_field_path;
 public:
-    SnippetModifierJuniperConverter(IJuniperConverter& orig_converter, FieldModifier& modifier)
-        : IJuniperConverter(),
-          _orig_converter(orig_converter),
+    SnippetModifierJuniperConverter(IJuniperConverter& juniper_converter, FieldModifier* modifier)
+        : IStringFieldConverter(),
+          _juniper_converter(juniper_converter),
           _modifier(modifier),
           _empty_field_path()
     {
     }
     ~SnippetModifierJuniperConverter() override = default;
-    void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter) override;
-    void insert_juniper_field(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) override;
+    void convert(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) override;
 };
 
-
-void
-SnippetModifierJuniperConverter::insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter)
-{
-    _orig_converter.insert_juniper_field(input, inserter);
-}
-
 void
-SnippetModifierJuniperConverter::insert_juniper_field(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter)
+SnippetModifierJuniperConverter::convert(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter)
 {
-    auto fv = _modifier.modify(input, _empty_field_path);
-    assert(fv);
-    auto& modified_input = dynamic_cast<const document::StringFieldValue &>(*fv);
-    _orig_converter.insert_juniper_field(modified_input.getValueRef(), inserter);
+    if (_modifier != nullptr) {
+        auto fv = _modifier->modify(input, _empty_field_path);
+        assert(fv);
+        auto& modified_input = dynamic_cast<const document::StringFieldValue &>(*fv);
+        _juniper_converter.convert(modified_input.getValueRef(), inserter);
+    } else {
+        _juniper_converter.convert(input.getValueRef(), inserter);
+    }
 }
 
 /**
@@ -155,22 +152,20 @@ DocsumStoreVsmDocument::insert_summary_field(const vespalib::string& field_name,
 void
 DocsumStoreVsmDocument::insert_juniper_field(const vespalib::string& field_name, vespalib::slime::Inserter& inserter, IJuniperConverter& converter) const
 {
-    // Markup for juniper has already been added due to FLATTENJUNIPER command in vsm summary config.
     auto field_value = get_field_value(field_name);
     if (field_value) {
+        FieldModifier* modifier = nullptr;
         if (is_struct_or_multivalue_field_type(*field_value->getDataType())) {
             auto entry_idx = _result_class.GetIndexFromName(field_name.c_str());
             if (entry_idx >= 0) {
                 assert((uint32_t) entry_idx < _result_class.GetNumEntries());
-                auto modifier = _docsum_filter.get_field_modifier(entry_idx);
-                if (modifier != nullptr) {
-                    SnippetModifierJuniperConverter stacked_converter(converter, *modifier);
-                    SummaryFieldConverter::insert_juniper_field(*field_value, inserter, stacked_converter);
-                    return;
-                }
+                modifier = _docsum_filter.get_field_modifier(entry_idx);
             }
+        } else {
+            // Markup for juniper has already been added due to FLATTENJUNIPER command in vsm summary config.
         }
-        SummaryFieldConverter::insert_juniper_field(*field_value, inserter, converter);
+        SnippetModifierJuniperConverter string_converter(converter, modifier);
+        SummaryFieldConverter::insert_juniper_field(*field_value, inserter, string_converter);
     }
 }