mgmt: Dispatcher: minor code cleanups

Change-Id: I0b092611caaf9f11e42a21061a50d4a3f3834903
diff --git a/src/mgmt/dispatcher.cpp b/src/mgmt/dispatcher.cpp
index 70661c1..df27aca 100644
--- a/src/mgmt/dispatcher.cpp
+++ b/src/mgmt/dispatcher.cpp
@@ -55,53 +55,45 @@
 Dispatcher::~Dispatcher()
 {
   std::vector<Name> topPrefixNames;
-
-  std::transform(m_topLevelPrefixes.begin(),
-                 m_topLevelPrefixes.end(),
+  std::transform(m_topLevelPrefixes.begin(), m_topLevelPrefixes.end(),
                  std::back_inserter(topPrefixNames),
                  [] (const std::unordered_map<Name, TopPrefixEntry>::value_type& entry) {
                    return entry.second.topPrefix;
                  });
 
-  for (auto&& name : topPrefixNames) {
+  for (const auto& name : topPrefixNames) {
     removeTopPrefix(name);
   }
 }
 
 void
-Dispatcher::addTopPrefix(const Name& prefix,
-                         bool wantRegister,
+Dispatcher::addTopPrefix(const Name& prefix, bool wantRegister,
                          const security::SigningInfo& signingInfo)
 {
-  bool hasOverlap = std::any_of(m_topLevelPrefixes.begin(),
-                                m_topLevelPrefixes.end(),
+  bool hasOverlap = std::any_of(m_topLevelPrefixes.begin(), m_topLevelPrefixes.end(),
                                 [&] (const std::unordered_map<Name, TopPrefixEntry>::value_type& x) {
                                   return x.first.isPrefixOf(prefix) || prefix.isPrefixOf(x.first);
                                 });
   if (hasOverlap) {
-    BOOST_THROW_EXCEPTION(std::out_of_range("Top-level Prefixes overlapped"));
+    BOOST_THROW_EXCEPTION(std::out_of_range("top-level prefix overlaps"));
   }
 
-  TopPrefixEntry& topPrefixEntry = m_topLevelPrefixes[prefix];;
+  TopPrefixEntry& topPrefixEntry = m_topLevelPrefixes[prefix];
   topPrefixEntry.topPrefix = prefix;
-  topPrefixEntry.wantRegister = wantRegister;
 
   if (wantRegister) {
-    RegisterPrefixFailureCallback failure = [] (const Name& name, const std::string& reason) {
-      BOOST_THROW_EXCEPTION(std::runtime_error(reason));
-    };
-    topPrefixEntry.registerPrefixId =
-      m_face.registerPrefix(prefix, bind([]{}), failure, signingInfo);
+    topPrefixEntry.registeredPrefixId = m_face.registerPrefix(prefix,
+      nullptr,
+      [] (const Name&, const std::string& reason) {
+        BOOST_THROW_EXCEPTION(std::runtime_error("prefix registration failed: " + reason));
+      },
+      signingInfo);
   }
 
-  for (auto&& entry : m_handlers) {
-    Name fullPrefix = prefix;
-    fullPrefix.append(entry.first);
-
-    const InterestFilterId* interestFilterId =
-      m_face.setInterestFilter(fullPrefix, std::bind(entry.second, prefix, _2));
-
-    topPrefixEntry.interestFilters.push_back(interestFilterId);
+  for (const auto& entry : m_handlers) {
+    Name fullPrefix = Name(prefix).append(entry.first);
+    const InterestFilterId* filter = m_face.setInterestFilter(fullPrefix, bind(entry.second, prefix, _2));
+    topPrefixEntry.interestFilters.push_back(filter);
   }
 }
 
@@ -114,11 +106,10 @@
   }
 
   const TopPrefixEntry& topPrefixEntry = it->second;
-  if (topPrefixEntry.wantRegister) {
-    m_face.unregisterPrefix(topPrefixEntry.registerPrefixId, bind([]{}), bind([]{}));
+  if (topPrefixEntry.registeredPrefixId) {
+    m_face.unregisterPrefix(*topPrefixEntry.registeredPrefixId, nullptr, nullptr);
   }
-
-  for (auto&& filter : topPrefixEntry.interestFilters) {
+  for (const auto& filter : topPrefixEntry.interestFilters) {
     m_face.unsetInterestFilter(filter);
   }
 
@@ -126,11 +117,11 @@
 }
 
 bool
-Dispatcher::isOverlappedWithOthers(const PartialName& relPrefix)
+Dispatcher::isOverlappedWithOthers(const PartialName& relPrefix) const
 {
   bool hasOverlapWithHandlers =
     std::any_of(m_handlers.begin(), m_handlers.end(),
-                [&] (const HandlerMap::value_type& entry) {
+                [&] (const std::unordered_map<PartialName, InterestHandler>::value_type& entry) {
                   return entry.first.isPrefixOf(relPrefix) || relPrefix.isPrefixOf(entry.first);
                 });
   bool hasOverlapWithStreams =
@@ -157,7 +148,8 @@
   auto data = m_storage.find(interest);
   if (data == nullptr) {
     // invoke missContinuation to process this Interest if the query fails.
-    missContinuation(prefix, interest);
+    if (missContinuation)
+      missContinuation(prefix, interest);
   }
   else {
     // send the fetched data through face if query succeeds.
@@ -169,7 +161,7 @@
 Dispatcher::sendData(const Name& dataName, const Block& content, const MetaInfo& metaInfo,
                      SendDestination option, time::milliseconds imsFresh)
 {
-  shared_ptr<Data> data = make_shared<Data>(dataName);
+  auto data = make_shared<Data>(dataName);
   data->setContent(content).setMetaInfo(metaInfo).setFreshnessPeriod(DEFAULT_FRESHNESS_PERIOD);
 
   m_keyChain.sign(*data, m_signingInfo);
@@ -241,16 +233,16 @@
 }
 
 void
-Dispatcher::sendControlResponse(const ControlResponse& resp, const Interest& interest,
-                                bool isNack)
+Dispatcher::sendControlResponse(const ControlResponse& resp, const Interest& interest, bool isNack)
 {
   MetaInfo metaInfo;
   if (isNack) {
     metaInfo.setType(tlv::ContentType_Nack);
   }
+
   // control response is always sent out through the face
-  sendData(interest.getName(), resp.wireEncode(), metaInfo, SendDestination::FACE,
-           DEFAULT_FRESHNESS_PERIOD);
+  sendData(interest.getName(), resp.wireEncode(), metaInfo,
+           SendDestination::FACE, DEFAULT_FRESHNESS_PERIOD);
 }
 
 void
@@ -263,12 +255,11 @@
   }
 
   if (isOverlappedWithOthers(relPrefix)) {
-    BOOST_THROW_EXCEPTION(std::out_of_range("relPrefix overlapped"));
+    BOOST_THROW_EXCEPTION(std::out_of_range("status dataset name overlaps"));
   }
 
   AuthorizationAcceptedCallback accepted =
-    bind(&Dispatcher::processAuthorizedStatusDatasetInterest, this,
-         _1, _2, _3, handler);
+    bind(&Dispatcher::processAuthorizedStatusDatasetInterest, this, _1, _2, _3, handler);
   AuthorizationRejectedCallback rejected =
     bind(&Dispatcher::afterAuthorizationRejected, this, _1, _2);
 
@@ -336,15 +327,14 @@
   }
 
   if (isOverlappedWithOthers(relPrefix)) {
-    BOOST_THROW_EXCEPTION(std::out_of_range("relPrefix overlaps with another relPrefix"));
+    BOOST_THROW_EXCEPTION(std::out_of_range("notification stream name overlaps"));
   }
 
-  // keep silent if Interest does not match a stored notification
-  InterestHandler missContinuation = bind([]{});
-
   // register a handler for the subscriber of this notification stream
-  m_handlers[relPrefix] = bind(&Dispatcher::queryStorage, this, _1, _2, missContinuation);
+  // keep silent if Interest does not match a stored notification
+  m_handlers[relPrefix] = bind(&Dispatcher::queryStorage, this, _1, _2, nullptr);
   m_streams[relPrefix] = 0;
+
   return bind(&Dispatcher::postNotification, this, _1, relPrefix);
 }
 
@@ -360,10 +350,9 @@
   streamName.append(relPrefix);
   streamName.appendSequenceNumber(m_streams[streamName]++);
 
-  // notification is sent out by the face after inserting into the in-memory storage,
+  // notification is sent out via the face after inserting into the in-memory storage,
   // because a request may be pending in the PIT
-  sendData(streamName, notification, MetaInfo(), SendDestination::FACE_AND_IMS,
-           DEFAULT_FRESHNESS_PERIOD);
+  sendData(streamName, notification, {}, SendDestination::FACE_AND_IMS, DEFAULT_FRESHNESS_PERIOD);
 }
 
 } // namespace mgmt
diff --git a/src/mgmt/dispatcher.hpp b/src/mgmt/dispatcher.hpp
index 2de6f78..4845b54 100644
--- a/src/mgmt/dispatcher.hpp
+++ b/src/mgmt/dispatcher.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -22,10 +22,10 @@
 #ifndef NDN_MGMT_DISPATCHER_HPP
 #define NDN_MGMT_DISPATCHER_HPP
 
-#include "../face.hpp"
 #include "../encoding/block.hpp"
-#include "../security/key-chain.hpp"
+#include "../face.hpp"
 #include "../ims/in-memory-storage-fifo.hpp"
+#include "../security/key-chain.hpp"
 #include "control-response.hpp"
 #include "control-parameters.hpp"
 #include "status-dataset-context.hpp"
@@ -57,7 +57,7 @@
 
 /** \brief a function to be called if authorization is rejected
  */
-typedef std::function<void(RejectReply act)> RejectContinuation;
+typedef std::function<void(RejectReply reply)> RejectContinuation;
 
 /** \brief a function that performs authorization
  *  \param prefix top-level prefix, e.g., "/localhost/nfd";
@@ -76,7 +76,7 @@
                            const AcceptContinuation& accept,
                            const RejectContinuation& reject)> Authorization;
 
-/** \return an Authorization that accepts all Interests, with empty string as requester
+/** \brief return an Authorization that accepts all Interests, with empty string as requester
  */
 Authorization
 makeAcceptAllAuthorization();
@@ -105,6 +105,7 @@
                            const ControlParameters& params,
                            const CommandContinuation& done)> ControlCommandHandler;
 
+// ---- STATUS DATASET ----
 
 /** \brief a function to handle a StatusDataset request
  *  \param prefix top-level prefix, e.g., "/localhost/nfd";
@@ -128,16 +129,6 @@
  */
 class Dispatcher : noncopyable
 {
-  class Error : public std::runtime_error
-  {
-  public:
-    explicit
-    Error(const std::string& what)
-      : std::runtime_error(what)
-    {
-    }
-  };
-
 public:
   /** \brief constructor
    *  \param face the Face on which the dispatcher operates
@@ -194,8 +185,7 @@
    *  \tparam CP subclass of ControlParameters used by this command
    *  \param relPrefix a prefix for this command, e.g., "faces/create";
    *                   relPrefixes in ControlCommands, StatusDatasets, NotificationStreams must be
-   *                   non-overlapping
-   *                   (no relPrefix is a prefix of another relPrefix)
+   *                   non-overlapping (no relPrefix is a prefix of another relPrefix)
    *  \param authorization Callback to authorize the incoming commands
    *  \param validateParams Callback to validate parameters of the incoming commands
    *  \param handler Callback to handle the commands
@@ -227,8 +217,7 @@
   /** \brief register a StatusDataset or a prefix under which StatusDatasets can be requested
    *  \param relPrefix a prefix for this dataset, e.g., "faces/list";
    *                   relPrefixes in ControlCommands, StatusDatasets, NotificationStreams must be
-   *                   non-overlapping
-   *                   (no relPrefix is a prefix of another relPrefix)
+   *                   non-overlapping (no relPrefix is a prefix of another relPrefix)
    *  \param authorization should set identity to Name() if the dataset is public
    *  \param handler Callback to process the incoming dataset requests
    *  \pre no top-level prefix has been added
@@ -264,8 +253,7 @@
   /** \brief register a NotificationStream
    *  \param relPrefix a prefix for this notification stream, e.g., "faces/events";
    *                   relPrefixes in ControlCommands, StatusDatasets, NotificationStreams must be
-   *                   non-overlapping
-   *                   (no relPrefix is a prefix of another relPrefix)
+   *                   non-overlapping (no relPrefix is a prefix of another relPrefix)
    *  \return a function into which notifications can be posted
    *  \pre no top-level prefix has been added
    *  \throw std::out_of_range \p relPrefix overlaps with an existing relPrefix
@@ -298,20 +286,17 @@
 
   /**
    * @brief the parser of extracting control parameters from name component.
-   *
-   * @param component name component that may encode control parameters.
+   * @param comp name component that may encode control parameters.
    * @return a shared pointer to the extracted control parameters.
    * @throw tlv::Error if the NameComponent cannot be parsed as ControlParameters
    */
-  typedef std::function<shared_ptr<ControlParameters>(const name::Component& component)>
-  ControlParametersParser;
+  typedef std::function<shared_ptr<ControlParameters>(const name::Component& comp)> ControlParametersParser;
 
   bool
-  isOverlappedWithOthers(const PartialName& relPrefix);
+  isOverlappedWithOthers(const PartialName& relPrefix) const;
 
   /**
    * @brief process unauthorized request
-   *
    * @param act action to reply
    * @param interest the incoming Interest
    */
@@ -338,14 +323,14 @@
   };
 
   /**
-   * @brief send data to the face or in-memory storage
+   * @brief send data to the face and/or in-memory storage
    *
-   * create a data packet with the given @p dataName, @p content, and @p metaInfo,
-   * set its FreshnessPeriod to DEFAULT_FRESHNESS_PERIOD, and then send it out through the face and/or
-   * insert it into the in-memory storage as specified in @p option.
+   * Create a Data packet with the given @p dataName, @p content, and @p metaInfo,
+   * set its FreshnessPeriod to DEFAULT_FRESHNESS_PERIOD, and then send it out through
+   * the face and/or insert it into the in-memory storage as specified in @p destination.
    *
-   * if it's toward the in-memory storage, set its CachePolicy to NO_CACHE and limit
-   * its FreshnessPeriod in the storage as @p imsFresh
+   * If it's toward the in-memory storage, set its CachePolicy to NO_CACHE and limit
+   * its FreshnessPeriod in the storage to @p imsFresh.
    *
    * @param dataName the name of this piece of data
    * @param content the content of this piece of data
@@ -455,9 +440,8 @@
   struct TopPrefixEntry
   {
     Name topPrefix;
-    bool wantRegister;
-    const ndn::RegisteredPrefixId* registerPrefixId;
-    std::vector<const ndn::InterestFilterId*> interestFilters;
+    optional<const RegisteredPrefixId*> registeredPrefixId = nullopt;
+    std::vector<const InterestFilterId*> interestFilters;
   };
   std::unordered_map<Name, TopPrefixEntry> m_topLevelPrefixes;
 
@@ -465,9 +449,7 @@
   KeyChain& m_keyChain;
   security::SigningInfo m_signingInfo;
 
-  typedef std::unordered_map<PartialName, InterestHandler> HandlerMap;
-  typedef HandlerMap::iterator HandlerMapIt;
-  HandlerMap m_handlers;
+  std::unordered_map<PartialName, InterestHandler> m_handlers;
 
   // NotificationStream name => next sequence number
   std::unordered_map<Name, uint64_t> m_streams;
@@ -491,9 +473,8 @@
     BOOST_THROW_EXCEPTION(std::out_of_range("relPrefix overlaps with another relPrefix"));
   }
 
-  ControlParametersParser parser =
-    [] (const name::Component& component) -> shared_ptr<ControlParameters> {
-    return make_shared<CP>(component.blockFromValue());
+  ControlParametersParser parser = [] (const name::Component& comp) -> shared_ptr<ControlParameters> {
+    return make_shared<CP>(comp.blockFromValue());
   };
 
   AuthorizationAcceptedCallback accepted =
@@ -509,4 +490,5 @@
 
 } // namespace mgmt
 } // namespace ndn
+
 #endif // NDN_MGMT_DISPATCHER_HPP