detail: improve ScopedCancelHandle
This also removes deprecated implicit conversion operators
from 'FooHandle' types to 'const FooId*'
Change-Id: I5b412bf833bb106b8f5aa483dbe09b6c58736e19
diff --git a/ndn-cxx/detail/cancel-handle.cpp b/ndn-cxx/detail/cancel-handle.cpp
index 8057249..4ce807c 100644
--- a/ndn-cxx/detail/cancel-handle.cpp
+++ b/ndn-cxx/detail/cancel-handle.cpp
@@ -24,11 +24,6 @@
namespace ndn {
namespace detail {
-CancelHandle::CancelHandle(function<void()> cancel)
- : m_cancel(std::move(cancel))
-{
-}
-
void
CancelHandle::cancel() const
{
@@ -38,42 +33,5 @@
}
}
-ScopedCancelHandle::ScopedCancelHandle(CancelHandle hdl)
- : m_hdl(std::move(hdl))
-{
-}
-
-ScopedCancelHandle::ScopedCancelHandle(ScopedCancelHandle&& other)
- : m_hdl(other.release())
-{
-}
-
-ScopedCancelHandle&
-ScopedCancelHandle::operator=(ScopedCancelHandle&& other)
-{
- cancel();
- m_hdl = other.release();
- return *this;
-}
-
-ScopedCancelHandle::~ScopedCancelHandle()
-{
- m_hdl.cancel();
-}
-
-void
-ScopedCancelHandle::cancel()
-{
- release().cancel();
-}
-
-CancelHandle
-ScopedCancelHandle::release()
-{
- CancelHandle hdl;
- std::swap(hdl, m_hdl);
- return hdl;
-}
-
} // namespace detail
} // namespace ndn
diff --git a/ndn-cxx/detail/cancel-handle.hpp b/ndn-cxx/detail/cancel-handle.hpp
index e2cb9b5..6415993 100644
--- a/ndn-cxx/detail/cancel-handle.hpp
+++ b/ndn-cxx/detail/cancel-handle.hpp
@@ -32,10 +32,13 @@
class CancelHandle
{
public:
- CancelHandle() noexcept = default;
+ CancelHandle() noexcept;
explicit
- CancelHandle(function<void()> cancel);
+ CancelHandle(std::function<void()> cancel) noexcept
+ : m_cancel(std::move(cancel))
+ {
+ }
/** \brief Cancel the operation.
*/
@@ -43,19 +46,29 @@
cancel() const;
private:
- mutable function<void()> m_cancel;
+ mutable std::function<void()> m_cancel;
};
+inline
+CancelHandle::CancelHandle() noexcept = default;
+
/** \brief Cancels an operation automatically upon destruction.
*/
+template<typename HandleT>
class ScopedCancelHandle
{
-public:
- ScopedCancelHandle() noexcept = default;
+ static_assert(std::is_convertible<HandleT*, CancelHandle*>::value,
+ "HandleT must publicly derive from CancelHandle");
- /** \brief Implicit constructor from CancelHandle.
+public:
+ ScopedCancelHandle() noexcept;
+
+ /** \brief Implicit constructor from HandleT.
*/
- ScopedCancelHandle(CancelHandle hdl);
+ ScopedCancelHandle(HandleT hdl) noexcept
+ : m_hdl(std::move(hdl))
+ {
+ }
/** \brief Copy construction is disallowed.
*/
@@ -63,7 +76,10 @@
/** \brief Move constructor.
*/
- ScopedCancelHandle(ScopedCancelHandle&& other);
+ ScopedCancelHandle(ScopedCancelHandle&& other) noexcept
+ : m_hdl(other.release())
+ {
+ }
/** \brief Copy assignment is disallowed.
*/
@@ -73,28 +89,50 @@
/** \brief Move assignment operator.
*/
ScopedCancelHandle&
- operator=(ScopedCancelHandle&& other);
+ operator=(ScopedCancelHandle&& other)
+ {
+ m_hdl.cancel();
+ m_hdl = other.release();
+ return *this;
+ }
/** \brief Cancel the operation.
*/
- ~ScopedCancelHandle();
+ ~ScopedCancelHandle()
+ {
+ m_hdl.cancel();
+ }
/** \brief Cancel the operation.
*/
void
- cancel();
+ cancel()
+ {
+ release().cancel();
+ }
/** \brief Release the operation so that it won't be cancelled when this ScopedCancelHandle is
* destructed.
- * \return the CancelHandle.
*/
- CancelHandle
- release();
+ HandleT
+ release() noexcept
+ {
+ return std::exchange(m_hdl, HandleT{});
+ }
+
+ explicit
+ operator bool() const noexcept
+ {
+ return !!m_hdl;
+ }
private:
- CancelHandle m_hdl;
+ HandleT m_hdl;
};
+template<typename T>
+ScopedCancelHandle<T>::ScopedCancelHandle() noexcept = default;
+
} // namespace detail
} // namespace ndn
diff --git a/ndn-cxx/face.cpp b/ndn-cxx/face.cpp
index 70fd8db..53ed411 100644
--- a/ndn-cxx/face.cpp
+++ b/ndn-cxx/face.cpp
@@ -396,7 +396,6 @@
PendingInterestHandle::PendingInterestHandle(Face& face, const PendingInterestId* id)
: CancelHandle([&face, id] { face.cancelPendingInterest(id); })
- , m_id(id)
{
}
@@ -427,7 +426,6 @@
InterestFilterHandle::InterestFilterHandle(Face& face, const InterestFilterId* id)
: CancelHandle([&face, id] { face.clearInterestFilter(id); })
- , m_id(id)
{
}
diff --git a/ndn-cxx/face.hpp b/ndn-cxx/face.hpp
index c14e556..6ee8949 100644
--- a/ndn-cxx/face.hpp
+++ b/ndn-cxx/face.hpp
@@ -551,15 +551,6 @@
PendingInterestHandle() noexcept = default;
PendingInterestHandle(Face& face, const PendingInterestId* id);
-
- [[deprecated]]
- operator const PendingInterestId*() const noexcept
- {
- return m_id;
- }
-
-private:
- const PendingInterestId* m_id = nullptr;
};
/** \brief A scoped handle of pending Interest.
@@ -577,7 +568,7 @@
* \warning Canceling a pending Interest after the face has been destructed may trigger undefined
* behavior.
*/
-using ScopedPendingInterestHandle = detail::ScopedCancelHandle;
+using ScopedPendingInterestHandle = detail::ScopedCancelHandle<PendingInterestHandle>;
/** \brief A handle of registered prefix.
*/
@@ -592,12 +583,6 @@
RegisteredPrefixHandle(Face& face, const RegisteredPrefixId* id);
- [[deprecated]]
- operator const RegisteredPrefixId*() const noexcept
- {
- return m_id;
- }
-
/** \brief Unregister the prefix.
* \warning Unregistering a prefix after the face has been destructed may trigger undefined
* behavior.
@@ -627,7 +612,7 @@
* \warning Unregistering a prefix after the face has been destructed may trigger undefined
* behavior.
*/
-using ScopedRegisteredPrefixHandle = detail::ScopedCancelHandle;
+using ScopedRegisteredPrefixHandle = detail::ScopedCancelHandle<RegisteredPrefixHandle>;
/** \brief A handle of registered Interest filter.
*
@@ -645,15 +630,6 @@
InterestFilterHandle() noexcept = default;
InterestFilterHandle(Face& face, const InterestFilterId* id);
-
- [[deprecated]]
- operator const InterestFilterId*() const noexcept
- {
- return m_id;
- }
-
-private:
- const InterestFilterId* m_id = nullptr;
};
/** \brief A scoped handle of registered Interest filter.
@@ -671,7 +647,7 @@
* \warning Unsetting an Interest filter after the face has been destructed may trigger
* undefined behavior.
*/
-using ScopedInterestFilterHandle = detail::ScopedCancelHandle;
+using ScopedInterestFilterHandle = detail::ScopedCancelHandle<InterestFilterHandle>;
} // namespace ndn
diff --git a/ndn-cxx/util/scheduler.hpp b/ndn-cxx/util/scheduler.hpp
index 7a2c2f3..887cf97 100644
--- a/ndn-cxx/util/scheduler.hpp
+++ b/ndn-cxx/util/scheduler.hpp
@@ -127,11 +127,7 @@
* \warning Canceling an event after the scheduler has been destructed may trigger undefined
* behavior.
*/
-class ScopedEventId : public detail::ScopedCancelHandle
-{
-public:
- using ScopedCancelHandle::ScopedCancelHandle;
-};
+using ScopedEventId = detail::ScopedCancelHandle<EventId>;
/** \brief Generic time-based scheduler
*/
diff --git a/tests/integrated/face.cpp b/tests/integrated/face.cpp
index ddc0cd3..aac5443 100644
--- a/tests/integrated/face.cpp
+++ b/tests/integrated/face.cpp
@@ -180,9 +180,9 @@
this->terminateAfter(4_s);
int nRegSuccess = 0, nUnregSuccess = 0;
- auto id = this->face.registerPrefix("/Hello/World",
+ auto handle = this->face.registerPrefix("/Hello/World",
[&] (const Name&) { ++nRegSuccess; },
- [] (const Name&, const std::string& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
+ [] (const Name&, const auto& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
this->sched.schedule(1_s, [&nRegSuccess] {
BOOST_CHECK_EQUAL(nRegSuccess, 1);
@@ -190,10 +190,10 @@
BOOST_CHECK(!output.empty());
});
- this->sched.schedule(2_s, [this, id, &nUnregSuccess] {
- this->face.unregisterPrefix(id,
+ this->sched.schedule(2_s, [&] {
+ handle.unregister(
[&] { ++nUnregSuccess; },
- [] (const std::string& msg) { BOOST_ERROR("unexpected unregister prefix failure: " << msg); });
+ [] (const auto& msg) { BOOST_ERROR("unexpected unregister prefix failure: " << msg); });
});
this->sched.schedule(3_s, [&nUnregSuccess] {
@@ -215,11 +215,11 @@
this->face.setInterestFilter("/Hello/World",
[&] (const InterestFilter&, const Interest&) { ++nInterests1; },
[&] (const Name&) { ++nRegSuccess1; },
- [] (const Name&, const std::string& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
+ [] (const Name&, const auto& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
this->face.setInterestFilter("/Los/Angeles/Lakers",
[&] (const InterestFilter&, const Interest&) { BOOST_ERROR("unexpected Interest"); },
[&] (const Name&) { ++nRegSuccess2; },
- [] (const Name&, const std::string& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
+ [] (const Name&, const auto& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
this->sched.schedule(500_ms, [] {
std::string output = executeCommand("nfdc route list | grep /Hello/World");
@@ -245,7 +245,7 @@
this->face.setInterestFilter(InterestFilter("/Hello/World", "<><b><c>?"),
[&] (const InterestFilter&, const Interest& interest) { receivedInterests.insert(interest.getName()); },
[&] (const Name&) { ++nRegSuccess; },
- [] (const Name&, const std::string& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
+ [] (const Name&, const auto& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
this->sched.schedule(700_ms, [] {
std::string output = executeCommand("nfdc route list | grep /Hello/World");
@@ -270,7 +270,7 @@
// no Interest shall arrive because prefix isn't registered in forwarder
this->face.setInterestFilter(InterestFilter("/Hello/World", "<><b><c>?"),
- [&] (const InterestFilter&, const Interest& interest) { BOOST_ERROR("unexpected Interest"); });
+ [&] (const InterestFilter&, const Interest&) { BOOST_ERROR("unexpected Interest"); });
this->sched.schedule(700_ms, [] {
// Boost.Test would fail if a child process exits with non-zero. http://stackoverflow.com/q/5325202
@@ -300,7 +300,7 @@
}
},
nullptr,
- [] (const Name&, const std::string& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
+ [] (const Name&, const auto& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
char outcome1, outcome2;
this->sendInterest(700_ms, *makeInterest("/Hello/World/data", false, 50_ms), outcome1);
@@ -320,7 +320,7 @@
this->face.put(*makeData(makeVeryLongName(interest.getName())));
},
nullptr,
- [] (const Name&, const std::string& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
+ [] (const Name&, const auto& msg) { BOOST_ERROR("unexpected register prefix failure: " << msg); });
this->sendInterest(1_s, *makeInterest("/Hello/World/oversized", true, 50_ms));
diff --git a/tests/unit/detail/cancel-handle.t.cpp b/tests/unit/detail/cancel-handle.t.cpp
index b87f9cd..2e951dd 100644
--- a/tests/unit/detail/cancel-handle.t.cpp
+++ b/tests/unit/detail/cancel-handle.t.cpp
@@ -55,11 +55,13 @@
BOOST_AUTO_TEST_SUITE(ScopedHandle)
+using ScopedTestHandle = ScopedCancelHandle<CancelHandle>;
+
BOOST_AUTO_TEST_CASE(ManualCancel)
{
int nCancels = 0;
{
- ScopedCancelHandle hdl = makeDummyCancelHandle(nCancels);
+ ScopedTestHandle hdl = makeDummyCancelHandle(nCancels);
BOOST_CHECK_EQUAL(nCancels, 0);
hdl.cancel();
@@ -72,7 +74,7 @@
{
int nCancels = 0;
{
- ScopedCancelHandle hdl = makeDummyCancelHandle(nCancels);
+ ScopedTestHandle hdl = makeDummyCancelHandle(nCancels);
BOOST_CHECK_EQUAL(nCancels, 0);
} // hdl goes out of scope
BOOST_CHECK_EQUAL(nCancels, 1);
@@ -82,7 +84,7 @@
{
int nCancels1 = 0, nCancels2 = 0;
{
- ScopedCancelHandle hdl = makeDummyCancelHandle(nCancels1);
+ ScopedTestHandle hdl = makeDummyCancelHandle(nCancels1);
hdl = makeDummyCancelHandle(nCancels2);
BOOST_CHECK_EQUAL(nCancels1, 1);
BOOST_CHECK_EQUAL(nCancels2, 0);
@@ -94,7 +96,7 @@
{
int nCancels = 0;
{
- ScopedCancelHandle hdl = makeDummyCancelHandle(nCancels);
+ ScopedTestHandle hdl = makeDummyCancelHandle(nCancels);
hdl.release();
hdl.cancel(); // no effect
} // hdl goes out of scope
@@ -104,10 +106,10 @@
BOOST_AUTO_TEST_CASE(MoveConstruct)
{
int nCancels = 0;
- unique_ptr<ScopedCancelHandle> hdl1;
+ unique_ptr<ScopedTestHandle> hdl1;
{
- ScopedCancelHandle hdl2 = makeDummyCancelHandle(nCancels);
- hdl1 = make_unique<ScopedCancelHandle>(std::move(hdl2));
+ ScopedTestHandle hdl2 = makeDummyCancelHandle(nCancels);
+ hdl1 = make_unique<ScopedTestHandle>(std::move(hdl2));
} // hdl2 goes out of scope
BOOST_CHECK_EQUAL(nCancels, 0);
hdl1.reset();
@@ -118,9 +120,9 @@
{
int nCancels = 0;
{
- ScopedCancelHandle hdl1;
+ ScopedTestHandle hdl1;
{
- ScopedCancelHandle hdl2 = makeDummyCancelHandle(nCancels);
+ ScopedTestHandle hdl2 = makeDummyCancelHandle(nCancels);
hdl1 = std::move(hdl2);
} // hdl2 goes out of scope
BOOST_CHECK_EQUAL(nCancels, 0);
diff --git a/tests/unit/face.t.cpp b/tests/unit/face.t.cpp
index 43b4db5..eb01f8f 100644
--- a/tests/unit/face.t.cpp
+++ b/tests/unit/face.t.cpp
@@ -267,25 +267,6 @@
} while (false));
}
-BOOST_AUTO_TEST_CASE(RemovePendingInterestId)
-{
- const PendingInterestId* interestId =
- face.expressInterest(*makeInterest("/Hello/World", true, 50_ms),
- bind([] { BOOST_FAIL("Unexpected data"); }),
- bind([] { BOOST_FAIL("Unexpected nack"); }),
- bind([] { BOOST_FAIL("Unexpected timeout"); }));
- advanceClocks(10_ms);
-
- face.removePendingInterest(interestId);
- advanceClocks(10_ms);
-
- face.receive(*makeData("/Hello/World/%21"));
- advanceClocks(200_ms, 5);
-
- // avoid "test case [...] did not check any assertions" message from Boost.Test
- BOOST_CHECK(true);
-}
-
BOOST_AUTO_TEST_CASE(CancelPendingInterestHandle)
{
auto hdl = face.expressInterest(*makeInterest("/Hello/World", true, 50_ms),
@@ -391,7 +372,7 @@
bool hasInterest1 = false, hasData = false;
// first InterestFilter allows loopback and should receive Interest
- face.setInterestFilter("/", [&] (const InterestFilter&, const Interest& interest) {
+ face.setInterestFilter("/", [&] (const InterestFilter&, const Interest&) {
hasInterest1 = true;
// do not respond with Data right away, so Face must send Interest to forwarder
});
@@ -534,11 +515,10 @@
{
size_t nInterests = 0;
size_t nRegs = 0;
- const RegisteredPrefixId* regPrefixId =
- face.setInterestFilter("/Hello/World",
- bind([&nInterests] { ++nInterests; }),
- bind([&nRegs] { ++nRegs; }),
- bind([] { BOOST_FAIL("Unexpected setInterestFilter failure"); }));
+ auto hdl = face.setInterestFilter("/Hello/World",
+ bind([&nInterests] { ++nInterests; }),
+ bind([&nRegs] { ++nRegs; }),
+ bind([] { BOOST_FAIL("Unexpected setInterestFilter failure"); }));
advanceClocks(25_ms, 4);
BOOST_CHECK_EQUAL(nRegs, 1);
BOOST_CHECK_EQUAL(nInterests, 0);
@@ -558,7 +538,7 @@
BOOST_CHECK_EQUAL(nInterests, 2);
// removing filter
- face.unsetInterestFilter(regPrefixId);
+ hdl.cancel();
advanceClocks(25_ms, 4);
face.receive(*makeInterest("/Hello/World/%21/3"));
@@ -585,10 +565,9 @@
BOOST_AUTO_TEST_CASE(SetUnsetInterestFilterWithoutSucessCallback)
{
size_t nInterests = 0;
- const RegisteredPrefixId* regPrefixId =
- face.setInterestFilter("/Hello/World",
- bind([&nInterests] { ++nInterests; }),
- bind([] { BOOST_FAIL("Unexpected setInterestFilter failure"); }));
+ auto hdl = face.setInterestFilter("/Hello/World",
+ bind([&nInterests] { ++nInterests; }),
+ bind([] { BOOST_FAIL("Unexpected setInterestFilter failure"); }));
advanceClocks(25_ms, 4);
BOOST_CHECK_EQUAL(nInterests, 0);
@@ -606,7 +585,7 @@
BOOST_CHECK_EQUAL(nInterests, 2);
// removing filter
- face.unsetInterestFilter(regPrefixId);
+ hdl.cancel();
advanceClocks(25_ms, 4);
face.receive(*makeInterest("/Hello/World/%21/3"));
@@ -650,18 +629,6 @@
BOOST_CHECK_EQUAL(nRegFailed, 1);
}
-BOOST_AUTO_TEST_CASE(RegisterUnregisterPrefixFunc)
-{
- const RegisteredPrefixId* regPrefixId = nullptr;
- BOOST_CHECK(runPrefixReg([&] (const auto& success, const auto& failure) {
- regPrefixId = face.registerPrefix("/Hello/World", success, failure);
- }));
-
- BOOST_CHECK(runPrefixUnreg([&] (const auto& success, const auto& failure) {
- face.unregisterPrefix(regPrefixId, success, failure);
- }));
-}
-
BOOST_FIXTURE_TEST_CASE(RegisterUnregisterPrefixFail, FaceFixture<NoPrefixRegReply>)
{
BOOST_CHECK(!runPrefixReg([&] (const auto& success, const auto& failure) {
diff --git a/tests/unit/util/scheduler.t.cpp b/tests/unit/util/scheduler.t.cpp
index d403a7c..021c1f0 100644
--- a/tests/unit/util/scheduler.t.cpp
+++ b/tests/unit/util/scheduler.t.cpp
@@ -249,7 +249,7 @@
BOOST_CHECK(eid == EventId{});
}
-BOOST_AUTO_TEST_CASE(Valid)
+BOOST_AUTO_TEST_CASE(OperatorBool)
{
EventId eid;
BOOST_CHECK_EQUAL(static_cast<bool>(eid), false);
@@ -366,6 +366,26 @@
BOOST_CHECK_EQUAL(hit, 2);
}
+BOOST_AUTO_TEST_CASE(OperatorBool)
+{
+ ScopedEventId se;
+ BOOST_CHECK_EQUAL(static_cast<bool>(se), false);
+ BOOST_CHECK_EQUAL(!se, true);
+
+ se = scheduler.schedule(10_ms, []{});
+ BOOST_CHECK_EQUAL(static_cast<bool>(se), true);
+ BOOST_CHECK_EQUAL(!se, false);
+
+ se.cancel();
+ BOOST_CHECK(!se);
+
+ se = scheduler.schedule(10_ms, []{});
+ BOOST_CHECK(se);
+
+ se.release();
+ BOOST_CHECK(!se);
+}
+
BOOST_AUTO_TEST_SUITE_END() // ScopedEventId
BOOST_AUTO_TEST_SUITE_END() // TestScheduler