From df689ff589e185f51dec5a6cfa70e4009ebc176a Mon Sep 17 00:00:00 2001 From: Arne H Juul Date: Thu, 14 Oct 2021 10:11:20 +0000 Subject: make DocsumReply a proper class --- searchcore/src/tests/proton/docsummary/docsummary.cpp | 19 +++++++++++-------- .../src/tests/proton/summaryengine/summaryengine.cpp | 5 +++-- .../proton/summaryengine/docsum_by_slime.cpp | 4 ++-- .../searchcore/proton/summaryengine/summaryengine.cpp | 12 ++++-------- 4 files changed, 20 insertions(+), 20 deletions(-) (limited to 'searchcore') diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp index 916b88c19c4..1584cf4cd02 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.hasSlime()); + 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..ca4f2329c19 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.hasSlime()); + 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->hasSlime()); } } 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..3c8e5715a28 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->hasSlime()) { + return reply->releaseSlime(); } else { LOG(warning, "got 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..0e01ee68cea 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) @@ -149,8 +145,8 @@ 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); + reply->setRequest(std::move(req)); + reply->setIssues(std::move(my_issues)); return reply; } -- cgit v1.2.3 From e28a07fab5f83fd06f84bee95f20f457738b7ddb Mon Sep 17 00:00:00 2001 From: Arne H Juul Date: Thu, 14 Oct 2021 14:12:44 +0000 Subject: more useful DocsumReply --- .../src/tests/proton/docsummary/docsummary.cpp | 2 +- .../tests/proton/summaryengine/summaryengine.cpp | 4 +-- .../proton/summaryengine/docsum_by_slime.cpp | 2 +- .../proton/summaryengine/summaryengine.cpp | 5 +--- .../proto_converter/proto_converter_test.cpp | 1 - .../proto_rpc_adapter/proto_rpc_adapter_test.cpp | 6 ++--- .../src/vespa/searchlib/common/unique_issues.h | 1 + .../src/vespa/searchlib/engine/docsumreply.cpp | 29 +++++++++++++++++++--- searchlib/src/vespa/searchlib/engine/docsumreply.h | 24 ++++++++++++------ .../src/vespa/searchlib/engine/proto_converter.cpp | 2 +- 10 files changed, 52 insertions(+), 24 deletions(-) (limited to 'searchcore') 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 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(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()); 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(std::make_unique()); - auto &list = reply->slime().setArray(); + auto slime = std::make_unique(); + 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(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(); 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 #include +using vespalib::Slime; +using vespalib::slime::Inspector; +using vespalib::slime::NixValue; + namespace search::engine { -DocsumReply::DocsumReply() { } +DocsumReply::DocsumReply(std::unique_ptr 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()) {} + +DocsumReply::DocsumReply(Slime::UP root) + : DocsumReply(std::move(root), {}, std::make_unique()) {} -DocsumReply::DocsumReply(vespalib::Slime::UP root) : _slime(std::move(root)) { } +DocsumReply::DocsumReply() + : DocsumReply(std::make_unique()) { } 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 DocsumReply::releaseSlime() { +std::unique_ptr 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 _slime; + DocsumRequest::UP _request; UniqueIssues::UP _issues; public: using UP = std::unique_ptr; - DocsumReply(); + DocsumReply(std::unique_ptr root, + DocsumRequest::UP request, + UniqueIssues::UP issues); + + DocsumReply(std::unique_ptr root, + DocsumRequest::UP request); + DocsumReply(std::unique_ptr 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 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); -- cgit v1.2.3 From 0f268db1367835ac3d748a3494ab152ac5f161b4 Mon Sep 17 00:00:00 2001 From: Arne H Juul Date: Fri, 15 Oct 2021 09:47:40 +0000 Subject: update after review --- .../src/tests/proton/docsummary/docsummary.cpp | 2 +- .../tests/proton/summaryengine/summaryengine.cpp | 4 ++-- .../proton/summaryengine/docsum_by_slime.cpp | 2 +- .../proton/summaryengine/summaryengine.cpp | 15 ++++++------ .../proto_converter/proto_converter_test.cpp | 27 ++++++++++++++-------- .../src/vespa/searchlib/engine/docsumreply.cpp | 11 ++++----- searchlib/src/vespa/searchlib/engine/docsumreply.h | 11 ++++----- .../src/vespa/searchlib/engine/proto_converter.cpp | 2 +- 8 files changed, 40 insertions(+), 34 deletions(-) (limited to 'searchcore') 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 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(); - - // TODO: Notify closed. - return ret; } if (_async) { @@ -127,8 +124,7 @@ SummaryEngine::getDocsums(DocsumRequest::UP req) auto my_issues = std::make_unique(); auto capture_issues = vespalib::Issue::listen(*my_issues); - DocsumReply::UP reply = std::make_unique(); - + 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(reply->releaseSlime(), std::move(req), std::move(my_issues)); + if (! reply) { + reply = std::make_unique(); + } + 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()}; + Slime &slime; + DocsumReply reply; Converter::ProtoDocsumReply proto; void convert() { Converter::docsum_reply_to_proto(reply, proto); } + DocsumReplyTest(std::unique_ptr slime_in) + : slime(*slime_in), reply(std::move(slime_in)) + {} + DocsumReplyTest() : DocsumReplyTest(std::make_unique()) {} }; 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(); + 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 root, {} DocsumReply::DocsumReply(Slime::UP root, DocsumRequest::UP request) - : DocsumReply(std::move(root), std::move(request), std::make_unique()) {} + : DocsumReply(std::move(root), std::move(request), {}) {} DocsumReply::DocsumReply(Slime::UP root) - : DocsumReply(std::move(root), {}, std::make_unique()) {} + : DocsumReply(std::move(root), {}, {}) {} -DocsumReply::DocsumReply() - : DocsumReply(std::make_unique()) { } +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 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 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); -- cgit v1.2.3