mgmt: remove one level of indirection in status dataset processing

Change-Id: I46bf2f2788300a76d64d835c24fe890b32798d0c

Author: Pesa

ndn-cxx/mgmt/dispatcher.cpp

@@ -166,8 +166,8 @@ Dispatcher::processCommand(const Name& prefix,
                            const Interest& interest,
                            const ControlParametersParser& parse,
                            const Authorization& authorize,
-                           const ValidateParameters& validateParams,
-                           const ControlCommandHandler& handler)
+                           ValidateParameters validate,
+                           ControlCommandHandler handler)
 {
   // /<prefix>/<relPrefix>/<parameters>
   size_t parametersLoc = prefix.size() + relPrefix.size();
@@ -181,8 +181,8 @@ Dispatcher::processCommand(const Name& prefix,
     return;
   }
 
-  AcceptContinuation accept = [=] (const auto& req) {
-    processAuthorizedCommand(req, prefix, interest, parameters, validateParams, handler);
+  AcceptContinuation accept = [=, v = std::move(validate), h = std::move(handler)] (const auto&) {
+    processAuthorizedCommand(prefix, interest, parameters, v, h);
   };
   RejectContinuation reject = [=] (RejectReply reply) {
     afterAuthorizationRejected(reply, interest);
@@ -191,14 +191,13 @@ Dispatcher::processCommand(const Name& prefix,
 }
 
 void
-Dispatcher::processAuthorizedCommand(const std::string& requester,
-                                     const Name& prefix,
+Dispatcher::processAuthorizedCommand(const Name& prefix,
                                      const Interest& interest,
                                      const shared_ptr<ControlParameters>& parameters,
-                                     const ValidateParameters& validateParams,
+                                     const ValidateParameters& validate,
                                      const ControlCommandHandler& handler)
 {
-  if (validateParams(*parameters)) {
+  if (validate(*parameters)) {
     handler(prefix, interest, *parameters,
             [=] (const auto& resp) { sendControlResponse(resp, interest); });
   }
@@ -221,36 +220,28 @@ Dispatcher::sendControlResponse(const ControlResponse& resp, const Interest& int
 
 void
 Dispatcher::addStatusDataset(const PartialName& relPrefix,
-                             Authorization auth,
-                             StatusDatasetHandler handler)
+                             Authorization authorize,
+                             StatusDatasetHandler handle)
 {
   checkPrefix(relPrefix);
 
-  AuthorizationAcceptedCallback accept =
-    [this, handler = std::move(handler)] (auto&&, const auto& prefix, const auto& interest, auto&&) {
-      processAuthorizedStatusDatasetInterest(prefix, interest, handler);
-    };
-  AuthorizationRejectedCallback reject =
-    [this] (auto&&... args) {
-      afterAuthorizationRejected(std::forward<decltype(args)>(args)...);
-    };
   // follow the general path if storage is a miss
-  InterestHandler missContinuation =
-    [this, auth = std::move(auth), accept = std::move(accept), reject = std::move(reject)] (auto&&... args) {
-      processStatusDatasetInterest(std::forward<decltype(args)>(args)..., auth, accept, reject);
-    };
+  InterestHandler afterMiss = [this,
+                               authorizer = std::move(authorize),
+                               handler = std::move(handle)] (const auto& prefix, const auto& interest) {
+    processStatusDatasetInterest(prefix, interest, authorizer, std::move(handler));
+  };
 
-  m_handlers[relPrefix] = [this, miss = std::move(missContinuation)] (auto&&... args) {
+  m_handlers[relPrefix] = [this, miss = std::move(afterMiss)] (auto&&... args) {
     queryStorage(std::forward<decltype(args)>(args)..., miss);
   };
 }
 
 void
 Dispatcher::processStatusDatasetInterest(const Name& prefix,
                                          const Interest& interest,
-                                         const Authorization& authorization,
-                                         const AuthorizationAcceptedCallback& accepted,
-                                         const AuthorizationRejectedCallback& rejected)
+                                         const Authorization& authorize,
+                                         StatusDatasetHandler handler)
 {
   const Name& interestName = interest.getName();
   bool endsWithVersionOrSegment = interestName.size() >= 1 &&
@@ -259,9 +250,13 @@ Dispatcher::processStatusDatasetInterest(const Name& prefix,
     return;
   }
 
-  AcceptContinuation accept = [=] (const auto& req) { accepted(req, prefix, interest, nullptr); };
-  RejectContinuation reject = [=] (RejectReply reply) { rejected(reply, interest); };
-  authorization(prefix, interest, nullptr, accept, reject);
+  AcceptContinuation accept = [=, h = std::move(handler)] (const auto&) {
+    processAuthorizedStatusDatasetInterest(prefix, interest, h);
+  };
+  RejectContinuation reject = [=] (RejectReply reply) {
+    afterAuthorizationRejected(reply, interest);
+  };
+  authorize(prefix, interest, nullptr, accept, reject);
 }
 
 void

ndn-cxx/mgmt/dispatcher.hpp

@@ -93,8 +93,9 @@ makeAcceptAllAuthorization();
  */
 using ValidateParameters = std::function<bool(const ControlParameters& params)>;
 
-/** \brief A function to be called after ControlCommandHandler completes.
- *  \param resp the response to be sent to requester
+/**
+ * \brief A function to be called after a ControlCommandHandler completes.
+ * \param resp The response that should be sent back to the requester.
  */
 using CommandContinuation = std::function<void(const ControlResponse& resp)>;
 
@@ -195,17 +196,17 @@ class Dispatcher : noncopyable
    * \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:
+   * Procedure for processing a ControlCommand registered through this function:
    *  1. Extract the NameComponent containing ControlParameters (the component after relPrefix),
    *     and parse ControlParameters into ParametersType; if parsing fails, abort these steps.
    *  2. Perform authorization; if the authorization is rejected, perform the RejectReply action
    *     and abort these steps.
-   *  3. Validate ControlParameters; if validation fails, create a ControlResponse with
+   *  3. Validate the ControlParameters; if validation fails, create a ControlResponse with
    *     StatusCode 400 and go to step 5.
    *  4. Invoke the command handler, wait until CommandContinuation is called.
    *  5. Encode the ControlResponse into one Data packet.
    *  6. Sign the Data packet.
-   *  7. If the Data packet is too large, abort these steps and log an error.
+   *  7. If the Data packet is too large, log an error and abort these steps.
    *  8. Send the signed Data packet.
    */
   template<typename ParametersType,
@@ -227,7 +228,8 @@ class Dispatcher : noncopyable
                              authorize = std::move(authorize),
                              validate = std::move(validate),
                              handle = std::move(handle)] (const auto& prefix, const auto& interest) {
-      processCommand(prefix, relPrefix, interest, parse, authorize, validate, handle);
+      processCommand(prefix, relPrefix, interest, parse, authorize,
+                     std::move(validate), std::move(handle));
     };
   }
 
@@ -296,13 +298,6 @@ class Dispatcher : noncopyable
 private:
   using InterestHandler = std::function<void(const Name& prefix, const Interest&)>;
 
-  using AuthorizationAcceptedCallback = std::function<void(const std::string& requester,
-                                                           const Name& prefix,
-                                                           const Interest&,
-                                                           const shared_ptr<ControlParameters>&)>;
-
-  using AuthorizationRejectedCallback = std::function<void(RejectReply, const Interest&)>;
-
   /**
    * @brief The parser for extracting control parameters from a name component.
    * @return A shared pointer to the extracted ControlParameters.
@@ -374,30 +369,28 @@ class Dispatcher : noncopyable
    * @param parse function to extract the control parameters from the command
    * @param authorize function to determine whether the command is authorized
    * @param validate function to validate the command parameters
-   * @param handler function to execute the command after authorization and validation
+   * @param handler function to execute the command after it is authorized and validated
    */
   void
   processCommand(const Name& prefix,
                  const Name& relPrefix,
                  const Interest& interest,
                  const ControlParametersParser& parse,
                  const Authorization& authorize,
-                 const ValidateParameters& validate,
-                 const ControlCommandHandler& handler);
+                 ValidateParameters validate,
+                 ControlCommandHandler handler);
 
   /**
    * @brief Process an authorized control command.
    *
-   * @param requester the requester
    * @param prefix the top-level prefix
    * @param interest the incoming Interest
    * @param parameters control parameters of this command
    * @param validate function to validate the command parameters
-   * @param handler function to execute the command after authorization and validation
+   * @param handler function to execute the command after its parameters are validated
    */
   void
-  processAuthorizedCommand(const std::string& requester,
-                           const Name& prefix,
+  processAuthorizedCommand(const Name& prefix,
                            const Interest& interest,
                            const shared_ptr<ControlParameters>& parameters,
                            const ValidateParameters& validate,
@@ -411,23 +404,21 @@ class Dispatcher : noncopyable
    *
    * @param prefix the top-level prefix
    * @param interest the incoming Interest
-   * @param authorize function to process verification
-   * @param accepted callback for successful authorization
-   * @param rejected callback for failed authorization
+   * @param authorize function to determine whether the request is authorized
+   * @param handler function to continue processing the request after it is authorized
    */
   void
   processStatusDatasetInterest(const Name& prefix,
                                const Interest& interest,
                                const Authorization& authorize,
-                               const AuthorizationAcceptedCallback& accepted,
-                               const AuthorizationRejectedCallback& rejected);
+                               StatusDatasetHandler handler);
 
   /**
    * @brief Process an authorized StatusDataset request.
    *
    * @param prefix the top-level prefix
    * @param interest the incoming Interest
-   * @param handler function to process this request
+   * @param handler function to process the dataset request
    */
   void
   processAuthorizedStatusDatasetInterest(const Name& prefix,

tests/unit/mgmt/dispatcher.t.cpp

@@ -32,18 +32,10 @@ using namespace ndn::mgmt;
 
 class DispatcherFixture : public IoKeyChainFixture
 {
-public:
-  DispatcherFixture()
-    : face(m_io, m_keyChain, {true, true})
-    , dispatcher(face, m_keyChain, security::SigningInfo())
-    , storage(dispatcher.m_storage)
-  {
-  }
-
-public:
-  DummyClientFace face;
-  mgmt::Dispatcher dispatcher;
-  InMemoryStorageFifo& storage;
+protected:
+  DummyClientFace face{m_io, m_keyChain, {true, true}};
+  Dispatcher dispatcher{face, m_keyChain};
+  InMemoryStorageFifo& storage{dispatcher.m_storage};
 };
 
 class VoidParameters : public mgmt::ControlParameters
@@ -266,16 +258,14 @@ BOOST_AUTO_TEST_CASE(ControlCommandAsyncAuthorization,
   * ut::description("test for bug #4059"))
 {
   AcceptContinuation authorizationAccept;
-  auto authorization =
-    [&authorizationAccept] (const Name&, const Interest&, const ControlParameters*,
-                            AcceptContinuation accept, RejectContinuation) {
-      authorizationAccept = std::move(accept);
-    };
-
-  auto validateParameters =
-    [] (const ControlParameters& params) {
-      return dynamic_cast<const StatefulParameters&>(params).check();
-    };
+  auto authorization = [&authorizationAccept] (const Name&, const Interest&, const ControlParameters*,
+                                               AcceptContinuation accept, RejectContinuation) {
+    authorizationAccept = std::move(accept);
+  };
+
+  auto validateParameters = [] (const ControlParameters& params) {
+    return dynamic_cast<const StatefulParameters&>(params).check();
+  };
 
   size_t nCallbackCalled = 0;
   dispatcher.addControlCommand<StatefulParameters>("test", authorization, validateParameters,