diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-02-03 08:46:49 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2023-02-03 08:46:49 +0000 |
commit | ab0f053be2cdb4f36b8866ca7c9ba557a676539d (patch) | |
tree | d76376ec569e7939f4654cdbbe4568f9182eb53f /messagebus | |
parent | 2b951244de137a3321c6aa70f52389664cab8efc (diff) |
- Use T && f() && to avoid moving temporaries.
- std::make_unique/make_shared
Diffstat (limited to 'messagebus')
29 files changed, 355 insertions, 437 deletions
diff --git a/messagebus/src/tests/advancedrouting/advancedrouting.cpp b/messagebus/src/tests/advancedrouting/advancedrouting.cpp index 1711122df8a..9b402de61e7 100644 --- a/messagebus/src/tests/advancedrouting/advancedrouting.cpp +++ b/messagebus/src/tests/advancedrouting/advancedrouting.cpp @@ -117,13 +117,13 @@ Test::testAdvanced(TestData &data) { const duration TIMEOUT = 60s; IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); + auto &simple = dynamic_cast<SimpleProtocol&>(*protocol); simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory(false, ErrorCode::NO_ADDRESS_FOR_SERVICE))); data._srcServer.mb.putProtocol(protocol); data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) .addHop(HopSpec("bar", "dst/bar")) .addHop(HopSpec("baz", "dst/baz")) - .addRoute(std::move(RouteSpec("baz").addHop("baz"))))); + .addRoute(RouteSpec("baz").addHop("baz")))); string route = vespalib::make_string("[Custom:%s,bar,route:baz,dst/cox,?dst/unknown]", data._fooSession->getConnectionSpec().c_str()); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse(route)).isAccepted()); @@ -134,13 +134,13 @@ Test::testAdvanced(TestData &data) data._fooSession->acknowledge(std::move(msg)); msg = data._barHandler.getMessage(TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::TRANSIENT_ERROR, "bar")); data._barSession->reply(std::move(reply)); msg = data._bazHandler.getMessage(TIMEOUT); ASSERT_TRUE(msg); - reply.reset(new EmptyReply()); + reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::TRANSIENT_ERROR, "baz1")); data._bazSession->reply(std::move(reply)); @@ -153,7 +153,7 @@ Test::testAdvanced(TestData &data) data._barSession->acknowledge(std::move(msg)); msg = data._bazHandler.getMessage(TIMEOUT); ASSERT_TRUE(msg); - reply.reset(new EmptyReply()); + reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::TRANSIENT_ERROR, "baz2")); data._bazSession->reply(std::move(reply)); @@ -165,7 +165,7 @@ Test::testAdvanced(TestData &data) ASSERT_FALSE(msg); msg = data._bazHandler.getMessage(TIMEOUT); ASSERT_TRUE(msg); - reply.reset(new EmptyReply()); + reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::FATAL_ERROR, "baz3")); data._bazSession->reply(std::move(reply)); diff --git a/messagebus/src/tests/configagent/configagent.cpp b/messagebus/src/tests/configagent/configagent.cpp index 06940c682f0..dee50cbe365 100644 --- a/messagebus/src/tests/configagent/configagent.cpp +++ b/messagebus/src/tests/configagent/configagent.cpp @@ -18,17 +18,19 @@ private: bool checkTables(uint32_t numTables); public: - ~Test() {} + ~Test() override; int Main() override; - bool setupRouting(const RoutingSpec &spec) override; + bool setupRouting(RoutingSpec spec) override; }; +Test::~Test() = default; + TEST_APPHOOK(Test); bool -Test::setupRouting(const RoutingSpec &spec) +Test::setupRouting(RoutingSpec spec) { - _spec = spec; + _spec = std::move(spec); return true; } diff --git a/messagebus/src/tests/context/context.cpp b/messagebus/src/tests/context/context.cpp index e91946aab05..b60dffddd79 100644 --- a/messagebus/src/tests/context/context.cpp +++ b/messagebus/src/tests/context/context.cpp @@ -33,7 +33,7 @@ RoutingSpec getRouting() { return RoutingSpec() .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("test", "test/session")) - .addRoute(std::move(RouteSpec("test").addHop("test")))); + .addRoute(RouteSpec("test").addHop("test"))); } TEST_SETUP(Test); diff --git a/messagebus/src/tests/error/error.cpp b/messagebus/src/tests/error/error.cpp index d97440f12e0..da518f959c0 100644 --- a/messagebus/src/tests/error/error.cpp +++ b/messagebus/src/tests/error/error.cpp @@ -20,7 +20,7 @@ RoutingSpec getRouting() { .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("pxy", "test/pxy/session")) .addHop(HopSpec("dst", "test/dst/session")) - .addRoute(std::move(RouteSpec("test").addHop("pxy").addHop("dst")))); + .addRoute(RouteSpec("test").addHop("pxy").addHop("dst"))); } TEST("error_test") { diff --git a/messagebus/src/tests/messagebus/messagebus.cpp b/messagebus/src/tests/messagebus/messagebus.cpp index ba466c4210e..932c2dcc073 100644 --- a/messagebus/src/tests/messagebus/messagebus.cpp +++ b/messagebus/src/tests/messagebus/messagebus.cpp @@ -30,14 +30,14 @@ struct Base { return RoutingSpec() .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("DocProc", "docproc/*/session")) - .addHop(std::move(HopSpec("Search", "search/[All]/[Hash]/session") + .addHop(HopSpec("Search", "search/[All]/[Hash]/session") .addRecipient("search/r.0/c.0/session") .addRecipient("search/r.0/c.1/session") .addRecipient("search/r.1/c.0/session") - .addRecipient("search/r.1/c.1/session"))) - .addRoute(std::move(RouteSpec("Index").addHop("DocProc").addHop("Search"))) - .addRoute(std::move(RouteSpec("DocProc").addHop("DocProc"))) - .addRoute(std::move(RouteSpec("Search").addHop("Search")))); + .addRecipient("search/r.1/c.1/session")) + .addRoute(RouteSpec("Index").addHop("DocProc").addHop("Search")) + .addRoute(RouteSpec("DocProc").addHop("DocProc")) + .addRoute(RouteSpec("Search").addHop("Search"))); } bool waitQueueSize(uint32_t size) { for (uint32_t i = 0; i < 1000; ++i) { diff --git a/messagebus/src/tests/messageordering/messageordering.cpp b/messagebus/src/tests/messageordering/messageordering.cpp index d83ef59be28..e2bfd284906 100644 --- a/messagebus/src/tests/messageordering/messageordering.cpp +++ b/messagebus/src/tests/messageordering/messageordering.cpp @@ -23,7 +23,7 @@ getRouting() return RoutingSpec() .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("dst", "test/dst/session")) - .addRoute(std::move(RouteSpec("test").addHop("dst")))); + .addRoute(RouteSpec("test").addHop("dst"))); } class MultiReceptor : public IMessageHandler diff --git a/messagebus/src/tests/routing/routing.cpp b/messagebus/src/tests/routing/routing.cpp index 2c4056b868d..6cbcb58b24a 100644 --- a/messagebus/src/tests/routing/routing.cpp +++ b/messagebus/src/tests/routing/routing.cpp @@ -14,6 +14,9 @@ #include <vespa/vespalib/testkit/testapp.h> #include <vespa/log/log.h> + +#include <memory> +#include <utility> LOG_SETUP("routing_test"); using namespace mbus; @@ -53,21 +56,19 @@ private: uint32_t _idxRemove; public: RemoveReplyPolicy(bool selectOnRetry, - const std::vector<uint32_t> consumableErrors, - const std::vector<Route> routes, + std::vector<uint32_t> consumableErrors, + std::vector<Route> routes, uint32_t idxRemove); void merge(RoutingContext &ctx) override; }; RemoveReplyPolicy::RemoveReplyPolicy(bool selectOnRetry, - const std::vector<uint32_t> consumableErrors, - const std::vector<Route> routes, + std::vector<uint32_t> consumableErrors, + std::vector<Route> routes, uint32_t idxRemove) : - CustomPolicy::CustomPolicy(selectOnRetry, consumableErrors, routes), + CustomPolicy::CustomPolicy(selectOnRetry, std::move(consumableErrors), std::move(routes)), _idxRemove(idxRemove) -{ - // empty -} +{ } void RemoveReplyPolicy::merge(RoutingContext &ctx) @@ -82,7 +83,7 @@ private: uint32_t _idxRemove; public: RemoveReplyPolicyFactory(bool selectOnRetry, - const std::vector<uint32_t> &consumableErrors, + std::vector<uint32_t> consumableErrors, uint32_t idxRemove); ~RemoveReplyPolicyFactory() override; IRoutingPolicy::UP create(const string ¶m) override; @@ -91,10 +92,10 @@ public: RemoveReplyPolicyFactory::~RemoveReplyPolicyFactory() = default; RemoveReplyPolicyFactory::RemoveReplyPolicyFactory(bool selectOnRetry, - const std::vector<uint32_t> &consumableErrors, + std::vector<uint32_t> consumableErrors, uint32_t idxRemove) : _selectOnRetry(selectOnRetry), - _consumableErrors(consumableErrors), + _consumableErrors(std::move(consumableErrors)), _idxRemove(idxRemove) { // empty @@ -103,39 +104,30 @@ RemoveReplyPolicyFactory::RemoveReplyPolicyFactory(bool selectOnRetry, IRoutingPolicy::UP RemoveReplyPolicyFactory::create(const string ¶m) { - std::vector<Route> routes; - CustomPolicyFactory::parseRoutes(param, routes); - return IRoutingPolicy::UP(new RemoveReplyPolicy(_selectOnRetry, _consumableErrors, routes, _idxRemove)); + return std::make_unique<RemoveReplyPolicy>(_selectOnRetry, _consumableErrors, + CustomPolicyFactory::parseRoutes(param), _idxRemove); } class ReuseReplyPolicy : public CustomPolicy { private: std::vector<uint32_t> _errorMask; public: - ReuseReplyPolicy(bool selectOnRetry, - const std::vector<uint32_t> &errorMask, - const std::vector<Route> &routes); + ReuseReplyPolicy(bool selectOnRetry, std::vector<uint32_t> errorMask, std::vector<Route> routes); void merge(RoutingContext &ctx) override; }; -ReuseReplyPolicy::ReuseReplyPolicy(bool selectOnRetry, - const std::vector<uint32_t> &errorMask, - const std::vector<Route> &routes) : - CustomPolicy::CustomPolicy(selectOnRetry, errorMask, routes), - _errorMask(errorMask) -{ - // empty -} +ReuseReplyPolicy::ReuseReplyPolicy(bool selectOnRetry, std::vector<uint32_t> errorMask, std::vector<Route> routes) : + CustomPolicy::CustomPolicy(selectOnRetry, errorMask, std::move(routes)), + _errorMask(std::move(errorMask)) +{ } void ReuseReplyPolicy::merge(RoutingContext &ctx) { - Reply::UP ret(new EmptyReply()); + auto ret = std::make_unique<EmptyReply>(); uint32_t idx = 0; int idxFirstOk = -1; - for (RoutingNodeIterator it = ctx.getChildIterator(); - it.isValid(); it.next(), ++idx) - { + for (RoutingNodeIterator it = ctx.getChildIterator(); it.isValid(); it.next(), ++idx) { const Reply &ref = it.getReplyRef(); if (!ref.hasErrors()) { if (idxFirstOk < 0) { @@ -181,9 +173,7 @@ ReuseReplyPolicyFactory::~ReuseReplyPolicyFactory() = default; IRoutingPolicy::UP ReuseReplyPolicyFactory::create(const string ¶m) { - std::vector<Route> routes; - CustomPolicyFactory::parseRoutes(param, routes); - return IRoutingPolicy::UP(new ReuseReplyPolicy(_selectOnRetry, _errorMask, routes)); + return std::make_unique<ReuseReplyPolicy>(_selectOnRetry, _errorMask, CustomPolicyFactory::parseRoutes(param)); } class SetReplyPolicy : public IRoutingPolicy { @@ -219,7 +209,7 @@ SetReplyPolicy::select(RoutingContext &ctx) if (err != ErrorCode::NONE) { ctx.setError(err, _param); } else { - ctx.setReply(Reply::UP(new EmptyReply())); + ctx.setReply(std::make_unique<EmptyReply>()); } ctx.setSelectOnRetry(_selectOnRetry); } @@ -227,7 +217,7 @@ SetReplyPolicy::select(RoutingContext &ctx) void SetReplyPolicy::merge(RoutingContext &ctx) { - Reply::UP reply(new EmptyReply()); + auto reply = std::make_unique<EmptyReply>(); reply->addError(Error(ErrorCode::FATAL_ERROR, "Merge should not be called when select() sets a reply.")); ctx.setReply(std::move(reply)); } @@ -256,11 +246,11 @@ SetReplyPolicyFactory::~SetReplyPolicyFactory() = default; IRoutingPolicy::UP SetReplyPolicyFactory::create(const string ¶m) { - return IRoutingPolicy::UP(new SetReplyPolicy(_selectOnRetry, _errors, param)); + return std::make_unique<SetReplyPolicy>(_selectOnRetry, _errors, param); } class TestException : public std::exception { - virtual const char* what() const throw() override { + virtual const char* what() const noexcept override { return "{test exception}"; } }; @@ -280,9 +270,8 @@ public: class SelectExceptionPolicyFactory : public SimpleProtocol::IPolicyFactory { public: ~SelectExceptionPolicyFactory() override; - IRoutingPolicy::UP create(const string ¶m) override { - (void)param; - return IRoutingPolicy::UP(new SelectExceptionPolicy()); + IRoutingPolicy::UP create(const string &) override { + return std::make_unique<SelectExceptionPolicy>(); } }; @@ -293,18 +282,15 @@ private: const string _select; public: - MergeExceptionPolicy(const string ¶m) + explicit MergeExceptionPolicy(const string ¶m) : _select(param) - { - // empty - } + { } void select(RoutingContext &ctx) override { ctx.addChild(Route::parse(_select)); } - void merge(RoutingContext &ctx) override { - (void)ctx; + void merge(RoutingContext &) override { throw TestException(); } }; @@ -313,7 +299,7 @@ class MergeExceptionPolicyFactory : public SimpleProtocol::IPolicyFactory { public: ~MergeExceptionPolicyFactory() override; IRoutingPolicy::UP create(const string ¶m) override { - return IRoutingPolicy::UP(new MergeExceptionPolicy(param)); + return std::make_unique<MergeExceptionPolicy>(param); } }; @@ -331,37 +317,17 @@ private: public: friend class MyPolicy; - MyPolicyFactory(const string &selectRoute, - uint32_t &selectError, - bool selectException, - bool mergeFromChild, - uint32_t mergeError, - bool mergeException) : - _selectRoute(selectRoute), - _selectError(selectError), - _selectException(selectException), - _mergeFromChild(mergeFromChild), - _mergeError(mergeError), - _mergeException(mergeException) - { - // empty - } + MyPolicyFactory(const string &selectRoute, uint32_t &selectError, bool selectException, + bool mergeFromChild, uint32_t mergeError, bool mergeException) noexcept; + ~MyPolicyFactory() override; - IRoutingPolicy::UP - create(const string ¶m) override; + IRoutingPolicy::UP create(const string ¶m) override; - static MyPolicyFactory::SP - newInstance(const string &selectRoute, - uint32_t selectError, - bool selectException, - bool mergeFromChild, - uint32_t mergeError, - bool mergeException) + static MyPolicyFactory::SP newInstance(const string &selectRoute, uint32_t selectError, bool selectException, + bool mergeFromChild, uint32_t mergeError, bool mergeException) { - MyPolicyFactory::SP ptr; - ptr.reset(new MyPolicyFactory(selectRoute, selectError, selectException, - mergeFromChild, mergeError, mergeException)); - return ptr; + return std::make_shared<MyPolicyFactory>(selectRoute, selectError, selectException, + mergeFromChild, mergeError, mergeException); } static MyPolicyFactory::SP @@ -419,12 +385,24 @@ public: } }; +MyPolicyFactory::MyPolicyFactory(const string &selectRoute, uint32_t &selectError, bool selectException, + bool mergeFromChild, uint32_t mergeError, bool mergeException) noexcept + : _selectRoute(selectRoute), + _selectError(selectError), + _selectException(selectException), + _mergeFromChild(mergeFromChild), + _mergeError(mergeError), + _mergeException(mergeException) +{ } + +MyPolicyFactory::~MyPolicyFactory() = default; + class MyPolicy : public IRoutingPolicy { private: const MyPolicyFactory &_parent; public: - MyPolicy(const MyPolicyFactory &parent) : + explicit MyPolicy(const MyPolicyFactory &parent) : _parent(parent) {} @@ -434,7 +412,7 @@ public: ctx.addChild(Route::parse(_parent._selectRoute)); } if (_parent._selectError != ErrorCode::NONE) { - Reply::UP reply(new EmptyReply()); + auto reply = std::make_unique<EmptyReply>(); reply->addError(Error(_parent._selectError, "err")); ctx.setReply(std::move(reply)); } @@ -446,7 +424,7 @@ public: void merge(RoutingContext &ctx) override { if (_parent._mergeError != ErrorCode::NONE) { - Reply::UP reply(new EmptyReply()); + auto reply = std::make_unique<EmptyReply>(); reply->addError(Error(_parent._mergeError, "err")); ctx.setReply(std::move(reply)); } else if (_parent._mergeFromChild) { @@ -459,10 +437,9 @@ public: }; IRoutingPolicy::UP -MyPolicyFactory::create(const string ¶m) +MyPolicyFactory::create(const string &) { - (void)param; - return IRoutingPolicy::UP(new MyPolicy(*this)); + return std::make_unique<MyPolicy>(*this); } //////////////////////////////////////////////////////////////////////////////// @@ -491,10 +468,10 @@ public: class Test : public vespalib::TestApp { private: Message::UP createMessage(const string &msg, uint32_t level = 9); - void setupRouting(TestData &data, const RoutingTableSpec &spec); + void setupRouting(TestData &data, RoutingTableSpec && spec); void setupPolicy(TestData &data, const string &policyName, SimpleProtocol::IPolicyFactory::SP policy); - bool testAcknowledge(TestData &data); + static bool testAcknowledge(TestData &data); bool testSend(TestData &data, const string &route, uint32_t level = 9); bool testTrace(TestData &data, const std::vector<string> &expected); bool testTrace(const std::vector<string> &expected, const Trace &trace); @@ -603,24 +580,24 @@ TestData::start() Message::UP Test::createMessage(const string &msg, uint32_t level) { - Message::UP ret(new SimpleMessage(msg)); + auto ret = std::make_unique<SimpleMessage>(msg); ret->getTrace().setLevel(level); return ret; } void -Test::setupRouting(TestData &data, const RoutingTableSpec &spec) +Test::setupRouting(TestData &data, RoutingTableSpec && spec) { - data._srcServer.mb.setupRouting(RoutingSpec().addTable(spec)); + data._srcServer.mb.setupRouting(RoutingSpec().addTable(std::move(spec))); } void Test::setupPolicy(TestData &data, const string &policyName, SimpleProtocol::IPolicyFactory::SP policy) { - IProtocol::SP ptr(new SimpleProtocol()); - static_cast<SimpleProtocol&>(*ptr).addPolicyFactory(policyName, policy); - data._srcServer.mb.putProtocol(ptr); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory(policyName, std::move(policy)); + data._srcServer.mb.putProtocol(protocol); } bool @@ -649,7 +626,7 @@ Test::testTrace(TestData &data, const std::vector<string> &expected) if (!EXPECT_TRUE(reply)) { return false; } - if (!EXPECT_TRUE(!reply->hasErrors())) { + if (!EXPECT_FALSE(reply->hasErrors())) { return false; } return testTrace(expected, reply->getTrace()); @@ -658,7 +635,7 @@ Test::testTrace(TestData &data, const std::vector<string> &expected) bool Test::testTrace(const std::vector<string> &expected, const Trace &trace) { - string version = vespalib::Vtag::currentVersion.toString(); + const string& version = vespalib::Vtag::currentVersion.toString(); string actual = trace.toString(); size_t pos = 0; for (uint32_t i = 0; i < expected.size(); ++i) { @@ -758,7 +735,7 @@ void Test::testNoRoutingTable(TestData &data) { Result res = data._srcSession->send(createMessage("msg"), "foo"); - EXPECT_TRUE(!res.isAccepted()); + EXPECT_FALSE(res.isAccepted()); EXPECT_EQUAL((uint32_t)ErrorCode::ILLEGAL_ROUTE, res.getError().getCode()); Message::UP msg = res.getMessage(); EXPECT_TRUE(msg); @@ -770,7 +747,7 @@ Test::testUnknownRoute(TestData &data) data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) .addHop(HopSpec("foo", "bar")))); Result res = data._srcSession->send(createMessage("msg"), "baz"); - EXPECT_TRUE(!res.isAccepted()); + EXPECT_FALSE(res.isAccepted()); EXPECT_EQUAL((uint32_t)ErrorCode::ILLEGAL_ROUTE, res.getError().getCode()); Message::UP msg = res.getMessage(); EXPECT_TRUE(msg); @@ -797,14 +774,14 @@ Test::testRecognizeHopName(TestData &data) data._dstSession->acknowledge(std::move(msg)); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); } void Test::testRecognizeRouteDirective(TestData &data) { data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) - .addRoute(std::move(RouteSpec("dst").addHop("dst/session"))) + .addRoute(RouteSpec("dst").addHop("dst/session")) .addHop(HopSpec("dir", "route:dst")))); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("dir")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); @@ -812,21 +789,21 @@ Test::testRecognizeRouteDirective(TestData &data) data._dstSession->acknowledge(std::move(msg)); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); } void Test::testRecognizeRouteName(TestData &data) { data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) - .addRoute(std::move(RouteSpec("dst").addHop("dst/session"))))); + .addRoute(RouteSpec("dst").addHop("dst/session")))); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("dst")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); data._dstSession->acknowledge(std::move(msg)); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); } void @@ -846,7 +823,7 @@ void Test::testRouteResolutionOverflow(TestData &data) { data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) - .addRoute(std::move(RouteSpec("foo").addHop("route:foo"))))); + .addRoute(RouteSpec("foo").addHop("route:foo")))); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), "foo").isAccepted()); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); @@ -858,7 +835,7 @@ void Test::testInsertRoute(TestData &data) { data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) - .addRoute(std::move(RouteSpec("foo").addHop("dst/session").addHop("bar"))))); + .addRoute(RouteSpec("foo").addHop("dst/session").addHop("bar")))); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("route:foo baz")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); @@ -868,14 +845,14 @@ Test::testInsertRoute(TestData &data) data._dstSession->acknowledge(std::move(msg)); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); } void Test::testErrorDirective(TestData &data) { Route route = Route::parse("foo/bar/baz"); - route.getHop(0).setDirective(1, IHopDirective::SP(new ErrorDirective("err"))); + route.getHop(0).setDirective(1, std::make_shared<ErrorDirective>("err")); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), route).isAccepted()); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); @@ -887,9 +864,8 @@ Test::testErrorDirective(TestData &data) void Test::testSelectError(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory())); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>()); data._srcServer.mb.putProtocol(protocol); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom: ]")).isAccepted()); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); @@ -903,9 +879,8 @@ Test::testSelectError(TestData &data) void Test::testSelectNone(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory())); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>()); data._srcServer.mb.putProtocol(protocol); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom]")).isAccepted()); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); @@ -917,9 +892,8 @@ Test::testSelectNone(TestData &data) void Test::testSelectOne(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory())); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>()); data._srcServer.mb.putProtocol(protocol); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:dst/session]")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); @@ -927,7 +901,7 @@ Test::testSelectOne(TestData &data) data._dstSession->acknowledge(std::move(msg)); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); } void @@ -937,13 +911,13 @@ Test::testResend1(TestData &data) EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("dst/session")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_TRANSIENT_ERROR, "err1")); data._dstSession->reply(std::move(reply)); msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - reply.reset(new EmptyReply()); + reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_TRANSIENT_ERROR, "err2")); data._dstSession->reply(std::move(reply)); @@ -952,7 +926,7 @@ Test::testResend1(TestData &data) data._dstSession->acknowledge(std::move(msg)); reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); EXPECT_TRUE(testTrace(StringList() .add("[APP_TRANSIENT_ERROR @ localhost]: err1") .add("-[APP_TRANSIENT_ERROR @ localhost]: err1") @@ -964,21 +938,20 @@ Test::testResend1(TestData &data) void Test::testResend2(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory())); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>()); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(true); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:dst/session]")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_TRANSIENT_ERROR, "err1")); data._dstSession->reply(std::move(reply)); msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - reply.reset(new EmptyReply()); + reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_TRANSIENT_ERROR, "err2")); data._dstSession->reply(std::move(reply)); @@ -987,7 +960,7 @@ Test::testResend2(TestData &data) data._dstSession->acknowledge(std::move(msg)); reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); EXPECT_TRUE(testTrace(StringList() .add("Source session accepted a 3 byte message. 1 message(s) now pending.") .add("Running routing policy 'Custom'.") @@ -1037,7 +1010,7 @@ Test::testNoResend(TestData &data) EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("dst/session")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_TRANSIENT_ERROR, "err1")); data._dstSession->reply(std::move(reply)); @@ -1050,15 +1023,14 @@ Test::testNoResend(TestData &data) void Test::testSelectOnResend(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory())); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>()); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(true); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:dst/session]")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_TRANSIENT_ERROR, "err")); data._dstSession->reply(std::move(reply)); @@ -1067,7 +1039,7 @@ Test::testSelectOnResend(TestData &data) data._dstSession->acknowledge(std::move(msg)); reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); EXPECT_TRUE(testTrace(StringList() .add("Selecting { 'dst/session' }.") .add("[APP_TRANSIENT_ERROR @ localhost]") @@ -1082,15 +1054,14 @@ Test::testSelectOnResend(TestData &data) void Test::testNoSelectOnResend(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory(false))); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>(false)); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(true); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:dst/session]")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_TRANSIENT_ERROR, "err")); data._dstSession->reply(std::move(reply)); @@ -1099,7 +1070,7 @@ Test::testNoSelectOnResend(TestData &data) data._dstSession->acknowledge(std::move(msg)); reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); EXPECT_TRUE(testTrace(StringList() .add("Selecting { 'dst/session' }.") .add("[APP_TRANSIENT_ERROR @ localhost]") @@ -1114,9 +1085,8 @@ Test::testNoSelectOnResend(TestData &data) void Test::testCanConsumeError(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory(true, ErrorCode::NO_ADDRESS_FOR_SERVICE))); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>(true, ErrorCode::NO_ADDRESS_FOR_SERVICE)); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(false); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:dst/session,dst/unknown]")).isAccepted()); @@ -1138,9 +1108,8 @@ Test::testCanConsumeError(TestData &data) void Test::testCantConsumeError(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory())); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>()); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(false); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:dst/unknown]")).isAccepted()); @@ -1159,9 +1128,8 @@ Test::testCantConsumeError(TestData &data) void Test::testNestedPolicies(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory(true, ErrorCode::NO_ADDRESS_FOR_SERVICE))); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>(true, ErrorCode::NO_ADDRESS_FOR_SERVICE)); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(false); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:[Custom:dst/session],[Custom:dst/unknown]]")).isAccepted()); @@ -1177,12 +1145,8 @@ Test::testNestedPolicies(TestData &data) void Test::testRemoveReply(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new RemoveReplyPolicyFactory( - true, - UIntList().add(ErrorCode::NO_ADDRESS_FOR_SERVICE), - 0))); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<RemoveReplyPolicyFactory>(true, UIntList().add(ErrorCode::NO_ADDRESS_FOR_SERVICE), 0)); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(false); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:[Custom:dst/session],[Custom:dst/unknown]]")).isAccepted()); @@ -1191,7 +1155,7 @@ Test::testRemoveReply(TestData &data) data._dstSession->acknowledge(std::move(msg)); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); EXPECT_TRUE(testTrace(StringList() .add("[NO_ADDRESS_FOR_SERVICE @ localhost]") .add("-[NO_ADDRESS_FOR_SERVICE @ localhost]") @@ -1203,10 +1167,9 @@ Test::testRemoveReply(TestData &data) void Test::testSetReply(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Select", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory(true, ErrorCode::APP_FATAL_ERROR))); - simple.addPolicyFactory("SetReply", SimpleProtocol::IPolicyFactory::SP(new SetReplyPolicyFactory(true, UIntList().add(ErrorCode::APP_FATAL_ERROR)))); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Select", std::make_shared<CustomPolicyFactory>(true, ErrorCode::APP_FATAL_ERROR)); + protocol->addPolicyFactory("SetReply", std::make_shared<SetReplyPolicyFactory>(true, UIntList().add(ErrorCode::APP_FATAL_ERROR))); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(false); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Select:[SetReply:foo],dst/session]")).isAccepted()); @@ -1223,20 +1186,15 @@ Test::testSetReply(TestData &data) void Test::testResendSetAndReuseReply(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("ReuseReply", SimpleProtocol::IPolicyFactory::SP(new ReuseReplyPolicyFactory( - false, - UIntList().add(ErrorCode::APP_FATAL_ERROR)))); - simple.addPolicyFactory("SetReply", SimpleProtocol::IPolicyFactory::SP(new SetReplyPolicyFactory( - false, - UIntList().add(ErrorCode::APP_FATAL_ERROR)))); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("ReuseReply", std::make_shared<ReuseReplyPolicyFactory>(false, UIntList().add(ErrorCode::APP_FATAL_ERROR))); + protocol->addPolicyFactory("SetReply", std::make_shared<SetReplyPolicyFactory>(false, UIntList().add(ErrorCode::APP_FATAL_ERROR))); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(true); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[ReuseReply:[SetReply:foo],dst/session]")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_TRANSIENT_ERROR, "dst")); data._dstSession->reply(std::move(reply)); @@ -1245,21 +1203,15 @@ Test::testResendSetAndReuseReply(TestData &data) data._dstSession->acknowledge(std::move(msg)); reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); } void Test::testResendSetAndRemoveReply(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("RemoveReply", SimpleProtocol::IPolicyFactory::SP(new RemoveReplyPolicyFactory( - false, - UIntList().add(ErrorCode::APP_TRANSIENT_ERROR), - 0))); - simple.addPolicyFactory("SetReply", SimpleProtocol::IPolicyFactory::SP(new SetReplyPolicyFactory( - false, - UIntList().add(ErrorCode::APP_TRANSIENT_ERROR).add(ErrorCode::APP_FATAL_ERROR)))); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("RemoveReply", std::make_shared<RemoveReplyPolicyFactory>(false, UIntList().add(ErrorCode::APP_TRANSIENT_ERROR), 0)); + protocol->addPolicyFactory("SetReply", std::make_shared<SetReplyPolicyFactory>(false, UIntList().add(ErrorCode::APP_TRANSIENT_ERROR).add(ErrorCode::APP_FATAL_ERROR))); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(true); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[RemoveReply:[SetReply:foo],dst/session]")).isAccepted()); @@ -1286,13 +1238,13 @@ Test::testHopIgnoresReply(TestData &data) EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("?dst/session")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_FATAL_ERROR, "dst")); data._dstSession->reply(std::move(reply)); reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); EXPECT_TRUE(testTrace(StringList() .add("Not waiting for a reply from 'dst/session'."), reply->getTrace())); @@ -1306,13 +1258,13 @@ Test::testHopBlueprintIgnoresReply(TestData &data) EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("foo")).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); - Reply::UP reply(new EmptyReply()); + Reply::UP reply = std::make_unique<EmptyReply>(); reply->swapState(*msg); reply->addError(Error(ErrorCode::APP_FATAL_ERROR, "dst")); data._dstSession->reply(std::move(reply)); reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); - EXPECT_TRUE(!reply->hasErrors()); + EXPECT_FALSE(reply->hasErrors()); EXPECT_TRUE(testTrace(StringList() .add("Not waiting for a reply from 'dst/session'."), reply->getTrace())); @@ -1334,15 +1286,12 @@ Test::testAcceptEmptyRoute(TestData &data) void Test::testAbortOnlyActiveNodes(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("Custom", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory(false))); - simple.addPolicyFactory("SetReply", SimpleProtocol::IPolicyFactory::SP(new SetReplyPolicyFactory( - false, - UIntList() - .add(ErrorCode::APP_TRANSIENT_ERROR) - .add(ErrorCode::APP_TRANSIENT_ERROR) - .add(ErrorCode::APP_FATAL_ERROR)))); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("Custom", std::make_shared<CustomPolicyFactory>(false)); + protocol->addPolicyFactory("SetReply", std::make_shared<SetReplyPolicyFactory>(false, + UIntList().add(ErrorCode::APP_TRANSIENT_ERROR) + .add(ErrorCode::APP_TRANSIENT_ERROR) + .add(ErrorCode::APP_FATAL_ERROR))); data._srcServer.mb.putProtocol(protocol); data._retryPolicy->setEnabled(true); EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[Custom:[SetReply:foo],?bar,dst/session]")).isAccepted()); @@ -1366,20 +1315,14 @@ Test::testUnknownPolicy(TestData &data) void Test::testSelectException(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("SelectException", - SimpleProtocol::IPolicyFactory::SP( - new SelectExceptionPolicyFactory())); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("SelectException", std::make_shared<SelectExceptionPolicyFactory>()); data._srcServer.mb.putProtocol(protocol); - EXPECT_TRUE(data._srcSession->send(createMessage("msg"), - Route::parse("[SelectException]")) - .isAccepted()); + EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("[SelectException]")).isAccepted()); Reply::UP reply = data._srcHandler.getReply(RECEPTOR_TIMEOUT); ASSERT_TRUE(reply); EXPECT_EQUAL(1u, reply->getNumErrors()); - EXPECT_EQUAL((uint32_t)ErrorCode::POLICY_ERROR, - reply->getError(0).getCode()); + EXPECT_EQUAL((uint32_t)ErrorCode::POLICY_ERROR, reply->getError(0).getCode()); EXPECT_EQUAL("Policy 'SelectException' threw an exception; {test exception}", reply->getError(0).getMessage()); } @@ -1387,15 +1330,11 @@ Test::testSelectException(TestData &data) void Test::testMergeException(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); - SimpleProtocol &simple = static_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("MergeException", - SimpleProtocol::IPolicyFactory::SP( - new MergeExceptionPolicyFactory())); + auto protocol = std::make_shared<SimpleProtocol>(); + protocol->addPolicyFactory("MergeException", std::make_shared<MergeExceptionPolicyFactory>()); data._srcServer.mb.putProtocol(protocol); Route route = Route::parse("[MergeException:dst/session]"); - EXPECT_TRUE(data._srcSession->send(createMessage("msg"), route) - .isAccepted()); + EXPECT_TRUE(data._srcSession->send(createMessage("msg"), route).isAccepted()); Message::UP msg = data._dstHandler.getMessage(RECEPTOR_TIMEOUT); ASSERT_TRUE(msg); data._dstSession->acknowledge(std::move(msg)); @@ -1419,7 +1358,7 @@ Test::requireThatIgnoreFlagPersistsThroughHopLookup(TestData &data) void Test::requireThatIgnoreFlagPersistsThroughRouteLookup(TestData &data) { - setupRouting(data, RoutingTableSpec(SimpleProtocol::NAME).addRoute(std::move(RouteSpec("foo").addHop("dst/unknown")))); + setupRouting(data, RoutingTableSpec(SimpleProtocol::NAME).addRoute(RouteSpec("foo").addHop("dst/unknown"))); ASSERT_TRUE(testSend(data, "?foo")); ASSERT_TRUE(testTrace(data, StringList().add("Ignoring errors in reply."))); } @@ -1442,7 +1381,7 @@ Test::requireThatIgnoreFlagIsSerializedWithMessage(TestData &data) EXPECT_EQUAL(2u, route.getNumHops()); Hop hop = route.getHop(0); EXPECT_EQUAL("foo", hop.toString()); - EXPECT_TRUE(!hop.getIgnoreResult()); + EXPECT_FALSE(hop.getIgnoreResult()); hop = route.getHop(1); EXPECT_EQUAL("?bar", hop.toString()); EXPECT_TRUE(hop.getIgnoreResult()); diff --git a/messagebus/src/tests/routingcontext/routingcontext.cpp b/messagebus/src/tests/routingcontext/routingcontext.cpp index c21725a482e..d2c8cfcb0db 100644 --- a/messagebus/src/tests/routingcontext/routingcontext.cpp +++ b/messagebus/src/tests/routingcontext/routingcontext.cpp @@ -15,12 +15,6 @@ using namespace mbus; -//////////////////////////////////////////////////////////////////////////////// -// -// Utilities -// -//////////////////////////////////////////////////////////////////////////////// - using vespalib::make_string; static const duration TIMEOUT = 120s; @@ -40,7 +34,7 @@ class CustomPolicyFactory : public SimpleProtocol::IPolicyFactory { private: friend class CustomPolicy; - bool _forward; + bool _forward; std::vector<string> _expectedAll; std::vector<string> _expectedMatched; @@ -64,11 +58,9 @@ public: void merge(RoutingContext &ctx) override; }; -CustomPolicy::CustomPolicy(CustomPolicyFactory &factory) : - _factory(factory) -{ - // empty -} +CustomPolicy::CustomPolicy(CustomPolicyFactory &factory) + : _factory(factory) +{ } void CustomPolicy::select(RoutingContext &ctx) @@ -79,13 +71,13 @@ CustomPolicy::select(RoutingContext &ctx) const std::vector<Route> &all = ctx.getAllRecipients(); if (_factory._expectedAll.size() == all.size()) { ctx.trace(1, make_string("Got %d expected recipients.", (uint32_t)all.size())); - for (const auto & it : all) { - if (find(_factory._expectedAll.begin(), _factory._expectedAll.end(), it.toString()) != _factory._expectedAll.end()) { - ctx.trace(1, make_string("Got expected recipient '%s'.", it.toString().c_str())); + for (const auto & route : all) { + if (find(_factory._expectedAll.begin(), _factory._expectedAll.end(), route.toString()) != _factory._expectedAll.end()) { + ctx.trace(1, make_string("Got expected recipient '%s'.", route.toString().c_str())); } else { reply->addError(Error(ErrorCode::APP_FATAL_ERROR, make_string("Matched recipient '%s' not expected.", - it.toString().c_str()))); + route.toString().c_str()))); } } } else { @@ -113,12 +105,12 @@ CustomPolicy::select(RoutingContext &ctx) ctx.getMatchedRecipients(matched); if (_factory._expectedMatched.size() == matched.size()) { ctx.trace(1, make_string("Got %d expected recipients.", (uint32_t)matched.size())); - for (auto & it : matched) { - if (find(_factory._expectedMatched.begin(), _factory._expectedMatched.end(), it.toString()) != _factory._expectedMatched.end()) { - ctx.trace(1, make_string("Got matched recipient '%s'.", it.toString().c_str())); + for (auto & route : matched) { + if (find(_factory._expectedMatched.begin(), _factory._expectedMatched.end(), route.toString()) != _factory._expectedMatched.end()) { + ctx.trace(1, make_string("Got matched recipient '%s'.", route.toString().c_str())); } else { reply->addError(Error(ErrorCode::APP_FATAL_ERROR, - make_string("Matched recipient '%s' not expected.", it.toString().c_str()))); + make_string("Matched recipient '%s' not expected.", route.toString().c_str()))); } } } else { @@ -129,8 +121,8 @@ CustomPolicy::select(RoutingContext &ctx) } if (!reply->hasErrors() && _factory._forward) { - for (auto & it : matched) { - ctx.addChild(it); + for (auto & route : matched) { + ctx.addChild(route); } } else { ctx.setReply(std::move(reply)); @@ -140,7 +132,7 @@ CustomPolicy::select(RoutingContext &ctx) void CustomPolicy::merge(RoutingContext &ctx) { - Reply::UP ret(new EmptyReply()); + auto ret = std::make_unique<EmptyReply>(); for (RoutingNodeIterator it = ctx.getChildIterator(); it.isValid(); it.next()) { @@ -202,7 +194,7 @@ public: class Test : public vespalib::TestApp { private: - Message::UP createMessage(const string &msg); + static Message::UP createMessage(const string &msg); public: int Main() override; @@ -289,11 +281,11 @@ Test::testSingleDirective(TestData &data) StringList().add("foo").add("bar")))); data._srcServer.mb.putProtocol(protocol); data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) - .addRoute(std::move(RouteSpec("myroute").addHop("myhop"))) - .addHop(std::move(HopSpec("myhop", "[Custom]") + .addRoute(RouteSpec("myroute").addHop("myhop")) + .addHop(HopSpec("myhop", "[Custom]") .addRecipient("foo") .addRecipient("bar") - .addRecipient("baz/cox"))))); + .addRecipient("baz/cox")))); for (uint32_t i = 0; i < 2; ++i) { EXPECT_TRUE(data._srcSession->send(createMessage("msg"), "myroute").isAccepted()); Reply::UP reply = data._srcHandler.getReply(); @@ -314,13 +306,13 @@ Test::testMoreDirectives(TestData &data) StringList().add("foo/bar0/baz").add("foo/bar1/baz")))); data._srcServer.mb.putProtocol(protocol); data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) - .addRoute(std::move(RouteSpec("myroute").addHop("myhop"))) - .addHop(std::move(HopSpec("myhop", "foo/[Custom]/baz") + .addRoute(RouteSpec("myroute").addHop("myhop")) + .addHop(HopSpec("myhop", "foo/[Custom]/baz") .addRecipient("foo") .addRecipient("foo/bar") .addRecipient("foo/bar0/baz") .addRecipient("foo/bar1/baz") - .addRecipient("foo/bar/baz/cox"))))); + .addRecipient("foo/bar/baz/cox")))); for (uint32_t i = 0; i < 2; ++i) { EXPECT_TRUE(data._srcSession->send(createMessage("msg"), "myroute").isAccepted()); Reply::UP reply = data._srcHandler.getReply(); @@ -333,21 +325,19 @@ Test::testMoreDirectives(TestData &data) void Test::testRecipientsRemain(TestData &data) { - IProtocol::SP protocol(new SimpleProtocol()); + auto protocol = std::make_shared<SimpleProtocol>(); auto &simple = dynamic_cast<SimpleProtocol&>(*protocol); - simple.addPolicyFactory("First", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory( - true, - StringList().add("foo/bar"), - StringList().add("foo/[Second]")))); - simple.addPolicyFactory("Second", SimpleProtocol::IPolicyFactory::SP(new CustomPolicyFactory( - false, - StringList().add("foo/bar"), - StringList().add("foo/bar")))); + simple.addPolicyFactory("First", std::make_shared<CustomPolicyFactory>(true, + StringList().add("foo/bar"), + StringList().add("foo/[Second]"))); + simple.addPolicyFactory("Second", std::make_shared<CustomPolicyFactory>(false, + StringList().add("foo/bar"), + StringList().add("foo/bar"))); data._srcServer.mb.putProtocol(protocol); data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) - .addRoute(std::move(RouteSpec("myroute").addHop("myhop"))) - .addHop(std::move(HopSpec("myhop", "[First]/[Second]") - .addRecipient("foo/bar"))))); + .addRoute(RouteSpec("myroute").addHop("myhop")) + .addHop(HopSpec("myhop", "[First]/[Second]") + .addRecipient("foo/bar")))); for (uint32_t i = 0; i < 2; ++i) { EXPECT_TRUE(data._srcSession->send(createMessage("msg"), "myroute").isAccepted()); Reply::UP reply = data._srcHandler.getReply(); @@ -363,11 +353,11 @@ Test::testConstRoute(TestData &data) auto protocol = std::make_shared<SimpleProtocol>(); auto &simple = dynamic_cast<SimpleProtocol&>(*protocol); simple.addPolicyFactory("DocumentRouteSelector", - std::make_unique<CustomPolicyFactory>(true, StringList().add("dst"), StringList().add("dst"))); + std::make_shared<CustomPolicyFactory>(true, StringList().add("dst"), StringList().add("dst"))); data._srcServer.mb.putProtocol(protocol); data._srcServer.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) - .addRoute(std::move(RouteSpec("default").addHop("indexing"))) - .addHop(std::move(HopSpec("indexing", "[DocumentRouteSelector]").addRecipient("dst"))) + .addRoute(RouteSpec("default").addHop("indexing")) + .addHop(HopSpec("indexing", "[DocumentRouteSelector]").addRecipient("dst")) .addHop(HopSpec("dst", "dst/session")))); for (uint32_t i = 0; i < 2; ++i) { EXPECT_TRUE(data._srcSession->send(createMessage("msg"), Route::parse("route:default")).isAccepted()); diff --git a/messagebus/src/tests/routingspec/routingspec.cpp b/messagebus/src/tests/routingspec/routingspec.cpp index e7f4dec4f48..24b370d6531 100644 --- a/messagebus/src/tests/routingspec/routingspec.cpp +++ b/messagebus/src/tests/routingspec/routingspec.cpp @@ -21,8 +21,8 @@ public: // empty } - bool setupRouting(const RoutingSpec &spec) override { - _routing = spec; + bool setupRouting(RoutingSpec spec) override { + _routing = std::move(spec); return true; } @@ -94,31 +94,31 @@ Test::testConstructors() { RoutingSpec spec = RoutingSpec() .addTable(RoutingTableSpec("foo") - .addHop(std::move(HopSpec("foo-h1", "foo-h1-sel") + .addHop(HopSpec("foo-h1", "foo-h1-sel") .addRecipient("foo-h1-r1") - .addRecipient("foo-h1-r2"))) - .addHop(std::move(HopSpec("foo-h2", "foo-h2-sel") + .addRecipient("foo-h1-r2")) + .addHop(HopSpec("foo-h2", "foo-h2-sel") .addRecipient("foo-h2-r1") - .addRecipient("foo-h2-r2"))) - .addRoute(std::move(RouteSpec("foo-r1") + .addRecipient("foo-h2-r2")) + .addRoute(RouteSpec("foo-r1") .addHop("foo-h1") - .addHop("foo-h2"))) - .addRoute(std::move(RouteSpec("foo-r2") + .addHop("foo-h2")) + .addRoute(RouteSpec("foo-r2") .addHop("foo-h2") - .addHop("foo-h1")))) + .addHop("foo-h1"))) .addTable(RoutingTableSpec("bar") - .addHop(std::move(HopSpec("bar-h1", "bar-h1-sel") + .addHop(HopSpec("bar-h1", "bar-h1-sel") .addRecipient("bar-h1-r1") - .addRecipient("bar-h1-r2"))) - .addHop(std::move(HopSpec("bar-h2", "bar-h2-sel") + .addRecipient("bar-h1-r2")) + .addHop(HopSpec("bar-h2", "bar-h2-sel") .addRecipient("bar-h2-r1") - .addRecipient("bar-h2-r2"))) - .addRoute(std::move(RouteSpec("bar-r1") + .addRecipient("bar-h2-r2")) + .addRoute(RouteSpec("bar-r1") .addHop("bar-h1") - .addHop("bar-h2"))) - .addRoute(std::move(RouteSpec("bar-r2") + .addHop("bar-h2")) + .addRoute(RouteSpec("bar-r2") .addHop("bar-h2") - .addHop("bar-h1")))); + .addHop("bar-h1"))); EXPECT_TRUE(testRouting(spec)); RoutingSpec specCopy = spec; @@ -184,20 +184,20 @@ Test::testConfigGeneration() .addHop(HopSpec("myhop1", "myselector1"))))); EXPECT_TRUE(testConfig(RoutingSpec().addTable(RoutingTableSpec("mytable1") .addHop(HopSpec("myhop1", "myselector1")) - .addRoute(std::move(RouteSpec("myroute1").addHop("myhop1")))))); + .addRoute(RouteSpec("myroute1").addHop("myhop1"))))); EXPECT_TRUE(testConfig(RoutingSpec().addTable(RoutingTableSpec("mytable1") .addHop(HopSpec("myhop1", "myselector1")) .addHop(HopSpec("myhop2", "myselector2")) - .addRoute(std::move(RouteSpec("myroute1").addHop("myhop1"))) - .addRoute(std::move(RouteSpec("myroute2").addHop("myhop2"))) - .addRoute(std::move(RouteSpec("myroute12").addHop("myhop1").addHop("myhop2")))))); + .addRoute(RouteSpec("myroute1").addHop("myhop1")) + .addRoute(RouteSpec("myroute2").addHop("myhop2")) + .addRoute(RouteSpec("myroute12").addHop("myhop1").addHop("myhop2"))))); EXPECT_TRUE(testConfig(RoutingSpec() .addTable(RoutingTableSpec("mytable1") .addHop(HopSpec("myhop1", "myselector1")) .addHop(HopSpec("myhop2", "myselector2")) - .addRoute(std::move(RouteSpec("myroute1").addHop("myhop1"))) - .addRoute(std::move(RouteSpec("myroute2").addHop("myhop2"))) - .addRoute(std::move(RouteSpec("myroute12").addHop("myhop1").addHop("myhop2")))) + .addRoute(RouteSpec("myroute1").addHop("myhop1")) + .addRoute(RouteSpec("myroute2").addHop("myhop2")) + .addRoute(RouteSpec("myroute12").addHop("myhop1").addHop("myhop2"))) .addTable(RoutingTableSpec("mytable2")))); EXPECT_EQUAL("routingtable[2]\n" @@ -223,10 +223,10 @@ Test::testConfigGeneration() .addTable(RoutingTableSpec("mytable2") .addHop(HopSpec("myhop1", "myselector1")) .addHop(std::move(HopSpec("myhop2", "myselector2").setIgnoreResult(true))) - .addHop(std::move(HopSpec("myhop1", "myselector3") + .addHop(HopSpec("myhop1", "myselector3") .addRecipient("myrecipient1") - .addRecipient("myrecipient2"))) - .addRoute(std::move(RouteSpec("myroute1").addHop("myhop1")))).toString()); + .addRecipient("myrecipient2")) + .addRoute(RouteSpec("myroute1").addHop("myhop1"))).toString()); } bool diff --git a/messagebus/src/tests/simple-roundtrip/simple-roundtrip.cpp b/messagebus/src/tests/simple-roundtrip/simple-roundtrip.cpp index 911c3d38e5e..43bae5403f3 100644 --- a/messagebus/src/tests/simple-roundtrip/simple-roundtrip.cpp +++ b/messagebus/src/tests/simple-roundtrip/simple-roundtrip.cpp @@ -18,7 +18,7 @@ RoutingSpec getRouting() { .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("pxy", "test/pxy/session")) .addHop(HopSpec("dst", "test/dst/session")) - .addRoute(std::move(RouteSpec("test").addHop("pxy").addHop("dst")))); + .addRoute(RouteSpec("test").addHop("pxy").addHop("dst"))); } int diff --git a/messagebus/src/tests/sourcesession/sourcesession.cpp b/messagebus/src/tests/sourcesession/sourcesession.cpp index a91c128b0b0..302f8495daf 100644 --- a/messagebus/src/tests/sourcesession/sourcesession.cpp +++ b/messagebus/src/tests/sourcesession/sourcesession.cpp @@ -44,14 +44,14 @@ RoutingSpec getRouting() { return RoutingSpec() .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("dst", "dst/session")) - .addRoute(std::move(RouteSpec("dst").addHop("dst")))); + .addRoute(RouteSpec("dst").addHop("dst"))); } RoutingSpec getBadRouting() { return RoutingSpec() .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("dst", "dst/session")) - .addRoute(std::move(RouteSpec("dst").addHop("dst")))); + .addRoute(RouteSpec("dst").addHop("dst"))); } bool waitQueueSize(RoutableQueue &queue, uint32_t size) { @@ -182,8 +182,8 @@ Test::testResendConnDown() RPCNetworkParams(slobrok.config())); src.mb.setupRouting(RoutingSpec().addTable(RoutingTableSpec(SimpleProtocol::NAME) .addHop(HopSpec("dst", "dst2/session")) - .addHop(std::move(HopSpec("pxy", "[All]").addRecipient("dst"))) - .addRoute(std::move(RouteSpec("dst").addHop("pxy"))))); + .addHop(HopSpec("pxy", "[All]").addRecipient("dst")) + .addRoute(RouteSpec("dst").addHop("pxy")))); RoutableQueue srcQ; SourceSession::UP ss = src.mb.createSourceSession(srcQ); diff --git a/messagebus/src/tests/throttling/throttling.cpp b/messagebus/src/tests/throttling/throttling.cpp index 3063e3ae8b5..15ff6211dd4 100644 --- a/messagebus/src/tests/throttling/throttling.cpp +++ b/messagebus/src/tests/throttling/throttling.cpp @@ -40,7 +40,7 @@ RoutingSpec getRouting() return RoutingSpec() .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("dst", "dst/session")) - .addRoute(std::move(RouteSpec("dst").addHop("dst")))); + .addRoute(RouteSpec("dst").addHop("dst"))); } bool waitQueueSize(RoutableQueue &queue, uint32_t size) diff --git a/messagebus/src/tests/trace-roundtrip/trace-roundtrip.cpp b/messagebus/src/tests/trace-roundtrip/trace-roundtrip.cpp index 7fbc3b096d6..35f232ba561 100644 --- a/messagebus/src/tests/trace-roundtrip/trace-roundtrip.cpp +++ b/messagebus/src/tests/trace-roundtrip/trace-roundtrip.cpp @@ -79,7 +79,7 @@ RoutingSpec getRouting() { .addTable(RoutingTableSpec("Simple") .addHop(HopSpec("pxy", "test/pxy/session")) .addHop(HopSpec("dst", "test/dst/session")) - .addRoute(std::move(RouteSpec("test").addHop("pxy").addHop("dst")))); + .addRoute(RouteSpec("test").addHop("pxy").addHop("dst"))); } int diff --git a/messagebus/src/vespa/messagebus/configagent.cpp b/messagebus/src/vespa/messagebus/configagent.cpp index 60c6690f7e3..83ddbc1543e 100644 --- a/messagebus/src/vespa/messagebus/configagent.cpp +++ b/messagebus/src/vespa/messagebus/configagent.cpp @@ -35,7 +35,7 @@ ConfigAgent::configure(std::unique_ptr<MessagebusConfig> config) } tableSpec.addRoute(std::move(routeSpec)); } - spec.addTable(tableSpec); + spec.addTable(std::move(tableSpec)); } _handler.setupRouting(spec); } diff --git a/messagebus/src/vespa/messagebus/iconfighandler.h b/messagebus/src/vespa/messagebus/iconfighandler.h index 5cfd26b6440..ef5d5b86a26 100644 --- a/messagebus/src/vespa/messagebus/iconfighandler.h +++ b/messagebus/src/vespa/messagebus/iconfighandler.h @@ -26,7 +26,7 @@ public: * @return true if new setup was accepted * @param spec spec of new routing setup **/ - virtual bool setupRouting(const RoutingSpec &spec) = 0; + virtual bool setupRouting(RoutingSpec spec) = 0; }; } // namespace mbus diff --git a/messagebus/src/vespa/messagebus/messagebus.cpp b/messagebus/src/vespa/messagebus/messagebus.cpp index 8012eabf9b1..acdc6374933 100644 --- a/messagebus/src/vespa/messagebus/messagebus.cpp +++ b/messagebus/src/vespa/messagebus/messagebus.cpp @@ -28,7 +28,7 @@ private: mbus::Resender *_resender; public: - ResenderTask(mbus::Resender &resender) + explicit ResenderTask(mbus::Resender &resender) : _resender(&resender) { // empty @@ -38,7 +38,7 @@ public: _resender->resendScheduled(); } - uint8_t priority() const override { + [[nodiscard]] uint8_t priority() const override { return 255; } }; @@ -73,7 +73,7 @@ public: _done = _msn.isEmpty(); } - uint8_t priority() const override { + [[nodiscard]] uint8_t priority() const override { return 255; } }; @@ -256,11 +256,10 @@ MessageBus::unregisterSession(const string &sessionName) RoutingTable::SP MessageBus::getRoutingTable(const string &protocol) { - using ITR = std::map<string, RoutingTable::SP>::iterator; std::lock_guard guard(_lock); - ITR itr = _routingTables.find(protocol); + auto itr = _routingTables.find(protocol); if (itr == _routingTables.end()) { - return RoutingTable::SP(); // not found + return {}; // not found } return itr->second; } @@ -293,7 +292,7 @@ MessageBus::handleMessage(Message::UP msg) } bool -MessageBus::setupRouting(const RoutingSpec &spec) +MessageBus::setupRouting(RoutingSpec spec) { std::map<string, RoutingTable::SP> rtm; for (uint32_t i = 0; i < spec.getNumTables(); ++i) { @@ -373,7 +372,7 @@ MessageBus::deliverMessage(Message::UP msg, const string &session) IMessageHandler *msgHandler = nullptr; { std::lock_guard guard(_lock); - std::map<string, IMessageHandler*>::iterator it = _sessions.find(session); + auto it = _sessions.find(session); if (it != _sessions.end()) { msgHandler = it->second; } diff --git a/messagebus/src/vespa/messagebus/messagebus.h b/messagebus/src/vespa/messagebus/messagebus.h index 2c27d281494..aa926a774d0 100644 --- a/messagebus/src/vespa/messagebus/messagebus.h +++ b/messagebus/src/vespa/messagebus/messagebus.h @@ -298,7 +298,7 @@ public: void handleMessage(Message::UP msg) override; // Implements IConfigHandler. - bool setupRouting(const RoutingSpec &spec) override; + bool setupRouting(RoutingSpec spec) override; // Implements INetworkOwner. IProtocol * getProtocol(const string &name) override; diff --git a/messagebus/src/vespa/messagebus/routing/hopspec.cpp b/messagebus/src/vespa/messagebus/routing/hopspec.cpp index 7c8980b33b1..61d6776ea4c 100644 --- a/messagebus/src/vespa/messagebus/routing/hopspec.cpp +++ b/messagebus/src/vespa/messagebus/routing/hopspec.cpp @@ -21,11 +21,17 @@ HopSpec & HopSpec::operator=(HopSpec && rhs) noexcept = default; HopSpec::~HopSpec() = default; HopSpec & -HopSpec::addRecipient(const string &recipient) { +HopSpec::addRecipient(const string &recipient) & { _recipients.push_back(recipient); return *this; } +HopSpec && +HopSpec::addRecipient(const string &recipient) && { + _recipients.push_back(recipient); + return std::move(*this); +} + void HopSpec::toConfig(string &cfg, const string &prefix) const { diff --git a/messagebus/src/vespa/messagebus/routing/hopspec.h b/messagebus/src/vespa/messagebus/routing/hopspec.h index fd2e4dfb431..11857cf4ba4 100644 --- a/messagebus/src/vespa/messagebus/routing/hopspec.h +++ b/messagebus/src/vespa/messagebus/routing/hopspec.h @@ -72,7 +72,8 @@ public: * @param recipient The recipient to add. * @return This, to allow chaining. */ - HopSpec &addRecipient(const string &recipient); + HopSpec & addRecipient(const string &recipient) &; + HopSpec && addRecipient(const string &recipient) &&; /** * Returns whether or not to ignore the result when routing through this hop. diff --git a/messagebus/src/vespa/messagebus/routing/routespec.cpp b/messagebus/src/vespa/messagebus/routing/routespec.cpp index 00b8d7ed322..f9c0eb701b7 100644 --- a/messagebus/src/vespa/messagebus/routing/routespec.cpp +++ b/messagebus/src/vespa/messagebus/routing/routespec.cpp @@ -20,11 +20,16 @@ RouteSpec & RouteSpec::operator = (RouteSpec &&) noexcept = default; RouteSpec::~RouteSpec() = default; RouteSpec & -RouteSpec::addHop(const string &hop) { +RouteSpec::addHop(const string &hop) & { _hops.push_back(hop); return *this; } +RouteSpec && +RouteSpec::addHop(const string &hop) && { + _hops.push_back(hop); + return std::move(*this); +} RouteSpec & RouteSpec::setHop(uint32_t i, const string &hop) { diff --git a/messagebus/src/vespa/messagebus/routing/routespec.h b/messagebus/src/vespa/messagebus/routing/routespec.h index 273836c94d5..ff5de577b83 100644 --- a/messagebus/src/vespa/messagebus/routing/routespec.h +++ b/messagebus/src/vespa/messagebus/routing/routespec.h @@ -62,7 +62,8 @@ public: * @param hop The hop to add. * @return This, to allow chaining. */ - RouteSpec & addHop(const string &hop); + RouteSpec & addHop(const string &hop) &; + RouteSpec && addHop(const string &hop) &&; /** * Sets the hop name for a given index. diff --git a/messagebus/src/vespa/messagebus/routing/routingspec.cpp b/messagebus/src/vespa/messagebus/routing/routingspec.cpp index c6c28ea24ab..a25f5c259e4 100644 --- a/messagebus/src/vespa/messagebus/routing/routingspec.cpp +++ b/messagebus/src/vespa/messagebus/routing/routingspec.cpp @@ -6,18 +6,22 @@ using vespalib::make_string; namespace mbus { -RoutingSpec::RoutingSpec() : - _tables() -{ - // empty +RoutingSpec::RoutingSpec() noexcept = default; +RoutingSpec::RoutingSpec(const RoutingSpec &) = default; +RoutingSpec::RoutingSpec(RoutingSpec &&) noexcept = default; +RoutingSpec & RoutingSpec::operator=(RoutingSpec &&) noexcept = default; +RoutingSpec::~RoutingSpec() = default; + +RoutingSpec & +RoutingSpec::addTable(RoutingTableSpec && table) & { + _tables.emplace_back(std::move(table)); + return *this; } -RoutingTableSpec -RoutingSpec::removeTable(uint32_t i) -{ - RoutingTableSpec ret = _tables[i]; - _tables.erase(_tables.begin() + i); - return ret; +RoutingSpec && +RoutingSpec::addTable(RoutingTableSpec && table) && { + _tables.emplace_back(std::move(table)); + return std::move(*this); } string @@ -25,17 +29,17 @@ RoutingSpec::toConfigString(const string &input) { string ret; ret.append("\""); - for (uint32_t i = 0, len = input.size(); i < len; ++i) { - if (input[i] == '\\') { + for (char i : input) { + if (i == '\\') { ret.append("\\\\"); - } else if (input[i] == '"') { + } else if (i == '"') { ret.append("\\\""); - } else if (input[i] == '\n') { + } else if (i == '\n') { ret.append("\\n"); - } else if (input[i] == 0) { + } else if (i == 0) { ret.append("\\x00"); } else { - ret += input[i]; + ret += i; } } ret.append("\""); diff --git a/messagebus/src/vespa/messagebus/routing/routingspec.h b/messagebus/src/vespa/messagebus/routing/routingspec.h index 39f407f691b..35443d33f72 100644 --- a/messagebus/src/vespa/messagebus/routing/routingspec.h +++ b/messagebus/src/vespa/messagebus/routing/routingspec.h @@ -21,24 +21,19 @@ private: std::vector<RoutingTableSpec> _tables; public: - /** - * Creates an empty specification. - */ - RoutingSpec(); - - /** - * Returns whether or not there are routing table specs contained in this. - * - * @return True if there is at least one table. - */ - bool hasTables() const { return !_tables.empty(); } + RoutingSpec() noexcept; + RoutingSpec(const RoutingSpec &); + RoutingSpec & operator=(const RoutingSpec &) = delete; + RoutingSpec(RoutingSpec &&) noexcept; + RoutingSpec & operator=(RoutingSpec &&) noexcept; + ~RoutingSpec(); /** * Returns the number of routing table specs that are contained in this. * * @return The number of routing tables. */ - uint32_t getNumTables() const { return _tables.size(); } + [[nodiscard]] uint32_t getNumTables() const { return _tables.size(); } /** * Returns the routing table spec at the given index. @@ -54,16 +49,7 @@ public: * @param i The index of the routing table to return. * @return The routing table at the given index. */ - const RoutingTableSpec &getTable(uint32_t i) const { return _tables[i]; } - - /** - * Sets the routing table spec at the given index. - * - * @param i The index at which to set the routing table. - * @param table The routing table to set. - * @return This, to allow chaining. - */ - RoutingSpec &setTable(uint32_t i, const RoutingTableSpec &table) { _tables[i] = table; return *this; } + [[nodiscard]] const RoutingTableSpec &getTable(uint32_t i) const { return _tables[i]; } /** * Adds a routing table spec to the list of tables. @@ -71,23 +57,8 @@ public: * @param table The routing table to add. * @return This, to allow chaining. */ - RoutingSpec &addTable(const RoutingTableSpec &table) { _tables.push_back(table); return *this; } - - /** - * Returns the routing table spec at the given index. - * - * @param i The index of the routing table to remove. - * @return The removed routing table. - */ - RoutingTableSpec removeTable(uint32_t i); - - /** - * Clears the list of routing table specs contained in this. - * - * @return This, to allow chaining. - */ - RoutingSpec &clearTables() { _tables.clear(); return *this; } - + RoutingSpec & addTable(RoutingTableSpec && table) &; + RoutingSpec && addTable(RoutingTableSpec && table) &&; /** * Appends the content of this to the given config string. * @@ -113,7 +84,7 @@ public: * * @return The string. */ - string toString() const; + [[nodiscard]] string toString() const; /** * Implements the equality operator. diff --git a/messagebus/src/vespa/messagebus/routing/routingtable.cpp b/messagebus/src/vespa/messagebus/routing/routingtable.cpp index 3483cd82bc6..73dd4ea73e0 100644 --- a/messagebus/src/vespa/messagebus/routing/routingtable.cpp +++ b/messagebus/src/vespa/messagebus/routing/routingtable.cpp @@ -3,9 +3,6 @@ #include "routingtable.h" #include "hop.h" #include "routingtablespec.h" -#include <vespa/messagebus/iprotocol.h> -#include <vespa/messagebus/message.h> -#include <vector> namespace mbus { @@ -47,7 +44,7 @@ RoutingTable::hasHop(const string &name) const const HopBlueprint * RoutingTable::getHop(const string &name) const { - std::map<string, HopBlueprint>::const_iterator it = _hops.find(name); + auto it = _hops.find(name); return it != _hops.end() ? &(it->second) : nullptr; } diff --git a/messagebus/src/vespa/messagebus/routing/routingtablespec.cpp b/messagebus/src/vespa/messagebus/routing/routingtablespec.cpp index 6b20bd4af9f..f62431a19f0 100644 --- a/messagebus/src/vespa/messagebus/routing/routingtablespec.cpp +++ b/messagebus/src/vespa/messagebus/routing/routingtablespec.cpp @@ -19,10 +19,15 @@ RoutingTableSpec::~RoutingTableSpec() = default; RoutingTableSpec& RoutingTableSpec::operator=(const RoutingTableSpec&) = default; RoutingTableSpec & -RoutingTableSpec::addHop(HopSpec && hop) { +RoutingTableSpec::addHop(HopSpec && hop) & { _hops.emplace_back(std::move(hop)); return *this; } +RoutingTableSpec && +RoutingTableSpec::addHop(HopSpec && hop) && { + _hops.emplace_back(std::move(hop)); + return std::move(*this); +} RoutingTableSpec & RoutingTableSpec::setHop(uint32_t i, HopSpec &&hop) { @@ -30,6 +35,18 @@ RoutingTableSpec::setHop(uint32_t i, HopSpec &&hop) { return *this; } +RoutingTableSpec & +RoutingTableSpec::addRoute(RouteSpec &&route) & { + _routes.emplace_back(std::move(route)); + return *this; +} + +RoutingTableSpec && +RoutingTableSpec::addRoute(RouteSpec &&route) && { + _routes.emplace_back(std::move(route)); + return std::move(*this); +} + void RoutingTableSpec::toConfig(string &cfg, const string &prefix) const { diff --git a/messagebus/src/vespa/messagebus/routing/routingtablespec.h b/messagebus/src/vespa/messagebus/routing/routingtablespec.h index 63f1a68ade2..45f0cec9b48 100644 --- a/messagebus/src/vespa/messagebus/routing/routingtablespec.h +++ b/messagebus/src/vespa/messagebus/routing/routingtablespec.h @@ -73,7 +73,8 @@ public: * @param hop The hop to add. * @return This, to allow chaining. */ - RoutingTableSpec &addHop(HopSpec && hop); + RoutingTableSpec & addHop(HopSpec && hop) &; + RoutingTableSpec && addHop(HopSpec && hop) &&; /** * Sets the hop spec at the given index. @@ -113,7 +114,8 @@ public: * @param route The route to add. * @return This, to allow chaining. */ - RoutingTableSpec &addRoute(RouteSpec &&route) { _routes.emplace_back(std::move(route)); return *this; } + RoutingTableSpec && addRoute(RouteSpec &&route) &&; + RoutingTableSpec & addRoute(RouteSpec &&route) &; /** * Sets the route spec at the given index. diff --git a/messagebus/src/vespa/messagebus/testlib/custompolicy.cpp b/messagebus/src/vespa/messagebus/testlib/custompolicy.cpp index 94e051b0752..db5f2d17973 100644 --- a/messagebus/src/vespa/messagebus/testlib/custompolicy.cpp +++ b/messagebus/src/vespa/messagebus/testlib/custompolicy.cpp @@ -12,16 +12,15 @@ LOG_SETUP(".custompolicy"); namespace mbus { CustomPolicy::CustomPolicy(bool selectOnRetry, - const std::vector<uint32_t> consumableErrors, - const std::vector<Route> &routes) : + std::vector<uint32_t> consumableErrors, + std::vector<Route> routes) : _selectOnRetry(selectOnRetry), - _consumableErrors(consumableErrors), - _routes(routes) + _consumableErrors(std::move(consumableErrors)), + _routes(std::move(routes)) { } -CustomPolicy::~CustomPolicy() { -} +CustomPolicy::~CustomPolicy() = default; void CustomPolicy::select(RoutingContext &context) @@ -38,10 +37,8 @@ CustomPolicy::select(RoutingContext &context) str.append(" }."); context.trace(1, str); context.setSelectOnRetry(_selectOnRetry); - for (std::vector<uint32_t>::iterator it = _consumableErrors.begin(); - it != _consumableErrors.end(); ++it) - { - context.addConsumableError(*it); + for (unsigned int & _consumableError : _consumableErrors) { + context.addConsumableError(_consumableError); } context.addChildren(_routes); } @@ -74,14 +71,7 @@ CustomPolicy::merge(RoutingContext &context) context.trace(1, str); } - -CustomPolicyFactory::CustomPolicyFactory() : - _selectOnRetry(true), - _consumableErrors() -{ -} - -CustomPolicyFactory::CustomPolicyFactory(bool selectOnRetry) : +CustomPolicyFactory::CustomPolicyFactory(bool selectOnRetry) noexcept : _selectOnRetry(selectOnRetry), _consumableErrors() { @@ -94,12 +84,14 @@ CustomPolicyFactory::CustomPolicyFactory(bool selectOnRetry, uint32_t consumable _consumableErrors.push_back(consumableError); } -CustomPolicyFactory::CustomPolicyFactory(bool selectOnRetry, const std::vector<uint32_t> consumableErrors) : +CustomPolicyFactory::CustomPolicyFactory(bool selectOnRetry, std::vector<uint32_t> consumableErrors) : _selectOnRetry(selectOnRetry), - _consumableErrors(consumableErrors) + _consumableErrors(std::move(consumableErrors)) { } +CustomPolicyFactory::~CustomPolicyFactory() = default; + IRoutingPolicy::UP CustomPolicyFactory::create(const string ¶m) { @@ -112,20 +104,16 @@ CustomPolicyFactory::create(const string ¶m) } str.append(" }"); - std::vector<Route> routes; - parseRoutes(param, routes); - LOG(info, "Creating custom policy; selectOnRetry = %d, consumableErrors = %s, param = '%s'.", _selectOnRetry, str.c_str(), param.c_str()); - IRoutingPolicy::UP ret(new CustomPolicy(_selectOnRetry, _consumableErrors, routes)); - return ret; + return std::make_unique<CustomPolicy>(_selectOnRetry, _consumableErrors, parseRoutes(param)); } -void -CustomPolicyFactory::parseRoutes(const string &str, - std::vector<Route> &routes) +std::vector<Route> +CustomPolicyFactory::parseRoutes(const string &str) { + std::vector<Route> routes; using Separator = boost::char_separator<char>; using Tokenizer = boost::tokenizer<Separator>; Separator separator(","); @@ -136,6 +124,7 @@ CustomPolicyFactory::parseRoutes(const string &str, { routes.push_back(Route::parse(*it)); } + return routes; } } // namespace mbus diff --git a/messagebus/src/vespa/messagebus/testlib/custompolicy.h b/messagebus/src/vespa/messagebus/testlib/custompolicy.h index 490d5d9fd03..9aeec9322f1 100644 --- a/messagebus/src/vespa/messagebus/testlib/custompolicy.h +++ b/messagebus/src/vespa/messagebus/testlib/custompolicy.h @@ -15,9 +15,9 @@ private: public: CustomPolicy(bool selectOnRetry, - const std::vector<uint32_t> consumableErrors, - const std::vector<Route> &routes); - ~CustomPolicy(); + std::vector<uint32_t> consumableErrors, + std::vector<Route> routes); + ~CustomPolicy() override; void select(RoutingContext &context) override; void merge(RoutingContext &context) override; @@ -27,15 +27,15 @@ class CustomPolicyFactory : public SimpleProtocol::IPolicyFactory { private: bool _selectOnRetry; std::vector<uint32_t> _consumableErrors; - public: - CustomPolicyFactory(); - CustomPolicyFactory(bool selectOnRetry); + CustomPolicyFactory() noexcept : CustomPolicyFactory(true) { } + explicit CustomPolicyFactory(bool selectOnRetry) noexcept; CustomPolicyFactory(bool selectOnRetry, uint32_t consumableError); - CustomPolicyFactory(bool selectOnRetry, const std::vector<uint32_t> consumableErrors); + CustomPolicyFactory(bool selectOnRetry, std::vector<uint32_t> consumableErrors); + ~CustomPolicyFactory() override; IRoutingPolicy::UP create(const string ¶m) override; - static void parseRoutes(const string &str, std::vector<Route> &routes); + [[nodiscard]] static std::vector<Route> parseRoutes(const string &str); }; } // namespace mbus diff --git a/messagebus/src/vespa/messagebus/testlib/simpleprotocol.cpp b/messagebus/src/vespa/messagebus/testlib/simpleprotocol.cpp index 4475851e94a..f27d4f9aae1 100644 --- a/messagebus/src/vespa/messagebus/testlib/simpleprotocol.cpp +++ b/messagebus/src/vespa/messagebus/testlib/simpleprotocol.cpp @@ -28,7 +28,7 @@ public: class AllPolicyFactory : public SimpleProtocol::IPolicyFactory { public: IRoutingPolicy::UP create(const string &) override { - return IRoutingPolicy::UP(new AllPolicy()); + return std::make_unique<AllPolicy>(); } ~AllPolicyFactory() override; }; @@ -41,7 +41,7 @@ public: std::vector<Route> recipients; ctx.getMatchedRecipients(recipients); if (!recipients.empty()) { - int i = static_cast<const SimpleMessage&>(ctx.getMessage()).getHash(); + int i = dynamic_cast<const SimpleMessage&>(ctx.getMessage()).getHash(); ctx.addChild(recipients[std::abs(i) % recipients.size()]); } } @@ -54,7 +54,7 @@ public: class HashPolicyFactory : public SimpleProtocol::IPolicyFactory { public: IRoutingPolicy::UP create(const string &) override { - return IRoutingPolicy::UP(new HashPolicy()); + return std::make_unique<HashPolicy>(); } ~HashPolicyFactory() override; }; @@ -64,20 +64,16 @@ HashPolicyFactory::~HashPolicyFactory() = default; SimpleProtocol::SimpleProtocol() : _policies() { - addPolicyFactory("All", IPolicyFactory::SP(new AllPolicyFactory)); - addPolicyFactory("Hash", IPolicyFactory::SP(new HashPolicyFactory)); + addPolicyFactory("All", std::make_unique<AllPolicyFactory>()); + addPolicyFactory("Hash", std::make_unique<HashPolicyFactory>()); } -SimpleProtocol::~SimpleProtocol() -{ - // empty -} +SimpleProtocol::~SimpleProtocol() = default; void -SimpleProtocol::addPolicyFactory(const string &name, - IPolicyFactory::SP factory) +SimpleProtocol::addPolicyFactory(const string &name, IPolicyFactory::SP factory) { - _policies.insert(FactoryMap::value_type(name, factory)); + _policies.emplace(name, std::move(factory)); } const string & @@ -87,14 +83,13 @@ SimpleProtocol::getName() const } IRoutingPolicy::UP -SimpleProtocol::createPolicy(const string &name, - const string ¶m) const +SimpleProtocol::createPolicy(const string &name, const string ¶m) const { - FactoryMap::const_iterator it = _policies.find(name); + auto it = _policies.find(name); if (it != _policies.end()) { return it->second->create(param); } - return IRoutingPolicy::UP(); + return {}; } Blob @@ -103,13 +98,13 @@ SimpleProtocol::encode(const vespalib::Version &version, const Routable &routabl (void)version; if (routable.getType() == MESSAGE) { string str = "M"; - str.append(static_cast<const SimpleMessage&>(routable).getValue()); + str.append(dynamic_cast<const SimpleMessage&>(routable).getValue()); Blob ret(str.size()); memcpy(ret.data(), str.data(), str.size()); return ret; } else if (routable.getType() == REPLY) { string str = "R"; - str.append(static_cast<const SimpleReply&>(routable).getValue()); + str.append(dynamic_cast<const SimpleReply&>(routable).getValue()); Blob ret(str.size()); memcpy(ret.data(), str.data(), str.size()); return ret; @@ -126,22 +121,22 @@ SimpleProtocol::decode(const vespalib::Version &version, BlobRef data) const const char *d = data.data(); uint32_t s = data.size(); if (s < 1) { - return Routable::UP(); // too short + return {}; // too short } string str(d + 1, s - 1); if (*d == 'M') { - return Routable::UP(new SimpleMessage(str)); + return std::make_unique<SimpleMessage>(str); } else if (*d == 'R') { - return Routable::UP(new SimpleReply(str)); + return std::make_unique<SimpleReply>(str); } else { - return Routable::UP(); // unknown type + return {}; // unknown type } } void SimpleProtocol::simpleMerge(RoutingContext &ctx) { - Reply::UP ret(new EmptyReply()); + auto ret = std::make_unique<EmptyReply>(); for (RoutingNodeIterator it = ctx.getChildIterator(); it.isValid(); it.next()) { |