management: Controller accepts CommandOptions

refs #2039

The following are deprecated:

* Controller::start overloads, except the overload taking CommandOptions
* Controller::Sign typedef
* Controller::getDefaultCommandTimeout
* ControlCommand::getPrefix
* ControlCommand::getRequestName(ControlParameters)

Change-Id: I8c405498597bdca115b71eec87acf23d28a266b1
diff --git a/src/management/nfd-control-command.cpp b/src/management/nfd-control-command.cpp
index fd9ec6b..d92245c 100644
--- a/src/management/nfd-control-command.cpp
+++ b/src/management/nfd-control-command.cpp
@@ -20,14 +20,15 @@
  */
 
 #include "nfd-control-command.hpp"
+#include "nfd-command-options.hpp" // only used in deprecated functions
 
 namespace ndn {
 namespace nfd {
 
 ControlCommand::ControlCommand(const std::string& module, const std::string& verb)
-  : m_prefix("ndn:/localhost/nfd")
+  : m_module(module)
+  , m_verb(verb)
 {
-  m_prefix.append(module).append(verb);
 }
 
 void
@@ -53,15 +54,35 @@
 }
 
 Name
-ControlCommand::getRequestName(const ControlParameters& parameters) const
+ControlCommand::getRequestName(const Name& commandPrefix,
+                               const ControlParameters& parameters) const
 {
   this->validateRequest(parameters);
 
-  Name name = m_prefix;
+  Name name = commandPrefix;
+  name.append(m_module).append(m_verb);
   name.append(parameters.wireEncode());
   return name;
 }
 
+const Name&
+ControlCommand::getPrefix() const
+{
+  // m_prefix is needed because we can't return a local variable as a reference,
+  // and changing the "const Name&" return type to "Name" may cause incompatibility
+  if (m_prefix.empty()) {
+    m_prefix.append(CommandOptions::DEFAULT_PREFIX);
+    m_prefix.append(m_module).append(m_verb);
+  }
+  return m_prefix;
+}
+
+Name
+ControlCommand::getRequestName(const ControlParameters& parameters) const
+{
+  return this->getRequestName(CommandOptions::DEFAULT_PREFIX, parameters);
+}
+
 ControlCommand::FieldValidator::FieldValidator()
   : m_required(CONTROL_PARAMETER_UBOUND)
   , m_optional(CONTROL_PARAMETER_UBOUND)
diff --git a/src/management/nfd-control-command.hpp b/src/management/nfd-control-command.hpp
index 8285a6d..e226486 100644
--- a/src/management/nfd-control-command.hpp
+++ b/src/management/nfd-control-command.hpp
@@ -47,16 +47,8 @@
     }
   };
 
-  /** \return Name prefix of this ControlCommand
-   */
-  const Name&
-  getPrefix() const
-  {
-    return m_prefix;
-  }
-
   /** \brief validate request parameters
-   *  \throw ArgumentError
+   *  \throw ArgumentError if parameters are invalid
    */
   virtual void
   validateRequest(const ControlParameters& parameters) const;
@@ -67,7 +59,7 @@
   applyDefaultsToRequest(ControlParameters& parameters) const;
 
   /** \brief validate response parameters
-   *  \throw ArgumentError
+   *  \throw ArgumentError if parameters are invalid
    */
   virtual void
   validateResponse(const ControlParameters& parameters) const;
@@ -78,6 +70,21 @@
   applyDefaultsToResponse(ControlParameters& parameters) const;
 
   /** \brief construct the Name for a request Interest
+   *  \throw ArgumentError if parameters are invalid
+   */
+  Name
+  getRequestName(const Name& commandPrefix, const ControlParameters& parameters) const;
+
+public: // deprecated
+  /** \return Name prefix of this ControlCommand
+   *  \deprecated use getRequestName
+   */
+  const Name&
+  getPrefix() const;
+
+  /** \brief construct the Name for a request Interest
+   *  \throw ArgumentError if parameters are invalid
+   *  \deprecated use the two-argument overload
    */
   Name
   getRequestName(const ControlParameters& parameters) const;
@@ -133,7 +140,12 @@
   FieldValidator m_responseValidator;
 
 private:
-  Name m_prefix;
+  name::Component m_module;
+  name::Component m_verb;
+
+  /** \deprecated kept to support getPrefix
+   */
+  mutable Name m_prefix;
 };
 
 
diff --git a/src/management/nfd-controller.cpp b/src/management/nfd-controller.cpp
index 2d5b348..1517522 100644
--- a/src/management/nfd-controller.cpp
+++ b/src/management/nfd-controller.cpp
@@ -25,6 +25,10 @@
 namespace ndn {
 namespace nfd {
 
+const uint32_t Controller::ERROR_TIMEOUT = 10060;
+const uint32_t Controller::ERROR_SERVER = 500;
+const uint32_t Controller::ERROR_LBOUND = 400;
+
 Controller::Controller(Face& face)
   : m_face(face)
   , m_internalKeyChain(make_shared<KeyChain>())
@@ -43,6 +47,38 @@
                          const ControlParameters& parameters,
                          const CommandSucceedCallback& onSuccess,
                          const CommandFailCallback& onFailure,
+                         const CommandOptions& options)
+{
+  Name requestName = command->getRequestName(options.getPrefix(), parameters);
+  Interest interest(requestName);
+  interest.setInterestLifetime(options.getTimeout());
+
+  switch (options.getSigningParamsKind()) {
+  case CommandOptions::SIGNING_PARAMS_DEFAULT:
+    m_keyChain.sign(interest);
+    break;
+  case CommandOptions::SIGNING_PARAMS_IDENTITY:
+    m_keyChain.signByIdentity(interest, options.getSigningIdentity());
+    break;
+  case CommandOptions::SIGNING_PARAMS_CERTIFICATE:
+    m_keyChain.sign(interest, options.getSigningCertificate());
+    break;
+  default:
+    BOOST_ASSERT(false);
+    break;
+  }
+
+  m_face.expressInterest(interest,
+                         bind(&Controller::processCommandResponse, this, _2,
+                              command, onSuccess, onFailure),
+                         bind(onFailure, ERROR_TIMEOUT, "request timed out"));
+}
+
+void
+Controller::startCommand(const shared_ptr<ControlCommand>& command,
+                         const ControlParameters& parameters,
+                         const CommandSucceedCallback& onSuccess,
+                         const CommandFailCallback& onFailure,
                          const Sign& sign,
                          const time::milliseconds& timeout)
 {
@@ -53,12 +89,10 @@
   interest.setInterestLifetime(timeout);
   sign(interest);
 
-  // http://msdn.microsoft.com/en-us/library/windows/desktop/ms740668.aspx
-  const uint32_t timeoutCode = 10060;
   m_face.expressInterest(interest,
                          bind(&Controller::processCommandResponse, this, _2,
                               command, onSuccess, onFailure),
-                         bind(onFailure, timeoutCode, "Command Interest timed out"));
+                         bind(onFailure, ERROR_TIMEOUT, "request timed out"));
 }
 
 void
@@ -69,21 +103,18 @@
 {
   /// \todo verify Data signature
 
-  const uint32_t serverErrorCode = 500;
-
   ControlResponse response;
   try {
     response.wireDecode(data.getContent().blockFromValue());
   }
-  catch (ndn::tlv::Error& e) {
+  catch (tlv::Error& e) {
     if (static_cast<bool>(onFailure))
-      onFailure(serverErrorCode, e.what());
+      onFailure(ERROR_SERVER, e.what());
     return;
   }
 
   uint32_t code = response.getCode();
-  const uint32_t errorCodeLowerBound = 400;
-  if (code >= errorCodeLowerBound) {
+  if (code >= ERROR_LBOUND) {
     if (static_cast<bool>(onFailure))
       onFailure(code, response.getText());
     return;
@@ -93,9 +124,9 @@
   try {
     parameters.wireDecode(response.getBody());
   }
-  catch (ndn::tlv::Error& e) {
+  catch (tlv::Error& e) {
     if (static_cast<bool>(onFailure))
-      onFailure(serverErrorCode, e.what());
+      onFailure(ERROR_SERVER, e.what());
     return;
   }
 
@@ -104,7 +135,7 @@
   }
   catch (ControlCommand::ArgumentError& e) {
     if (static_cast<bool>(onFailure))
-      onFailure(serverErrorCode, e.what());
+      onFailure(ERROR_SERVER, e.what());
     return;
   }
 
diff --git a/src/management/nfd-controller.hpp b/src/management/nfd-controller.hpp
index f5eff7c..cd8db1d 100644
--- a/src/management/nfd-controller.hpp
+++ b/src/management/nfd-controller.hpp
@@ -25,6 +25,7 @@
 #include "nfd-control-command.hpp"
 #include "../face.hpp"
 #include "../security/key-chain.hpp"
+#include "nfd-command-options.hpp"
 
 namespace ndn {
 namespace nfd {
@@ -49,6 +50,7 @@
   typedef function<void(uint32_t/*code*/,const std::string&/*reason*/)> CommandFailCallback;
 
   /** \brief a function to sign the request Interest
+   *  \deprecated arbitrary signing function is no longer supported
    */
   typedef function<void(Interest&)> Sign;
 
@@ -70,11 +72,26 @@
   start(const ControlParameters& parameters,
         const CommandSucceedCallback& onSuccess,
         const CommandFailCallback& onFailure,
-        const time::milliseconds& timeout = getDefaultCommandTimeout())
+        const CommandOptions& options = CommandOptions())
   {
-    start<Command>(parameters, onSuccess, onFailure,
-                   bind(&KeyChain::sign<Interest>, &m_keyChain, _1),
-                   timeout);
+    shared_ptr<ControlCommand> command = make_shared<Command>();
+    this->startCommand(command, parameters, onSuccess, onFailure, options);
+  }
+
+  /** \brief start command execution
+   *  \deprecated use the overload taking CommandOptions
+   */
+  template<typename Command>
+  void
+  start(const ControlParameters& parameters,
+        const CommandSucceedCallback& onSuccess,
+        const CommandFailCallback& onFailure,
+        const time::milliseconds& timeout)
+  {
+    CommandOptions options;
+    options.setTimeout(timeout);
+
+    this->start<Command>(parameters, onSuccess, onFailure, options);
   }
 
   /** \brief start command execution
@@ -84,6 +101,8 @@
    *  \note IdentityCertificate() creates a certificate with an empty name, which is an
    *        invalid certificate.  A valid IdentityCertificate has at least 4 name components,
    *        as it follows `<...>/KEY/<...>/<key-id>/ID-CERT/<version>` naming model.
+   *
+   *  \deprecated use the overload taking CommandOptions
    */
   template<typename Command>
   void
@@ -93,19 +112,21 @@
         const IdentityCertificate& certificate,
         const time::milliseconds& timeout = getDefaultCommandTimeout())
   {
+    CommandOptions options;
     if (certificate.getName().empty()) {
-      start<Command>(parameters, onSuccess, onFailure, timeout);
+      options.setSigningDefault();
     }
     else {
-      start<Command>(parameters, onSuccess, onFailure,
-        bind(static_cast<void(KeyChain::*)(Interest&,const Name&)>(&KeyChain::sign<Interest>),
-             &m_keyChain, _1, cref(certificate.getName())),
-        timeout);
+      options.setSigningCertificate(certificate);
     }
+    options.setTimeout(timeout);
+
+    this->start<Command>(parameters, onSuccess, onFailure, options);
   }
 
   /** \brief start command execution
    *  \param identity the identity used to sign request Interests
+   *  \deprecated use the overload taking CommandOptions
    */
   template<typename Command>
   void
@@ -115,13 +136,16 @@
         const Name& identity,
         const time::milliseconds& timeout = getDefaultCommandTimeout())
   {
-    start<Command>(parameters, onSuccess, onFailure,
-                   bind(&KeyChain::signByIdentity<Interest>, &m_keyChain, _1, cref(identity)),
-                   timeout);
+    CommandOptions options;
+    options.setSigningIdentity(identity);
+    options.setTimeout(timeout);
+
+    this->start<Command>(parameters, onSuccess, onFailure, options);
   }
 
   /** \brief start command execution
    *  \param sign a function to sign request Interests
+   *  \deprecated arbitrary signing function is no longer supported
    */
   template<typename Command>
   void
@@ -141,6 +165,15 @@
                const ControlParameters& parameters,
                const CommandSucceedCallback& onSuccess,
                const CommandFailCallback& onFailure,
+               const CommandOptions& options);
+
+  /** \deprecated This is to support arbitrary signing function.
+   */
+  void
+  startCommand(const shared_ptr<ControlCommand>& command,
+               const ControlParameters& parameters,
+               const CommandSucceedCallback& onSuccess,
+               const CommandFailCallback& onFailure,
                const Sign& sign,
                const time::milliseconds& timeout);
 
@@ -151,12 +184,28 @@
                          const CommandFailCallback& onFailure);
 
 public:
+  /** \deprecated use CommandOptions::DEFAULT_TIMEOUT
+   */
   static time::milliseconds
   getDefaultCommandTimeout()
   {
     return time::milliseconds(10000);
   }
 
+public:
+  /** \brief error code for timeout
+   *  \note comes from http://msdn.microsoft.com/en-us/library/windows/desktop/ms740668.aspx
+   */
+  static const uint32_t ERROR_TIMEOUT;
+
+  /** \brief error code for server error
+   */
+  static const uint32_t ERROR_SERVER;
+
+  /** \brief inclusive lower bound of error codes
+   */
+  static const uint32_t ERROR_LBOUND;
+
 protected:
   Face& m_face;
   shared_ptr<KeyChain> m_internalKeyChain;