more useful DocsumReply

10 files changed, 52 insertions, 24 deletions

diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp
index 1584cf4cd02..fca5e4dfbda 100644
--- a/searchcore/src/tests/proton/docsummary/docsummary.cpp
+++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp
@@ -344,7 +344,7 @@ bool assertSlime(const std::string &exp, const DocsumReply &reply) {
     vespalib::Slime expSlime;
     size_t used = JsonFormat::decode(exp, expSlime);
     EXPECT_TRUE(used > 0);
-    ASSERT_TRUE(reply.hasSlime());
+    ASSERT_TRUE(reply.hasResults());
     return (EXPECT_EQUAL(expSlime, reply.slime()));
 }
 
diff --git a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
index ca4f2329c19..b59aaf33908 100644
--- a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
+++ b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
@@ -135,7 +135,7 @@ void assertSlime(const std::string &exp, const DocsumReply &reply) {
     vespalib::Slime expSlime;
     size_t used = JsonFormat::decode(exp, expSlime);
     EXPECT_TRUE(used > 0);
-    ASSERT_TRUE(reply.hasSlime());
+    ASSERT_TRUE(reply.hasResults());
     EXPECT_EQUAL(expSlime, reply.slime());
 }
 
@@ -160,7 +160,7 @@ TEST("requireThatGetDocsumsExecute") {
         DocsumRequest::Source request(createRequest());
         DocsumReply::UP reply = engine.getDocsums(std::move(request), client);
         EXPECT_TRUE(reply);
-        EXPECT_FALSE(reply->hasSlime());
+        EXPECT_FALSE(reply->hasResults());
     }
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/summaryengine/docsum_by_slime.cpp b/searchcore/src/vespa/searchcore/proton/summaryengine/docsum_by_slime.cpp
index 3c8e5715a28..1860f3ab989 100644
--- a/searchcore/src/vespa/searchcore/proton/summaryengine/docsum_by_slime.cpp
+++ b/searchcore/src/vespa/searchcore/proton/summaryengine/docsum_by_slime.cpp
@@ -93,7 +93,7 @@ vespalib::Slime::UP
 DocsumBySlime::getDocsums(const Inspector & req)
 {
     DocsumReply::UP reply = _docsumServer.getDocsums(slimeToRequest(req));
-    if (reply && reply->hasSlime()) {
+    if (reply && reply->hasResults()) {
         return reply->releaseSlime();
     } else {
         LOG(warning, "got <null> docsum reply from back-end");
diff --git a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
index 0e01ee68cea..8a4ac98643d 100644
--- a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
@@ -145,10 +145,7 @@ SummaryEngine::getDocsums(DocsumRequest::UP req)
         }
         updateDocsumMetrics(vespalib::to_s(req->getTimeUsed()), getNumDocs(*reply));
     }
-    reply->setRequest(std::move(req));
-    reply->setIssues(std::move(my_issues));
-
-    return reply;
+    return std::make_unique<DocsumReply>(reply->releaseSlime(), std::move(req), std::move(my_issues));
 }
 
 void
diff --git a/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp b/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp
index 7eb7d1d0d80..23b95f489c6 100644
--- a/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp
+++ b/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp
@@ -501,7 +501,6 @@ TEST_F(DocsumReplyTest, require_that_missing_root_slime_gives_empty_payload) {
 }
 
 TEST_F(DocsumReplyTest, require_that_issues_are_converted_to_errors) {
-    reply.setIssues(std::make_unique<UniqueIssues>());
     reply.issues().handle(vespalib::Issue("a"));
     reply.issues().handle(vespalib::Issue("b"));
     reply.issues().handle(vespalib::Issue("c"));
diff --git a/searchlib/src/tests/engine/proto_rpc_adapter/proto_rpc_adapter_test.cpp b/searchlib/src/tests/engine/proto_rpc_adapter/proto_rpc_adapter_test.cpp
index 7aa72b50ba4..33a40b76a51 100644
--- a/searchlib/src/tests/engine/proto_rpc_adapter/proto_rpc_adapter_test.cpp
+++ b/searchlib/src/tests/engine/proto_rpc_adapter/proto_rpc_adapter_test.cpp
@@ -49,11 +49,11 @@ struct MyDocsumServer : DocsumServer {
     DocsumReply::UP getDocsums(DocsumRequest::Source src, DocsumClient &client) override {
         auto req = src.release();
         assert(req);
-        auto reply = std::make_unique<DocsumReply>(std::make_unique<Slime>());
-        auto &list = reply->slime().setArray();
+        auto slime = std::make_unique<Slime>();
+        auto &list = slime->setArray();
         list.addObject().setBool("use_root_slime", true);
         list.addObject().setString("ranking", req->ranking);
-        reply->setRequest(std::move(req));
+        auto reply = std::make_unique<DocsumReply>(std::move(slime), std::move(req));
         std::this_thread::sleep_for(std::chrono::milliseconds(5));
         client.getDocsumsDone(std::move(reply)); // simplified async response
         return std::unique_ptr<DocsumReply>();
diff --git a/searchlib/src/vespa/searchlib/common/unique_issues.h b/searchlib/src/vespa/searchlib/common/unique_issues.h
index 5b25820c408..79cb7a9aa23 100644
--- a/searchlib/src/vespa/searchlib/common/unique_issues.h
+++ b/searchlib/src/vespa/searchlib/common/unique_issues.h
@@ -23,6 +23,7 @@ public:
             fun(msg);
         }
     }
+    size_t size() const { return _messages.size(); }
 };
 
 }
diff --git a/searchlib/src/vespa/searchlib/engine/docsumreply.cpp b/searchlib/src/vespa/searchlib/engine/docsumreply.cpp
index e8baad19a47..ba738a51183 100644
--- a/searchlib/src/vespa/searchlib/engine/docsumreply.cpp
+++ b/searchlib/src/vespa/searchlib/engine/docsumreply.cpp
@@ -4,17 +4,38 @@
 #include <vespa/vespalib/data/slime/slime.h>
 #include <cassert>
 
+using vespalib::Slime;
+using vespalib::slime::Inspector;
+using vespalib::slime::NixValue;
+
 namespace search::engine {
 
-DocsumReply::DocsumReply() { }
+DocsumReply::DocsumReply(std::unique_ptr<vespalib::Slime> root,
+                         DocsumRequest::UP request,
+                         UniqueIssues::UP issues)
+    : _slime(std::move(root)),
+      _request(std::move(request)),
+      _issues(std::move(issues))
+{}
+
+DocsumReply::DocsumReply(Slime::UP root, DocsumRequest::UP request)
+    : DocsumReply(std::move(root), std::move(request), std::make_unique<UniqueIssues>()) {}
+
+DocsumReply::DocsumReply(Slime::UP root)
+    : DocsumReply(std::move(root), {}, std::make_unique<UniqueIssues>()) {}
 
-DocsumReply::DocsumReply(vespalib::Slime::UP root) : _slime(std::move(root)) { }
+DocsumReply::DocsumReply()
+    : DocsumReply(std::make_unique<Slime>()) { }
 
 vespalib::slime::Inspector & DocsumReply::root() const {
-    return _slime ? _slime->get() : *vespalib::slime::NixValue::invalid();
+    return _slime ? _slime->get() : *NixValue::invalid();
+}
+
+bool DocsumReply::hasResults() const {
+    return (root().children() > 0);
 }
 
-std::unique_ptr<vespalib::Slime> DocsumReply::releaseSlime() {
+std::unique_ptr<Slime> DocsumReply::releaseSlime() {
     return std::move(_slime);
 }
 
diff --git a/searchlib/src/vespa/searchlib/engine/docsumreply.h b/searchlib/src/vespa/searchlib/engine/docsumreply.h
index 6098dfe197f..f8fccc2c436 100644
--- a/searchlib/src/vespa/searchlib/engine/docsumreply.h
+++ b/searchlib/src/vespa/searchlib/engine/docsumreply.h
@@ -16,29 +16,39 @@ namespace search::engine {
 
 class DocsumReply {
 private:
-    DocsumRequest::UP _request;
     std::unique_ptr<vespalib::Slime> _slime;
+    DocsumRequest::UP _request;
     UniqueIssues::UP _issues;
 public:
     using UP = std::unique_ptr<DocsumReply>;
 
-    DocsumReply();
+    DocsumReply(std::unique_ptr<vespalib::Slime> root,
+                DocsumRequest::UP request,
+                UniqueIssues::UP issues);
+
+    DocsumReply(std::unique_ptr<vespalib::Slime> root,
+                DocsumRequest::UP request);
+
     DocsumReply(std::unique_ptr<vespalib::Slime> root);
 
-    bool hasSlime() const { return _slime.get() != nullptr; }
-    bool hasRequest() const { return _request.get() != nullptr; }
-    bool hasIssues() const { return _issues.get() != nullptr; }
+    DocsumReply();
+
+    bool hasResults() const;
+    bool hasRequest() const { return (_request.get() != nullptr); }
+    bool hasIssues() const { return _issues && (_issues->size() > 0); }
 
-    vespalib::Slime & slime() const { assert(hasSlime()); return *_slime; }
+    vespalib::Slime & slime() const { assert(_slime.get()); return *_slime; }
     DocsumRequest& request() const { assert(hasRequest()); return *_request; }
-    UniqueIssues & issues() const { assert(hasIssues()); return *_issues; }
+    UniqueIssues & issues() const { return *_issues; }
 
+/*
     void setRequest(DocsumRequest::UP request) {
         _request = std::move(request);
     }
     void setIssues(UniqueIssues::UP issues) {
         _issues = std::move(issues);
     }
+*/
 
     std::unique_ptr<vespalib::Slime> releaseSlime();
 
diff --git a/searchlib/src/vespa/searchlib/engine/proto_converter.cpp b/searchlib/src/vespa/searchlib/engine/proto_converter.cpp
index 52b247ae19e..732ddecd357 100644
--- a/searchlib/src/vespa/searchlib/engine/proto_converter.cpp
+++ b/searchlib/src/vespa/searchlib/engine/proto_converter.cpp
@@ -168,7 +168,7 @@ ProtoConverter::docsum_request_from_proto(const ProtoDocsumRequest &proto, Docsu
 void
 ProtoConverter::docsum_reply_to_proto(const DocsumReply &reply, ProtoDocsumReply &proto)
 {
-    if (reply.hasSlime()) {
+    if (reply.hasResults()) {
         vespalib::SmartBuffer buf(4_Ki);
         vespalib::slime::BinaryFormat::encode(reply.slime(), buf);
         proto.set_slime_summaries(buf.obtain().data, buf.obtain().size);