tools: simplify autoconfig MulticastDiscovery stage

refs #4158

Change-Id: Ib44804a0ac97b59d567fe8c0493c7d9ec4915137
diff --git a/tests/tools/ndn-autoconfig/multicast-discovery.t.cpp b/tests/tools/ndn-autoconfig/multicast-discovery.t.cpp
new file mode 100644
index 0000000..8905d58
--- /dev/null
+++ b/tests/tools/ndn-autoconfig/multicast-discovery.t.cpp
@@ -0,0 +1,136 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/*
+ * Copyright (c) 2014-2017,  Regents of the University of California,
+ *                           Arizona Board of Regents,
+ *                           Colorado State University,
+ *                           University Pierre & Marie Curie, Sorbonne University,
+ *                           Washington University in St. Louis,
+ *                           Beijing Institute of Technology,
+ *                           The University of Memphis.
+ *
+ * This file is part of NFD (Named Data Networking Forwarding Daemon).
+ * See AUTHORS.md for complete list of NFD authors and contributors.
+ *
+ * NFD is free software: you can redistribute it and/or modify it under the terms
+ * of the GNU General Public License as published by the Free Software Foundation,
+ * either version 3 of the License, or (at your option) any later version.
+ *
+ * NFD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * NFD, e.g., in COPYING.md file.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "ndn-autoconfig/multicast-discovery.hpp"
+
+#include "../mock-nfd-mgmt-fixture.hpp"
+#include <ndn-cxx/encoding/block-helpers.hpp>
+#include <ndn-cxx/encoding/tlv-nfd.hpp>
+
+namespace ndn {
+namespace tools {
+namespace autoconfig {
+namespace tests {
+
+using namespace ::nfd::tools::tests;
+using nfd::ControlParameters;
+
+BOOST_AUTO_TEST_SUITE(NdnAutoconfig)
+BOOST_FIXTURE_TEST_SUITE(TestMulticastDiscovery, MockNfdMgmtFixture)
+
+BOOST_AUTO_TEST_CASE(Normal)
+{
+  this->processInterest = [this] (const Interest& interest) {
+    if (Name("/localhost/nfd/faces/query").isPrefixOf(interest.getName())) {
+      nfd::FaceStatus payload1, payload2;
+      payload1.setFaceId(860)
+              .setRemoteUri("ether://[ff:ff:ff:ff:ff:ff]")
+              .setLocalUri("dev://eth0")
+              .setFaceScope(nfd::FACE_SCOPE_NON_LOCAL)
+              .setFacePersistency(nfd::FACE_PERSISTENCY_PERMANENT)
+              .setLinkType(nfd::LINK_TYPE_MULTI_ACCESS);
+      payload2.setFaceId(861)
+              .setRemoteUri("ether://[ff:ff:ff:ff:ff:ff]")
+              .setLocalUri("dev://eth1")
+              .setFaceScope(nfd::FACE_SCOPE_NON_LOCAL)
+              .setFacePersistency(nfd::FACE_PERSISTENCY_PERMANENT)
+              .setLinkType(nfd::LINK_TYPE_MULTI_ACCESS);
+      this->sendDataset(interest.getName(), payload1, payload2);
+      return;
+    }
+
+    optional<ControlParameters> req = parseCommand(interest, "/localhost/nfd/rib/register");
+    if (req) {
+      BOOST_REQUIRE(req->hasName());
+      BOOST_CHECK_EQUAL(req->getName(), "/localhop/ndn-autoconf/hub");
+      BOOST_REQUIRE(req->hasFaceId());
+
+      if (req->getFaceId() == 860) {
+        ControlParameters resp;
+        resp.setName("/localhop/ndn-autoconf/hub")
+            .setFaceId(860)
+            .setOrigin(nfd::ROUTE_ORIGIN_APP)
+            .setCost(1)
+            .setFlags(0);
+        this->succeedCommand(interest, resp);
+      }
+      else if (req->getFaceId() == 861) {
+        // no reply, but stage should continue when there is at least one successful registration
+      }
+      else {
+        BOOST_ERROR("unexpected rib/register command");
+      }
+      return;
+    }
+
+    req = parseCommand(interest, "/localhost/nfd/strategy-choice/set");
+    if (req) {
+      BOOST_REQUIRE(req->hasName());
+      BOOST_CHECK_EQUAL(req->getName(), "/localhop/ndn-autoconf/hub");
+      BOOST_REQUIRE(req->hasStrategy());
+      BOOST_CHECK_EQUAL(req->getStrategy(), "/localhost/nfd/strategy/multicast");
+
+      this->succeedCommand(interest, *req);
+      return;
+    }
+
+    if (interest.getName() == "/localhop/ndn-autoconf/hub") {
+      const char FACEURI[] = "udp://router.example.net";
+      auto data = makeData(Name("/localhop/ndn-autoconf/hub").appendVersion());
+      data->setContent(makeBinaryBlock(tlv::nfd::Uri, FACEURI, ::strlen(FACEURI)));
+      face.receive(*data);
+      return;
+    }
+
+    BOOST_ERROR("unexpected Interest " << interest);
+  };
+
+  nfd::Controller controller(face, m_keyChain);
+  MulticastDiscovery stage(face, controller);
+
+  bool hasSuccess = false;
+  stage.onSuccess.connect([&hasSuccess] (const FaceUri& u) {
+    BOOST_CHECK(!hasSuccess);
+    hasSuccess = true;
+
+    BOOST_CHECK(u == FaceUri("udp://router.example.net"));
+  });
+
+  stage.onFailure.connect([] (const std::string& reason) {
+    BOOST_ERROR("unexpected failure: " << reason);
+  });
+
+  stage.start();
+  face.processEvents(time::seconds(20));
+  BOOST_CHECK(hasSuccess);
+}
+
+BOOST_AUTO_TEST_SUITE_END() // TestMulticastDiscovery
+BOOST_AUTO_TEST_SUITE_END() // NdnAutoconfig
+
+} // namespace tests
+} // namespace autoconfig
+} // namespace tools
+} // namespace ndn
diff --git a/tools/ndn-autoconfig/multicast-discovery.cpp b/tools/ndn-autoconfig/multicast-discovery.cpp
index f9b4797..c74d5f3 100644
--- a/tools/ndn-autoconfig/multicast-discovery.cpp
+++ b/tools/ndn-autoconfig/multicast-discovery.cpp
@@ -24,146 +24,132 @@
  */
 
 #include "multicast-discovery.hpp"
-
+#include <boost/lexical_cast.hpp>
 #include <ndn-cxx/encoding/tlv-nfd.hpp>
 
 namespace ndn {
 namespace tools {
 namespace autoconfig {
 
-static const Name LOCALHOP_HUB_DISCOVERY_PREFIX = "/localhop/ndn-autoconf/hub";
+using nfd::ControlParameters;
+using nfd::ControlResponse;
+
+static const Name HUB_DISCOVERY_PREFIX("/localhop/ndn-autoconf/hub");
+static const uint64_t HUB_DISCOVERY_ROUTE_COST(1);
+static const time::milliseconds HUB_DISCOVERY_ROUTE_EXPIRATION = time::seconds(30);
+static const time::milliseconds HUB_DISCOVERY_INTEREST_LIFETIME = time::seconds(4);
 
 MulticastDiscovery::MulticastDiscovery(Face& face, nfd::Controller& controller)
   : m_face(face)
   , m_controller(controller)
-  , m_nRequestedRegs(0)
-  , m_nFinishedRegs(0)
 {
 }
 
 void
 MulticastDiscovery::doStart()
 {
-  this->collectMulticastFaces();
-}
-
-void
-MulticastDiscovery::collectMulticastFaces()
-{
   nfd::FaceQueryFilter filter;
   filter.setLinkType(nfd::LINK_TYPE_MULTI_ACCESS);
+
   m_controller.fetch<nfd::FaceQueryDataset>(
     filter,
     bind(&MulticastDiscovery::registerHubDiscoveryPrefix, this, _1),
-    bind(&MulticastDiscovery::fail, this, _2)
-  );
+    [this] (uint32_t code, const std::string& reason) {
+      this->fail("Error " + to_string(code) + " when querying multi-access faces: " + reason);
+    });
 }
 
 void
 MulticastDiscovery::registerHubDiscoveryPrefix(const std::vector<nfd::FaceStatus>& dataset)
 {
-  std::vector<uint64_t> multicastFaces;
-  std::transform(dataset.begin(), dataset.end(), std::back_inserter(multicastFaces),
-                 [] (const nfd::FaceStatus& faceStatus) { return faceStatus.getFaceId(); });
+  if (dataset.empty()) {
+    this->fail("No multi-access faces available");
+    return;
+  }
 
-  if (multicastFaces.empty()) {
-    this->fail("No multicast faces available, skipping multicast discovery stage");
+  m_nRegs = dataset.size();
+  m_nRegSuccess = 0;
+  m_nRegFailure = 0;
+
+  for (const auto& faceStatus : dataset) {
+    ControlParameters parameters;
+    parameters.setName(HUB_DISCOVERY_PREFIX)
+              .setFaceId(faceStatus.getFaceId())
+              .setCost(HUB_DISCOVERY_ROUTE_COST)
+              .setExpirationPeriod(HUB_DISCOVERY_ROUTE_EXPIRATION);
+
+    m_controller.start<nfd::RibRegisterCommand>(
+      parameters,
+      [this] (const ControlParameters&) {
+        ++m_nRegSuccess;
+        afterReg();
+      },
+      [this, faceStatus] (const ControlResponse& resp) {
+        std::cerr << "Error " << resp.getCode() << " when registering hub discovery prefix "
+                  << "for face " << faceStatus.getFaceId() << " (" << faceStatus.getRemoteUri()
+                  << "): " << resp.getText() << std::endl;
+        ++m_nRegFailure;
+        afterReg();
+      });
+  }
+}
+
+void
+MulticastDiscovery::afterReg()
+{
+  if (m_nRegSuccess + m_nRegFailure < m_nRegs) {
+    return; // continue waiting
+  }
+  if (m_nRegSuccess > 0) {
+    this->setStrategy();
   }
   else {
-    nfd::ControlParameters parameters;
-    parameters
-      .setName(LOCALHOP_HUB_DISCOVERY_PREFIX)
-      .setCost(1)
-      .setExpirationPeriod(time::seconds(30));
-
-    m_nRequestedRegs = multicastFaces.size();
-    m_nFinishedRegs = 0;
-
-    for (const auto& face : multicastFaces) {
-      parameters.setFaceId(face);
-      m_controller.start<nfd::RibRegisterCommand>(
-        parameters,
-        bind(&MulticastDiscovery::onRegisterSuccess, this),
-        bind(&MulticastDiscovery::onRegisterFailure, this, _1));
-    }
-  }
-}
-
-void
-MulticastDiscovery::onRegisterSuccess()
-{
-  ++m_nFinishedRegs;
-
-  if (m_nRequestedRegs == m_nFinishedRegs) {
-    MulticastDiscovery::setStrategy();
-  }
-}
-
-void
-MulticastDiscovery::onRegisterFailure(const nfd::ControlResponse& response)
-{
-  std::cerr << "ERROR: " << response.getText() << " (code: " << response.getCode() << ")" << std::endl;
-  --m_nRequestedRegs;
-
-  if (m_nRequestedRegs == m_nFinishedRegs) {
-    if (m_nRequestedRegs > 0) {
-      MulticastDiscovery::setStrategy();
-    }
-    else {
-      this->fail("Failed to register " + LOCALHOP_HUB_DISCOVERY_PREFIX.toUri() +
-                 " for all multicast faces, skipping multicast discovery stage");
-    }
+    this->fail("Cannot register hub discovery prefix for any face");
   }
 }
 
 void
 MulticastDiscovery::setStrategy()
 {
-  nfd::ControlParameters parameters;
-  parameters
-    .setName(LOCALHOP_HUB_DISCOVERY_PREFIX)
-    .setStrategy("/localhost/nfd/strategy/multicast");
+  ControlParameters parameters;
+  parameters.setName(HUB_DISCOVERY_PREFIX)
+            .setStrategy("/localhost/nfd/strategy/multicast"),
 
   m_controller.start<nfd::StrategyChoiceSetCommand>(
     parameters,
     bind(&MulticastDiscovery::requestHubData, this),
-    bind(&MulticastDiscovery::onSetStrategyFailure, this, _1));
-}
-
-void
-MulticastDiscovery::onSetStrategyFailure(const nfd::ControlResponse& response)
-{
-  this->fail("Failed to set multicast strategy for " +
-             LOCALHOP_HUB_DISCOVERY_PREFIX.toUri() + " namespace (" + response.getText() + "). "
-             "Skipping multicast discovery stage");
+    [this] (const ControlResponse& resp) {
+      this->fail("Error " + to_string(resp.getCode()) + " when setting multicast strategy: " +
+                 resp.getText());
+    });
 }
 
 void
 MulticastDiscovery::requestHubData()
 {
-  Interest interest(LOCALHOP_HUB_DISCOVERY_PREFIX);
-  interest.setInterestLifetime(time::milliseconds(4000)); // 4 seconds
+  Interest interest(HUB_DISCOVERY_PREFIX);
+  interest.setInterestLifetime(HUB_DISCOVERY_INTEREST_LIFETIME);
   interest.setMustBeFresh(true);
 
   m_face.expressInterest(interest,
-                         bind(&MulticastDiscovery::onSuccess, this, _2),
-                         bind(&MulticastDiscovery::fail, this, "HUB Data not received: nacked"),
-                         bind(&MulticastDiscovery::fail, this, "HUB Data not received: timeout"));
-}
+    [this] (const Interest&, const Data& data) {
+      const Block& content = data.getContent();
+      content.parse();
 
-void
-MulticastDiscovery::onSuccess(const Data& data)
-{
-  const Block& content = data.getContent();
-  content.parse();
+      auto i = content.find(tlv::nfd::Uri);
+      if (i == content.elements_end()) {
+        this->fail("Malformed hub Data: missing Uri element");
+        return;
+      }
 
-  // Get Uri
-  Block::element_const_iterator blockValue = content.find(tlv::nfd::Uri);
-  if (blockValue == content.elements_end()) {
-    this->fail("Incorrect reply to multicast discovery stage");
-    return;
-  }
-  this->provideHubFaceUri(std::string(reinterpret_cast<const char*>(blockValue->value()), blockValue->value_size()));
+      this->provideHubFaceUri(std::string(reinterpret_cast<const char*>(i->value()), i->value_size()));
+    },
+    [this] (const Interest&, const lp::Nack& nack) {
+      this->fail("Nack-" + boost::lexical_cast<std::string>(nack.getReason()) + " when retrieving hub Data");
+    },
+    [this] (const Interest&) {
+      this->fail("Timeout when retrieving hub Data");
+    });
 }
 
 } // namespace autoconfig
diff --git a/tools/ndn-autoconfig/multicast-discovery.hpp b/tools/ndn-autoconfig/multicast-discovery.hpp
index d714306..a1d5884 100644
--- a/tools/ndn-autoconfig/multicast-discovery.hpp
+++ b/tools/ndn-autoconfig/multicast-discovery.hpp
@@ -35,31 +35,28 @@
 namespace tools {
 namespace autoconfig {
 
-/**
- * @brief Multicast discovery stage
+/** \brief multicast discovery stage
  *
- * - Request
+ *  This stage locates an NDN gateway router, commonly known as a "hub", in the local network by
+ *  sending a hub discovery Interest ndn:/localhop/ndn-autoconf/hub via multicast. This class
+ *  configures routes and strategy on local NFD, so that this Interest is multicast to all
+ *  multi-access faces.
  *
- *     The end host sends an Interest over a multicast face.
+ *  If an NDN gateway router is present in the local network, it should reply with a Data
+ *  containing its own FaceUri. The Data payload contains a Uri element, and the value of this
+ *  element is an ASCII-encoded string of the router's FaceUri. The router may use
+ *  ndn-autoconfig-server program to serve this Data.
  *
- *     Interest Name is /localhop/ndn-autoconf/hub.
- *
- * - Response
- *
- *     A producer app on the HUB answer this Interest with a Data packet that contains a
- *     TLV-encoded Uri block.  The value of this block is the URI for the HUB, preferably a
- *     UDP tunnel.
+ *  Signature on this Data is currently not verified. This stage succeeds when the Data is
+ *  successfully decoded.
  */
 class MulticastDiscovery : public Stage
 {
 public:
-  /**
-   * @brief Create multicast discovery stage
-   */
   MulticastDiscovery(Face& face, nfd::Controller& controller);
 
   const std::string&
-  getName() const override
+  getName() const final
   {
     static const std::string STAGE_NAME("multicast discovery");
     return STAGE_NAME;
@@ -67,38 +64,27 @@
 
 private:
   void
-  doStart() override;
-
-  void
-  collectMulticastFaces();
+  doStart() final;
 
   void
   registerHubDiscoveryPrefix(const std::vector<nfd::FaceStatus>& dataset);
 
   void
-  onRegisterSuccess();
-
-  void
-  onRegisterFailure(const nfd::ControlResponse& response);
+  afterReg();
 
   void
   setStrategy();
 
   void
-  onSetStrategyFailure(const nfd::ControlResponse& response);
-
-  // Start to look for a hub (NDN hub discovery first stage)
-  void
   requestHubData();
 
-  void
-  onSuccess(const Data& data);
-
 private:
   Face& m_face;
   nfd::Controller& m_controller;
-  size_t m_nRequestedRegs;
-  size_t m_nFinishedRegs;
+
+  int m_nRegs = 0;
+  int m_nRegSuccess = 0;
+  int m_nRegFailure = 0;
 };
 
 } // namespace autoconfig