tools: nfdc route remove command

refs #3866

Change-Id: Iffbcf3face8758a52d35a854408f9910f4ba6c0b
diff --git a/docs/manpages/nfdc-route.rst b/docs/manpages/nfdc-route.rst
index 7b01c44..0c2f0e0 100644
--- a/docs/manpages/nfdc-route.rst
+++ b/docs/manpages/nfdc-route.rst
@@ -27,7 +27,7 @@
 it is updated with the specified cost, route inheritance flags, and expiration period.
 This command returns when the request has been accepted, but does not wait for RIB update completion.
 
-The **nfdc unregister** command removes a route with matching prefix, nexthop, and origin.
+The **nfdc route remove** command removes a route with matching prefix, nexthop, and origin.
 
 OPTIONS
 -------
@@ -40,7 +40,8 @@
 
 <FACEURI>
     An URI representing the remote endpoint of a face.
-    It must uniquely match an existing face.
+    In **nfdc route add** command, it must uniquely match an existing face.
+    In **nfdc route remove** command, it must match one or more existing faces.
 
 <ORIGIN>
     Origin of the route, i.e. who is announcing the route.
@@ -70,9 +71,9 @@
 
 2: Malformed command line
 
-3: Face not found (**nfdc route add** only)
+3: Face not found
 
-4: FaceUri canonization failed (**nfdc route add** only)
+4: FaceUri canonization failed
 
 5: Ambiguous: multiple matching faces are found (**nfdc route add** only)
 
diff --git a/tests/tools/nfdc/mock-nfd-mgmt-fixture.hpp b/tests/tools/nfdc/mock-nfd-mgmt-fixture.hpp
index d1b8274..319a9f1 100644
--- a/tests/tools/nfdc/mock-nfd-mgmt-fixture.hpp
+++ b/tests/tools/nfdc/mock-nfd-mgmt-fixture.hpp
@@ -59,45 +59,73 @@
   }
 
 protected: // ControlCommand
-  /** \brief check the last Interest is a command with specified prefix
+  /** \brief check the Interest is a command with specified prefix
    *  \retval nullopt last Interest is not the expected command
    *  \return command parameters
    */
-  ndn::optional<ControlParameters>
-  getCommand(const Name& expectedPrefix) const
+  static ndn::optional<ControlParameters>
+  parseCommand(const Interest& interest, const Name& expectedPrefix)
   {
-    if (face.sentInterests.empty() ||
-        !expectedPrefix.isPrefixOf(face.sentInterests.back().getName())) {
+    if (!expectedPrefix.isPrefixOf(interest.getName())) {
       return ndn::nullopt;
     }
-    return ControlParameters(face.sentInterests.back().getName()
-                             .at(expectedPrefix.size()).blockFromValue());
+    return ControlParameters(interest.getName().at(expectedPrefix.size()).blockFromValue());
+  }
+
+  DEPRECATED(
+  ndn::optional<ControlParameters>
+  getCommand(const Name& expectedPrefix) const)
+  {
+    if (face.sentInterests.empty()) {
+      return ndn::nullopt;
+    }
+    return parseCommand(face.sentInterests.back(), expectedPrefix);
   }
 
   /** \brief respond to the last command
-   *  \pre last Interest is a command
    */
   void
-  succeedCommand(const ControlParameters& parameters)
+  succeedCommand(const Interest& interest, const ControlParameters& parameters)
   {
     ndn::nfd::ControlResponse resp(200, "OK");
     resp.setBody(parameters.wireEncode());
-    this->sendCommandReply(resp);
+    this->sendCommandReply(interest, resp);
+  }
+
+  DEPRECATED(
+  void
+  succeedCommand(const ControlParameters& parameters))
+  {
+    this->succeedCommand(face.sentInterests.back(), parameters);
   }
 
   /** \brief respond to the last command
    *  \pre last Interest is a command
    */
   void
-  failCommand(uint32_t code, const std::string& text)
+  failCommand(const Interest& interest, uint32_t code, const std::string& text)
   {
-    this->sendCommandReply({code, text});
+    this->sendCommandReply(interest, {code, text});
+  }
+
+  DEPRECATED(
+  void
+  failCommand(uint32_t code, const std::string& text))
+  {
+    this->sendCommandReply(face.sentInterests.back(), {code, text});
   }
 
   void
-  failCommand(uint32_t code, const std::string& text, const ControlParameters& resp)
+  failCommand(const Interest& interest, uint32_t code, const std::string& text, const ControlParameters& body)
   {
-    this->sendCommandReply(ndn::nfd::ControlResponse(code, text).setBody(resp.wireEncode()));
+    this->sendCommandReply(interest, code, text, body.wireEncode());
+  }
+
+  DEPRECATED(
+  void
+  failCommand(uint32_t code, const std::string& text, const ControlParameters& body))
+  {
+    this->failCommand(face.sentInterests.back(), code, text, body);
   }
 
 protected: // StatusDataset
@@ -218,13 +246,21 @@
   }
 
   void
-  sendCommandReply(const ndn::nfd::ControlResponse& resp)
+  sendCommandReply(const Interest& interest, const ndn::nfd::ControlResponse& resp)
   {
-    auto data = makeData(face.sentInterests.back().getName());
+    auto data = makeData(interest.getName());
     data->setContent(resp.wireEncode());
     face.receive(*data);
   }
 
+  void
+  sendCommandReply(const Interest& interest, uint32_t code, const std::string& text,
+                   const Block& body)
+  {
+    this->sendCommandReply(interest,
+                           ndn::nfd::ControlResponse(code, text).setBody(body));
+  }
+
   /** \brief send a payload in reply to StatusDataset request
    *  \param name dataset prefix without version and segment
    *  \param contentArgs passed to Data::setContent
@@ -269,13 +305,23 @@
 } // namespace tools
 } // namespace nfd
 
+/** \brief require the command in \p interest has expected prefix
+ *  \note This must be used in processInterest lambda, and the Interest must be named 'interest'.
+ */
+#define MOCK_NFD_MGMT_REQUIRE_COMMAND_IS(expectedPrefix) \
+  [interest] { \
+    auto params = parseCommand(interest, (expectedPrefix)); \
+    BOOST_REQUIRE_MESSAGE(params, "Interest " << interest.getName() << \
+                          " does not match command prefix " << (expectedPrefix)); \
+    return *params; \
+  } ()
+
+///\deprecated use MOCK_NFD_MGMT_REQUIRE_COMMAND_IS
 #define MOCK_NFD_MGMT_REQUIRE_LAST_COMMAND_IS(expectedPrefix) \
   [this] { \
     BOOST_REQUIRE_MESSAGE(!face.sentInterests.empty(), "no Interest expressed"); \
-    auto params = this->getCommand(expectedPrefix); \
-    BOOST_REQUIRE_MESSAGE(params, "last Interest " << face.sentInterests.back().getName() << \
-                          " does not match command prefix " << expectedPrefix); \
-    return *params; \
+    const Interest& interest = face.sentInterests.back(); \
+    return MOCK_NFD_MGMT_REQUIRE_COMMAND_IS(expectedPrefix); \
   } ()
 
 #endif // NFD_TESTS_TOOLS_NFDC_MOCK_NFD_MGMT_FIXTURE_HPP
diff --git a/tests/tools/nfdc/rib-module.t.cpp b/tests/tools/nfdc/rib-module.t.cpp
index 04c2b5f..df0a696 100644
--- a/tests/tools/nfdc/rib-module.t.cpp
+++ b/tests/tools/nfdc/rib-module.t.cpp
@@ -164,6 +164,124 @@
 
 BOOST_AUTO_TEST_SUITE_END() // AddCommand
 
+BOOST_FIXTURE_TEST_SUITE(RemoveCommand, ExecuteCommandFixture)
+
+BOOST_AUTO_TEST_CASE(NormalByFaceId)
+{
+  this->processInterest = [this] (const Interest& interest) {
+    if (this->respondFaceQuery(interest)) {
+      return;
+    }
+
+    ControlParameters req = MOCK_NFD_MGMT_REQUIRE_COMMAND_IS("/localhost/nfd/rib/unregister");
+    ndn::nfd::RibUnregisterCommand cmd;
+    cmd.validateRequest(req);
+    cmd.applyDefaultsToRequest(req);
+    BOOST_CHECK_EQUAL(req.getName(), "/2B5NUGjpt");
+    BOOST_CHECK_EQUAL(req.getFaceId(), 10156);
+    BOOST_CHECK_EQUAL(req.getOrigin(), ndn::nfd::ROUTE_ORIGIN_STATIC);
+
+    this->succeedCommand(interest, req);
+  };
+
+  this->execute("route remove /2B5NUGjpt 10156");
+  BOOST_CHECK_EQUAL(exitCode, 0);
+  BOOST_CHECK(out.is_equal("route-removed prefix=/2B5NUGjpt nexthop=10156 origin=255\n"));
+  BOOST_CHECK(err.is_empty());
+}
+
+BOOST_AUTO_TEST_CASE(NormalByFaceUri)
+{
+  this->processInterest = [this] (const Interest& interest) {
+    if (this->respondFaceQuery(interest)) {
+      return;
+    }
+
+    ControlParameters req = MOCK_NFD_MGMT_REQUIRE_COMMAND_IS("/localhost/nfd/rib/unregister");
+    ndn::nfd::RibUnregisterCommand cmd;
+    cmd.validateRequest(req);
+    cmd.applyDefaultsToRequest(req);
+    BOOST_CHECK_EQUAL(req.getName(), "/wHdNn0BtUF");
+    BOOST_CHECK_EQUAL(req.getFaceId(), 2249);
+    BOOST_CHECK_EQUAL(req.getOrigin(), 15246);
+
+    this->succeedCommand(interest, req);
+  };
+
+  this->execute("route remove /wHdNn0BtUF tcp4://32.121.182.82:6363 origin 15246");
+  BOOST_CHECK_EQUAL(exitCode, 0);
+  BOOST_CHECK(out.is_equal("route-removed prefix=/wHdNn0BtUF nexthop=2249 origin=15246\n"));
+  BOOST_CHECK(err.is_empty());
+}
+
+BOOST_AUTO_TEST_CASE(MultipleFaces)
+{
+  std::set<uint64_t> faceIds{6720, 31066};
+  this->processInterest = [this, &faceIds] (const Interest& interest) {
+    if (this->respondFaceQuery(interest)) {
+      return;
+    }
+
+    ControlParameters req = MOCK_NFD_MGMT_REQUIRE_COMMAND_IS("/localhost/nfd/rib/unregister");
+    ndn::nfd::RibUnregisterCommand cmd;
+    cmd.validateRequest(req);
+    cmd.applyDefaultsToRequest(req);
+    BOOST_CHECK_EQUAL(req.getName(), "/nm5y8X8b2");
+    BOOST_CHECK_MESSAGE(faceIds.erase(req.getFaceId()), "expected face " + std::to_string(req.getFaceId()));
+    BOOST_CHECK_EQUAL(req.getOrigin(), ndn::nfd::ROUTE_ORIGIN_STATIC);
+
+    this->succeedCommand(interest, req);
+  };
+
+  this->execute("route remove /nm5y8X8b2 udp4://225.131.75.231:56363");
+  BOOST_CHECK(faceIds.empty());
+  BOOST_CHECK_EQUAL(exitCode, 0);
+  BOOST_CHECK(out.is_equal("route-removed prefix=/nm5y8X8b2 nexthop=6720 origin=255\n"
+                           "route-removed prefix=/nm5y8X8b2 nexthop=31066 origin=255\n"));
+  BOOST_CHECK(err.is_empty());
+}
+
+BOOST_AUTO_TEST_CASE(FaceNotExist)
+{
+  this->processInterest = [this] (const Interest& interest) {
+    BOOST_CHECK(this->respondFaceQuery(interest));
+  };
+
+  this->execute("route remove /HeGRjzwFM 23728");
+  BOOST_CHECK_EQUAL(exitCode, 3);
+  BOOST_CHECK(out.is_empty());
+  BOOST_CHECK(err.is_equal("Face not found\n"));
+}
+
+BOOST_AUTO_TEST_CASE(ErrorDataset)
+{
+  this->processInterest = nullptr; // no response to dataset or command
+
+  this->execute("route remove /YX4xQQN3v5 udp://26.97.248.3");
+  BOOST_CHECK_EQUAL(exitCode, 1);
+  BOOST_CHECK(out.is_empty());
+  BOOST_CHECK(err.is_equal("Error 10060 when querying face: Timeout\n"));
+}
+
+BOOST_AUTO_TEST_CASE(ErrorCommand)
+{
+  this->processInterest = [this] (const Interest& interest) {
+    if (this->respondFaceQuery(interest)) {
+      return;
+    }
+
+    MOCK_NFD_MGMT_REQUIRE_COMMAND_IS("/localhost/nfd/rib/unregister");
+    // no response to command
+  };
+
+  this->execute("route remove /mvGRoxD2 10156");
+  BOOST_CHECK_EQUAL(exitCode, 1);
+  BOOST_CHECK(out.is_empty());
+  BOOST_CHECK(err.is_equal("Error 10060 when removing route: request timed out\n"));
+}
+
+BOOST_AUTO_TEST_SUITE_END() // RemoveCommand
+
 const std::string STATUS_XML = stripXmlSpaces(R"XML(
   <rib>
     <ribEntry>
diff --git a/tools/nfdc/find-face.cpp b/tools/nfdc/find-face.cpp
index d877e81..5645e26 100644
--- a/tools/nfdc/find-face.cpp
+++ b/tools/nfdc/find-face.cpp
@@ -43,7 +43,7 @@
 {
   FaceQueryFilter filter;
   filter.setRemoteUri(faceUri.toString());
-  return this->execute(filter);
+  return this->execute(filter, allowMulti);
 }
 
 FindFace::Code
@@ -55,14 +55,14 @@
 }
 
 FindFace::Code
-FindFace::execute(const boost::any& faceIdOrUri)
+FindFace::execute(const boost::any& faceIdOrUri, bool allowMulti)
 {
   const uint64_t* faceId = boost::any_cast<uint64_t>(&faceIdOrUri);
   if (faceId != nullptr) {
     return this->execute(*faceId);
   }
   else {
-    return this->execute(boost::any_cast<FaceUri>(faceIdOrUri));
+    return this->execute(boost::any_cast<FaceUri>(faceIdOrUri), allowMulti);
   }
 }
 
diff --git a/tools/nfdc/find-face.hpp b/tools/nfdc/find-face.hpp
index 45c3f1e..2661aa6 100644
--- a/tools/nfdc/find-face.hpp
+++ b/tools/nfdc/find-face.hpp
@@ -72,11 +72,11 @@
 
   /** \brief find face by FaceId or FaceUri
    *  \param faceIdOrUri a boost::any that contains uint64_t or FaceUri
-   *  \note allowMulti will be set to false
+   *  \param allowMulti effective only if \p faceIdOrUri contains a FaceUri
    *  \throw boost::bad_any_cast faceIdOrUri is neither uint64_t nor FaceUri
    */
   Code
-  execute(const boost::any& faceIdOrUri);
+  execute(const boost::any& faceIdOrUri, bool allowMulti = false);
 
   /** \brief find face by FaceQueryFilter
    *  \pre execute has not been invoked
diff --git a/tools/nfdc/rib-module.cpp b/tools/nfdc/rib-module.cpp
index b85410c..295ad3f 100644
--- a/tools/nfdc/rib-module.cpp
+++ b/tools/nfdc/rib-module.cpp
@@ -45,6 +45,14 @@
     .addArg("capture", ArgValueType::NONE, Required::NO, Positional::NO)
     .addArg("expires", ArgValueType::UNSIGNED, Required::NO, Positional::NO);
   parser.addCommand(defRouteAdd, &RibModule::add);
+
+  CommandDefinition defRouteRemove("route", "remove");
+  defRouteRemove
+    .setTitle("remove a route")
+    .addArg("prefix", ArgValueType::NAME, Required::YES, Positional::YES)
+    .addArg("nexthop", ArgValueType::FACE_ID_OR_URI, Required::YES, Positional::YES)
+    .addArg("origin", ArgValueType::UNSIGNED, Required::NO, Positional::NO);
+  parser.addCommand(defRouteRemove, &RibModule::remove);
 }
 
 void
@@ -116,6 +124,54 @@
 }
 
 void
+RibModule::remove(ExecuteContext& ctx)
+{
+  auto prefix = ctx.args.get<Name>("prefix");
+  const boost::any& nexthop = ctx.args.at("nexthop");
+  auto origin = ctx.args.get<uint64_t>("origin", ndn::nfd::ROUTE_ORIGIN_STATIC);
+
+  FindFace findFace(ctx);
+  FindFace::Code res = findFace.execute(nexthop, true);
+
+  ctx.exitCode = static_cast<int>(res);
+  switch (res) {
+    case FindFace::Code::OK:
+      break;
+    case FindFace::Code::ERROR:
+    case FindFace::Code::CANONIZE_ERROR:
+    case FindFace::Code::NOT_FOUND:
+      ctx.err << findFace.getErrorReason() << '\n';
+      return;
+    default:
+      BOOST_ASSERT_MSG(false, "unexpected FindFace result");
+      return;
+  }
+
+  for (const FaceStatus& faceStatus : findFace.getResults()) {
+    ControlParameters unregisterParams;
+    unregisterParams
+      .setName(prefix)
+      .setFaceId(faceStatus.getFaceId())
+      .setOrigin(origin);
+
+    ctx.controller.start<ndn::nfd::RibUnregisterCommand>(
+      unregisterParams,
+      [&] (const ControlParameters& resp) {
+        ctx.out << "route-removed ";
+        text::ItemAttributes ia;
+        ctx.out << ia("prefix") << resp.getName()
+                << ia("nexthop") << resp.getFaceId()
+                << ia("origin") << resp.getOrigin()
+                << '\n';
+      },
+      ctx.makeCommandFailureHandler("removing route"),
+      ctx.makeCommandOptions());
+  }
+
+  ctx.face.processEvents();
+}
+
+void
 RibModule::fetchStatus(Controller& controller,
                        const function<void()>& onSuccess,
                        const Controller::DatasetFailCallback& onFailure,
diff --git a/tools/nfdc/rib-module.hpp b/tools/nfdc/rib-module.hpp
index cca1ef4..aee2c8b 100644
--- a/tools/nfdc/rib-module.hpp
+++ b/tools/nfdc/rib-module.hpp
@@ -52,6 +52,11 @@
   static void
   add(ExecuteContext& ctx);
 
+  /** \brief the 'route remove' command
+   */
+  static void
+  remove(ExecuteContext& ctx);
+
   void
   fetchStatus(Controller& controller,
               const function<void()>& onSuccess,