Revert "Revert "Use commands from summary config to setup document field rewriters.""

11 files changed, 70 insertions, 136 deletions

diff --git a/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt b/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt
index 963c94f7796..8070bed8b03 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt
+++ b/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt
@@ -7,7 +7,6 @@ vespa_add_library(searchsummary_docsummary OBJECT
     attributedfw.cpp
     check_undefined_value_visitor.cpp
     copy_dfw.cpp
-    docsumconfig.cpp
     docsum_field_writer.cpp
     docsum_field_writer_factory.cpp
     docsum_store_document.cpp
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp
deleted file mode 100644
index 0c2adcbfaa5..00000000000
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include "docsumconfig.h"
-#include "docsum_field_writer_factory.h"
-#include "docsumwriter.h"
-#include <vespa/vespalib/util/exceptions.h>
-
-namespace search::docsummary {
-
-using vespalib::IllegalArgumentException;
-using vespalib::make_string;
-
-const ResultConfig &
-DynamicDocsumConfig::getResultConfig() const {
-    return *_writer->GetResultConfig();
-}
-
-std::unique_ptr<IDocsumFieldWriterFactory>
-DynamicDocsumConfig::make_docsum_field_writer_factory()
-{
-    return std::make_unique<DocsumFieldWriterFactory>(getResultConfig().useV8geoPositions(), getEnvironment());
-}
-
-void
-DynamicDocsumConfig::configure(const vespa::config::search::SummarymapConfig &cfg)
-{
-    std::vector<string> strCfg;
-    auto docsum_field_writer_factory = make_docsum_field_writer_factory();
-    for (const auto & o : cfg.override) {
-        bool rc(false);
-        auto fieldWriter = docsum_field_writer_factory->create_docsum_field_writer(o.field, o.command, o.arguments, rc);
-        if (rc && fieldWriter) {
-            rc = _writer->Override(o.field.c_str(), std::move(fieldWriter)); // OBJECT HAND-OVER
-        }
-        if (!rc) {
-            throw IllegalArgumentException(o.command + " override operation failed during initialization");
-        }
-    }
-}
-
-}
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.h
deleted file mode 100644
index 2b0f90a8e8b..00000000000
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.h
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <vespa/config-summarymap.h>
-
-namespace search { class MatchingElementsFields; }
-namespace search::docsummary {
-
-class IDocsumEnvironment;
-class IDocsumFieldWriterFactory;
-class DocsumFieldWriter;
-class DynamicDocsumWriter;
-class ResultConfig;
-
-class DynamicDocsumConfig
-{
-public:
-    DynamicDocsumConfig(const IDocsumEnvironment& env, DynamicDocsumWriter * writer) :
-        _env(env),
-        _writer(writer)
-    { }
-    virtual ~DynamicDocsumConfig() = default;
-    void configure(const vespa::config::search::SummarymapConfig &cfg);
-protected:
-    using string = vespalib::string;
-    const IDocsumEnvironment& getEnvironment() { return _env; }
-    const ResultConfig & getResultConfig() const;
-
-    virtual std::unique_ptr<IDocsumFieldWriterFactory> make_docsum_field_writer_factory();
-private:
-    const IDocsumEnvironment& _env;
-    DynamicDocsumWriter * _writer;
-};
-
-}
-
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp
index 89de20b6a81..181af0ca9a3 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp
@@ -60,7 +60,7 @@ DynamicDocsumWriter::insertDocsum(const ResolveClassInfo & rci, uint32_t docid,
         vespalib::slime::Cursor & docsum = topInserter.insertObject();
         for (uint32_t i = 0; i < rci.outputClass->GetNumEntries(); ++i) {
             const ResConfigEntry *resCfg = rci.outputClass->GetEntry(i);
-            const DocsumFieldWriter *writer = _overrideTable[resCfg->_enumValue].get();
+            const DocsumFieldWriter *writer = resCfg->_docsum_field_writer.get();
             if (state->_args.needField(resCfg->_bindname) && ! writer->isDefaultValue(docid, state)) {
                 const Memory field_name(resCfg->_bindname.data(), resCfg->_bindname.size());
                 ObjectInserter inserter(docsum, field_name);
@@ -78,7 +78,7 @@ DynamicDocsumWriter::insertDocsum(const ResolveClassInfo & rci, uint32_t docid,
         for (uint32_t i = 0; i < rci.outputClass->GetNumEntries(); ++i) {
             const ResConfigEntry *outCfg = rci.outputClass->GetEntry(i);
             if ( ! state->_args.needField(outCfg->_bindname)) continue;
-            const DocsumFieldWriter *writer = _overrideTable[outCfg->_enumValue].get();
+            const DocsumFieldWriter *writer = outCfg->_docsum_field_writer.get();
             const Memory field_name(outCfg->_bindname.data(), outCfg->_bindname.size());
             ObjectInserter inserter(docsum, field_name);
             if (writer != nullptr) {
@@ -96,60 +96,31 @@ DynamicDocsumWriter::insertDocsum(const ResolveClassInfo & rci, uint32_t docid,
 
 DynamicDocsumWriter::DynamicDocsumWriter(std::unique_ptr<ResultConfig> config, std::unique_ptr<KeywordExtractor> extractor)
     : _resultConfig(std::move(config)),
-      _keywordExtractor(std::move(extractor)),
-      _numFieldWriterStates(0),
-      _overrideTable(_resultConfig->GetFieldNameEnum().GetNumEntries())
+      _keywordExtractor(std::move(extractor))
 {
 }
 
 
 DynamicDocsumWriter::~DynamicDocsumWriter() = default;
 
-bool
-DynamicDocsumWriter::Override(const char *fieldName, std::unique_ptr<DocsumFieldWriter> writer)
-{
-    uint32_t fieldEnumValue = _resultConfig->GetFieldNameEnum().Lookup(fieldName);
-
-    if (fieldEnumValue >= _overrideTable.size() || _overrideTable[fieldEnumValue]) {
-        if (fieldEnumValue >= _overrideTable.size()) {
-            LOG(warning, "cannot override docsum field '%s'; undefined field name", fieldName);
-        } else if (_overrideTable[fieldEnumValue] != nullptr) {
-            LOG(warning, "cannot override docsum field '%s'; already overridden", fieldName);
-        }
-        return false;
-    }
-
-    writer->setIndex(fieldEnumValue);
-    auto writer_ptr = writer.get();
-    _overrideTable[fieldEnumValue] = std::move(writer);
-    if (writer_ptr->setFieldWriterStateIndex(_numFieldWriterStates)) {
-        ++_numFieldWriterStates;
-    }
-
-    bool generated = writer_ptr->IsGenerated();
-    for (auto & result_class : *_resultConfig) {
-        if (result_class.GetIndexFromEnumValue(fieldEnumValue) >= 0) {
-            result_class.getDynamicInfo().update_override_counts(generated);
-        }
-    }
-
-    return true;
-}
-
-
 void
-DynamicDocsumWriter::InitState(const IAttributeManager & attrMan, GetDocsumsState *state)
+DynamicDocsumWriter::InitState(const IAttributeManager & attrMan, GetDocsumsState& state, const ResolveClassInfo& rci)
 {
-    state->_kwExtractor = _keywordExtractor.get();
-    state->_attrCtx = attrMan.createContext();
-    state->_attributes.resize(_overrideTable.size());
-    state->_fieldWriterStates.resize(_numFieldWriterStates);
-    for (size_t i(0); i < state->_attributes.size(); i++) {
-        const DocsumFieldWriter *fw = _overrideTable[i].get();
+    state._kwExtractor = _keywordExtractor.get();
+    state._attrCtx = attrMan.createContext();
+    auto result_class = rci.outputClass;
+    if (result_class == nullptr) {
+        return;
+    }
+    size_t num_entries = result_class->GetNumEntries();
+    state._attributes.resize(num_entries);
+    state._fieldWriterStates.resize(result_class->get_num_field_writer_states());
+    for (size_t i(0); i < num_entries; i++) {
+        const DocsumFieldWriter *fw = result_class->GetEntry(i)->_docsum_field_writer.get();
         if (fw) {
             const vespalib::string & attributeName = fw->getAttributeName();
             if (!attributeName.empty()) {
-                state->_attributes[i] = state->_attrCtx->getAttribute(attributeName);
+                state._attributes[i] = state._attrCtx->getAttribute(attributeName);
             }
         }
     }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.h
index 9ae44623475..c0579638593 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.h
@@ -36,7 +36,7 @@ public:
     };
 
     virtual ~IDocsumWriter() = default;
-    virtual void InitState(const search::IAttributeManager & attrMan, GetDocsumsState *state) = 0;
+    virtual void InitState(const search::IAttributeManager & attrMan, GetDocsumsState& state, const ResolveClassInfo& rci) = 0;
     virtual void insertDocsum(const ResolveClassInfo & rci, uint32_t docid, GetDocsumsState *state,
                               IDocsumStore *docinfos, Inserter & target) = 0;
     virtual ResolveClassInfo resolveClassInfo(vespalib::stringref outputClassName) const = 0;
@@ -49,8 +49,6 @@ class DynamicDocsumWriter : public IDocsumWriter
 private:
     std::unique_ptr<ResultConfig>                         _resultConfig;
     std::unique_ptr<KeywordExtractor>                     _keywordExtractor;
-    uint32_t                                              _numFieldWriterStates;
-    std::vector<std::unique_ptr<const DocsumFieldWriter>> _overrideTable;
 
     ResolveClassInfo resolveOutputClass(vespalib::stringref outputClassName) const;
 
@@ -62,8 +60,7 @@ public:
 
     const ResultConfig *GetResultConfig() { return _resultConfig.get(); }
 
-    bool Override(const char *fieldName, std::unique_ptr<DocsumFieldWriter> writer);
-    void InitState(const search::IAttributeManager & attrMan, GetDocsumsState *state) override;
+    void InitState(const search::IAttributeManager & attrMan, GetDocsumsState& state, const ResolveClassInfo& rci) override;
     void insertDocsum(const ResolveClassInfo & outputClassInfo, uint32_t docid, GetDocsumsState *state,
                       IDocsumStore *docinfos, Inserter & inserter) override;
 
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/res_config_entry.cpp b/searchsummary/src/vespa/searchsummary/docsummary/res_config_entry.cpp
index 668fb9b519b..b00b5d975a2 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/res_config_entry.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/res_config_entry.cpp
@@ -1,16 +1,20 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "res_config_entry.h"
+#include "docsum_field_writer.h"
 
 namespace search::docsummary {
 
 ResConfigEntry::ResConfigEntry() noexcept
     : _type(RES_BAD),
       _bindname(),
-      _enumValue(0)
+      _enumValue(0),
+      _docsum_field_writer()
 {
 }
 
 ResConfigEntry::~ResConfigEntry() = default;
 
+ResConfigEntry::ResConfigEntry(ResConfigEntry&&) noexcept = default;
+
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/res_config_entry.h b/searchsummary/src/vespa/searchsummary/docsummary/res_config_entry.h
index 771125b1f45..6277e955c3d 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/res_config_entry.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/res_config_entry.h
@@ -4,9 +4,12 @@
 
 #include "res_type.h"
 #include <vespa/vespalib/stllike/string.h>
+#include <memory>
 
 namespace search::docsummary {
 
+class DocsumFieldWriter;
+
 /**
  * This struct describes a single docsum field (name and type).
  **/
@@ -14,8 +17,10 @@ struct ResConfigEntry {
     ResType          _type;
     vespalib::string _bindname;
     int              _enumValue;
+    std::unique_ptr<DocsumFieldWriter> _docsum_field_writer;
     ResConfigEntry() noexcept;
     ~ResConfigEntry();
+    ResConfigEntry(ResConfigEntry&&) noexcept;
 };
 
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp
index 9930c39cb7b..78a9804ccd4 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "resultclass.h"
+#include "docsum_field_writer.h"
 #include "resultconfig.h"
 #include <vespa/vespalib/stllike/hashtable.hpp>
 #include <cassert>
@@ -14,7 +15,8 @@ ResultClass::ResultClass(const char *name, util::StringEnum & fieldEnum)
       _fieldEnum(fieldEnum),
       _enumMap(),
       _dynInfo(),
-      _omit_summary_features(false)
+      _omit_summary_features(false),
+      _num_field_writer_states(0)
 { }
 
 
@@ -28,7 +30,7 @@ ResultClass::GetIndexFromName(const char* name) const
 }
 
 bool
-ResultClass::AddConfigEntry(const char *name, ResType type)
+ResultClass::AddConfigEntry(const char *name, ResType type, std::unique_ptr<DocsumFieldWriter> docsum_field_writer)
 {
     if (_nameMap.find(name) != _nameMap.end())
         return false;
@@ -39,10 +41,25 @@ ResultClass::AddConfigEntry(const char *name, ResType type)
     e._bindname  = name;
     e._enumValue = _fieldEnum.Add(name);
     assert(e._enumValue >= 0);
-    _entries.push_back(e);
+    if (docsum_field_writer) {
+        docsum_field_writer->setIndex(_entries.size());
+        bool generated = docsum_field_writer->IsGenerated();
+        getDynamicInfo().update_override_counts(generated);
+        if (docsum_field_writer->setFieldWriterStateIndex(_num_field_writer_states)) {
+            ++_num_field_writer_states;
+        }
+    }
+    e._docsum_field_writer = std::move(docsum_field_writer);
+    _entries.push_back(std::move(e));
     return true;
 }
 
+bool
+ResultClass::AddConfigEntry(const char *name, ResType type)
+{
+    return AddConfigEntry(name, type, {});
+}
+
 void
 ResultClass::CreateEnumMap()
 {
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.h b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.h
index 39e94e5050e..e11032d1aac 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.h
@@ -50,6 +50,7 @@ private:
     // Whether or not summary features should be omitted when filling this summary class.
     // As default, summary features are always included.
     bool                       _omit_summary_features;
+    size_t                     _num_field_writer_states;
 
 public:
     typedef std::unique_ptr<ResultClass> UP;
@@ -95,10 +96,13 @@ public:
      * @return true(success)/false(fail)
      * @param name the name of the field to add.
      * @param type the type of the field to add.
+     * @param docsum_field_writer field writer for writing field
      **/
+    bool AddConfigEntry(const char *name, ResType type, std::unique_ptr<DocsumFieldWriter> docsum_field_writer);
     bool AddConfigEntry(const char *name, ResType type);
 
 
+
     /**
      * This method may be called to create an internal mapping from
      * field name enumerated value to field index. When building up a
@@ -169,6 +173,8 @@ public:
     bool omit_summary_features() const {
         return _omit_summary_features;
     }
+
+    size_t get_num_field_writer_states() const noexcept { return _num_field_writer_states; }
 };
 
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.cpp b/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.cpp
index f5e4c5d34cf..63ccf80d4e7 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.cpp
@@ -1,6 +1,8 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "resultconfig.h"
+#include "docsum_field_writer.h"
+#include "docsum_field_writer_factory.h"
 #include "resultclass.h"
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/stllike/hash_map.hpp>
@@ -99,7 +101,7 @@ bool ResultConfig::wantedV8geoPositions() {
 }
 
 bool
-ResultConfig::ReadConfig(const vespa::config::search::SummaryConfig &cfg, const char *configId)
+ResultConfig::ReadConfig(const vespa::config::search::SummaryConfig &cfg, const char *configId, IDocsumFieldWriterFactory& docsum_field_writer_factory)
 {
     bool rc = true;
     Reset();
@@ -129,10 +131,20 @@ ResultConfig::ReadConfig(const vespa::config::search::SummaryConfig &cfg, const
         for (unsigned int j = 0; rc && (j < cfg_class.fields.size()); j++) {
             const char *fieldtype = cfg_class.fields[j].type.c_str();
             const char *fieldname = cfg_class.fields[j].name.c_str();
+            vespalib::string override_name = cfg_class.fields[j].command;
+            vespalib::string source_name = cfg_class.fields[j].source;
             auto res_type = ResTypeUtils::get_res_type(fieldtype);
             LOG(debug, "Reconfiguring class '%s' field '%s' of type '%s'", cfg_class.name.c_str(), fieldname, fieldtype);
             if (res_type != RES_BAD) {
-                rc = resClass->AddConfigEntry(fieldname, res_type);
+                std::unique_ptr<DocsumFieldWriter> docsum_field_writer;
+                if (!override_name.empty()) {
+                    docsum_field_writer = docsum_field_writer_factory.create_docsum_field_writer(fieldname, override_name, source_name, rc);
+                    if (!rc) {
+                        LOG(error, "%s override operation failed during initialization", override_name.c_str());
+                        break;
+                    }
+                }
+                rc = resClass->AddConfigEntry(fieldname, res_type, std::move(docsum_field_writer));
             } else {
                 LOG(error, "%s %s.fields[%d]: unknown type '%s'", configId, cfg_class.name.c_str(), j, fieldtype);
                 rc = false;
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.h b/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.h
index 220c988b634..9aaf6fa65b3 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.h
@@ -8,6 +8,7 @@
 
 namespace search::docsummary {
 
+class IDocsumFieldWriterFactory;
 class ResultClass;
 
 /**
@@ -176,7 +177,7 @@ public:
      * @return true(success)/false(fail)
      * @param configId reference on server
      **/
-    bool ReadConfig(const vespa::config::search::SummaryConfig &cfg, const char *configId);
+    bool ReadConfig(const vespa::config::search::SummaryConfig &cfg, const char *configId, IDocsumFieldWriterFactory& docsum_field_writer_factory);
 };
 
 }