mgmt: add new Dispatcher API for control commands

The new registration API is slightly higher level, while providing
better separation of concerns between Dispatcher and ControlCommand.
The latter is now solely responsible for encoding, decoding, and
validation of the command parameters. This should simplify the
implementation of alternative encoding formats for command parameters
in the future.

Change-Id: I3296e7ddf5db2f2def3ae676f66b7c50b6fba957

Author: Pesa

ndn-cxx/mgmt/dispatcher.cpp

@@ -93,7 +93,7 @@ void
 Dispatcher::checkPrefix(const PartialName& relPrefix) const
 {
   if (!m_topLevelPrefixes.empty()) {
-    NDN_THROW(std::domain_error("one or more top-level prefix has been added"));
+    NDN_THROW(std::domain_error("one or more top-level prefixes have already been added"));
   }
 
   bool hasOverlap = std::any_of(m_handlers.begin(), m_handlers.end(), [&] (const auto& entry) {
@@ -162,22 +162,18 @@ Dispatcher::sendOnFace(const Data& data)
 
 void
 Dispatcher::processCommand(const Name& prefix,
-                           const Name& relPrefix,
                            const Interest& interest,
-                           const ControlParametersParser& parse,
+                           const ParametersParser& parse,
                            const Authorization& authorize,
                            ValidateParameters validate,
                            ControlCommandHandler handler)
 {
-  // /<prefix>/<relPrefix>/<parameters>
-  size_t parametersLoc = prefix.size() + relPrefix.size();
-  const name::Component& pc = interest.getName().get(parametersLoc);
-
   shared_ptr<ControlParameters> parameters;
   try {
-    parameters = parse(pc);
+    parameters = parse(prefix, interest);
   }
-  catch (const tlv::Error&) {
+  catch (const std::exception& e) {
+    NDN_LOG_DEBUG("malformed command " << interest.getName() << ": " << e.what());
     return;
   }
 
@@ -197,9 +193,18 @@ Dispatcher::processAuthorizedCommand(const Name& prefix,
                                      const ValidateParameters& validate,
                                      const ControlCommandHandler& handler)
 {
-  if (validate(*parameters)) {
-    handler(prefix, interest, *parameters,
-            [=] (const auto& resp) { sendControlResponse(resp, interest); });
+  bool ok = false;
+  try {
+    ok = validate(*parameters);
+  }
+  catch (const std::exception& e) {
+    NDN_LOG_DEBUG("invalid parameters for command " << interest.getName() << ": " << e.what());
+  }
+
+  if (ok) {
+    handler(prefix, interest, *parameters, [this, interest] (const auto& resp) {
+      sendControlResponse(resp, interest);
+    });
   }
   else {
     sendControlResponse(ControlResponse(400, "failed in validating parameters"), interest);

ndn-cxx/mgmt/dispatcher.hpp

@@ -87,11 +87,12 @@ makeAcceptAllAuthorization();
 
 // ---- CONTROL COMMAND ----
 
-/** \brief A function to validate input ControlParameters.
- *  \param params parsed ControlParameters;
- *                This is guaranteed to have correct type for the command.
+/**
+ * \brief A function to validate and normalize the incoming request parameters.
+ * \param params The parsed ControlParameters; guaranteed to be of the correct (sub-)type
+ *               for the command.
  */
-using ValidateParameters = std::function<bool(const ControlParameters& params)>;
+using ValidateParameters = std::function<bool(ControlParameters& params)>;
 
 /**
  * \brief A function to be called after a ControlCommandHandler completes.
@@ -183,7 +184,7 @@ class Dispatcher : noncopyable
 
 public: // ControlCommand
   /**
-   * \brief Register a ControlCommand.
+   * \brief Register a ControlCommand (old style).
    * \tparam ParametersType Concrete subclass of ControlParameters used by this command.
    * \param relPrefix The name prefix for this command relative to the top-level prefix,
    *                  e.g., "faces/create". The prefixes across all ControlCommands,
@@ -219,17 +220,69 @@ class Dispatcher : noncopyable
   {
     checkPrefix(relPrefix);
 
-    ControlParametersParser parse = [] (const name::Component& comp) -> shared_ptr<ControlParameters> {
+    auto relPrefixLen = relPrefix.size();
+    ParametersParser parse = [relPrefixLen] (const Name& prefix,
+                                             const auto& interest) -> shared_ptr<ControlParameters> {
+      const name::Component& comp = interest.getName().get(prefix.size() + relPrefixLen);
       return make_shared<ParametersType>(comp.blockFromValue());
     };
 
-    m_handlers[relPrefix] = [this, relPrefix,
-                             parse = std::move(parse),
-                             authorize = std::move(authorize),
-                             validate = std::move(validate),
-                             handle = std::move(handle)] (const auto& prefix, const auto& interest) {
-      processCommand(prefix, relPrefix, interest, parse, authorize,
-                     std::move(validate), std::move(handle));
+    m_handlers[relPrefix] = [this,
+                             parser = std::move(parse),
+                             authorizer = std::move(authorize),
+                             validator = std::move(validate),
+                             handler = std::move(handle)] (const auto& prefix, const auto& interest) {
+      processCommand(prefix, interest, parser, authorizer, std::move(validator), std::move(handler));
+    };
+  }
+
+  /**
+   * \brief Register a ControlCommand (new style).
+   * \tparam Command The type of ControlCommand to register.
+   * \param authorize Callback to authorize the incoming commands.
+   * \param handle Callback to handle the commands.
+   * \pre No top-level prefix has been added.
+   * \throw std::out_of_range \p relPrefix overlaps with an existing relPrefix.
+   * \throw std::domain_error One or more top-level prefixes have been added.
+   *
+   * Procedure for processing a ControlCommand registered through this function:
+   *  1. Extract the parameters from the request by invoking `Command::parseRequest` on the
+   *     incoming Interest; if parsing fails, abort these steps.
+   *  2. Perform authorization; if the authorization is rejected, perform the RejectReply action
+   *     and abort these steps.
+   *  3. Validate the parameters with `Command::validateRequest`.
+   *  4. Normalize the parameters with `Command::applyDefaultsToRequest`.
+   *  5. If either step 3 or 4 fails, create a ControlResponse with StatusCode 400 and go to step 7.
+   *  6. Invoke the command handler, wait until CommandContinuation is called.
+   *  7. Encode the ControlResponse into one Data packet.
+   *  8. Sign the Data packet.
+   *  9. If the Data packet is too large, log an error and abort these steps.
+   * 10. Send the signed Data packet.
+   */
+  template<typename Command>
+  void
+  addControlCommand(Authorization authorize, ControlCommandHandler handle)
+  {
+    auto relPrefix = Command::getName();
+    checkPrefix(relPrefix);
+
+    ParametersParser parse = [] (const Name& prefix, const auto& interest) {
+      return Command::parseRequest(interest, prefix.size());
+    };
+    ValidateParameters validate = [] (auto& params) {
+      auto& reqParams = static_cast<typename Command::RequestParameters&>(params);
+      Command::validateRequest(reqParams);
+      Command::applyDefaultsToRequest(reqParams);
+      // for compatibility with ValidateParameters signature; consider refactoring in the future
+      return true;
+    };
+
+    m_handlers[relPrefix] = [this,
+                             parser = std::move(parse),
+                             authorizer = std::move(authorize),
+                             validator = std::move(validate),
+                             handler = std::move(handle)] (const auto& prefix, const auto& interest) {
+      processCommand(prefix, interest, parser, authorizer, std::move(validator), std::move(handler));
     };
   }
 
@@ -299,11 +352,11 @@ class Dispatcher : noncopyable
   using InterestHandler = std::function<void(const Name& prefix, const Interest&)>;
 
   /**
-   * @brief The parser for extracting control parameters from a name component.
+   * @brief The parser for extracting the parameters from a command request.
    * @return A shared pointer to the extracted ControlParameters.
-   * @throw tlv::Error if the name component cannot be parsed as ControlParameters
+   * @throw tlv::Error The request parameters cannot be parsed.
    */
-  using ControlParametersParser = std::function<shared_ptr<ControlParameters>(const name::Component&)>;
+  using ParametersParser = std::function<shared_ptr<ControlParameters>(const Name& prefix, const Interest&)>;
 
   void
   checkPrefix(const PartialName& relPrefix) const;
@@ -364,7 +417,6 @@ class Dispatcher : noncopyable
    * @brief Process an incoming control command Interest before authorization.
    *
    * @param prefix the top-level prefix
-   * @param relPrefix the relative prefix
    * @param interest the incoming Interest
    * @param parse function to extract the control parameters from the command
    * @param authorize function to determine whether the command is authorized
@@ -373,9 +425,8 @@ class Dispatcher : noncopyable
    */
   void
   processCommand(const Name& prefix,
-                 const Name& relPrefix,
                  const Interest& interest,
-                 const ControlParametersParser& parse,
+                 const ParametersParser& parse,
                  const Authorization& authorize,
                  ValidateParameters validate,
                  ControlCommandHandler handler);

ndn-cxx/mgmt/nfd/control-command.cpp

@@ -47,8 +47,15 @@ ControlParametersCommandFormat::validate(const ControlParameters& parameters) co
   }
 }
 
+shared_ptr<ControlParameters>
+ControlParametersCommandFormat::decode(const Interest& interest, size_t prefixLen)
+{
+  auto block = interest.getName().at(prefixLen).blockFromValue();
+  return make_shared<ControlParameters>(block);
+}
+
 void
-ControlParametersCommandFormat::encode(Interest& interest, const ControlParameters& params) const
+ControlParametersCommandFormat::encode(Interest& interest, const ControlParameters& params)
 {
   auto name = interest.getName();
   name.append(params.wireEncode());

ndn-cxx/mgmt/nfd/control-command.hpp

@@ -42,7 +42,12 @@ class ArgumentError : public std::invalid_argument
 
 /**
  * \ingroup management
- * \brief Implements encoding and validation of the ControlParameters format for control commands.
+ * \brief Implements decoding, encoding, and validation of ControlParameters in control commands.
+ *
+ * According to this format, the request parameters are encoded as a single GenericNameComponent
+ * in the Interest name, immediately after the command module and command verb components.
+ *
+ * \sa https://redmine.named-data.net/projects/nfd/wiki/ControlCommand
  */
 class ControlParametersCommandFormat
 {
@@ -78,11 +83,17 @@ class ControlParametersCommandFormat
   validate(const ControlParameters& params) const;
 
   /**
-   * \brief Serializes the parameters into the request \p interest.
+   * \brief Extract the parameters from the request \p interest.
+   */
+  static shared_ptr<ControlParameters>
+  decode(const Interest& interest, size_t prefixLen);
+
+  /**
+   * \brief Serialize the parameters into the request \p interest.
    * \pre \p params are valid.
    */
-  void
-  encode(Interest& interest, const ControlParameters& params) const;
+  static void
+  encode(Interest& interest, const ControlParameters& params);
 
 private:
   std::bitset<CONTROL_PARAMETER_UBOUND> m_required;
@@ -115,6 +126,15 @@ class ControlCommand : noncopyable
 
   ControlCommand() = delete;
 
+  /**
+   * \brief Return the command name (module + verb).
+   */
+  static PartialName
+  getName()
+  {
+    return PartialName().append(Derived::s_module).append(Derived::s_verb);
+  }
+
   /**
    * \brief Construct request Interest.
    * \throw ArgumentError if parameters are invalid
@@ -129,6 +149,16 @@ class ControlCommand : noncopyable
     return request;
   }
 
+  /**
+   * \brief Extract parameters from request Interest.
+   */
+  static shared_ptr<mgmt::ControlParameters>
+  parseRequest(const Interest& interest, size_t prefixLen)
+  {
+    // /<prefix>/<module>/<verb>
+    return Derived::s_requestFormat.decode(interest, prefixLen + 2);
+  }
+
   /**
    * \brief Validate request parameters.
    * \throw ArgumentError if parameters are invalid

tests/unit/mgmt/control-response.t.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2023 Regents of the University of California.
+ * Copyright (c) 2013-2025 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -36,7 +36,7 @@ static_assert(std::is_convertible_v<ControlResponse::Error*, tlv::Error*>,
 BOOST_AUTO_TEST_SUITE(Mgmt)
 BOOST_AUTO_TEST_SUITE(TestControlResponse)
 
-static const uint8_t WIRE[] = {
+const uint8_t WIRE[] = {
   0x65, 0x17, // ControlResponse
         0x66, 0x02, // StatusCode
               0x01, 0x94,
@@ -46,17 +46,48 @@ static const uint8_t WIRE[] = {
 
 BOOST_AUTO_TEST_CASE(Encode)
 {
-  ControlResponse cr(404, "Nothing not found");
-  const Block& wire = cr.wireEncode();
-  BOOST_CHECK_EQUAL_COLLECTIONS(WIRE, WIRE + sizeof(WIRE),
-                                wire.begin(), wire.end());
+  ControlResponse cr1(404, "Nothing not found");
+  BOOST_TEST(cr1.wireEncode() == WIRE, boost::test_tools::per_element());
+
+  ControlResponse cr2;
+  cr2.setCode(404);
+  cr2.setText("Nothing not found");
+  BOOST_TEST(cr2.wireEncode() == WIRE, boost::test_tools::per_element());
+
+  ControlResponse cr3(cr1);
+  cr3.setCode(405);
+  BOOST_TEST(cr3.wireEncode() != Block{WIRE});
+
+  ControlResponse cr4(cr1);
+  cr4.setText("foo");
+  BOOST_TEST(cr4.wireEncode() != Block{WIRE});
 }
 
 BOOST_AUTO_TEST_CASE(Decode)
 {
   ControlResponse cr(Block{WIRE});
   BOOST_CHECK_EQUAL(cr.getCode(), 404);
   BOOST_CHECK_EQUAL(cr.getText(), "Nothing not found");
+
+  // wrong outer TLV type
+  BOOST_CHECK_EXCEPTION(cr.wireDecode("6406660201946700"_block), tlv::Error, [] (const auto& e) {
+    return e.what() == "Expecting ControlResponse element, but TLV has type 100"sv;
+  });
+
+  // empty TLV
+  BOOST_CHECK_EXCEPTION(cr.wireDecode("6500"_block), tlv::Error, [] (const auto& e) {
+    return e.what() == "missing StatusCode sub-element"sv;
+  });
+
+  // missing StatusCode
+  BOOST_CHECK_EXCEPTION(cr.wireDecode("65026700"_block), tlv::Error, [] (const auto& e) {
+    return e.what() == "missing StatusCode sub-element"sv;
+  });
+
+  // missing StatusText
+  BOOST_CHECK_EXCEPTION(cr.wireDecode("650466020194"_block), tlv::Error, [] (const auto& e) {
+    return e.what() == "missing StatusText sub-element"sv;
+  });
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestControlResponse