management: Controller::CommandFailCallback exposes ControlResponse

refs #3739

Change-Id: Ib4b66cd99647ab930450fc3b472be565084be160
diff --git a/src/detail/face-impl.hpp b/src/detail/face-impl.hpp
index 92f102f..a7c332d 100644
--- a/src/detail/face-impl.hpp
+++ b/src/detail/face-impl.hpp
@@ -45,8 +45,6 @@
 
 namespace ndn {
 
-using ndn::nfd::ControlParameters;
-
 /**
  * @brief implementation detail of Face
  */
@@ -188,7 +186,7 @@
                  uint64_t flags,
                  const nfd::CommandOptions& options)
   {
-    ControlParameters params;
+    nfd::ControlParameters params;
     params.setName(prefix);
     params.setFlags(flags);
 
@@ -196,8 +194,8 @@
 
     m_face.m_nfdController->start<nfd::RibRegisterCommand>(
       params,
-      [=] (const ControlParameters&) { this->afterPrefixRegistered(prefixToRegister, onSuccess); },
-      [=] (uint32_t code, const std::string& reason) { onFailure(prefixToRegister->getPrefix(), reason); },
+      [=] (const nfd::ControlParameters&) { this->afterPrefixRegistered(prefixToRegister, onSuccess); },
+      [=] (const nfd::ControlResponse& resp) { onFailure(prefixToRegister->getPrefix(), resp.getText()); },
       options);
 
     return reinterpret_cast<const RegisteredPrefixId*>(prefixToRegister.get());
@@ -236,12 +234,12 @@
         m_interestFilterTable.remove(filter);
       }
 
-      ControlParameters params;
+      nfd::ControlParameters params;
       params.setName(record.getPrefix());
       m_face.m_nfdController->start<nfd::RibUnregisterCommand>(
         params,
-        [=] (const ControlParameters&) { this->finalizeUnregisterPrefix(i, onSuccess); },
-        [=] (uint32_t code, const std::string& reason) { onFailure(reason); },
+        [=] (const nfd::ControlParameters&) { this->finalizeUnregisterPrefix(i, onSuccess); },
+        [=] (const nfd::ControlResponse& resp) { onFailure(resp.getText()); },
         record.getCommandOptions());
     }
     else {
diff --git a/src/management/nfd-controller.cpp b/src/management/nfd-controller.cpp
index 72299f0..b395d6b 100644
--- a/src/management/nfd-controller.cpp
+++ b/src/management/nfd-controller.cpp
@@ -20,7 +20,8 @@
  */
 
 #include "nfd-controller.hpp"
-#include "nfd-control-response.hpp"
+#include "../face.hpp"
+#include "../security/key-chain.hpp"
 #include "../util/segment-fetcher.hpp"
 
 namespace ndn {
@@ -52,7 +53,7 @@
   const CommandSucceedCallback& onSuccess = onSuccess1 ?
     onSuccess1 : [] (const ControlParameters&) {};
   const CommandFailCallback& onFailure = onFailure1 ?
-    onFailure1 : [] (uint32_t, const std::string&) {};
+    onFailure1 : [] (const ControlResponse&) {};
 
   Name requestName = command->getRequestName(options.getPrefix(), parameters);
   Interest interest(requestName);
@@ -60,10 +61,15 @@
   m_keyChain.sign(interest, options.getSigningInfo());
 
   m_face.expressInterest(interest,
-                         bind(&Controller::processCommandResponse, this, _2,
-                              command, onSuccess, onFailure),
-                         bind(onFailure, ERROR_NACK, "network Nack received"),
-                         bind(onFailure, ERROR_TIMEOUT, "request timed out"));
+    [=] (const Interest&, const Data& data) {
+      this->processCommandResponse(data, command, onSuccess, onFailure);
+    },
+    [=] (const Interest&, const lp::Nack&) {
+      onFailure(ControlResponse(Controller::ERROR_NACK, "network Nack received"));
+    },
+    [=] (const Interest&) {
+      onFailure(ControlResponse(Controller::ERROR_TIMEOUT, "request timed out"));
+    });
 }
 
 void
@@ -77,7 +83,7 @@
       this->processValidatedCommandResponse(*data, command, onSuccess, onFailure);
     },
     [=] (const shared_ptr<const Data>&, const std::string& msg) {
-      onFailure(ERROR_VALIDATION, msg);
+      onFailure(ControlResponse(ERROR_VALIDATION, msg));
     }
   );
 }
@@ -93,13 +99,13 @@
     response.wireDecode(data.getContent().blockFromValue());
   }
   catch (const tlv::Error& e) {
-    onFailure(ERROR_SERVER, e.what());
+    onFailure(ControlResponse(ERROR_SERVER, e.what()));
     return;
   }
 
   uint32_t code = response.getCode();
   if (code >= ERROR_LBOUND) {
-    onFailure(code, response.getText());
+    onFailure(response);
     return;
   }
 
@@ -108,7 +114,7 @@
     parameters.wireDecode(response.getBody());
   }
   catch (const tlv::Error& e) {
-    onFailure(ERROR_SERVER, e.what());
+    onFailure(ControlResponse(ERROR_SERVER, e.what()));
     return;
   }
 
@@ -116,7 +122,7 @@
     command->validateResponse(parameters);
   }
   catch (const ControlCommand::ArgumentError& e) {
-    onFailure(ERROR_SERVER, e.what());
+    onFailure(ControlResponse(ERROR_SERVER, e.what()));
     return;
   }
 
@@ -126,7 +132,7 @@
 void
 Controller::fetchDataset(const Name& prefix,
                          const std::function<void(const ConstBufferPtr&)>& processResponse,
-                         const CommandFailCallback& onFailure,
+                         const DatasetFailCallback& onFailure,
                          const CommandOptions& options)
 {
   Interest baseInterest(prefix);
@@ -137,7 +143,7 @@
 }
 
 void
-Controller::processDatasetFetchError(const CommandFailCallback& onFailure,
+Controller::processDatasetFetchError(const DatasetFailCallback& onFailure,
                                      uint32_t code, std::string msg)
 {
   switch (static_cast<SegmentFetcher::ErrorCode>(code)) {
diff --git a/src/management/nfd-controller.hpp b/src/management/nfd-controller.hpp
index f1d6df9..297f113 100644
--- a/src/management/nfd-controller.hpp
+++ b/src/management/nfd-controller.hpp
@@ -23,13 +23,19 @@
 #define NDN_MANAGEMENT_NFD_CONTROLLER_HPP
 
 #include "nfd-control-command.hpp"
+#include "nfd-control-response.hpp"
 #include "nfd-status-dataset.hpp"
 #include "nfd-command-options.hpp"
-#include "../face.hpp"
-#include "../security/key-chain.hpp"
 #include "../security/validator-null.hpp"
 
 namespace ndn {
+
+namespace security {
+class KeyChain;
+} // namespace security
+class Face;
+class Validator;
+
 namespace nfd {
 
 /**
@@ -39,7 +45,8 @@
 
 /**
  * \ingroup management
- * \brief NFD Management protocol - ControlCommand client
+ * \brief NFD Management protocol client
+ * \sa https://redmine.named-data.net/projects/nfd/wiki/Management
  */
 class Controller : noncopyable
 {
@@ -50,12 +57,16 @@
 
   /** \brief a callback on command failure
    */
-  typedef function<void(uint32_t code, const std::string& reason)> CommandFailCallback;
+  typedef function<void(const ControlResponse&)> CommandFailCallback;
+
+  /** \brief a callback on dataset retrieval failure
+   */
+  typedef function<void(uint32_t code, const std::string& reason)> DatasetFailCallback;
 
   /** \brief construct a Controller that uses face for transport,
    *         and uses the passed KeyChain to sign commands
    */
-  Controller(Face& face, KeyChain& keyChain, Validator& validator = s_validatorNull);
+  Controller(Face& face, security::KeyChain& keyChain, Validator& validator = s_validatorNull);
 
   /** \brief start command execution
    */
@@ -75,7 +86,7 @@
   template<typename Dataset>
   typename std::enable_if<std::is_default_constructible<Dataset>::value>::type
   fetch(const std::function<void(typename Dataset::ResultType)>& onSuccess,
-        const CommandFailCallback& onFailure,
+        const DatasetFailCallback& onFailure,
         const CommandOptions& options = CommandOptions())
   {
     this->fetchDataset(make_shared<Dataset>(), onSuccess, onFailure, options);
@@ -87,7 +98,7 @@
   void
   fetch(const ParamType& param,
         const std::function<void(typename Dataset::ResultType)>& onSuccess,
-        const CommandFailCallback& onFailure,
+        const DatasetFailCallback& onFailure,
         const CommandOptions& options = CommandOptions())
   {
     this->fetchDataset(make_shared<Dataset>(param), onSuccess, onFailure, options);
@@ -117,24 +128,24 @@
   void
   fetchDataset(shared_ptr<Dataset> dataset,
                const std::function<void(typename Dataset::ResultType)>& onSuccess,
-               const CommandFailCallback& onFailure,
+               const DatasetFailCallback& onFailure,
                const CommandOptions& options);
 
   void
   fetchDataset(const Name& prefix,
                const std::function<void(const ConstBufferPtr&)>& processResponse,
-               const CommandFailCallback& onFailure,
+               const DatasetFailCallback& onFailure,
                const CommandOptions& options);
 
   template<typename Dataset>
   void
   processDatasetResponse(shared_ptr<Dataset> dataset,
                          const std::function<void(typename Dataset::ResultType)>& onSuccess,
-                         const CommandFailCallback& onFailure,
+                         const DatasetFailCallback& onFailure,
                          ConstBufferPtr payload);
 
   void
-  processDatasetFetchError(const CommandFailCallback& onFailure, uint32_t code, std::string msg);
+  processDatasetFetchError(const DatasetFailCallback& onFailure, uint32_t code, std::string msg);
 
 public:
   /** \brief error code for timeout
@@ -159,7 +170,7 @@
 
 protected:
   Face& m_face;
-  KeyChain& m_keyChain;
+  security::KeyChain& m_keyChain;
   Validator& m_validator;
 
 private:
@@ -170,12 +181,12 @@
 inline void
 Controller::fetchDataset(shared_ptr<Dataset> dataset,
                          const std::function<void(typename Dataset::ResultType)>& onSuccess1,
-                         const CommandFailCallback& onFailure1,
+                         const DatasetFailCallback& onFailure1,
                          const CommandOptions& options)
 {
   const std::function<void(typename Dataset::ResultType)>& onSuccess = onSuccess1 ?
     onSuccess1 : [] (const typename Dataset::ResultType&) {};
-  const CommandFailCallback& onFailure = onFailure1 ?
+  const DatasetFailCallback& onFailure = onFailure1 ?
     onFailure1 : [] (uint32_t, const std::string&) {};
 
   Name prefix = dataset->getDatasetPrefix(options.getPrefix());
@@ -189,15 +200,15 @@
 inline void
 Controller::processDatasetResponse(shared_ptr<Dataset> dataset,
                                    const std::function<void(typename Dataset::ResultType)>& onSuccess,
-                                   const CommandFailCallback& onFailure,
+                                   const DatasetFailCallback& onFailure,
                                    ConstBufferPtr payload)
 {
   typename Dataset::ResultType result;
   try {
     result = dataset->parseResult(payload);
   }
-  catch (const tlv::Error& ex) {
-    onFailure(ERROR_SERVER, ex.what());
+  catch (const tlv::Error& e) {
+    onFailure(ERROR_SERVER, e.what());
     return;
   }
 
diff --git a/tests/unit-tests/management/nfd-controller-fixture.hpp b/tests/unit-tests/management/nfd-controller-fixture.hpp
index 492cbc2..d722dda 100644
--- a/tests/unit-tests/management/nfd-controller-fixture.hpp
+++ b/tests/unit-tests/management/nfd-controller-fixture.hpp
@@ -41,7 +41,8 @@
   ControllerFixture()
     : face(io, m_keyChain)
     , controller(face, m_keyChain, m_validator)
-    , failCallback(bind(&ControllerFixture::fail, this, _1, _2))
+    , commandFailCallback(bind(&ControllerFixture::recordCommandFail, this, _1))
+    , datasetFailCallback(bind(&ControllerFixture::recordDatasetFail, this, _1, _2))
   {
     Name identityName("/localhost/ControllerFixture");
     if (this->addIdentity(identityName)) {
@@ -65,7 +66,13 @@
 
 private:
   void
-  fail(uint32_t code, const std::string& reason)
+  recordCommandFail(const ControlResponse& response)
+  {
+    failCodes.push_back(response.getCode());
+  }
+
+  void
+  recordDatasetFail(uint32_t code, const std::string& reason)
   {
     failCodes.push_back(code);
   }
@@ -73,7 +80,8 @@
 protected:
   ndn::util::DummyClientFace face;
   Controller controller;
-  Controller::CommandFailCallback failCallback;
+  Controller::CommandFailCallback commandFailCallback;
+  Controller::DatasetFailCallback datasetFailCallback;
   std::vector<uint32_t> failCodes;
 
 private:
diff --git a/tests/unit-tests/management/nfd-controller.t.cpp b/tests/unit-tests/management/nfd-controller.t.cpp
index 8a944f8..13031fb 100644
--- a/tests/unit-tests/management/nfd-controller.t.cpp
+++ b/tests/unit-tests/management/nfd-controller.t.cpp
@@ -66,7 +66,7 @@
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
-                         parameters, succeedCallback, failCallback));
+                         parameters, succeedCallback, commandFailCallback));
   this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
@@ -107,7 +107,7 @@
   ControlParameters parameters;
   parameters.setUri("tcp4://192.0.2.1:6363");
 
-  controller.start<FaceCreateCommand>(parameters, nullptr, failCallback);
+  controller.start<FaceCreateCommand>(parameters, nullptr, commandFailCallback);
   this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
@@ -139,7 +139,7 @@
   options.setPrefix("/localhop/net/example/router1/nfd");
 
   BOOST_CHECK_NO_THROW(controller.start<RibRegisterCommand>(
-                         parameters, succeedCallback, failCallback, options));
+                         parameters, succeedCallback, commandFailCallback, options));
   this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
@@ -157,7 +157,7 @@
   // Uri is missing
 
   BOOST_CHECK_THROW(controller.start<FaceCreateCommand>(
-                      parameters, succeedCallback, failCallback),
+                      parameters, succeedCallback, commandFailCallback),
                     ControlCommand::ArgumentError);
 }
 
@@ -169,7 +169,7 @@
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
-                         parameters, succeedCallback, failCallback));
+                         parameters, succeedCallback, commandFailCallback));
   this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
@@ -198,7 +198,7 @@
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
-                         parameters, succeedCallback, failCallback));
+                         parameters, succeedCallback, commandFailCallback));
   this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
@@ -222,7 +222,7 @@
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
-                         parameters, succeedCallback, failCallback));
+                         parameters, succeedCallback, commandFailCallback));
   this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
@@ -250,7 +250,7 @@
   parameters.setUri("tcp4://192.0.2.1:6363");
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
-                         parameters, succeedCallback, failCallback));
+                         parameters, succeedCallback, commandFailCallback));
   this->advanceClocks(time::milliseconds(1));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
@@ -273,7 +273,7 @@
   options.setTimeout(time::milliseconds(50));
 
   BOOST_CHECK_NO_THROW(controller.start<FaceCreateCommand>(
-                         parameters, succeedCallback, failCallback, options));
+                         parameters, succeedCallback, commandFailCallback, options));
   this->advanceClocks(time::milliseconds(1), 101); // Face's PIT granularity is 100ms
 
   BOOST_REQUIRE_EQUAL(failCodes.size(), 1);
diff --git a/tests/unit-tests/management/nfd-status-dataset.t.cpp b/tests/unit-tests/management/nfd-status-dataset.t.cpp
index a836746..e2f62a3 100644
--- a/tests/unit-tests/management/nfd-status-dataset.t.cpp
+++ b/tests/unit-tests/management/nfd-status-dataset.t.cpp
@@ -112,7 +112,7 @@
   options.setTimeout(time::milliseconds(3000));
   controller.fetch<FaceDataset>(
     [] (const std::vector<FaceStatus>& result) { BOOST_FAIL("fetchDataset should not succeed"); },
-    failCallback,
+    datasetFailCallback,
     options);
   this->advanceClocks(time::milliseconds(500), 7);
 
@@ -124,7 +124,7 @@
 {
   controller.fetch<FaceDataset>(
     [] (const std::vector<FaceStatus>& result) { BOOST_FAIL("fetchDataset should not succeed"); },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   face.receive(*makeData("/localhost/nfd/faces/list/%FD%00"));
@@ -140,7 +140,7 @@
 
   controller.fetch<FaceDataset>(
     [] (const std::vector<FaceStatus>& result) { BOOST_FAIL("fetchDataset should not succeed"); },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   FaceStatus payload;
@@ -156,7 +156,7 @@
 {
   controller.fetch<FaceDataset>(
     [] (const std::vector<FaceStatus>& result) { BOOST_FAIL("fetchDataset should not succeed"); },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 1);
@@ -171,7 +171,7 @@
 {
   controller.fetch<FaceDataset>(
     [] (const std::vector<FaceStatus>& result) { BOOST_FAIL("fetchDataset should not succeed"); },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   Name payload; // Name is not valid FaceStatus
@@ -186,7 +186,7 @@
 {
   controller.fetch<FaceDataset>(
     [] (const std::vector<FaceStatus>& result) { BOOST_FAIL("fetchDataset should not succeed"); },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   FaceStatus payload1;
@@ -207,7 +207,7 @@
 {
   controller.fetch<FaceDataset>(
     nullptr,
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   FaceStatus payload;
@@ -241,7 +241,7 @@
       hasResult = true;
       BOOST_CHECK_EQUAL(result.getNfdVersion(), "0.4.2");
     },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   ForwarderStatus payload;
@@ -262,7 +262,7 @@
       BOOST_CHECK_EQUAL(result.size(), 2);
       BOOST_CHECK_EQUAL(result.front().getFaceId(), 24485);
     },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   FaceStatus payload1;
@@ -288,7 +288,7 @@
       BOOST_CHECK_EQUAL(result.size(), 1);
       BOOST_CHECK_EQUAL(result.front().getFaceId(), 8795);
     },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   Name prefix("/localhost/nfd/faces/query");
@@ -316,7 +316,7 @@
       BOOST_CHECK_EQUAL(result.size(), 1);
       BOOST_CHECK_EQUAL(result.front().getFaceId(), 14022);
     },
-    failCallback,
+    datasetFailCallback,
     options);
   this->advanceClocks(time::milliseconds(500));
 
@@ -340,7 +340,7 @@
       BOOST_CHECK_EQUAL(result.size(), 2);
       BOOST_CHECK_EQUAL(result.front().getLocalUri(), "tcp4://192.0.2.1:6363");
     },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   ChannelStatus payload1;
@@ -363,7 +363,7 @@
       BOOST_CHECK_EQUAL(result.size(), 2);
       BOOST_CHECK_EQUAL(result.front().getPrefix(), "/wYs7fzYcfG");
     },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   FibEntry payload1;
@@ -386,7 +386,7 @@
       BOOST_CHECK_EQUAL(result.size(), 2);
       BOOST_CHECK_EQUAL(result.front().getName(), "/8MLz6N3B");
     },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   StrategyChoice payload1;
@@ -409,7 +409,7 @@
       BOOST_CHECK_EQUAL(result.size(), 2);
       BOOST_CHECK_EQUAL(result.front().getName(), "/zXxBth97ee");
     },
-    failCallback);
+    datasetFailCallback);
   this->advanceClocks(time::milliseconds(500));
 
   RibEntry payload1;
@@ -434,7 +434,7 @@
       BOOST_CHECK_EQUAL(result.size(), 1);
       BOOST_CHECK_EQUAL(result.front().getName(), "/e6L5K4ascd");
     },
-    failCallback,
+    datasetFailCallback,
     options);
   this->advanceClocks(time::milliseconds(500));