management: Controller allows empty callbacks

refs #3653

Change-Id: Id112960fa3f9f52854687b798c4632a922297ff2
diff --git a/src/management/nfd-controller.cpp b/src/management/nfd-controller.cpp
index 0a56777..5fc755d 100644
--- a/src/management/nfd-controller.cpp
+++ b/src/management/nfd-controller.cpp
@@ -45,10 +45,15 @@
 void
 Controller::startCommand(const shared_ptr<ControlCommand>& command,
                          const ControlParameters& parameters,
-                         const CommandSucceedCallback& onSuccess,
-                         const CommandFailCallback& onFailure,
+                         const CommandSucceedCallback& onSuccess1,
+                         const CommandFailCallback& onFailure1,
                          const CommandOptions& options)
 {
+  const CommandSucceedCallback& onSuccess = onSuccess1 ?
+    onSuccess1 : [] (const ControlParameters&) {};
+  const CommandFailCallback& onFailure = onFailure1 ?
+    onFailure1 : [] (uint32_t, const std::string&) {};
+
   Name requestName = command->getRequestName(options.getPrefix(), parameters);
   Interest interest(requestName);
   interest.setInterestLifetime(options.getTimeout());
@@ -73,16 +78,14 @@
   try {
     response.wireDecode(data.getContent().blockFromValue());
   }
-  catch (tlv::Error& e) {
-    if (static_cast<bool>(onFailure))
-      onFailure(ERROR_SERVER, e.what());
+  catch (const tlv::Error& e) {
+    onFailure(ERROR_SERVER, e.what());
     return;
   }
 
   uint32_t code = response.getCode();
   if (code >= ERROR_LBOUND) {
-    if (static_cast<bool>(onFailure))
-      onFailure(code, response.getText());
+    onFailure(code, response.getText());
     return;
   }
 
@@ -90,23 +93,20 @@
   try {
     parameters.wireDecode(response.getBody());
   }
-  catch (tlv::Error& e) {
-    if (static_cast<bool>(onFailure))
-      onFailure(ERROR_SERVER, e.what());
+  catch (const tlv::Error& e) {
+    onFailure(ERROR_SERVER, e.what());
     return;
   }
 
   try {
     command->validateResponse(parameters);
   }
-  catch (ControlCommand::ArgumentError& e) {
-    if (static_cast<bool>(onFailure))
-      onFailure(ERROR_SERVER, e.what());
+  catch (const ControlCommand::ArgumentError& e) {
+    onFailure(ERROR_SERVER, e.what());
     return;
   }
 
-  if (static_cast<bool>(onSuccess))
-    onSuccess(parameters);
+  onSuccess(parameters);
 }
 
 void
diff --git a/src/management/nfd-controller.hpp b/src/management/nfd-controller.hpp
index 3ae1cf2..9fa721b 100644
--- a/src/management/nfd-controller.hpp
+++ b/src/management/nfd-controller.hpp
@@ -164,10 +164,15 @@
 template<typename Dataset>
 inline void
 Controller::fetchDataset(shared_ptr<Dataset> dataset,
-                         const std::function<void(typename Dataset::ResultType)>& onSuccess,
-                         const CommandFailCallback& onFailure,
+                         const std::function<void(typename Dataset::ResultType)>& onSuccess1,
+                         const CommandFailCallback& onFailure1,
                          const CommandOptions& options)
 {
+  const std::function<void(typename Dataset::ResultType)>& onSuccess = onSuccess1 ?
+    onSuccess1 : [] (const typename Dataset::ResultType&) {};
+  const CommandFailCallback& onFailure = onFailure1 ?
+    onFailure1 : [] (uint32_t, const std::string&) {};
+
   Name prefix = dataset->getDatasetPrefix(options.getPrefix());
   this->fetchDataset(prefix,
                      bind(&Controller::processDatasetResponse<Dataset>, this, dataset, onSuccess, onFailure, _1),
@@ -190,6 +195,7 @@
     onFailure(ERROR_SERVER, ex.what());
     return;
   }
+
   onSuccess(result);
 }
 
diff --git a/tests/unit-tests/management/nfd-controller.t.cpp b/tests/unit-tests/management/nfd-controller.t.cpp
index c2e2b6c..1beb5db 100644
--- a/tests/unit-tests/management/nfd-controller.t.cpp
+++ b/tests/unit-tests/management/nfd-controller.t.cpp
@@ -56,16 +56,19 @@
   std::vector<ControlParameters> succeeds;
 };
 
+// This test suite focuses on ControlCommand functionality of Controller.
+// Individual commands are tested in nfd-control-command.t.cpp
+// StatusDataset functionality is tested in nfd-status-dataset.t.cpp
 BOOST_FIXTURE_TEST_SUITE(TestNfdController, CommandFixture)
 
-BOOST_AUTO_TEST_CASE(CommandSuccess)
+BOOST_AUTO_TEST_CASE(Success)
 {
   ControlParameters parameters;
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
                          parameters, succeedCallback, failCallback));
-  advanceClocks(time::milliseconds(1));
+  this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
   const Interest& requestInterest = face.sentInterests[0];
@@ -92,7 +95,7 @@
   responseData->setContent(responsePayload.wireEncode());
   face.receive(*responseData);
 
-  advanceClocks(time::milliseconds(1));
+  this->advanceClocks(time::milliseconds(1));
 
   BOOST_CHECK_EQUAL(failCodes.size(), 0);
   BOOST_REQUIRE_EQUAL(succeeds.size(), 1);
@@ -100,7 +103,55 @@
   BOOST_CHECK_EQUAL(succeeds.back().getFaceId(), responseBody.getFaceId());
 }
 
-BOOST_AUTO_TEST_CASE(CommandInvalidRequest)
+BOOST_AUTO_TEST_CASE(SuccessNoCallback)
+{
+  ControlParameters parameters;
+  parameters.setUri("tcp4://192.0.2.1:6363");
+
+  controller.start<FaceCreateCommand>(parameters, nullptr, failCallback);
+  this->advanceClocks(time::milliseconds(1));
+
+  BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
+  const Interest& requestInterest = face.sentInterests[0];
+
+  ControlParameters responseBody;
+  responseBody.setUri("tcp4://192.0.2.1:6363")
+              .setFaceId(22)
+              .setFacePersistency(ndn::nfd::FacePersistency::FACE_PERSISTENCY_PERSISTENT);
+  ControlResponse responsePayload(201, "created");
+  responsePayload.setBody(responseBody.wireEncode());
+
+  auto responseData = makeData(requestInterest.getName());
+  responseData->setContent(responsePayload.wireEncode());
+  face.receive(*responseData);
+
+  BOOST_CHECK_NO_THROW(this->advanceClocks(time::milliseconds(1)));
+
+  BOOST_CHECK_EQUAL(failCodes.size(), 0);
+}
+
+BOOST_AUTO_TEST_CASE(OptionsPrefix)
+{
+  ControlParameters parameters;
+  parameters.setName("/ndn/com/example");
+  parameters.setFaceId(400);
+
+  CommandOptions options;
+  options.setPrefix("/localhop/net/example/router1/nfd");
+
+  BOOST_CHECK_NO_THROW(controller.start<RibRegisterCommand>(
+                         parameters, succeedCallback, failCallback, options));
+  this->advanceClocks(time::milliseconds(1));
+
+  BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
+  const Interest& requestInterest = face.sentInterests[0];
+
+  FaceCreateCommand command;
+  BOOST_CHECK(Name("/localhop/net/example/router1/nfd/rib/register").isPrefixOf(
+              requestInterest.getName()));
+}
+
+BOOST_AUTO_TEST_CASE(InvalidRequest)
 {
   ControlParameters parameters;
   parameters.setName("ndn:/should-not-have-this-field");
@@ -111,14 +162,14 @@
                     ControlCommand::ArgumentError);
 }
 
-BOOST_AUTO_TEST_CASE(CommandErrorCode)
+BOOST_AUTO_TEST_CASE(ErrorCode)
 {
   ControlParameters parameters;
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
                          parameters, succeedCallback, failCallback));
-  advanceClocks(time::milliseconds(1));
+  this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
   const Interest& requestInterest = face.sentInterests[0];
@@ -128,21 +179,21 @@
   auto responseData = makeData(requestInterest.getName());
   responseData->setContent(responsePayload.wireEncode());
   face.receive(*responseData);
-  advanceClocks(time::milliseconds(1));
+  this->advanceClocks(time::milliseconds(1));
 
   BOOST_CHECK_EQUAL(succeeds.size(), 0);
   BOOST_REQUIRE_EQUAL(failCodes.size(), 1);
   BOOST_CHECK_EQUAL(failCodes.back(), 401);
 }
 
-BOOST_AUTO_TEST_CASE(CommandInvalidResponse)
+BOOST_AUTO_TEST_CASE(InvalidResponse)
 {
   ControlParameters parameters;
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
                          parameters, succeedCallback, failCallback));
-  advanceClocks(time::milliseconds(1));
+  this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
   const Interest& requestInterest = face.sentInterests[0];
@@ -157,54 +208,33 @@
   auto responseData = makeData(requestInterest.getName());
   responseData->setContent(responsePayload.wireEncode());
   face.receive(*responseData);
-  advanceClocks(time::milliseconds(1));
+  this->advanceClocks(time::milliseconds(1));
 
   BOOST_CHECK_EQUAL(succeeds.size(), 0);
   BOOST_REQUIRE_EQUAL(failCodes.size(), 1);
 }
 
-BOOST_AUTO_TEST_CASE(CommandNack)
+BOOST_AUTO_TEST_CASE(Nack)
 {
   ControlParameters parameters;
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
                          parameters, succeedCallback, failCallback));
-  advanceClocks(time::milliseconds(1));
+  this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
   const Interest& requestInterest = face.sentInterests[0];
 
   auto responseNack = makeNack(requestInterest, lp::NackReason::NO_ROUTE);
   face.receive(responseNack);
-  advanceClocks(time::milliseconds(1));
+  this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(failCodes.size(), 1);
   BOOST_CHECK_EQUAL(failCodes.back(), Controller::ERROR_NACK);
 }
 
-BOOST_AUTO_TEST_CASE(OptionsPrefix)
-{
-  ControlParameters parameters;
-  parameters.setName("/ndn/com/example");
-  parameters.setFaceId(400);
-
-  CommandOptions options;
-  options.setPrefix("/localhop/net/example/router1/nfd");
-
-  BOOST_CHECK_NO_THROW(controller.start<RibRegisterCommand>(
-                         parameters, succeedCallback, failCallback, options));
-  advanceClocks(time::milliseconds(1));
-
-  BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
-  const Interest& requestInterest = face.sentInterests[0];
-
-  FaceCreateCommand command;
-  BOOST_CHECK(Name("/localhop/net/example/router1/nfd/rib/register").isPrefixOf(
-              requestInterest.getName()));
-}
-
-BOOST_AUTO_TEST_CASE(OptionsTimeout)
+BOOST_AUTO_TEST_CASE(Timeout)
 {
   ControlParameters parameters;
   parameters.setUri("tcp4://192.0.2.1:6363");
@@ -214,16 +244,29 @@
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
                          parameters, succeedCallback, failCallback, options));
-  advanceClocks(time::milliseconds(1), 101); // Face's PIT granularity is 100ms
+  this->advanceClocks(time::milliseconds(1), 101); // Face's PIT granularity is 100ms
 
   BOOST_REQUIRE_EQUAL(failCodes.size(), 1);
   BOOST_CHECK_EQUAL(failCodes.back(), Controller::ERROR_TIMEOUT);
 }
 
-BOOST_AUTO_TEST_SUITE_END() // TestController
-BOOST_AUTO_TEST_SUITE_END() // Management
+BOOST_AUTO_TEST_CASE(FailureNoCallback)
+{
+  ControlParameters parameters;
+  parameters.setUri("tcp4://192.0.2.1:6363");
 
-// Controller::fetch<Dataset> has a separate test suite in nfd-status-dataset.t.cpp
+  CommandOptions options;
+  options.setTimeout(time::milliseconds(50));
+
+  controller.start<FaceCreateCommand>(parameters, succeedCallback, nullptr, options);
+  BOOST_CHECK_NO_THROW(this->advanceClocks(time::milliseconds(100), 10));
+  // timeout
+
+  BOOST_CHECK_EQUAL(succeeds.size(), 0);
+}
+
+BOOST_AUTO_TEST_SUITE_END() // TestNfdController
+BOOST_AUTO_TEST_SUITE_END() // Management
 
 } // namespace tests
 } // namespace nfd
diff --git a/tests/unit-tests/management/nfd-status-dataset.t.cpp b/tests/unit-tests/management/nfd-status-dataset.t.cpp
index bc4b495..4288d4a 100644
--- a/tests/unit-tests/management/nfd-status-dataset.t.cpp
+++ b/tests/unit-tests/management/nfd-status-dataset.t.cpp
@@ -202,6 +202,36 @@
 
 BOOST_AUTO_TEST_SUITE_END() // Failures
 
+BOOST_AUTO_TEST_SUITE(NoCallback)
+
+BOOST_AUTO_TEST_CASE(Success)
+{
+  controller.fetch<FaceDataset>(
+    nullptr,
+    failCallback);
+  this->advanceClocks(time::milliseconds(500));
+
+  FaceStatus payload;
+  payload.setFaceId(2577);
+  this->sendDataset("/localhost/nfd/faces/list", payload);
+  BOOST_CHECK_NO_THROW(this->advanceClocks(time::milliseconds(500)));
+
+  BOOST_CHECK_EQUAL(failCodes.size(), 0);
+}
+
+BOOST_AUTO_TEST_CASE(Failure)
+{
+  CommandOptions options;
+  options.setTimeout(time::milliseconds(3000));
+  controller.fetch<FaceDataset>(
+    [] (const std::vector<FaceStatus>& result) { BOOST_FAIL("fetchDataset should not succeed"); },
+    nullptr,
+    options);
+  BOOST_CHECK_NO_THROW(this->advanceClocks(time::milliseconds(500), 7));
+}
+
+BOOST_AUTO_TEST_SUITE_END() // NoCallback
+
 BOOST_AUTO_TEST_SUITE(Datasets)
 
 BOOST_AUTO_TEST_CASE(StatusGeneral)