mgmt: refactoring and cleanup in StatusDatasetContext
* remove {get,set}Expiry(): it's not the responsibility
of StatusDatasetContext to deal with in-memory storage
* increase the max payload size of each segment to 8k bytes
* minimize the number of memory allocations
* really check that the prefix does not end with a segment
component, as stated in the docs
* throw std::logic_error instead of std::domain_error
* improve exception messages
* cleanup doxygen
Change-Id: Id6990f3dd064cc90a45eab4ec1c52141f5212be5
diff --git a/tests/unit/mgmt/dispatcher.t.cpp b/tests/unit/mgmt/dispatcher.t.cpp
index 79d3dd3..d89da1b 100644
--- a/tests/unit/mgmt/dispatcher.t.cpp
+++ b/tests/unit/mgmt/dispatcher.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2020 Regents of the University of California.
+ * Copyright (c) 2013-2021 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -74,11 +74,8 @@
static Authorization
makeTestAuthorization()
{
- return [] (const Name& prefix,
- const Interest& interest,
- const ControlParameters* params,
- AcceptContinuation accept,
- RejectContinuation reject) {
+ return [] (const Name&, const Interest& interest, const ControlParameters*,
+ AcceptContinuation accept, RejectContinuation reject) {
if (interest.getName()[-1] == name::Component("valid")) {
accept("");
}
@@ -251,7 +248,7 @@
}
void
- wireDecode(const Block& wire) final
+ wireDecode(const Block&) final
{
m_state = EXPECTED_STATE;
}
@@ -271,9 +268,9 @@
{
AcceptContinuation authorizationAccept;
auto authorization =
- [&authorizationAccept] (const Name& prefix, const Interest& interest, const ControlParameters* params,
- AcceptContinuation accept, RejectContinuation reject) {
- authorizationAccept = accept;
+ [&authorizationAccept] (const Name&, const Interest&, const ControlParameters*,
+ AcceptContinuation accept, RejectContinuation) {
+ authorizationAccept = std::move(accept);
};
auto validateParameters =
@@ -282,11 +279,8 @@
};
size_t nCallbackCalled = 0;
- dispatcher
- .addControlCommand<StatefulParameters>("test",
- authorization,
- validateParameters,
- bind([&nCallbackCalled] { ++nCallbackCalled; }));
+ dispatcher.addControlCommand<StatefulParameters>("test", authorization, validateParameters,
+ bind([&nCallbackCalled] { ++nCallbackCalled; }));
dispatcher.addTopPrefix("/root");
advanceClocks(1_ms);
@@ -305,20 +299,15 @@
{
const uint8_t smallBuf[] = {0x81, 0x01, 0x01};
const Block smallBlock(smallBuf, sizeof(smallBuf));
- Block largeBlock;
- {
- EncodingBuffer encoder;
- for (size_t i = 0; i < 2500; ++i) {
- encoder.prependByte(1);
- }
- encoder.prependVarNumber(2500);
- encoder.prependVarNumber(129);
- largeBlock = encoder.block();
- }
+ const Block largeBlock = [] {
+ Block b(129, std::make_shared<const Buffer>(3000));
+ b.encode();
+ return b;
+ }();
dispatcher.addStatusDataset("test/small",
makeTestAuthorization(),
- [&smallBlock] (const Name& prefix, const Interest& interest,
+ [&smallBlock] (const Name&, const Interest&,
StatusDatasetContext& context) {
context.append(smallBlock);
context.append(smallBlock);
@@ -328,7 +317,7 @@
dispatcher.addStatusDataset("test/large",
makeTestAuthorization(),
- [&largeBlock] (const Name& prefix, const Interest& interest,
+ [&largeBlock] (const Name&, const Interest&,
StatusDatasetContext& context) {
context.append(largeBlock);
context.append(largeBlock);
@@ -338,8 +327,7 @@
dispatcher.addStatusDataset("test/reject",
makeTestAuthorization(),
- [] (const Name& prefix, const Interest& interest,
- StatusDatasetContext& context) {
+ [] (const Name&, const Interest&, StatusDatasetContext& context) {
context.reject();
});
@@ -351,8 +339,8 @@
face.receive(*makeInterest("/root/test/small/%80%00/invalid")); // returns 403
face.receive(*makeInterest("/root/test/small/%80%00/silent")); // silently ignored
advanceClocks(1_ms, 20);
- BOOST_CHECK_EQUAL(face.sentData.size(), 2);
+ BOOST_REQUIRE_EQUAL(face.sentData.size(), 2);
BOOST_CHECK_EQUAL(face.sentData[0].getContentType(), tlv::ContentType_Blob);
BOOST_CHECK_EQUAL(ControlResponse(face.sentData[0].getContent().blockFromValue()).getCode(), 403);
BOOST_CHECK_EQUAL(face.sentData[1].getContentType(), tlv::ContentType_Blob);
@@ -365,7 +353,7 @@
advanceClocks(1_ms, 10);
// one data packet is generated and sent to both places
- BOOST_CHECK_EQUAL(face.sentData.size(), 1);
+ BOOST_REQUIRE_EQUAL(face.sentData.size(), 1);
BOOST_CHECK_EQUAL(storage.size(), 1);
auto fetchedData = storage.find(interestSmall);
@@ -375,7 +363,7 @@
face.receive(*makeInterest(Name("/root/test/small/valid").appendVersion(10))); // should be ignored
face.receive(*makeInterest(Name("/root/test/small/valid").appendSegment(20))); // should be ignored
advanceClocks(1_ms, 10);
- BOOST_CHECK_EQUAL(face.sentData.size(), 1);
+ BOOST_REQUIRE_EQUAL(face.sentData.size(), 1);
BOOST_CHECK_EQUAL(storage.size(), 1);
Block content = face.sentData[0].getContent();
@@ -393,8 +381,8 @@
// two data packets are generated, the first one will be sent to both places
// while the second one will only be inserted into the in-memory storage
- BOOST_CHECK_EQUAL(face.sentData.size(), 1);
- BOOST_CHECK_EQUAL(storage.size(), 2);
+ BOOST_REQUIRE_EQUAL(face.sentData.size(), 1);
+ BOOST_REQUIRE_EQUAL(storage.size(), 2);
// segment0 should be sent through the face
const auto& component = face.sentData[0].getName().at(-1);
@@ -429,7 +417,8 @@
face.sentData.clear();
face.receive(*makeInterest("/root/test/reject/%80%00/valid")); // returns nack
advanceClocks(1_ms);
- BOOST_CHECK_EQUAL(face.sentData.size(), 1);
+
+ BOOST_REQUIRE_EQUAL(face.sentData.size(), 1);
BOOST_CHECK_EQUAL(face.sentData[0].getContentType(), tlv::ContentType_Nack);
BOOST_CHECK_EQUAL(ControlResponse(face.sentData[0].getContent().blockFromValue()).getCode(), 400);
BOOST_CHECK_EQUAL(storage.size(), 0); // the nack packet will not be inserted into the in-memory storage
diff --git a/tests/unit/mgmt/status-dataset-context.t.cpp b/tests/unit/mgmt/status-dataset-context.t.cpp
index c46292c..58c8382 100644
--- a/tests/unit/mgmt/status-dataset-context.t.cpp
+++ b/tests/unit/mgmt/status-dataset-context.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2020 Regents of the University of California.
+ * Copyright (c) 2013-2021 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -36,7 +36,6 @@
{
Name dataName;
Block content;
- time::milliseconds imsFresh;
bool isFinalBlock;
};
@@ -45,15 +44,12 @@
: interest(makeInterest("/test/context/interest"))
, contentBlock(makeStringBlock(tlv::Content, "/test/data/content"))
, context(*interest,
- [this] (const Name& dataName, const Block& content,
- time::milliseconds imsFresh, bool isFinalBlock) {
- SendDataArgs args{dataName, content, imsFresh, isFinalBlock};
- sendDataHistory.push_back(args);
+ [this] (auto&&... args) {
+ sendDataHistory.push_back({std::forward<decltype(args)>(args)...});
},
- [this] (const ControlResponse& resp) {
+ [this] (const auto& resp) {
sendNackHistory.push_back(resp);
})
- , defaultImsFresh(1000_ms)
{
}
@@ -79,12 +75,11 @@
}
protected:
- std::vector<SendDataArgs> sendDataHistory;
- std::vector<ControlResponse> sendNackHistory;
shared_ptr<Interest> interest;
Block contentBlock;
- mgmt::StatusDatasetContext context;
- time::milliseconds defaultImsFresh;
+ StatusDatasetContext context;
+ std::vector<SendDataArgs> sendDataHistory;
+ std::vector<ControlResponse> sendNackHistory;
};
BOOST_AUTO_TEST_SUITE(Mgmt)
@@ -103,95 +98,95 @@
{
Name validPrefix = Name(interest->getName()).append("/valid");
BOOST_CHECK_NO_THROW(context.setPrefix(validPrefix));
- BOOST_CHECK(context.getPrefix()[-1].isVersion());
BOOST_CHECK_EQUAL(context.getPrefix().getPrefix(-1), validPrefix);
+ BOOST_CHECK(context.getPrefix()[-1].isVersion());
+
+ // trailing version component is preserved
+ validPrefix.appendVersion(42);
+ BOOST_CHECK_NO_THROW(context.setPrefix(validPrefix));
+ BOOST_CHECK_EQUAL(context.getPrefix(), validPrefix);
}
BOOST_AUTO_TEST_CASE(SetInvalid)
{
+ // Interest name is not a prefix of invalidPrefix
Name invalidPrefix = Name(interest->getName()).getPrefix(-1).append("/invalid");
- BOOST_CHECK_THROW(context.setPrefix(invalidPrefix), std::invalid_argument);
+ BOOST_CHECK_EXCEPTION(context.setPrefix(invalidPrefix), std::invalid_argument, [] (const auto& e) {
+ return e.what() == "prefix must start with the Interest's name"s;
+ });
+
+ // invalidPrefix contains a segment component
+ invalidPrefix = Name(interest->getName()).appendSegment(1);
+ BOOST_CHECK_EXCEPTION(context.setPrefix(invalidPrefix), std::invalid_argument, [] (const auto& e) {
+ return e.what() == "prefix must not contain a segment component"s;
+ });
}
-BOOST_AUTO_TEST_CASE(SetValidWithAppendCalled)
+BOOST_AUTO_TEST_CASE(SetValidAfterAppend)
{
Name validPrefix = Name(interest->getName()).append("/valid");
context.append(contentBlock);
- BOOST_CHECK_THROW(context.setPrefix(validPrefix), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.setPrefix(validPrefix), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call setPrefix() after append/end/reject"s;
+ });
}
-BOOST_AUTO_TEST_CASE(SetValidWithEndCalled)
+BOOST_AUTO_TEST_CASE(SetValidAfterEnd)
{
Name validPrefix = Name(interest->getName()).append("/valid");
context.end();
- BOOST_CHECK_THROW(context.setPrefix(validPrefix), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.setPrefix(validPrefix), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call setPrefix() after append/end/reject"s;
+ });
}
-BOOST_AUTO_TEST_CASE(SetValidWithRejectCalled)
+BOOST_AUTO_TEST_CASE(SetValidAfterReject)
{
Name validPrefix = Name(interest->getName()).append("/valid");
context.reject();
- BOOST_CHECK_THROW(context.setPrefix(validPrefix), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.setPrefix(validPrefix), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call setPrefix() after append/end/reject"s;
+ });
}
BOOST_AUTO_TEST_SUITE_END() // Prefix
-BOOST_AUTO_TEST_SUITE(Expiry)
-
-BOOST_AUTO_TEST_CASE(GetAndSet)
-{
- auto period = 9527_ms;
- BOOST_CHECK_EQUAL(context.getExpiry(), 1000_ms);
- BOOST_CHECK_EQUAL(context.setExpiry(period).getExpiry(), period);
-}
-
-BOOST_AUTO_TEST_SUITE_END() // Expiry
-
BOOST_AUTO_TEST_SUITE(Respond)
BOOST_AUTO_TEST_CASE(Basic)
{
- BOOST_CHECK_NO_THROW(context.append(contentBlock));
- BOOST_CHECK(sendDataHistory.empty()); // does not call end yet
+ context.append(contentBlock);
+ BOOST_CHECK(sendDataHistory.empty()); // end() not called yet
- BOOST_CHECK_NO_THROW(context.end());
-
+ context.end();
BOOST_REQUIRE_EQUAL(sendDataHistory.size(), 1);
- const auto& args = sendDataHistory[0];
+ const auto& args = sendDataHistory[0];
BOOST_CHECK_EQUAL(args.dataName, makeSegmentName(0));
BOOST_CHECK_EQUAL(args.content.blockFromValue(), contentBlock);
- BOOST_CHECK_EQUAL(args.imsFresh, defaultImsFresh);
BOOST_CHECK_EQUAL(args.isFinalBlock, true);
}
BOOST_AUTO_TEST_CASE(Large)
{
- static Block largeBlock = [] () -> Block {
- EncodingBuffer encoder;
- size_t maxBlockSize = MAX_NDN_PACKET_SIZE >> 1;
- for (size_t i = 0; i < maxBlockSize; ++i) {
- encoder.prependByte(1);
- }
- encoder.prependVarNumber(maxBlockSize);
- encoder.prependVarNumber(tlv::Content);
- return encoder.block();
+ const Block largeBlock = [] {
+ Block b(tlv::Content, std::make_shared<const Buffer>(10000));
+ b.encode();
+ return b;
}();
- BOOST_CHECK_NO_THROW(context.append(largeBlock));
- BOOST_CHECK_NO_THROW(context.end());
+ context.append(largeBlock);
+ BOOST_CHECK_EQUAL(sendDataHistory.size(), 1);
- // two segments are generated
+ context.end();
BOOST_REQUIRE_EQUAL(sendDataHistory.size(), 2);
- // check segment0
+ // check segment 0
BOOST_CHECK_EQUAL(sendDataHistory[0].dataName, makeSegmentName(0));
- BOOST_CHECK_EQUAL(sendDataHistory[0].imsFresh, defaultImsFresh);
BOOST_CHECK_EQUAL(sendDataHistory[0].isFinalBlock, false);
- // check segment1
+ // check segment 1
BOOST_CHECK_EQUAL(sendDataHistory[1].dataName, makeSegmentName(1));
- BOOST_CHECK_EQUAL(sendDataHistory[1].imsFresh, defaultImsFresh);
BOOST_CHECK_EQUAL(sendDataHistory[1].isFinalBlock, true);
// check data content
@@ -203,39 +198,34 @@
BOOST_AUTO_TEST_CASE(MultipleSmall)
{
- size_t nBlocks = 100;
+ const size_t nBlocks = 100;
for (size_t i = 0 ; i < nBlocks ; i ++) {
- BOOST_CHECK_NO_THROW(context.append(contentBlock));
+ context.append(contentBlock);
}
- BOOST_CHECK_NO_THROW(context.end());
+ context.end();
// check data to in-memory storage
BOOST_REQUIRE_EQUAL(sendDataHistory.size(), 1);
BOOST_CHECK_EQUAL(sendDataHistory[0].dataName, makeSegmentName(0));
- BOOST_CHECK_EQUAL(sendDataHistory[0].imsFresh, defaultImsFresh);
BOOST_CHECK_EQUAL(sendDataHistory[0].isFinalBlock, true);
auto contentMultiBlocks = concatenateDataContent();
- BOOST_CHECK_NO_THROW(contentMultiBlocks.parse());
+ contentMultiBlocks.parse();
BOOST_CHECK_EQUAL(contentMultiBlocks.elements().size(), nBlocks);
- for (auto&& element : contentMultiBlocks.elements()) {
+ for (const auto& element : contentMultiBlocks.elements()) {
BOOST_CHECK_EQUAL(element, contentBlock);
}
}
BOOST_AUTO_TEST_SUITE_END() // Respond
-BOOST_AUTO_TEST_SUITE(Reject)
-
-BOOST_AUTO_TEST_CASE(Basic)
+BOOST_AUTO_TEST_CASE(Reject)
{
BOOST_CHECK_NO_THROW(context.reject());
BOOST_REQUIRE_EQUAL(sendNackHistory.size(), 1);
BOOST_CHECK_EQUAL(sendNackHistory[0].getCode(), 400);
}
-BOOST_AUTO_TEST_SUITE_END() // Reject
-
class AbnormalStateTestFixture
{
protected:
@@ -245,7 +235,7 @@
}
protected:
- mgmt::StatusDatasetContext context;
+ StatusDatasetContext context;
};
BOOST_FIXTURE_TEST_SUITE(AbnormalState, AbnormalStateTestFixture)
@@ -254,7 +244,9 @@
{
const uint8_t buf[] = {0x82, 0x01, 0x02};
BOOST_CHECK_NO_THROW(context.append(Block(buf, sizeof(buf))));
- BOOST_CHECK_THROW(context.reject(), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.reject(), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call reject() after append/end"s;
+ });
}
BOOST_AUTO_TEST_CASE(AppendEndReject)
@@ -262,40 +254,51 @@
const uint8_t buf[] = {0x82, 0x01, 0x02};
BOOST_CHECK_NO_THROW(context.append(Block(buf, sizeof(buf))));
BOOST_CHECK_NO_THROW(context.end());
- BOOST_CHECK_THROW(context.reject(), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.reject(), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call reject() after append/end"s;
+ });
}
BOOST_AUTO_TEST_CASE(EndAppend)
{
BOOST_CHECK_NO_THROW(context.end());
- // end, append -> error
const uint8_t buf[] = {0x82, 0x01, 0x02};
- BOOST_CHECK_THROW(context.append(Block(buf, sizeof(buf))), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.append(Block(buf, sizeof(buf))), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call append() on a finalized context"s;
+ });
}
BOOST_AUTO_TEST_CASE(EndEnd)
{
BOOST_CHECK_NO_THROW(context.end());
- BOOST_CHECK_THROW(context.end(), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.end(), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call end() on a finalized context"s;
+ });
}
BOOST_AUTO_TEST_CASE(EndReject)
{
BOOST_CHECK_NO_THROW(context.end());
- BOOST_CHECK_THROW(context.reject(), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.reject(), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call reject() after append/end"s;
+ });
}
BOOST_AUTO_TEST_CASE(RejectAppend)
{
BOOST_CHECK_NO_THROW(context.reject());
const uint8_t buf[] = {0x82, 0x01, 0x02};
- BOOST_CHECK_THROW(context.append(Block(buf, sizeof(buf))), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.append(Block(buf, sizeof(buf))), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call append() on a finalized context"s;
+ });
}
BOOST_AUTO_TEST_CASE(RejectEnd)
{
BOOST_CHECK_NO_THROW(context.reject());
- BOOST_CHECK_THROW(context.end(), std::domain_error);
+ BOOST_CHECK_EXCEPTION(context.end(), std::logic_error, [] (const auto& e) {
+ return e.what() == "cannot call end() on a finalized context"s;
+ });
}
BOOST_AUTO_TEST_SUITE_END() // AbnormalState