diff options
34 files changed, 164 insertions, 268 deletions
diff --git a/documentapi/src/tests/messages/messages60test.cpp b/documentapi/src/tests/messages/messages60test.cpp index c7bb1015e02..8b895fb2b59 100644 --- a/documentapi/src/tests/messages/messages60test.cpp +++ b/documentapi/src/tests/messages/messages60test.cpp @@ -323,6 +323,7 @@ Messages60Test::testGetDocumentMessage() { GetDocumentMessage tmp(document::DocumentId("id:ns:testdoc::"), "foo bar"); + EXPECT_EQUAL(360u, sizeof(GetDocumentMessage)); EXPECT_EQUAL(MESSAGE_BASE_LENGTH + (size_t)31, serialize("GetDocumentMessage", tmp)); for (uint32_t lang = 0; lang < NUM_LANGUAGES; ++lang) { @@ -400,6 +401,7 @@ Messages60Test::testPutDocumentMessage() msg.setTimestamp(666); msg.setCondition(TestAndSetCondition("There's just one condition")); + EXPECT_EQUAL(280u, sizeof(PutDocumentMessage)); EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 45u + serializedLength(msg.getCondition().getSelection()), @@ -447,6 +449,7 @@ Messages60Test::testPutDocumentReply() reply.setHighestModificationTimestamp(30); EXPECT_EQUAL(13u, serialize("PutDocumentReply", reply)); + EXPECT_EQUAL(112u, sizeof(WriteDocumentReply)); for (uint32_t lang = 0; lang < NUM_LANGUAGES; ++lang) { mbus::Routable::UP obj = deserialize("PutDocumentReply", DocumentProtocol::REPLY_PUTDOCUMENT, lang); @@ -466,6 +469,7 @@ Messages60Test::testUpdateDocumentReply() reply.setHighestModificationTimestamp(30); EXPECT_EQUAL(14u, serialize("UpdateDocumentReply", reply)); + EXPECT_EQUAL(120u, sizeof(UpdateDocumentReply)); for (uint32_t lang = 0; lang < NUM_LANGUAGES; ++lang) { mbus::Routable::UP obj = deserialize("UpdateDocumentReply", DocumentProtocol::REPLY_UPDATEDOCUMENT, lang); @@ -485,12 +489,13 @@ Messages60Test::testRemoveDocumentMessage() msg.setCondition(TestAndSetCondition("There's just one condition")); + EXPECT_EQUAL(360u, sizeof(RemoveDocumentMessage)); EXPECT_EQUAL(MESSAGE_BASE_LENGTH + size_t(20) + serializedLength(msg.getCondition().getSelection()), serialize("RemoveDocumentMessage", msg)); for (uint32_t lang = 0; lang < NUM_LANGUAGES; ++lang) { auto routablePtr = deserialize("RemoveDocumentMessage", DocumentProtocol::MESSAGE_REMOVEDOCUMENT, lang); - if (EXPECT_TRUE(routablePtr.get() != nullptr)) { + if (EXPECT_TRUE(routablePtr)) { auto & ref = static_cast<RemoveDocumentMessage &>(*routablePtr); EXPECT_EQUAL(string("id:ns:testdoc::"), ref.getDocumentId().toString()); EXPECT_EQUAL(msg.getCondition().getSelection(), ref.getCondition().getSelection()); @@ -506,6 +511,7 @@ Messages60Test::testRemoveDocumentReply() std::vector<uint64_t> ts; reply.setWasFound(false); reply.setHighestModificationTimestamp(30); + EXPECT_EQUAL(120u, sizeof(RemoveDocumentReply)); EXPECT_EQUAL(14u, serialize("RemoveDocumentReply", reply)); @@ -663,12 +669,13 @@ Messages60Test::testUpdateDocumentMessage() msg.setNewTimestamp(777u); msg.setCondition(TestAndSetCondition("There's just one condition")); + EXPECT_EQUAL(288u, sizeof(UpdateDocumentMessage)); EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 93u + serializedLength(msg.getCondition().getSelection()), serialize("UpdateDocumentMessage", msg)); for (uint32_t lang = 0; lang < NUM_LANGUAGES; ++lang) { auto routableUp = deserialize("UpdateDocumentMessage", DocumentProtocol::MESSAGE_UPDATEDOCUMENT, lang); - if (EXPECT_TRUE(routableUp.get() != nullptr)) { + if (EXPECT_TRUE(routableUp)) { auto & deserializedMsg = static_cast<UpdateDocumentMessage &>(*routableUp); EXPECT_EQUAL(msg.getDocumentUpdate(), deserializedMsg.getDocumentUpdate()); EXPECT_EQUAL(msg.getOldTimestamp(), deserializedMsg.getOldTimestamp()); diff --git a/documentapi/src/tests/replymerger/replymerger_test.cpp b/documentapi/src/tests/replymerger/replymerger_test.cpp index 4626ccd0a60..f74ad23da1d 100644 --- a/documentapi/src/tests/replymerger/replymerger_test.cpp +++ b/documentapi/src/tests/replymerger/replymerger_test.cpp @@ -8,6 +8,7 @@ #include <vespa/documentapi/messagebus/messages/updatedocumentreply.h> #include <vespa/documentapi/messagebus/messages/getdocumentreply.h> #include <vespa/messagebus/emptyreply.h> +#include <vespa/messagebus/error.h> using namespace documentapi; diff --git a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp index dcfc0fa5f6e..a957ce5e4ff 100644 --- a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp @@ -8,6 +8,7 @@ #include <vespa/document/util/stringutil.h> #include <vespa/documentapi/documentapi.h> #include <vespa/vespalib/util/exceptions.h> +#include <vespa/messagebus/error.h> #include <sstream> #include <cassert> @@ -31,16 +32,16 @@ DocumentProtocol::DocumentProtocol(const LoadTypeSet& loadTypes, string cfg = (configId.empty() ? "client" : configId); // When adding factories to this list, please KEEP THEM ORDERED alphabetically like they are now. - putRoutingPolicyFactory("AND", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::AndPolicyFactory())); - putRoutingPolicyFactory("Content", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::ContentPolicyFactory())); - putRoutingPolicyFactory("MessageType", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::MessageTypePolicyFactory())); - putRoutingPolicyFactory("DocumentRouteSelector", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::DocumentRouteSelectorPolicyFactory(*_repo, cfg))); - putRoutingPolicyFactory("Extern", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::ExternPolicyFactory())); - putRoutingPolicyFactory("LocalService", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::LocalServicePolicyFactory())); - putRoutingPolicyFactory("RoundRobin", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::RoundRobinPolicyFactory())); - putRoutingPolicyFactory("Storage", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::StoragePolicyFactory())); - putRoutingPolicyFactory("SubsetService", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::SubsetServicePolicyFactory())); - putRoutingPolicyFactory("LoadBalancer", IRoutingPolicyFactory::SP(new RoutingPolicyFactories::LoadBalancerPolicyFactory())); + putRoutingPolicyFactory("AND", std::make_shared<RoutingPolicyFactories::AndPolicyFactory>()); + putRoutingPolicyFactory("Content", std::make_shared<RoutingPolicyFactories::ContentPolicyFactory>()); + putRoutingPolicyFactory("MessageType", std::make_shared<RoutingPolicyFactories::MessageTypePolicyFactory>()); + putRoutingPolicyFactory("DocumentRouteSelector", std::make_shared<RoutingPolicyFactories::DocumentRouteSelectorPolicyFactory>(*_repo, cfg)); + putRoutingPolicyFactory("Extern", std::make_shared<RoutingPolicyFactories::ExternPolicyFactory>()); + putRoutingPolicyFactory("LocalService", std::make_shared<RoutingPolicyFactories::LocalServicePolicyFactory>()); + putRoutingPolicyFactory("RoundRobin", std::make_shared<RoutingPolicyFactories::RoundRobinPolicyFactory>()); + putRoutingPolicyFactory("Storage", std::make_shared<RoutingPolicyFactories::StoragePolicyFactory>()); + putRoutingPolicyFactory("SubsetService", std::make_shared<RoutingPolicyFactories::SubsetServicePolicyFactory>()); + putRoutingPolicyFactory("LoadBalancer", std::make_shared<RoutingPolicyFactories::LoadBalancerPolicyFactory>()); // Prepare version specifications to use when adding routable factories. vespalib::VersionSpecification version6(6, 221); @@ -48,42 +49,42 @@ DocumentProtocol::DocumentProtocol(const LoadTypeSet& loadTypes, std::vector<vespalib::VersionSpecification> from6 = { version6 }; // Add 6.x serialization - putRoutableFactory(MESSAGE_CREATEVISITOR, IRoutableFactory::SP(new RoutableFactories60::CreateVisitorMessageFactory()), from6); - putRoutableFactory(MESSAGE_DESTROYVISITOR, IRoutableFactory::SP(new RoutableFactories60::DestroyVisitorMessageFactory()), from6); - putRoutableFactory(MESSAGE_DOCUMENTLIST, IRoutableFactory::SP(new RoutableFactories60::DocumentListMessageFactory(*_repo)), from6); - putRoutableFactory(MESSAGE_DOCUMENTSUMMARY, IRoutableFactory::SP(new RoutableFactories60::DocumentSummaryMessageFactory()), from6); - putRoutableFactory(MESSAGE_EMPTYBUCKETS, IRoutableFactory::SP(new RoutableFactories60::EmptyBucketsMessageFactory()), from6); - putRoutableFactory(MESSAGE_GETBUCKETLIST, IRoutableFactory::SP(new RoutableFactories60::GetBucketListMessageFactory()), from6); - putRoutableFactory(MESSAGE_GETBUCKETSTATE, IRoutableFactory::SP(new RoutableFactories60::GetBucketStateMessageFactory()), from6); - putRoutableFactory(MESSAGE_GETDOCUMENT, IRoutableFactory::SP(new RoutableFactories60::GetDocumentMessageFactory()), from6); - putRoutableFactory(MESSAGE_MAPVISITOR, IRoutableFactory::SP(new RoutableFactories60::MapVisitorMessageFactory()), from6); - putRoutableFactory(MESSAGE_PUTDOCUMENT, IRoutableFactory::SP(new RoutableFactories60::PutDocumentMessageFactory(*_repo)), from6); - putRoutableFactory(MESSAGE_QUERYRESULT, IRoutableFactory::SP(new RoutableFactories60::QueryResultMessageFactory()), from6); - putRoutableFactory(MESSAGE_REMOVEDOCUMENT, IRoutableFactory::SP(new RoutableFactories60::RemoveDocumentMessageFactory()), from6); - putRoutableFactory(MESSAGE_REMOVELOCATION, IRoutableFactory::SP(new RoutableFactories60::RemoveLocationMessageFactory(*_repo)), from6); - putRoutableFactory(MESSAGE_SEARCHRESULT, IRoutableFactory::SP(new RoutableFactories60::SearchResultMessageFactory()), from6); - putRoutableFactory(MESSAGE_STATBUCKET, IRoutableFactory::SP(new RoutableFactories60::StatBucketMessageFactory()), from6); - putRoutableFactory(MESSAGE_UPDATEDOCUMENT, IRoutableFactory::SP(new RoutableFactories60::UpdateDocumentMessageFactory(*_repo)), from6); - putRoutableFactory(MESSAGE_VISITORINFO, IRoutableFactory::SP(new RoutableFactories60::VisitorInfoMessageFactory()), from6); - putRoutableFactory(REPLY_CREATEVISITOR, IRoutableFactory::SP(new RoutableFactories60::CreateVisitorReplyFactory()), from6); - putRoutableFactory(REPLY_DESTROYVISITOR, IRoutableFactory::SP(new RoutableFactories60::DestroyVisitorReplyFactory()), from6); - putRoutableFactory(REPLY_DOCUMENTIGNORED, IRoutableFactory::SP(new RoutableFactories60::DocumentIgnoredReplyFactory()), from6); - putRoutableFactory(REPLY_DOCUMENTLIST, IRoutableFactory::SP(new RoutableFactories60::DocumentListReplyFactory()), from6); - putRoutableFactory(REPLY_DOCUMENTSUMMARY, IRoutableFactory::SP(new RoutableFactories60::DocumentSummaryReplyFactory()), from6); - putRoutableFactory(REPLY_EMPTYBUCKETS, IRoutableFactory::SP(new RoutableFactories60::EmptyBucketsReplyFactory()), from6); - putRoutableFactory(REPLY_GETBUCKETLIST, IRoutableFactory::SP(new RoutableFactories60::GetBucketListReplyFactory()), from6); - putRoutableFactory(REPLY_GETBUCKETSTATE, IRoutableFactory::SP(new RoutableFactories60::GetBucketStateReplyFactory()), from6); - putRoutableFactory(REPLY_GETDOCUMENT, IRoutableFactory::SP(new RoutableFactories60::GetDocumentReplyFactory(*_repo)), from6); - putRoutableFactory(REPLY_MAPVISITOR, IRoutableFactory::SP(new RoutableFactories60::MapVisitorReplyFactory()), from6); - putRoutableFactory(REPLY_PUTDOCUMENT, IRoutableFactory::SP(new RoutableFactories60::PutDocumentReplyFactory()), from6); - putRoutableFactory(REPLY_QUERYRESULT, IRoutableFactory::SP(new RoutableFactories60::QueryResultReplyFactory()), from6); - putRoutableFactory(REPLY_REMOVEDOCUMENT, IRoutableFactory::SP(new RoutableFactories60::RemoveDocumentReplyFactory()), from6); - putRoutableFactory(REPLY_REMOVELOCATION, IRoutableFactory::SP(new RoutableFactories60::RemoveLocationReplyFactory()), from6); - putRoutableFactory(REPLY_SEARCHRESULT, IRoutableFactory::SP(new RoutableFactories60::SearchResultReplyFactory()), from6); - putRoutableFactory(REPLY_STATBUCKET, IRoutableFactory::SP(new RoutableFactories60::StatBucketReplyFactory()), from6); - putRoutableFactory(REPLY_UPDATEDOCUMENT, IRoutableFactory::SP(new RoutableFactories60::UpdateDocumentReplyFactory()), from6); - putRoutableFactory(REPLY_VISITORINFO, IRoutableFactory::SP(new RoutableFactories60::VisitorInfoReplyFactory()), from6); - putRoutableFactory(REPLY_WRONGDISTRIBUTION, IRoutableFactory::SP(new RoutableFactories60::WrongDistributionReplyFactory()), from6); + putRoutableFactory(MESSAGE_CREATEVISITOR, std::make_shared<RoutableFactories60::CreateVisitorMessageFactory>(), from6); + putRoutableFactory(MESSAGE_DESTROYVISITOR, std::make_shared<RoutableFactories60::DestroyVisitorMessageFactory>(), from6); + putRoutableFactory(MESSAGE_DOCUMENTLIST, std::make_shared<RoutableFactories60::DocumentListMessageFactory>(*_repo), from6); + putRoutableFactory(MESSAGE_DOCUMENTSUMMARY, std::make_shared<RoutableFactories60::DocumentSummaryMessageFactory>(), from6); + putRoutableFactory(MESSAGE_EMPTYBUCKETS, std::make_shared<RoutableFactories60::EmptyBucketsMessageFactory>(), from6); + putRoutableFactory(MESSAGE_GETBUCKETLIST, std::make_shared<RoutableFactories60::GetBucketListMessageFactory>(), from6); + putRoutableFactory(MESSAGE_GETBUCKETSTATE, std::make_shared<RoutableFactories60::GetBucketStateMessageFactory>(), from6); + putRoutableFactory(MESSAGE_GETDOCUMENT, std::make_shared<RoutableFactories60::GetDocumentMessageFactory>(), from6); + putRoutableFactory(MESSAGE_MAPVISITOR, std::make_shared<RoutableFactories60::MapVisitorMessageFactory>(), from6); + putRoutableFactory(MESSAGE_PUTDOCUMENT, std::make_shared<RoutableFactories60::PutDocumentMessageFactory>(*_repo), from6); + putRoutableFactory(MESSAGE_QUERYRESULT, std::make_shared<RoutableFactories60::QueryResultMessageFactory>(), from6); + putRoutableFactory(MESSAGE_REMOVEDOCUMENT, std::make_shared<RoutableFactories60::RemoveDocumentMessageFactory>(), from6); + putRoutableFactory(MESSAGE_REMOVELOCATION, std::make_shared<RoutableFactories60::RemoveLocationMessageFactory>(*_repo), from6); + putRoutableFactory(MESSAGE_SEARCHRESULT, std::make_shared<RoutableFactories60::SearchResultMessageFactory>(), from6); + putRoutableFactory(MESSAGE_STATBUCKET, std::make_shared<RoutableFactories60::StatBucketMessageFactory>(), from6); + putRoutableFactory(MESSAGE_UPDATEDOCUMENT, std::make_shared<RoutableFactories60::UpdateDocumentMessageFactory>(*_repo), from6); + putRoutableFactory(MESSAGE_VISITORINFO, std::make_shared<RoutableFactories60::VisitorInfoMessageFactory>(), from6); + putRoutableFactory(REPLY_CREATEVISITOR, std::make_shared<RoutableFactories60::CreateVisitorReplyFactory>(), from6); + putRoutableFactory(REPLY_DESTROYVISITOR, std::make_shared<RoutableFactories60::DestroyVisitorReplyFactory>(), from6); + putRoutableFactory(REPLY_DOCUMENTIGNORED, std::make_shared<RoutableFactories60::DocumentIgnoredReplyFactory>(), from6); + putRoutableFactory(REPLY_DOCUMENTLIST, std::make_shared<RoutableFactories60::DocumentListReplyFactory>(), from6); + putRoutableFactory(REPLY_DOCUMENTSUMMARY, std::make_shared<RoutableFactories60::DocumentSummaryReplyFactory>(), from6); + putRoutableFactory(REPLY_EMPTYBUCKETS, std::make_shared<RoutableFactories60::EmptyBucketsReplyFactory>(), from6); + putRoutableFactory(REPLY_GETBUCKETLIST, std::make_shared<RoutableFactories60::GetBucketListReplyFactory>(), from6); + putRoutableFactory(REPLY_GETBUCKETSTATE, std::make_shared<RoutableFactories60::GetBucketStateReplyFactory>(), from6); + putRoutableFactory(REPLY_GETDOCUMENT, std::make_shared<RoutableFactories60::GetDocumentReplyFactory>(*_repo), from6); + putRoutableFactory(REPLY_MAPVISITOR, std::make_shared<RoutableFactories60::MapVisitorReplyFactory>(), from6); + putRoutableFactory(REPLY_PUTDOCUMENT, std::make_shared<RoutableFactories60::PutDocumentReplyFactory>(), from6); + putRoutableFactory(REPLY_QUERYRESULT, std::make_shared<RoutableFactories60::QueryResultReplyFactory>(), from6); + putRoutableFactory(REPLY_REMOVEDOCUMENT, std::make_shared<RoutableFactories60::RemoveDocumentReplyFactory>(), from6); + putRoutableFactory(REPLY_REMOVELOCATION, std::make_shared<RoutableFactories60::RemoveLocationReplyFactory>(), from6); + putRoutableFactory(REPLY_SEARCHRESULT, std::make_shared<RoutableFactories60::SearchResultReplyFactory>(), from6); + putRoutableFactory(REPLY_STATBUCKET, std::make_shared<RoutableFactories60::StatBucketReplyFactory>(), from6); + putRoutableFactory(REPLY_UPDATEDOCUMENT, std::make_shared<RoutableFactories60::UpdateDocumentReplyFactory>(), from6); + putRoutableFactory(REPLY_VISITORINFO, std::make_shared<RoutableFactories60::VisitorInfoReplyFactory>(), from6); + putRoutableFactory(REPLY_WRONGDISTRIBUTION, std::make_shared<RoutableFactories60::WrongDistributionReplyFactory>(), from6); } DocumentProtocol::~DocumentProtocol() = default; diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/documentmessage.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/documentmessage.cpp index 199d83749c2..892562ccd3a 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/documentmessage.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/messages/documentmessage.cpp @@ -2,23 +2,22 @@ #include "documentmessage.h" #include <vespa/documentapi/messagebus/documentprotocol.h> -#include <cassert> namespace documentapi { DocumentMessage::DocumentMessage() : mbus::Message(), - _priority(Priority::PRI_NORMAL_3), _loadType(LoadType::DEFAULT), + _priority(Priority::PRI_NORMAL_3), _approxSize(1024) {} +DocumentMessage::~DocumentMessage() = default; + mbus::Reply::UP DocumentMessage::createReply() const { - mbus::Reply::UP ret(doCreateReply().release()); - assert(ret.get() != nullptr); - return ret; + return doCreateReply(); } const mbus::string& diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/documentmessage.h b/documentapi/src/vespa/documentapi/messagebus/messages/documentmessage.h index 6e1507068eb..77a4cc23b69 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/documentmessage.h +++ b/documentapi/src/vespa/documentapi/messagebus/messages/documentmessage.h @@ -4,15 +4,14 @@ #include "documentreply.h" #include <vespa/documentapi/loadtypes/loadtype.h> #include <vespa/messagebus/message.h> -#include <vespa/messagebus/reply.h> namespace documentapi { class DocumentMessage : public mbus::Message { private: + LoadType _loadType; Priority::Value _priority; - LoadType _loadType; - uint32_t _approxSize; // Not sent on wire; set by deserializer or by caller. + uint32_t _approxSize; // Not sent on wire; set by deserializer or by caller. protected: /** @@ -30,15 +29,8 @@ public: typedef std::unique_ptr<DocumentMessage> UP; typedef std::shared_ptr<DocumentMessage> SP; - /** - * Constructs a new document message with no content. - */ DocumentMessage(); - - /** - * Virtual destructor required for inheritance. - */ - virtual ~DocumentMessage() { } + ~DocumentMessage() override; /** * Creates and returns a reply to this message. This method uses the internal {@link #doCreateReply()} to @@ -47,7 +39,7 @@ public: * * @return The created reply. */ - mbus::Reply::UP createReply() const; + std::unique_ptr<mbus::Reply> createReply() const; /** * Returns the priority of this message. diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/documentreply.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/documentreply.cpp index 5a2478cbd65..d16f5d5cd66 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/documentreply.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/messages/documentreply.cpp @@ -9,9 +9,9 @@ DocumentReply::DocumentReply(uint32_t type) : mbus::Reply(), _type(type), _priority(Priority::PRI_NORMAL_3) -{ - // empty -} +{ } + +DocumentReply::~DocumentReply() = default; const mbus::string& DocumentReply::getProtocol() const diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/documentreply.h b/documentapi/src/vespa/documentapi/messagebus/messages/documentreply.h index c166a4acb73..136c4983516 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/documentreply.h +++ b/documentapi/src/vespa/documentapi/messagebus/messages/documentreply.h @@ -32,7 +32,7 @@ public: /** * Virtual destructor required for inheritance. */ - ~DocumentReply() { } + ~DocumentReply() override; /** * Returns the priority tag for this message. This is an optional tag added for VDS that is not interpreted by the diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/testandsetmessage.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/testandsetmessage.cpp index f9a48cb0755..2c6a0d08032 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/testandsetmessage.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/messages/testandsetmessage.cpp @@ -4,6 +4,7 @@ namespace documentapi { -TestAndSetMessage::~TestAndSetMessage() { } +TestAndSetMessage::TestAndSetMessage() = default; +TestAndSetMessage::~TestAndSetMessage() = default; } diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/testandsetmessage.h b/documentapi/src/vespa/documentapi/messagebus/messages/testandsetmessage.h index d3e912549c4..2e623121c85 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/testandsetmessage.h +++ b/documentapi/src/vespa/documentapi/messagebus/messages/testandsetmessage.h @@ -12,7 +12,8 @@ private: TestAndSetCondition _condition; public: - ~TestAndSetMessage(); + TestAndSetMessage(); + ~TestAndSetMessage() override; void setCondition(const TestAndSetCondition & condition) { _condition = condition; } const TestAndSetCondition & getCondition() const { return _condition; } }; diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/andpolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/andpolicy.cpp index baca64353fc..0683c0179cc 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/andpolicy.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/policies/andpolicy.cpp @@ -1,8 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "andpolicy.h" -#include <vespa/messagebus/error.h> -#include <vespa/messagebus/errorcode.h> -#include <vespa/messagebus/emptyreply.h> #include <vespa/messagebus/routing/routingcontext.h> #include <vespa/documentapi/messagebus/documentprotocol.h> @@ -18,10 +15,7 @@ ANDPolicy::ANDPolicy(const string ¶m) } } -ANDPolicy::~ANDPolicy() -{ - // empty -} +ANDPolicy::~ANDPolicy() = default; void ANDPolicy::select(mbus::RoutingContext &context) @@ -29,11 +23,9 @@ ANDPolicy::select(mbus::RoutingContext &context) if (_hops.empty()) { context.addChildren(context.getAllRecipients()); } else { - for (std::vector<mbus::Hop>::iterator it = _hops.begin(); - it != _hops.end(); ++it) - { + for (auto & hop : _hops) { mbus::Route route = context.getRoute(); - route.setHop(0, *it); + route.setHop(0, hop); context.addChild(route); } } diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/asyncinitializationpolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/asyncinitializationpolicy.cpp index 82cda6c773f..21b09f32419 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/asyncinitializationpolicy.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/policies/asyncinitializationpolicy.cpp @@ -9,6 +9,7 @@ #include "asyncinitializationpolicy.h" #include <vespa/vespalib/util/threadstackexecutor.h> #include <vespa/messagebus/emptyreply.h> +#include <vespa/messagebus/error.h> #include <vespa/documentapi/messagebus/documentprotocol.h> #include <vespa/vespalib/text/stringtokenizer.h> @@ -50,20 +51,23 @@ AsyncInitializationPolicy::initSynchronous() _state = State::DONE; } +namespace { + mbus::Error -AsyncInitializationPolicy::currentPolicyInitError() const -{ +currentPolicyInitError(vespalib::stringref error) { // If an init error has been recorded for the last init attempt, report // it back until we've managed to successfully complete the init step. - if (_error.empty()) { + if (error.empty()) { return mbus::Error(DocumentProtocol::ERROR_NODE_NOT_READY, "Waiting to initialize policy"); } else { return mbus::Error(DocumentProtocol::ERROR_POLICY_FAILURE, - "Error when creating policy: " + _error); + "Error when creating policy: " + error); } } +} + void AsyncInitializationPolicy::select(mbus::RoutingContext& context) { @@ -87,7 +91,7 @@ AsyncInitializationPolicy::select(mbus::RoutingContext& context) if (_state != State::DONE) { auto reply = std::make_unique<mbus::EmptyReply>(); - reply->addError(currentPolicyInitError()); + reply->addError(currentPolicyInitError(_error)); context.setReply(std::move(reply)); return; } @@ -96,7 +100,7 @@ AsyncInitializationPolicy::select(mbus::RoutingContext& context) // deadlock (executor will stall until all its tasks have finished // executing, and any queued tasks would attempt to take the mutex // we're currently holding, deadlocking both threads). - _executor.reset(nullptr); + _executor.reset(); } doSelect(context); diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/asyncinitializationpolicy.h b/documentapi/src/vespa/documentapi/messagebus/policies/asyncinitializationpolicy.h index 0e30da8e7c8..3061eb3d337 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/asyncinitializationpolicy.h +++ b/documentapi/src/vespa/documentapi/messagebus/policies/asyncinitializationpolicy.h @@ -2,7 +2,6 @@ #pragma once #include <vespa/messagebus/routing/iroutingpolicy.h> -#include <vespa/messagebus/error.h> #include <vespa/vespalib/util/executor.h> #include <vespa/documentapi/common.h> #include <map> @@ -42,8 +41,6 @@ public: void needAsynchronousInit() { _syncInit = false; } private: - mbus::Error currentPolicyInitError() const; - class Task : public vespalib::Executor::Task { public: diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/loadbalancerpolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/loadbalancerpolicy.cpp index 3bf17213b26..feee7db8640 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/loadbalancerpolicy.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/policies/loadbalancerpolicy.cpp @@ -2,6 +2,7 @@ #include "loadbalancerpolicy.h" #include <vespa/messagebus/emptyreply.h> #include <vespa/messagebus/errorcode.h> +#include <vespa/messagebus/error.h> #include <vespa/messagebus/routing/ihopdirective.h> #include <vespa/messagebus/routing/routingcontext.h> #include <vespa/messagebus/routing/verbatimdirective.h> diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/roundrobinpolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/roundrobinpolicy.cpp index a58f3439df2..e780806c8c0 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/roundrobinpolicy.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/policies/roundrobinpolicy.cpp @@ -3,7 +3,7 @@ #include "roundrobinpolicy.h" #include <vespa/documentapi/messagebus/documentprotocol.h> #include <vespa/messagebus/emptyreply.h> -#include <vespa/messagebus/routing/verbatimdirective.h> +#include <vespa/messagebus/error.h> namespace documentapi { @@ -18,7 +18,7 @@ RoundRobinPolicy::RoundRobinPolicy(const string &) : _cache() {} -RoundRobinPolicy::~RoundRobinPolicy() {} +RoundRobinPolicy::~RoundRobinPolicy() = default; void RoundRobinPolicy::select(mbus::RoutingContext &ctx) diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp index e49d412fee1..3fc1df0352a 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp @@ -4,7 +4,7 @@ #include <vespa/document/base/documentid.h> #include <vespa/document/update/documentupdate.h> #include <vespa/messagebus/emptyreply.h> -#include <vespa/messagebus/routing/verbatimdirective.h> +#include <vespa/messagebus/error.h> #include <vespa/documentapi/documentapi.h> #include <vespa/vdslib/state/clusterstate.h> #include <vespa/vespalib/stllike/asciistream.h> diff --git a/documentapi/src/vespa/documentapi/messagebus/routablefactories60.h b/documentapi/src/vespa/documentapi/messagebus/routablefactories60.h index 0a997f3ffd9..579abbda291 100644 --- a/documentapi/src/vespa/documentapi/messagebus/routablefactories60.h +++ b/documentapi/src/vespa/documentapi/messagebus/routablefactories60.h @@ -137,7 +137,7 @@ public: virtual bool encodeBucketSpace(vespalib::stringref bucketSpace, vespalib::GrowableByteBuffer& buf) const; virtual string decodeBucketSpace(document::ByteBuffer&) const; public: - CreateVisitorMessageFactory() : DocumentMessageFactory() {} + CreateVisitorMessageFactory() noexcept : DocumentMessageFactory() {} }; class CreateVisitorReplyFactory : public DocumentReplyFactory { protected: @@ -164,7 +164,7 @@ public: DocumentMessage::UP doDecode(document::ByteBuffer &buf) const override; bool doEncode(const DocumentMessage &msg, vespalib::GrowableByteBuffer &buf) const override; public: - DocumentListMessageFactory(const document::DocumentTypeRepo &r) + DocumentListMessageFactory(const document::DocumentTypeRepo &r) noexcept : _repo(r) {} }; class DocumentListReplyFactory : public DocumentReplyFactory { @@ -224,14 +224,14 @@ public: DocumentReply::UP doDecode(document::ByteBuffer &buf) const override; bool doEncode(const DocumentReply &msg, vespalib::GrowableByteBuffer &buf) const override; public: - GetDocumentReplyFactory(const document::DocumentTypeRepo &r) : _repo(r) {} + GetDocumentReplyFactory(const document::DocumentTypeRepo &r) noexcept : _repo(r) {} }; class MapVisitorMessageFactory : public DocumentMessageFactory { protected: DocumentMessage::UP doDecode(document::ByteBuffer &buf) const override; bool doEncode(const DocumentMessage &msg, vespalib::GrowableByteBuffer &buf) const override; public: - MapVisitorMessageFactory() : DocumentMessageFactory() {} + MapVisitorMessageFactory() noexcept : DocumentMessageFactory() {} }; class MapVisitorReplyFactory : public DocumentReplyFactory { protected: @@ -248,7 +248,7 @@ public: bool doEncode(const DocumentMessage &msg, vespalib::GrowableByteBuffer &buf) const override; public: void decodeInto(PutDocumentMessage & msg, document::ByteBuffer & buf) const; - PutDocumentMessageFactory(const document::DocumentTypeRepo &r) : _repo(r) {} + PutDocumentMessageFactory(const document::DocumentTypeRepo &r) noexcept : _repo(r) {} }; class PutDocumentReplyFactory : public DocumentReplyFactory { protected: @@ -275,7 +275,7 @@ public: DocumentMessage::UP doDecode(document::ByteBuffer &buf) const override; bool doEncode(const DocumentMessage &msg, vespalib::GrowableByteBuffer &buf) const override; public: - RemoveLocationMessageFactory(const document::DocumentTypeRepo &r) : _repo(r) {} + RemoveLocationMessageFactory(const document::DocumentTypeRepo &r) noexcept : _repo(r) {} }; class RemoveLocationReplyFactory : public DocumentReplyFactory { protected: @@ -324,7 +324,7 @@ public: bool doEncode(const DocumentMessage &msg, vespalib::GrowableByteBuffer &buf) const override; public: void decodeInto(UpdateDocumentMessage & msg, document::ByteBuffer & buf) const; - UpdateDocumentMessageFactory(const document::DocumentTypeRepo &r) : _repo(r) {} + UpdateDocumentMessageFactory(const document::DocumentTypeRepo &r) noexcept : _repo(r) {} }; class UpdateDocumentReplyFactory : public DocumentReplyFactory { protected: diff --git a/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp b/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp index bc4fc80abad..fda3075fc28 100644 --- a/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp +++ b/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp @@ -1,6 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/messagebus/testlib/receptor.h> #include <vespa/messagebus/testlib/simpleprotocol.h> #include <vespa/messagebus/testlib/simplemessage.h> #include <vespa/messagebus/testlib/simplereply.h> @@ -24,6 +23,9 @@ Test::Main() SimpleProtocol protocol; EXPECT_TRUE(protocol.getName() == "Simple"); + EXPECT_EQUAL(152u, sizeof(Result)); + EXPECT_EQUAL(136u, sizeof(Error)); + EXPECT_EQUAL(56u, sizeof(Routable)); { // test protocol IRoutingPolicy::UP bogus = protocol.createPolicy("bogus", ""); @@ -32,7 +34,6 @@ Test::Main() TEST_FLUSH(); { // test SimpleMessage - EXPECT_EQUAL(56u, sizeof(Routable)); EXPECT_EQUAL(104u, sizeof(Message)); EXPECT_EQUAL(184u, sizeof(SimpleMessage)); auto msg = std::make_unique<SimpleMessage>("test"); @@ -52,7 +53,6 @@ Test::Main() TEST_FLUSH(); { // test SimpleReply - EXPECT_EQUAL(56u, sizeof(Routable)); EXPECT_EQUAL(96u, sizeof(Reply)); EXPECT_EQUAL(160u, sizeof(SimpleReply)); auto reply = std::make_unique<SimpleReply>("reply"); diff --git a/messagebus/src/vespa/messagebus/error.cpp b/messagebus/src/vespa/messagebus/error.cpp index 85c9b92eb19..85eec140db5 100644 --- a/messagebus/src/vespa/messagebus/error.cpp +++ b/messagebus/src/vespa/messagebus/error.cpp @@ -3,7 +3,7 @@ #include "errorcode.h" #include <vespa/vespalib/util/stringfmt.h> -using vespalib::make_string; +using vespalib::make_string_short::fmt; namespace mbus { @@ -13,9 +13,9 @@ Error::Error() _service() { } -Error::~Error() {} +Error::~Error() = default; -Error::Error(uint32_t c, const string &m, const string &s) +Error::Error(uint32_t c, vespalib::stringref m, vespalib::stringref s) : _code(c), _msg(m), _service(s) @@ -26,9 +26,9 @@ Error::toString() const { string name(ErrorCode::getName(_code)); if (name.empty()) { - name = make_string("%u", _code); + name = fmt("%u", _code); } - return make_string("[%s @ %s]: %s", name.c_str(), _service.empty() ? "localhost" : _service.c_str(), _msg.c_str()); + return fmt("[%s @ %s]: %s", name.c_str(), _service.empty() ? "localhost" : _service.c_str(), _msg.c_str()); } } // namespace mbus diff --git a/messagebus/src/vespa/messagebus/error.h b/messagebus/src/vespa/messagebus/error.h index 638463e3665..74936962113 100644 --- a/messagebus/src/vespa/messagebus/error.h +++ b/messagebus/src/vespa/messagebus/error.h @@ -36,7 +36,7 @@ public: * @param m error message * @param s error service **/ - Error(uint32_t c, const string &m, const string &s = ""); + Error(uint32_t c, vespalib::stringref m, vespalib::stringref s = ""); /** * Obtain the error code of this error. diff --git a/messagebus/src/vespa/messagebus/message.cpp b/messagebus/src/vespa/messagebus/message.cpp index 305ffb06aa0..8d17fd6b0ff 100644 --- a/messagebus/src/vespa/messagebus/message.cpp +++ b/messagebus/src/vespa/messagebus/message.cpp @@ -5,6 +5,7 @@ #include "ireplyhandler.h" #include "emptyreply.h" #include "errorcode.h" +#include "error.h" #include <vespa/vespalib/util/backtrace.h> #include <vespa/log/log.h> diff --git a/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp b/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp index a8f7b47f3e3..775c90ea9ee 100644 --- a/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp +++ b/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp @@ -4,6 +4,7 @@ #include "rpcnetwork.h" #include "rpcserviceaddress.h" #include <vespa/messagebus/emptyreply.h> +#include <vespa/messagebus/error.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/fnet/frt/reflection.h> diff --git a/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp b/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp index 16175fea1c2..a2e3310046d 100644 --- a/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp +++ b/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp @@ -4,6 +4,7 @@ #include "rpcnetwork.h" #include "rpcserviceaddress.h" #include <vespa/messagebus/emptyreply.h> +#include <vespa/messagebus/error.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/databuffer.h> @@ -249,7 +250,7 @@ RPCSendV2::createResponse(FRT_Values & ret, const string & version, Reply & repl Cursor & error = array.addObject(); error.setLong(CODE_F, reply.getError(i).getCode()); error.setString(MSG_F, reply.getError(i).getMessage()); - error.setString(SERVICE_F, reply.getError(i).getService().c_str()); + error.setString(SERVICE_F, reply.getError(i).getService()); } } @@ -263,7 +264,6 @@ RPCSendV2::createResponse(FRT_Values & ret, const string & version, Reply & repl ret.AddInt32(toCompress.size()); assert(buf.getDataLen() <= INT32_MAX); ret.AddData(std::move(buf)); - } } // namespace mbus diff --git a/messagebus/src/vespa/messagebus/reply.cpp b/messagebus/src/vespa/messagebus/reply.cpp index bf12698c86f..68bb7db1191 100644 --- a/messagebus/src/vespa/messagebus/reply.cpp +++ b/messagebus/src/vespa/messagebus/reply.cpp @@ -5,6 +5,7 @@ #include "ireplyhandler.h" #include "message.h" #include "tracelevel.h" +#include "error.h" #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/util/backtrace.h> @@ -54,12 +55,6 @@ Reply::swapState(Routable &rhs) } } -bool -Reply::isReply() const -{ - return true; -} - void Reply::addError(const Error &e) { @@ -72,16 +67,25 @@ Reply::addError(const Error &e) bool Reply::hasFatalErrors() const { - for (std::vector<Error>::const_iterator it = _errors.begin(); - it != _errors.end(); ++it) + for (const Error & error : _errors) { - if (it->getCode() >= ErrorCode::FATAL_ERROR) { + if (error.getCode() >= ErrorCode::FATAL_ERROR) { return true; } } return false; } +const Error & +Reply::getError(uint32_t i) const { + return _errors[i]; +} + +uint32_t +Reply::getNumErrors() const { + return _errors.size(); +} + void Reply::setMessage(Message::UP msg) { _msg = std::move(msg); diff --git a/messagebus/src/vespa/messagebus/reply.h b/messagebus/src/vespa/messagebus/reply.h index 64b9f5c0b13..72493ec0ae6 100644 --- a/messagebus/src/vespa/messagebus/reply.h +++ b/messagebus/src/vespa/messagebus/reply.h @@ -1,13 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include "error.h" #include "routable.h" #include <vector> namespace mbus { class Message; +class Error; /** * A reply is a response to a message that has been sent throught the @@ -43,7 +43,7 @@ public: ~Reply() override; void swapState(Routable &rhs) override; - bool isReply() const override; + bool isReply() const override { return true; } /** * Add an Error to this Reply @@ -72,14 +72,14 @@ public: * @param i The index of the error to return. * @return The error at the given index. */ - const Error &getError(uint32_t i) const { return _errors[i]; } + const Error &getError(uint32_t i) const; /** * Returns the number of errors that this reply contains. * * @return The number of replies. */ - uint32_t getNumErrors() const { return _errors.size(); } + uint32_t getNumErrors() const; /** * Attach a Message to this Reply. If a Reply contains errors, messagebus diff --git a/messagebus/src/vespa/messagebus/result.cpp b/messagebus/src/vespa/messagebus/result.cpp index e99120d8c2d..970bb72e638 100644 --- a/messagebus/src/vespa/messagebus/result.cpp +++ b/messagebus/src/vespa/messagebus/result.cpp @@ -1,80 +1,22 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "result.h" +#include "message.h" namespace mbus { -Result::Handover::Handover(bool a, const Error &e, Message *m) - : _accepted(a), - _error(e), - _msg(m) -{ } - Result::Result() : _accepted(true), _error(), _msg() { } -Result::Result(const Error &err, Message::UP msg) +Result::Result(const Error &err, std::unique_ptr<Message> msg) : _accepted(false), _error(err), _msg(std::move(msg)) { } -Result::Result(Result &&rhs) - : _accepted(rhs._accepted), - _error(rhs._error), - _msg(std::move(rhs._msg)) -{ } - -Result::Result(const Handover &rhs) - : _accepted(rhs._accepted), - _error(rhs._error), - _msg(rhs._msg) -{ } - -Result::~Result() {} - -bool -Result::isAccepted() const -{ - return _accepted; -} - -const Error & -Result::getError() const -{ - return _error; -} - -Message::UP -Result::getMessage() -{ - return std::move(_msg); -} - -Result::operator Handover() -{ - return Handover(_accepted, _error, _msg.release()); -} - -Result & -Result::operator=(Result &&rhs) -{ - _accepted = rhs._accepted; - _error = rhs._error; - _msg = std::move(rhs._msg); - return *this; -} - -Result & -Result::operator=(const Handover &rhs) -{ - _accepted = rhs._accepted; - _error = rhs._error; - _msg.reset(rhs._msg); - return *this; -} +Result::~Result() = default; } // namespace mbus diff --git a/messagebus/src/vespa/messagebus/result.h b/messagebus/src/vespa/messagebus/result.h index adb9c1ea81a..dcf6e4964e5 100644 --- a/messagebus/src/vespa/messagebus/result.h +++ b/messagebus/src/vespa/messagebus/result.h @@ -3,10 +3,11 @@ #pragma once #include "error.h" -#include "message.h" namespace mbus { +class Message; + /** * A Result object is used as return value when trying to send a * Message on a SourceSession. It says whether messagebus has accepted @@ -24,24 +25,10 @@ class Result private: bool _accepted; Error _error; - Message::UP _msg; + std::unique_ptr<Message> _msg; public: /** - * This inner class is used to implement destructive copy for - * return values. - **/ - class Handover { - friend class Result; - bool _accepted; - Error _error; - Message *_msg; - Handover(bool a, const Error &e, Message *m); - Handover(const Handover &); // not implemented - Handover &operator=(const Handover &); // not implemented - }; - - /** * Create a Result indicating that messagebus has accepted the * Message. **/ @@ -54,23 +41,11 @@ public: * @param err the reason for not accepting the Message * @param msg the message that did not get accepted **/ - Result(const Error &err, Message::UP msg); + Result(const Error &err, std::unique_ptr<Message> msg); - /** - * Move constructor - * - * @param rhs the original object - **/ - Result(Result &&rhs); - - /** - * Construct a new Result from an internal Handover object that - * has destructed the original Result. - * - * @param rhs handover object - **/ - Result(const Handover &rhs); + Result(Result &&rhs) = default; + Result &operator=(Result &&rhs) = default; ~Result(); /** @@ -78,14 +53,14 @@ public: * * @return true if the Message was accepted **/ - bool isAccepted() const; + bool isAccepted() const { return _accepted; } /** * Obtain the error causing the message not to be accepted. * * @return error **/ - const Error &getError() const; + const Error &getError() const { return _error; } /** * If the message was not accepted, this method may be used to get @@ -95,28 +70,7 @@ public: * * @return the Message that was not accepted **/ - Message::UP getMessage(); - - /** - * Perform an implicit typecast to support destructive copy of - * return values. - **/ - operator Handover(); - - /** - * Moving assignment operator - * - * @param rhs the original object - **/ - Result &operator=(Result &&rhs); - - /** - * Assign a Result from an internal Handover object that has - * destructed the original Result. - * - * @param rhs handover object - **/ - Result &operator=(const Handover &rhs); + std::unique_ptr<Message> getMessage() { return std::move(_msg); } }; } // namespace mbus diff --git a/messagebus_test/src/tests/speed/cpp-client.cpp b/messagebus_test/src/tests/speed/cpp-client.cpp index 2c20b35c597..d7e120680d4 100644 --- a/messagebus_test/src/tests/speed/cpp-client.cpp +++ b/messagebus_test/src/tests/speed/cpp-client.cpp @@ -7,6 +7,7 @@ #include <vespa/messagebus/testlib/simplemessage.h> #include <vespa/messagebus/testlib/simpleprotocol.h> #include <vespa/messagebus/testlib/simplereply.h> +#include <vespa/messagebus/error.h> #include <vespa/vespalib/util/time.h> #include <thread> #include <vespa/fastos/app.h> @@ -76,7 +77,7 @@ Client::handleReply(Reply::UP reply) { for (uint32_t i = 0; i < reply->getNumErrors(); ++i) { fprintf(stderr, "ERR[%d]: code=%d, msg=%s\n", i, reply->getError(i).getCode(), - reply->getError(i).getMessage().c_str()); + reply->getError(i).getMessage().data()); } std::lock_guard guard(_lock); ++_failCnt; diff --git a/storage/src/tests/storageserver/documentapiconvertertest.cpp b/storage/src/tests/storageserver/documentapiconvertertest.cpp index bc52d7508dc..a1293a842c1 100644 --- a/storage/src/tests/storageserver/documentapiconvertertest.cpp +++ b/storage/src/tests/storageserver/documentapiconvertertest.cpp @@ -10,6 +10,7 @@ #include <vespa/document/update/documentupdate.h> #include <vespa/documentapi/documentapi.h> #include <vespa/messagebus/emptyreply.h> +#include <vespa/messagebus/error.h> #include <vespa/storage/common/bucket_resolver.h> #include <vespa/storage/storageserver/documentapiconverter.h> #include <vespa/storageapi/message/datagram.h> diff --git a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp index f0c987ee333..760fde8323a 100644 --- a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp +++ b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp @@ -14,6 +14,7 @@ #include <vespa/storageapi/message/searchresult.h> #include <vespa/storageapi/message/stat.h> #include <vespa/storageapi/message/visitor.h> +#include <vespa/messagebus/error.h> #include <vespa/log/log.h> LOG_SETUP(".documentapiconverter"); diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp index 5e8e6809422..7225b0fd1ed 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp +++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp @@ -5,6 +5,7 @@ #include <vespa/storage/common/nodestateupdater.h> #include <vespa/storage/persistence/messages.h> #include <vespa/messagebus/message.h> +#include <vespa/messagebus/error.h> #include <vespa/config/common/exceptions.h> #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/vespalib/util/stringfmt.h> diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_2.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_2.cpp index b6d0d297a69..ee577881d82 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_2.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_2.cpp @@ -5,8 +5,7 @@ #include "storagecommand.h" #include "serializationhelper.h" -namespace storage { -namespace mbusprot { +namespace storage::mbusprot { using documentapi::TestAndSetCondition; @@ -62,5 +61,4 @@ void ProtocolSerialization5_2::encodeTasCondition(GBBuf & buf, const api::Storag buf.putString(cmd.getCondition().getSelection()); } -} // mbusprot -} // storage +} diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_2.h b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_2.h index 42e9c17e192..f6f9443248c 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_2.h +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_2.h @@ -7,8 +7,7 @@ #include <vespa/vespalib/util/growablebytebuffer.h> #include <vespa/storageapi/message/persistence.h> -namespace storage { -namespace mbusprot { +namespace storage::mbusprot { class ProtocolSerialization5_2 : public ProtocolSerialization5_1 { @@ -31,5 +30,4 @@ protected: static void encodeTasCondition(GBBuf & buf, const api::StorageCommand & cmd); }; -} // mbusprot -} // storage +} diff --git a/storageserver/src/tests/storageservertest.cpp b/storageserver/src/tests/storageservertest.cpp index 2f165c69470..363dc6240dd 100644 --- a/storageserver/src/tests/storageservertest.cpp +++ b/storageserver/src/tests/storageservertest.cpp @@ -5,6 +5,7 @@ #include <vespa/storage/storageserver/servicelayernode.h> #include <vespa/storageserver/app/distributorprocess.h> #include <vespa/storageserver/app/dummyservicelayerprocess.h> +#include <vespa/messagebus/message.h> #include <vespa/vespalib/gtest/gtest.h> #include <vespa/log/log.h> @@ -21,7 +22,7 @@ struct StorageServerTest : public ::testing::Test { std::unique_ptr<vdstestlib::DirConfig> storConfig; StorageServerTest(); - ~StorageServerTest(); + ~StorageServerTest() override; void SetUp() override; void TearDown() override; @@ -34,7 +35,7 @@ StorageServerTest::~StorageServerTest() = default; namespace { struct Node { - virtual ~Node() {} + virtual ~Node() = default; virtual StorageNode& getNode() = 0; virtual StorageNodeContext& getContext() = 0; }; @@ -43,10 +44,10 @@ struct Distributor : public Node { DistributorProcess _process; Distributor(vdstestlib::DirConfig& config); - ~Distributor(); + ~Distributor() override; - virtual StorageNode& getNode() override { return _process.getNode(); } - virtual StorageNodeContext& getContext() override { return _process.getContext(); } + StorageNode& getNode() override { return _process.getNode(); } + StorageNodeContext& getContext() override { return _process.getContext(); } }; struct Storage : public Node { @@ -54,10 +55,10 @@ struct Storage : public Node { StorageComponent::UP _component; Storage(vdstestlib::DirConfig& config); - ~Storage(); + ~Storage() override; - virtual StorageNode& getNode() override { return _process.getNode(); } - virtual StorageNodeContext& getContext() override { return _process.getContext(); } + StorageNode& getNode() override { return _process.getNode(); } + StorageNodeContext& getContext() override { return _process.getContext(); } }; Distributor::Distributor(vdstestlib::DirConfig& config) @@ -74,8 +75,8 @@ Storage::Storage(vdstestlib::DirConfig& config) { _process.setupConfig(60000ms); _process.createNode(); - _component.reset(new StorageComponent( - getContext().getComponentRegister(), "test")); + _component = std::make_unique<StorageComponent>( + getContext().getComponentRegister(), "test"); } Storage::~Storage() = default; @@ -87,9 +88,9 @@ StorageServerTest::SetUp() { [[maybe_unused]] int systemResult = system("chmod -R 755 vdsroot"); systemResult = system("rm -rf vdsroot*"); - slobrok.reset(new mbus::Slobrok); - distConfig.reset(new vdstestlib::DirConfig(getStandardConfig(false))); - storConfig.reset(new vdstestlib::DirConfig(getStandardConfig(true))); + slobrok = std::make_unique<mbus::Slobrok>(); + distConfig = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(false)); + storConfig = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true)); addSlobrokConfig(*distConfig, *slobrok); addSlobrokConfig(*storConfig, *slobrok); storConfig->getConfig("stor-filestor").set("fail_disk_after_error_count", "1"); diff --git a/vespalib/src/vespa/vespalib/data/slime/cursor.h b/vespalib/src/vespa/vespalib/data/slime/cursor.h index 6815ad3ba83..34d7028a027 100644 --- a/vespalib/src/vespa/vespalib/data/slime/cursor.h +++ b/vespalib/src/vespa/vespalib/data/slime/cursor.h @@ -5,8 +5,7 @@ #include "inspector.h" #include "external_memory.h" -namespace vespalib { -namespace slime { +namespace vespalib::slime { struct Cursor : public Inspector { virtual Cursor &operator[](size_t idx) const override = 0; @@ -46,6 +45,4 @@ struct Cursor : public Inspector { virtual Symbol resolve(Memory symbol_name) = 0; }; -} // namespace vespalib::slime -} // namespace vespalib - +} |