update after review

8 files changed, 40 insertions, 34 deletions

diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp
index fca5e4dfbda..ffe720059c6 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.hasResults());
+    ASSERT_TRUE(reply.hasResult());
     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 b59aaf33908..d942abd28c2 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.hasResults());
+    ASSERT_TRUE(reply.hasResult());
     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->hasResults());
+        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 1860f3ab989..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,7 +93,7 @@ vespalib::Slime::UP
 DocsumBySlime::getDocsums(const Inspector & req)
 {
     DocsumReply::UP reply = _docsumServer.getDocsums(slimeToRequest(req));
-    if (reply && reply->hasResults()) {
+    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 8a4ac98643d..ec5c8c9b72d 100644
--- a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
@@ -106,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) {
@@ -127,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) {
@@ -145,7 +141,12 @@ SummaryEngine::getDocsums(DocsumRequest::UP req)
         }
         updateDocsumMetrics(vespalib::to_s(req->getTimeUsed()), getNumDocs(*reply));
     }
-    return std::make_unique<DocsumReply>(reply->releaseSlime(), std::move(req), std::move(my_issues));
+    if (! reply) {
+        reply = std::make_unique<DocsumReply>();
+    }
+    reply->setRequest(std::move(req));
+    reply->setIssues(std::move(my_issues));
+    return reply;
 }
 
 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 23b95f489c6..d0c356bf879 100644
--- a/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp
+++ b/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp
@@ -478,20 +478,25 @@ TEST_F(DocsumRequestTest, require_that_global_ids_are_converted) {
 //-----------------------------------------------------------------------------
 
 struct DocsumReplyTest : ::testing::Test {
-    DocsumReply reply{std::make_unique<vespalib::Slime>()};
+    Slime &slime;
+    DocsumReply reply;
     Converter::ProtoDocsumReply proto;
     void convert() { Converter::docsum_reply_to_proto(reply, proto); }
+    DocsumReplyTest(std::unique_ptr<Slime> slime_in)
+        : slime(*slime_in), reply(std::move(slime_in))
+    {}
+    DocsumReplyTest() : DocsumReplyTest(std::make_unique<Slime>()) {}
 };
 
 TEST_F(DocsumReplyTest, require_that_slime_summaries_are_converted) {
-    auto &list = reply.slime().setArray();
+    auto &list = slime.setArray();
     auto &doc0 = list.addObject();
     doc0.setLong("my_field", 42);
     convert();
     const auto &mem = proto.slime_summaries();
-    Slime slime;
-    EXPECT_EQ(BinaryFormat::decode(Memory(mem.data(), mem.size()), slime), mem.size());
-    EXPECT_EQ(slime.get()[0]["my_field"].asLong(), 42);
+    Slime decoded;
+    EXPECT_EQ(BinaryFormat::decode(Memory(mem.data(), mem.size()), decoded), mem.size());
+    EXPECT_EQ(decoded.get()[0]["my_field"].asLong(), 42);
 }
 
 TEST_F(DocsumReplyTest, require_that_missing_root_slime_gives_empty_payload) {
@@ -501,11 +506,13 @@ TEST_F(DocsumReplyTest, require_that_missing_root_slime_gives_empty_payload) {
 }
 
 TEST_F(DocsumReplyTest, require_that_issues_are_converted_to_errors) {
-    reply.issues().handle(vespalib::Issue("a"));
-    reply.issues().handle(vespalib::Issue("b"));
-    reply.issues().handle(vespalib::Issue("c"));
-    reply.issues().handle(vespalib::Issue("a"));
-    reply.issues().handle(vespalib::Issue("b"));
+    auto my_issues = std::make_unique<UniqueIssues>();
+    my_issues->handle(vespalib::Issue("a"));
+    my_issues->handle(vespalib::Issue("b"));
+    my_issues->handle(vespalib::Issue("c"));
+    my_issues->handle(vespalib::Issue("a"));
+    my_issues->handle(vespalib::Issue("b"));
+    reply.setIssues(std::move(my_issues));
     convert();
     ASSERT_EQ(proto.errors_size(), 3);
     EXPECT_EQ(proto.errors(0).message(), "a");
diff --git a/searchlib/src/vespa/searchlib/engine/docsumreply.cpp b/searchlib/src/vespa/searchlib/engine/docsumreply.cpp
index ba738a51183..550398d225c 100644
--- a/searchlib/src/vespa/searchlib/engine/docsumreply.cpp
+++ b/searchlib/src/vespa/searchlib/engine/docsumreply.cpp
@@ -19,20 +19,19 @@ DocsumReply::DocsumReply(std::unique_ptr<vespalib::Slime> root,
 {}
 
 DocsumReply::DocsumReply(Slime::UP root, DocsumRequest::UP request)
-    : DocsumReply(std::move(root), std::move(request), std::make_unique<UniqueIssues>()) {}
+    : DocsumReply(std::move(root), std::move(request), {}) {}
 
 DocsumReply::DocsumReply(Slime::UP root)
-    : DocsumReply(std::move(root), {}, std::make_unique<UniqueIssues>()) {}
+    : DocsumReply(std::move(root), {}, {}) {}
 
-DocsumReply::DocsumReply()
-    : DocsumReply(std::make_unique<Slime>()) { }
+DocsumReply::DocsumReply() = default;
 
 vespalib::slime::Inspector & DocsumReply::root() const {
     return _slime ? _slime->get() : *NixValue::invalid();
 }
 
-bool DocsumReply::hasResults() const {
-    return (root().children() > 0);
+bool DocsumReply::hasResult() const {
+    return root().valid();
 }
 
 std::unique_ptr<Slime> DocsumReply::releaseSlime() {
diff --git a/searchlib/src/vespa/searchlib/engine/docsumreply.h b/searchlib/src/vespa/searchlib/engine/docsumreply.h
index f8fccc2c436..0caa848e4ef 100644
--- a/searchlib/src/vespa/searchlib/engine/docsumreply.h
+++ b/searchlib/src/vespa/searchlib/engine/docsumreply.h
@@ -33,22 +33,21 @@ public:
 
     DocsumReply();
 
-    bool hasResults() const;
+    bool hasResult() const;
     bool hasRequest() const { return (_request.get() != nullptr); }
     bool hasIssues() const { return _issues && (_issues->size() > 0); }
 
-    vespalib::Slime & slime() const { assert(_slime.get()); return *_slime; }
-    DocsumRequest& request() const { assert(hasRequest()); return *_request; }
-    UniqueIssues & issues() const { return *_issues; }
+    const vespalib::Slime & slime() const { assert(_slime.get()); return *_slime; }
+    const DocsumRequest& request() const { assert(_request.get()); return *_request; }
+    const UniqueIssues & issues() const { assert(_issues.get()); 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 732ddecd357..8b949d9ed4e 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.hasResults()) {
+    if (reply.hasResult()) {
         vespalib::SmartBuffer buf(4_Ki);
         vespalib::slime::BinaryFormat::encode(reply.slime(), buf);
         proto.set_slime_summaries(buf.obtain().data, buf.obtain().size);