update: migrate to new Dispatcher API for control commands

Change-Id: I87481be651c93050e23cf7429520023f29494acd

src/nlsr.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2023,  The University of Memphis,
+ * Copyright (c) 2014-2025,  The University of Memphis,
  *                           Regents of the University of California,
  *                           Arizona Board of Regents.
  *
@@ -161,11 +161,11 @@ Nlsr::registerPrefix(const ndn::Name& prefix)
 {
   m_face.registerPrefix(prefix,
     [] (const ndn::Name& name) {
-      NLSR_LOG_DEBUG("Successfully registered prefix: " << name);
+      NLSR_LOG_DEBUG("Successfully registered prefix " << name);
     },
     [] (const ndn::Name& name, const std::string& reason) {
-      NLSR_LOG_ERROR("ERROR: Failed to register prefix " << name << " in local hub's daemon");
-      NDN_THROW(Error("Error: Prefix registration failed: " + reason));
+      NLSR_LOG_ERROR("Failed to register prefix " << name << " (" << reason << ")");
+      NDN_THROW(Error("Prefix registration failed: " + reason));
     });
 }
 

src/update/manager-base.cpp

@@ -34,12 +34,6 @@ ManagerBase::ManagerBase(ndn::mgmt::Dispatcher& dispatcher,
 {
 }
 
-ndn::PartialName
-ManagerBase::makeRelPrefix(const std::string& verb) const
-{
-  return ndn::PartialName(m_module).append(verb);
-}
-
 CommandManagerBase::CommandManagerBase(ndn::mgmt::Dispatcher& dispatcher,
                                       NamePrefixList& namePrefixList,
                                       Lsdb& lsdb,
@@ -51,16 +45,14 @@ CommandManagerBase::CommandManagerBase(ndn::mgmt::Dispatcher& dispatcher,
 }
 
 void
-CommandManagerBase::advertiseAndInsertPrefix(const ndn::Name& prefix,
-                                             const ndn::Interest& interest,
-                                             const ndn::mgmt::ControlParametersBase& parameters,
+CommandManagerBase::advertiseAndInsertPrefix(const ndn::mgmt::ControlParametersBase& parameters,
                                              const ndn::mgmt::CommandContinuation& done)
 {
   const auto& castParams = static_cast<const ndn::nfd::ControlParameters&>(parameters);
 
   // Only build a Name LSA if the added name is new
   if (m_namePrefixList.insert(castParams.getName())) {
-    NLSR_LOG_INFO("Advertising name: " << castParams.getName() << "\n");
+    NLSR_LOG_INFO("Advertising name: " << castParams.getName());
     m_lsdb.buildAndInstallOwnNameLsa();
     if (castParams.hasFlags() && castParams.getFlags() == PREFIX_FLAG) {
       NLSR_LOG_INFO("Saving name to the configuration file ");
@@ -77,7 +69,7 @@ CommandManagerBase::advertiseAndInsertPrefix(const ndn::Name& prefix,
   else {
     if (castParams.hasFlags() && castParams.getFlags() == PREFIX_FLAG) {
       // Save an already advertised prefix
-      NLSR_LOG_INFO("Saving an already advertised name: " << castParams.getName() << "\n");
+      NLSR_LOG_INFO("Saving an already advertised name: " << castParams.getName());
       if (afterAdvertise(castParams.getName()) == true) {
         return done(ndn::nfd::ControlResponse(205, "OK").setBody(parameters.wireEncode()));
       }
@@ -92,16 +84,14 @@ CommandManagerBase::advertiseAndInsertPrefix(const ndn::Name& prefix,
 }
 
 void
-CommandManagerBase::withdrawAndRemovePrefix(const ndn::Name& prefix,
-                                            const ndn::Interest& interest,
-                                            const ndn::mgmt::ControlParametersBase& parameters,
+CommandManagerBase::withdrawAndRemovePrefix(const ndn::mgmt::ControlParametersBase& parameters,
                                             const ndn::mgmt::CommandContinuation& done)
 {
   const auto& castParams = static_cast<const ndn::nfd::ControlParameters&>(parameters);
 
   // Only build a Name LSA if the added name is new
   if (m_namePrefixList.erase(castParams.getName())) {
-    NLSR_LOG_INFO("Withdrawing/Removing name: " << castParams.getName() << "\n");
+    NLSR_LOG_INFO("Withdrawing/Removing name: " << castParams.getName());
     m_lsdb.buildAndInstallOwnNameLsa();
     if (castParams.hasFlags() && castParams.getFlags() == PREFIX_FLAG) {
       if (afterWithdraw(castParams.getName()) == true) {
@@ -117,7 +107,7 @@ CommandManagerBase::withdrawAndRemovePrefix(const ndn::Name& prefix,
   else {
     if (castParams.hasFlags() && castParams.getFlags() == PREFIX_FLAG) {
       // Delete an already withdrawn prefix
-      NLSR_LOG_INFO("Deleting an already withdrawn name: " << castParams.getName() << "\n");
+      NLSR_LOG_INFO("Deleting an already withdrawn name: " << castParams.getName());
       if (afterWithdraw(castParams.getName()) == true) {
         return done(ndn::nfd::ControlResponse(205, "OK").setBody(parameters.wireEncode()));
       }

src/update/manager-base.hpp

@@ -24,7 +24,6 @@
 
 #include "lsdb.hpp"
 #include "name-prefix-list.hpp"
-#include "test-access-control.hpp"
 
 #include <ndn-cxx/face.hpp>
 #include <ndn-cxx/interest.hpp>
@@ -34,7 +33,6 @@
 #include <ndn-cxx/mgmt/nfd/control-response.hpp>
 
 #include <boost/noncopyable.hpp>
-#include <iostream>
 #include <optional>
 
 namespace nlsr {
@@ -57,31 +55,6 @@ class ManagerBase : boost::noncopyable
 protected:
   ManagerBase(ndn::mgmt::Dispatcher& m_dispatcher, const std::string& module);
 
-  /*! \brief Generate the relative prefix for a handler by appending the verb name to the module name.
-   */
-  ndn::PartialName
-  makeRelPrefix(const std::string& verb) const;
-
-PUBLIC_WITH_TESTS_ELSE_PROTECTED:
-  /*! \brief Validate the parameters for a given command.
-   */
-  template<typename Command>
-  static bool
-  validateParameters(const ndn::mgmt::ControlParametersBase& parameters)
-  {
-    try {
-      Command::validateRequest(dynamic_cast<const ndn::nfd::ControlParameters&>(parameters));
-    }
-    catch (const ndn::nfd::ArgumentError&) {
-      throw;
-    }
-    catch (const std::exception& e) {
-      std::cerr << e.what() << std::endl;
-      return false;
-    }
-    return true;
-  }
-
 protected:
   ndn::mgmt::Dispatcher& m_dispatcher;
 
@@ -100,35 +73,31 @@ class CommandManagerBase : public ManagerBase
   virtual
   ~CommandManagerBase() = default;
 
-  /*! \brief add desired name prefix to the advertised name prefix list
+  /*! \brief Add desired name prefix to the advertised name prefix list
    *         or insert a prefix into the FIB if parameters is valid.
    */
   void
-  advertiseAndInsertPrefix(const ndn::Name& prefix,
-                           const ndn::Interest& interest,
-                           const ndn::mgmt::ControlParametersBase& parameters,
+  advertiseAndInsertPrefix(const ndn::mgmt::ControlParametersBase& parameters,
                            const ndn::mgmt::CommandContinuation& done);
 
-  /*! \brief remove desired name prefix from the advertised name prefix list
+  /*! \brief Remove desired name prefix from the advertised name prefix list
    *         or remove a prefix from the FIB if parameters is valid.
    */
   void
-  withdrawAndRemovePrefix(const ndn::Name& prefix,
-                          const ndn::Interest& interest,
-                          const ndn::mgmt::ControlParametersBase& parameters,
+  withdrawAndRemovePrefix(const ndn::mgmt::ControlParametersBase& parameters,
                           const ndn::mgmt::CommandContinuation& done);
 
-  /*! \brief save an advertised prefix to the nlsr configuration file
-   *         returns bool from the overridden function while nullopt here
+  /*! \brief Save an advertised prefix to the nlsr configuration file.
+   *  \return bool from the overridden function while nullopt here
    */
   virtual std::optional<bool>
   afterAdvertise(const ndn::Name& prefix)
   {
     return std::nullopt;
   }
 
-  /*! \brief save an advertised prefix to the nlsr configuration file
-   *         returns bool from the overridden function while nullopt here
+  /*! \brief Save an advertised prefix to the nlsr configuration file.
+   *  \return bool from the overridden function while nullopt here
    */
   virtual std::optional<bool>
   afterWithdraw(const ndn::Name& prefix)

src/update/nfd-rib-command-processor.cpp

@@ -28,15 +28,13 @@ NfdRibCommandProcessor::NfdRibCommandProcessor(ndn::mgmt::Dispatcher& dispatcher
                                                Lsdb& lsdb)
   : CommandManagerBase(dispatcher, namePrefixList, lsdb, "rib")
 {
-  m_dispatcher.addControlCommand<ndn::nfd::ControlParameters>(makeRelPrefix("register"),
+  m_dispatcher.addControlCommand<ndn::nfd::RibRegisterCommand>(
     ndn::mgmt::makeAcceptAllAuthorization(),
-    [] (const auto& p) { return validateParameters<ndn::nfd::RibRegisterCommand>(p); },
-    std::bind(&NfdRibCommandProcessor::advertiseAndInsertPrefix, this, _1, _2, _3, _4));
+    std::bind(&NfdRibCommandProcessor::advertiseAndInsertPrefix, this, _3, _4));
 
-  m_dispatcher.addControlCommand<ndn::nfd::ControlParameters>(makeRelPrefix("unregister"),
+  m_dispatcher.addControlCommand<ndn::nfd::RibUnregisterCommand>(
     ndn::mgmt::makeAcceptAllAuthorization(),
-    [] (const auto& p) { return validateParameters<ndn::nfd::RibUnregisterCommand>(p); },
-    std::bind(&NfdRibCommandProcessor::withdrawAndRemovePrefix, this, _1, _2, _3, _4));
+    std::bind(&NfdRibCommandProcessor::withdrawAndRemovePrefix, this, _3, _4));
 }
 
 } // namespace nlsr::update

src/update/prefix-update-commands.hpp

@@ -40,12 +40,12 @@ namespace nlsr::update {
 
 class AdvertisePrefixCommand : public ndn::nfd::ControlCommand<AdvertisePrefixCommand>
 {
-  NDN_CXX_CONTROL_COMMAND("nlsr", "advertise");
+  NDN_CXX_CONTROL_COMMAND("prefix-update", "advertise");
 };
 
 class WithdrawPrefixCommand : public ndn::nfd::ControlCommand<WithdrawPrefixCommand>
 {
-  NDN_CXX_CONTROL_COMMAND("nlsr", "withdraw");
+  NDN_CXX_CONTROL_COMMAND("prefix-update", "withdraw");
 };
 
 } // namespace nlsr::update

src/update/prefix-update-processor.cpp

@@ -22,11 +22,9 @@
 #include "prefix-update-processor.hpp"
 #include "logger.hpp"
 #include "lsdb.hpp"
-#include "nlsr.hpp"
 #include "prefix-update-commands.hpp"
 
 #include <ndn-cxx/face.hpp>
-#include <ndn-cxx/mgmt/nfd/control-response.hpp>
 
 #include <boost/algorithm/string.hpp>
 #include <fstream>
@@ -61,18 +59,13 @@ PrefixUpdateProcessor::PrefixUpdateProcessor(ndn::mgmt::Dispatcher& dispatcher,
   , m_validator(validator)
   , m_confFileNameDynamic(configFileName)
 {
-  NLSR_LOG_DEBUG("Setting dispatcher to capture Interests for: "
-    << ndn::Name(Nlsr::LOCALHOST_PREFIX).append("prefix-update"));
-
-  m_dispatcher.addControlCommand<ndn::nfd::ControlParameters>(makeRelPrefix("advertise"),
+  m_dispatcher.addControlCommand<AdvertisePrefixCommand>(
     makeAuthorization(),
-    [] (const auto& p) { return validateParameters<AdvertisePrefixCommand>(p); },
-    std::bind(&PrefixUpdateProcessor::advertiseAndInsertPrefix, this, _1, _2, _3, _4));
+    std::bind(&PrefixUpdateProcessor::advertiseAndInsertPrefix, this, _3, _4));
 
-  m_dispatcher.addControlCommand<ndn::nfd::ControlParameters>(makeRelPrefix("withdraw"),
+  m_dispatcher.addControlCommand<WithdrawPrefixCommand>(
     makeAuthorization(),
-    [] (const auto& p) { return validateParameters<WithdrawPrefixCommand>(p); },
-    std::bind(&PrefixUpdateProcessor::withdrawAndRemovePrefix, this, _1, _2, _3, _4));
+    std::bind(&PrefixUpdateProcessor::withdrawAndRemovePrefix, this, _3, _4));
 }
 
 ndn::mgmt::Authorization
@@ -84,15 +77,14 @@ PrefixUpdateProcessor::makeAuthorization()
               const ndn::mgmt::RejectContinuation& reject) {
     m_validator.validate(interest,
       [accept] (const ndn::Interest& request) {
-
         auto signer1 = getSignerFromTag(request);
         std::string signer = signer1.value_or("*");
         NLSR_LOG_DEBUG("accept " << request.getName() << " signer=" << signer);
         accept(signer);
       },
       [reject] (const ndn::Interest& request, const ndn::security::ValidationError& error) {
         NLSR_LOG_DEBUG("reject " << request.getName() << " signer=" <<
-                        getSignerFromTag(request).value_or("?") << ' ' << error);
+                       getSignerFromTag(request).value_or("?") << ' ' << error);
         reject(ndn::mgmt::RejectReply::STATUS403);
       });
   };
@@ -120,7 +112,6 @@ PrefixUpdateProcessor::checkForPrefixInFile(const std::string prefix)
       return true;
     }
   }
-  fp.close();
   return false;
 }
 

tests/update/test-nfd-rib-command-processor.cpp

@@ -27,7 +27,6 @@
 #include "tests/test-common.hpp"
 
 #include <boost/lexical_cast.hpp>
-#include <boost/mp11/list.hpp>
 
 namespace nlsr::tests {
 
@@ -53,7 +52,7 @@ class NfdRibCommandProcessorFixture : public IoKeyChainFixture
   void
   sendCommand(ndn::Name prefix, const ndn::nfd::ControlParameters& parameters)
   {
-    ndn::Interest interest(prefix.append(ndn::tlv::GenericNameComponent, parameters.wireEncode()));
+    ndn::Interest interest(prefix.append(parameters.wireEncode()));
     face.receive(interest);
     this->advanceClocks(ndn::time::milliseconds(10), 10);
   }
@@ -76,13 +75,9 @@ class NfdRibCommandProcessorFixture : public IoKeyChainFixture
     sendInterestForPublishedData();
 
     const ndn::Name& lsaPrefix = conf.getLsaPrefix();
-
-    const auto& it = std::find_if(face.sentData.begin(), face.sentData.end(),
-      [&] (const ndn::Data& data) {
-        return lsaPrefix.isPrefixOf(data.getName());
-      });
-
-    return (it != face.sentData.end());
+    auto it = std::find_if(face.sentData.begin(), face.sentData.end(),
+                           [&] (const auto& data) { return lsaPrefix.isPrefixOf(data.getName()); });
+    return it != face.sentData.end();
   }
 
 public:
@@ -96,29 +91,8 @@ class NfdRibCommandProcessorFixture : public IoKeyChainFixture
   uint64_t nameLsaSeqNoBeforeInterest;
 };
 
-using Commands = boost::mp11::mp_list<
-  ndn::nfd::RibRegisterCommand,
-  ndn::nfd::RibUnregisterCommand
->;
-
 BOOST_FIXTURE_TEST_SUITE(TestNfdRibCommandProcessor, NfdRibCommandProcessorFixture)
 
-BOOST_AUTO_TEST_CASE_TEMPLATE(ValidateParametersSuccess, NfdRibCommand, Commands)
-{
-  ndn::nfd::ControlParameters parameters;
-  parameters.setName("/test/prefixA");
-
-  BOOST_CHECK(processor.validateParameters<NfdRibCommand>(parameters));
-}
-
-BOOST_AUTO_TEST_CASE_TEMPLATE(ValidateParametersFailure, NfdRibCommand, Commands)
-{
-  ndn::nfd::ControlParameters parameters;
-  parameters.setName("/test/prefixA").setMtu(500);
-
-  BOOST_CHECK_THROW(processor.validateParameters<NfdRibCommand>(parameters), std::invalid_argument);
-}
-
 BOOST_AUTO_TEST_CASE(OnReceiveInterestRegisterCommand)
 {
   ndn::Name name("/localhost/nlsr/rib/register");