Merge pull request #19555 from vespa-engine/arnej/docsumreply-class

make DocsumReply a proper class

4 files changed, 25 insertions, 27 deletions

diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp
index 916b88c19c4..ffe720059c6 100644
--- a/searchcore/src/tests/proton/docsummary/docsummary.cpp
+++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp
@@ -324,18 +324,19 @@ void
 assertTensor(const vespalib::eval::Value::UP & exp, const std::string & fieldName,
              const DocsumReply & reply, uint32_t id)
 {
+    const auto & root = reply.root();
     if (exp) {
-        EXPECT_TRUE(reply.root()["docsums"].valid());
-        EXPECT_TRUE(reply.root()["docsums"][id].valid());
-        EXPECT_TRUE(reply.root()["docsums"][id]["docsum"].valid());
-        EXPECT_TRUE(reply.root()["docsums"][id]["docsum"][fieldName].valid());
-        vespalib::Memory data = reply.root()["docsums"][id]["docsum"][fieldName].asData();
+        EXPECT_TRUE(root["docsums"].valid());
+        EXPECT_TRUE(root["docsums"][id].valid());
+        EXPECT_TRUE(root["docsums"][id]["docsum"].valid());
+        EXPECT_TRUE(root["docsums"][id]["docsum"][fieldName].valid());
+        vespalib::Memory data = root["docsums"][id]["docsum"][fieldName].asData();
         vespalib::nbostream x(data.data, data.size);
         auto tensor = SimpleValue::from_stream(x);
         EXPECT_TRUE(tensor.get() != nullptr);
         EXPECT_EQUAL(*exp, *tensor);
     } else {
-        EXPECT_FALSE(reply.root()["docsums"][id][fieldName].valid());
+        EXPECT_FALSE(root["docsums"][id][fieldName].valid());
     }
 }
 
@@ -343,7 +344,8 @@ bool assertSlime(const std::string &exp, const DocsumReply &reply) {
     vespalib::Slime expSlime;
     size_t used = JsonFormat::decode(exp, expSlime);
     EXPECT_TRUE(used > 0);
-    return (EXPECT_EQUAL(expSlime.get(), reply.root()));
+    ASSERT_TRUE(reply.hasResult());
+    return (EXPECT_EQUAL(expSlime, reply.slime()));
 }
 
 TEST_F("requireThatAdapterHandlesAllFieldTypes", Fixture)
@@ -564,7 +566,8 @@ TEST("requireThatSummariesTimeout")
     req.resultClassName = "class2";
     req.hits.push_back(DocsumRequest::Hit(gid1));
     DocsumReply::UP rep = dc._ddb->getDocsums(req);
-    const auto & field = rep->root()["errors"];
+    const auto & root = rep->root();
+    const auto & field = root["errors"];
     EXPECT_TRUE(field.valid());
     EXPECT_EQUAL(field[0]["type"].asString(), "timeout");
     auto bufstring = field[0]["message"].asString();
diff --git a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
index 6660b7bfd55..d942abd28c2 100644
--- a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
+++ b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
@@ -135,7 +135,8 @@ void assertSlime(const std::string &exp, const DocsumReply &reply) {
     vespalib::Slime expSlime;
     size_t used = JsonFormat::decode(exp, expSlime);
     EXPECT_TRUE(used > 0);
-    EXPECT_EQUAL(expSlime.get(), reply.root());
+    ASSERT_TRUE(reply.hasResult());
+    EXPECT_EQUAL(expSlime, reply.slime());
 }
 
 TEST("requireThatGetDocsumsExecute") {
@@ -159,7 +160,7 @@ TEST("requireThatGetDocsumsExecute") {
         DocsumRequest::Source request(createRequest());
         DocsumReply::UP reply = engine.getDocsums(std::move(request), client);
         EXPECT_TRUE(reply);
-        EXPECT_FALSE(reply->_root);
+        EXPECT_FALSE(reply->hasResult());
     }
 }
 
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 bd6a09c1387..a5ad7618c84 100644
--- a/searchcore/src/vespa/searchcore/proton/summaryengine/docsum_by_slime.cpp
+++ b/searchcore/src/vespa/searchcore/proton/summaryengine/docsum_by_slime.cpp
@@ -93,8 +93,8 @@ vespalib::Slime::UP
 DocsumBySlime::getDocsums(const Inspector & req)
 {
     DocsumReply::UP reply = _docsumServer.getDocsums(slimeToRequest(req));
-    if (reply && reply->_root) {
-        return std::move(reply->_root);
+    if (reply && reply->hasResult()) {
+        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 91533011b40..ec5c8c9b72d 100644
--- a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
@@ -35,12 +35,8 @@ public:
 };
 
 uint32_t getNumDocs(const DocsumReply &reply) {
-    if (reply._root) {
-        const Inspector &root = reply._root->get();
-        return root[DOCSUMS].entries();
-    } else {
-        return 0;
-    }
+    const Inspector &root = reply.root();
+    return root[DOCSUMS].entries();
 }
 
 VESPA_THREAD_STACK_TAG(summary_engine_executor)
@@ -110,11 +106,8 @@ DocsumReply::UP
 SummaryEngine::getDocsums(DocsumRequest::Source request, DocsumClient & client)
 {
     if (_closed) {
-        LOG(warning, "Receiving docsumrequest after engine has been shutdown");
+        vespalib::Issue::report("Received docsum request after engine has been shutdown");
         auto ret = std::make_unique<DocsumReply>();
-
-        // TODO: Notify closed.
-
         return ret;
     }
     if (_async) {
@@ -131,8 +124,7 @@ SummaryEngine::getDocsums(DocsumRequest::UP req)
     auto my_issues = std::make_unique<search::UniqueIssues>();
     auto capture_issues = vespalib::Issue::listen(*my_issues);
 
-    DocsumReply::UP reply = std::make_unique<DocsumReply>();
-
+    DocsumReply::UP reply;
     if (req) {
         ISearchHandler::SP searchHandler = getSearchHandler(DocTypeName(*req));
         if (searchHandler) {
@@ -149,9 +141,11 @@ SummaryEngine::getDocsums(DocsumRequest::UP req)
         }
         updateDocsumMetrics(vespalib::to_s(req->getTimeUsed()), getNumDocs(*reply));
     }
-    reply->request = std::move(req);
-    reply->my_issues = std::move(my_issues);
-
+    if (! reply) {
+        reply = std::make_unique<DocsumReply>();
+    }
+    reply->setRequest(std::move(req));
+    reply->setIssues(std::move(my_issues));
     return reply;
 }