Unify how errors during setup is handled in DocsumFieldWriterFactory.

1 files changed, 58 insertions, 41 deletions

diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_factory.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_factory.cpp
index b3fa6c68b87..fb00b759574 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_factory.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_factory.cpp
@@ -34,65 +34,83 @@ DocsumFieldWriterFactory::has_attribute_manager() const noexcept
     return getEnvironment().getAttributeManager() != nullptr;
 }
 
+namespace {
+
+void
+throw_if_nullptr(const std::unique_ptr<DocsumFieldWriter>& writer,
+                 const vespalib::string& command)
+{
+    if (writer.get() == nullptr) {
+        throw IllegalArgumentException("Failed to create docsum field writer for command '" + command + "'.");
+    }
+}
+
+void
+throw_missing_source(const vespalib::string& command)
+{
+    throw IllegalArgumentException("Missing source for command '" + command + "'.");
+}
+
+}
+
 std::unique_ptr<DocsumFieldWriter>
-DocsumFieldWriterFactory::create_docsum_field_writer(const vespalib::string& fieldName, const vespalib::string& overrideName, const vespalib::string& argument, bool& rc)
+DocsumFieldWriterFactory::create_docsum_field_writer(const vespalib::string& field_name,
+                                                     const vespalib::string& command,
+                                                     const vespalib::string& source)
 {
-    rc = false;
     std::unique_ptr<DocsumFieldWriter> fieldWriter;
-    if (overrideName == "dynamicteaser") {
-        if ( ! argument.empty() ) {
+    if (command == "dynamicteaser") {
+        if ( ! source.empty() ) {
             auto fw = std::make_unique<DynamicTeaserDFW>(getEnvironment().getJuniper());
             auto fw_ptr = fw.get();
             fieldWriter = std::move(fw);
-            rc = fw_ptr->Init(fieldName.c_str(), argument);
+            if (!fw_ptr->Init(field_name.c_str(), source)) {
+                throw IllegalArgumentException("Failed to initialize DynamicTeaserDFW.");
+            }
         } else {
-            throw IllegalArgumentException("Missing argument");
+            throw_missing_source(command);
         }
-    } else if (overrideName == "summaryfeatures") {
+    } else if (command == "summaryfeatures") {
         fieldWriter = std::make_unique<SummaryFeaturesDFW>();
-        rc = true;
-    } else if (overrideName == "rankfeatures") {
+    } else if (command == "rankfeatures") {
         fieldWriter = std::make_unique<RankFeaturesDFW>();
-        rc = true;
-    } else if (overrideName == "empty") {
+    } else if (command == "empty") {
         fieldWriter = std::make_unique<EmptyDFW>();
-        rc = true;
-    } else if (overrideName == "copy") {
-        if ( ! argument.empty() ) {
-            fieldWriter = std::make_unique<CopyDFW>(argument);
-            rc = true;
+    } else if (command == "copy") {
+        if ( ! source.empty() ) {
+            fieldWriter = std::make_unique<CopyDFW>(source);
         } else {
-            throw IllegalArgumentException("Missing argument");
+            throw_missing_source(command);
         }
-    } else if (overrideName == "absdist") {
+    } else if (command == "absdist") {
         if (has_attribute_manager()) {
-            fieldWriter = AbsDistanceDFW::create(argument.c_str(), getEnvironment().getAttributeManager());
-            rc = static_cast<bool>(fieldWriter);
+            fieldWriter = AbsDistanceDFW::create(source.c_str(), getEnvironment().getAttributeManager());
+            throw_if_nullptr(fieldWriter, command);
         }
-    } else if (overrideName == "positions") {
+    } else if (command == "positions") {
         if (has_attribute_manager()) {
-            fieldWriter = PositionsDFW::create(argument.c_str(), getEnvironment().getAttributeManager(), _use_v8_geo_positions);
-            rc = static_cast<bool>(fieldWriter);
+            fieldWriter = PositionsDFW::create(source.c_str(), getEnvironment().getAttributeManager(), _use_v8_geo_positions);
+            throw_if_nullptr(fieldWriter, command);
         }
-    } else if (overrideName == "geopos") {
+    } else if (command == "geopos") {
         if (has_attribute_manager()) {
-            fieldWriter = GeoPositionDFW::create(argument.c_str(), getEnvironment().getAttributeManager(), _use_v8_geo_positions);
-            rc = static_cast<bool>(fieldWriter);
+            fieldWriter = GeoPositionDFW::create(source.c_str(), getEnvironment().getAttributeManager(), _use_v8_geo_positions);
+            throw_if_nullptr(fieldWriter, command);
         }
-    } else if (overrideName == "attribute") {
+    } else if (command == "attribute") {
         if (has_attribute_manager()) {
-            fieldWriter = AttributeDFWFactory::create(*getEnvironment().getAttributeManager(), argument);
-            rc = true; // Allow missing attribute vector
+            fieldWriter = AttributeDFWFactory::create(*getEnvironment().getAttributeManager(), source);
+            // Missing attribute vector is allowed, so throw_if_nullptr() is NOT used.
         }
-    } else if (overrideName == "attributecombiner") {
+    } else if (command == "attributecombiner") {
         if (has_attribute_manager()) {
             auto attr_ctx = getEnvironment().getAttributeManager()->createContext();
-            const vespalib::string& source_field = argument.empty() ? fieldName : argument;
+            const vespalib::string& source_field = source.empty() ? field_name : source;
             fieldWriter = AttributeCombinerDFW::create(source_field, *attr_ctx, false, std::shared_ptr<MatchingElementsFields>());
-            rc = static_cast<bool>(fieldWriter);
+            throw_if_nullptr(fieldWriter, command);
         }
-    } else if (overrideName == "matchedattributeelementsfilter") {
-        const vespalib::string& source_field = argument.empty() ? fieldName : argument;
+    } else if (command == "matchedattributeelementsfilter") {
+        const vespalib::string& source_field = source.empty() ? field_name : source;
         if (has_attribute_manager()) {
             auto attr_ctx = getEnvironment().getAttributeManager()->createContext();
             if (attr_ctx->getAttribute(source_field) != nullptr) {
@@ -100,20 +118,19 @@ DocsumFieldWriterFactory::create_docsum_field_writer(const vespalib::string& fie
             } else {
                 fieldWriter = AttributeCombinerDFW::create(source_field, *attr_ctx, true, _matching_elems_fields);
             }
-            rc = static_cast<bool>(fieldWriter);
+            throw_if_nullptr(fieldWriter, command);
         }
-    } else if (overrideName == "matchedelementsfilter") {
-        const vespalib::string& source_field = argument.empty() ? fieldName : argument;
+    } else if (command == "matchedelementsfilter") {
+        const vespalib::string& source_field = source.empty() ? field_name : source;
         if (has_attribute_manager()) {
             auto attr_ctx = getEnvironment().getAttributeManager()->createContext();
             fieldWriter = MatchedElementsFilterDFW::create(source_field,*attr_ctx, _matching_elems_fields);
-            rc = static_cast<bool>(fieldWriter);
+            throw_if_nullptr(fieldWriter, command);
         }
-    } else if (overrideName == "documentid") {
+    } else if (command == "documentid") {
         fieldWriter = std::make_unique<DocumentIdDFW>();
-        rc = true;
     } else {
-        throw IllegalArgumentException("unknown override operation '" + overrideName + "' for field '" + fieldName + "'.");
+        throw IllegalArgumentException("Unknown command '" + command + "'.");
     }
     return fieldWriter;
 }