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
diff --git a/tests/unit/mgmt/control-response.t.cpp b/tests/unit/mgmt/control-response.t.cpp
index 7e72c26..6bda463 100644
--- a/tests/unit/mgmt/control-response.t.cpp
+++ b/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 @@
 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,10 +46,21 @@
 
 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)
@@ -57,6 +68,26 @@
   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
diff --git a/tests/unit/mgmt/dispatcher.t.cpp b/tests/unit/mgmt/dispatcher.t.cpp
index ec6f90f..6a80012 100644
--- a/tests/unit/mgmt/dispatcher.t.cpp
+++ b/tests/unit/mgmt/dispatcher.t.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2024 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).
  *
@@ -20,7 +20,7 @@
  */
 
 #include "ndn-cxx/mgmt/dispatcher.hpp"
-#include "ndn-cxx/mgmt/nfd/control-parameters.hpp"
+#include "ndn-cxx/mgmt/nfd/control-command.hpp"
 #include "ndn-cxx/util/dummy-client-face.hpp"
 
 #include "tests/test-common.hpp"
@@ -85,46 +85,39 @@
 
 BOOST_AUTO_TEST_CASE(Basic)
 {
-  BOOST_CHECK_NO_THROW(dispatcher
-                         .addControlCommand<VoidParameters>("test/1", makeAcceptAllAuthorization(),
-                                                            std::bind([] { return true; }),
-                                                            std::bind([]{})));
-  BOOST_CHECK_NO_THROW(dispatcher
-                         .addControlCommand<VoidParameters>("test/2", makeAcceptAllAuthorization(),
-                                                            std::bind([] { return true; }),
-                                                            std::bind([]{})));
+  auto nop = [] (auto&&...) {};
+  auto tautology = [] (auto&&...) { return true; };
 
+  BOOST_CHECK_NO_THROW(dispatcher
+                       .addControlCommand<VoidParameters>("test/1", makeAcceptAllAuthorization(),
+                                                          tautology, nop));
+  BOOST_CHECK_NO_THROW(dispatcher
+                       .addControlCommand<VoidParameters>("test/2", makeAcceptAllAuthorization(),
+                                                          tautology, nop));
   BOOST_CHECK_THROW(dispatcher
-                      .addControlCommand<VoidParameters>("test", makeAcceptAllAuthorization(),
-                                                         std::bind([] { return true; }),
-                                                         std::bind([]{})),
+                    .addControlCommand<VoidParameters>("test", makeAcceptAllAuthorization(),
+                                                       tautology, nop),
                     std::out_of_range);
 
-  BOOST_CHECK_NO_THROW(dispatcher.addStatusDataset("status/1",
-                                                   makeAcceptAllAuthorization(), std::bind([]{})));
-  BOOST_CHECK_NO_THROW(dispatcher.addStatusDataset("status/2",
-                                                   makeAcceptAllAuthorization(), std::bind([]{})));
-  BOOST_CHECK_THROW(dispatcher.addStatusDataset("status",
-                                                makeAcceptAllAuthorization(), std::bind([]{})),
+  BOOST_CHECK_NO_THROW(dispatcher.addStatusDataset("status/1", makeAcceptAllAuthorization(), nop));
+  BOOST_CHECK_NO_THROW(dispatcher.addStatusDataset("status/2", makeAcceptAllAuthorization(), nop));
+  BOOST_CHECK_THROW(dispatcher.addStatusDataset("status", makeAcceptAllAuthorization(), nop),
                     std::out_of_range);
 
   BOOST_CHECK_NO_THROW(dispatcher.addNotificationStream("stream/1"));
   BOOST_CHECK_NO_THROW(dispatcher.addNotificationStream("stream/2"));
   BOOST_CHECK_THROW(dispatcher.addNotificationStream("stream"), std::out_of_range);
 
-
   BOOST_CHECK_NO_THROW(dispatcher.addTopPrefix("/root/1"));
   BOOST_CHECK_NO_THROW(dispatcher.addTopPrefix("/root/2"));
   BOOST_CHECK_THROW(dispatcher.addTopPrefix("/root"), std::out_of_range);
 
   BOOST_CHECK_THROW(dispatcher
-                      .addControlCommand<VoidParameters>("test/3", makeAcceptAllAuthorization(),
-                                                         std::bind([] { return true; }),
-                                                         std::bind([]{})),
+                    .addControlCommand<VoidParameters>("test/3", makeAcceptAllAuthorization(),
+                                                       tautology, nop),
                     std::domain_error);
 
-  BOOST_CHECK_THROW(dispatcher.addStatusDataset("status/3",
-                                                makeAcceptAllAuthorization(), std::bind([]{})),
+  BOOST_CHECK_THROW(dispatcher.addStatusDataset("status/3", makeAcceptAllAuthorization(), nop),
                     std::domain_error);
 
   BOOST_CHECK_THROW(dispatcher.addNotificationStream("stream/3"), std::domain_error);
@@ -135,13 +128,13 @@
   std::map<std::string, size_t> nCallbackCalled;
   dispatcher
     .addControlCommand<VoidParameters>("test/1", makeAcceptAllAuthorization(),
-                                       std::bind([] { return true; }),
-                                       std::bind([&nCallbackCalled] { ++nCallbackCalled["test/1"]; }));
+                                       [] (auto&&...) { return true; },
+                                       [&nCallbackCalled] (auto&&...) { ++nCallbackCalled["test/1"]; });
 
   dispatcher
     .addControlCommand<VoidParameters>("test/2", makeAcceptAllAuthorization(),
-                                       std::bind([] { return true; }),
-                                       std::bind([&nCallbackCalled] { ++nCallbackCalled["test/2"]; }));
+                                       [] (auto&&...) { return true; },
+                                       [&nCallbackCalled] (auto&&...) { ++nCallbackCalled["test/2"]; });
 
   face.receive(*makeInterest("/root/1/test/1/%80%00"));
   advanceClocks(1_ms);
@@ -190,18 +183,17 @@
   BOOST_CHECK_EQUAL(nCallbackCalled["test/1"], 4);
 }
 
-BOOST_AUTO_TEST_CASE(ControlCommand)
+BOOST_AUTO_TEST_CASE(ControlCommandOld,
+  * ut::description("test old-style ControlCommand registration"))
 {
   size_t nCallbackCalled = 0;
-  dispatcher
-    .addControlCommand<VoidParameters>("test",
-                                       makeTestAuthorization(),
-                                       std::bind([] { return true; }),
-                                       std::bind([&nCallbackCalled] { ++nCallbackCalled; }));
+  auto handler = [&nCallbackCalled] (auto&&...) { ++nCallbackCalled; };
+  dispatcher.addControlCommand<VoidParameters>("test", makeTestAuthorization(),
+                                               [] (auto&&...) { return true; }, std::move(handler));
 
   dispatcher.addTopPrefix("/root");
   advanceClocks(1_ms);
-  face.sentData.clear();
+  BOOST_REQUIRE_EQUAL(face.sentData.size(), 0);
 
   face.receive(*makeInterest("/root/test/%80%00")); // returns 403
   face.receive(*makeInterest("/root/test/%80%00/invalid")); // returns 403
@@ -210,7 +202,7 @@
   face.receive(*makeInterest("/root/test/.../valid"));  // silently ignored (wrong format)
   advanceClocks(1_ms, 20);
   BOOST_CHECK_EQUAL(nCallbackCalled, 0);
-  BOOST_CHECK_EQUAL(face.sentData.size(), 2);
+  BOOST_REQUIRE_EQUAL(face.sentData.size(), 2);
 
   BOOST_CHECK_EQUAL(face.sentData[0].getContentType(), tlv::ContentType_Blob);
   BOOST_CHECK_EQUAL(ControlResponse(face.sentData[0].getContent().blockFromValue()).getCode(), 403);
@@ -222,6 +214,96 @@
   BOOST_CHECK_EQUAL(nCallbackCalled, 1);
 }
 
+BOOST_AUTO_TEST_CASE(ControlCommandNew,
+  * ut::description("test new-style ControlCommand registration"))
+{
+  size_t nHandlerCalled = 0;
+  auto handler = [&nHandlerCalled] (auto&&...) { ++nHandlerCalled; };
+
+  // test addControlCommand()
+  dispatcher.addControlCommand<nfd::FaceCreateCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::FaceUpdateCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::FaceDestroyCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::FibAddNextHopCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::FibRemoveNextHopCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::CsConfigCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::CsEraseCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::StrategyChoiceSetCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::StrategyChoiceUnsetCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::RibRegisterCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addControlCommand<nfd::RibUnregisterCommand>(makeAcceptAllAuthorization(), handler);
+
+  BOOST_CHECK_THROW(dispatcher.addControlCommand<nfd::CsConfigCommand>(makeAcceptAllAuthorization(),
+                                                                       [] (auto&&...) {}),
+                    std::out_of_range);
+
+  dispatcher.addTopPrefix("/root");
+  advanceClocks(1_ms);
+  BOOST_REQUIRE_EQUAL(face.sentData.size(), 0);
+
+  BOOST_CHECK_THROW(dispatcher.addControlCommand<nfd::CsConfigCommand>(makeAcceptAllAuthorization(),
+                                                                       [] (auto&&...) {}),
+                    std::domain_error);
+
+  // we pick FaceDestroyCommand as an example for the following tests
+
+  // malformed request (missing ControlParameters) => silently ignored
+  auto baseName = Name("/root").append(nfd::FaceDestroyCommand::getName());
+  auto interest = makeInterest(baseName);
+  face.receive(*interest);
+  advanceClocks(1_ms);
+  BOOST_CHECK_EQUAL(nHandlerCalled, 0);
+  BOOST_CHECK_EQUAL(face.sentData.size(), 0);
+
+  // ControlParameters present but invalid (missing required field) => reply with error 400
+  nfd::ControlParameters params;
+  interest->setName(Name(baseName).append(params.wireEncode()));
+  face.receive(*interest);
+  advanceClocks(1_ms);
+  BOOST_CHECK_EQUAL(nHandlerCalled, 0);
+  BOOST_REQUIRE_EQUAL(face.sentData.size(), 1);
+  BOOST_CHECK_EQUAL(face.sentData[0].getContentType(), tlv::ContentType_Blob);
+  BOOST_CHECK_EQUAL(ControlResponse(face.sentData[0].getContent().blockFromValue()).getCode(), 400);
+
+  // valid request
+  params.setFaceId(42);
+  interest->setName(Name(baseName).append(params.wireEncode()));
+  face.receive(*interest);
+  advanceClocks(1_ms);
+  BOOST_CHECK_EQUAL(nHandlerCalled, 1);
+  BOOST_CHECK_EQUAL(face.sentData.size(), 1);
+}
+
+BOOST_AUTO_TEST_CASE(ControlCommandResponse)
+{
+  auto handler = [] (const Name& prefix, const Interest& interest,
+                     const ControlParameters&, const CommandContinuation& done) {
+    BOOST_CHECK_EQUAL(prefix, "/root");
+    BOOST_CHECK_EQUAL(interest.getName().getPrefix(3),
+                      Name("/root").append(nfd::CsConfigCommand::getName()));
+    done(ControlResponse(42, "the answer"));
+  };
+
+  // use CsConfigCommand as an example
+  dispatcher.addControlCommand<nfd::CsConfigCommand>(makeAcceptAllAuthorization(), handler);
+  dispatcher.addTopPrefix("/root");
+  advanceClocks(1_ms);
+  BOOST_REQUIRE_EQUAL(face.sentData.size(), 0);
+
+  nfd::ControlParameters params;
+  auto interest = makeInterest(Name("/root")
+                               .append(nfd::CsConfigCommand::getName())
+                               .append(params.wireEncode()));
+  face.receive(*interest);
+  advanceClocks(1_ms, 10);
+
+  BOOST_REQUIRE_EQUAL(face.sentData.size(), 1);
+  BOOST_CHECK_EQUAL(face.sentData[0].getContentType(), tlv::ContentType_Blob);
+  ControlResponse resp(face.sentData[0].getContent().blockFromValue());
+  BOOST_CHECK_EQUAL(resp.getCode(), 42);
+  BOOST_CHECK_EQUAL(resp.getText(), "the answer");
+}
+
 class StatefulParameters : public mgmt::ControlParameters
 {
 public:
@@ -269,7 +351,7 @@
 
   size_t nCallbackCalled = 0;
   dispatcher.addControlCommand<StatefulParameters>("test", authorization, validateParameters,
-                                                   std::bind([&nCallbackCalled] { ++nCallbackCalled; }));
+                                                   [&nCallbackCalled] (auto&&...) { ++nCallbackCalled; });
 
   dispatcher.addTopPrefix("/root");
   advanceClocks(1_ms);