mgmt: moved signing into AppFace, code streamlining, and bug fixes

tests/management: improved callback validation

Fixed bad shared_ptr NULL checks
Fixed bad iterator decrement in InternalFace.sendInterest
Removed semi-useless constants in FibManager
Clarified ControlResponse text messages

Change-Id: Ic327a0b6b57827e401c7c3115d0ee92bae996a34
refs: #1138
diff --git a/tests/mgmt/fib-manager.cpp b/tests/mgmt/fib-manager.cpp
index fec1f63..b07d11b 100644
--- a/tests/mgmt/fib-manager.cpp
+++ b/tests/mgmt/fib-manager.cpp
@@ -27,6 +27,12 @@
 {
 public:
 
+  FibManagerFixture()
+    : m_callbackFired(false)
+  {
+
+  }
+
   shared_ptr<Face>
   getFace(FaceId id)
   {
@@ -44,31 +50,47 @@
     m_faces.push_back(face);
   }
 
+  void
+  validateControlResponse(const Data& response,
+                          uint32_t expectedCode,
+                          const std::string& expectedText)
+  {
+    m_callbackFired = true;
+    Block controlRaw = response.getContent().blockFromValue();
+
+    ndn::ControlResponse control;
+    control.wireDecode(controlRaw);
+
+    NFD_LOG_DEBUG("received control response"
+                  << " Name: " << response.getName()
+                  << " code: " << control.getCode()
+                  << " text: " << control.getText());
+
+    BOOST_REQUIRE(control.getCode() == expectedCode);
+    BOOST_REQUIRE(control.getText() == expectedText);
+  }
+
+  bool
+  didCallbackFire()
+  {
+    return m_callbackFired;
+  }
+
+  void
+  resetCallbackFired()
+  {
+    m_callbackFired = false;
+  }
+
 private:
   std::vector<shared_ptr<Face> > m_faces;
+  bool m_callbackFired;
 };
 
 
 BOOST_AUTO_TEST_SUITE(MgmtFibManager)
 
-void
-validateControlResponse(const Data& response,
-                        uint32_t expectedCode,
-                        const std::string& expectedText)
-{
-  Block controlRaw = response.getContent().blockFromValue();
 
-  ndn::ControlResponse control;
-  control.wireDecode(controlRaw);
-
-  NFD_LOG_DEBUG("received control response"
-                << " Name: " << response.getName()
-                << " code: " << control.getCode()
-                << " text: " << control.getText());
-
-  BOOST_REQUIRE(control.getCode() == expectedCode);
-  BOOST_REQUIRE(control.getText() == expectedText);
-}
 
 bool
 foundNextHop(FaceId id, uint32_t cost, const fib::NextHop& next)
@@ -81,7 +103,7 @@
 {
   shared_ptr<fib::Entry> entry = fib.findLongestPrefixMatch(prefix);
 
-  if (entry)
+  if (static_cast<bool>(entry))
     {
       const fib::NextHopList& hops = entry->getNextHops();
       return hops.size() == oldSize + 1 &&
@@ -101,7 +123,7 @@
                           face);
 
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 400, "MALFORMED");
+    bind(&FibManagerFixture::validateControlResponse, &fixture,  _1, 400, "Malformed command");
 
   Interest command("/localhost/nfd/fib");
   face->sendInterest(command);
@@ -117,25 +139,28 @@
                           &fixture, _1),
                           face);
 
+  BOOST_REQUIRE(fixture.didCallbackFire() == false);
+
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 400, "MALFORMED");
+    bind(&FibManagerFixture::validateControlResponse, &fixture, _1, 400, "Malformed command");
 
   Interest command("/localhost/nfd/fib");
   manager.onFibRequest(command);
+
+  BOOST_REQUIRE(fixture.didCallbackFire());
 }
 
-BOOST_AUTO_TEST_CASE(UnsupportedVerb)
+BOOST_FIXTURE_TEST_CASE(UnsupportedVerb, FibManagerFixture)
 {
-  FibManagerFixture fixture;
   shared_ptr<InternalFace> face(make_shared<InternalFace>());
   Fib fib;
   FibManager manager(fib,
                      bind(&FibManagerFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
 
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 404, "UNSUPPORTED");
+    bind(&FibManagerFixture::validateControlResponse, this, _1, 501, "Unsupported command");
 
   ndn::FibManagementOptions options;
   options.setName("/hello");
@@ -150,23 +175,24 @@
 
   Interest command(commandName);
   manager.onFibRequest(command);
+
+  BOOST_REQUIRE(didCallbackFire());
 }
 
-BOOST_AUTO_TEST_CASE(UnsignedCommand)
+BOOST_FIXTURE_TEST_CASE(UnsignedCommand, FibManagerFixture)
 {
-  FibManagerFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(make_shared<InternalFace>());
   Fib fib;
   FibManager manager(fib,
                      bind(&FibManagerFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                      face);
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 200, "OK");
+    bind(&FibManagerFixture::validateControlResponse, this, _1, 200, "OK");
   /// \todo enable once sig checking implement
-    // bind(&validateControlResponse, _1, 401, "SIGREG");
+    // bind(&FibManagerFixture::validateControlResponse, this, _1, 401, "SIGREG");
 
   ndn::FibManagementOptions options;
   options.setName("/hello");
@@ -182,24 +208,24 @@
   Interest command(commandName);
   manager.onFibRequest(command);
 
+  BOOST_REQUIRE(didCallbackFire());
   BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", 0, 101));
 }
 
-BOOST_AUTO_TEST_CASE(UnauthorizedCommand)
+BOOST_FIXTURE_TEST_CASE(UnauthorizedCommand, FibManagerFixture)
 {
-  FibManagerFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(make_shared<InternalFace>());
   Fib fib;
   FibManager manager(fib,
                      bind(&FibManagerFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                      face);
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 200, "OK");
+    bind(&FibManagerFixture::validateControlResponse, this, _1, 200, "OK");
   /// \todo enable once sig checking implement
-    // bind(&validateControlResponse, _1, 403, "UNAUTHORIZED");
+    // bind(&FibManagerFixture::validateControlResponse, this, _1, 403, "Unauthorized command");
 
   ndn::FibManagementOptions options;
   options.setName("/hello");
@@ -215,23 +241,23 @@
   Interest command(commandName);
   manager.onFibRequest(command);
 
+  BOOST_REQUIRE(didCallbackFire());
   BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", 0, 101));
 }
 
-BOOST_AUTO_TEST_CASE(BadOptionParse)
+BOOST_FIXTURE_TEST_CASE(BadOptionParse, FibManagerFixture)
 {
-  FibManagerFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(make_shared<InternalFace>());
   Fib fib;
   FibManager manager(fib,
                      bind(&FibManagerFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                      face);
 
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 400, "MALFORMED");
+    bind(&FibManagerFixture::validateControlResponse, this, _1, 400, "Malformed command");
 
   Name commandName("/localhost/nfd/fib");
   commandName.append("add-nexthop");
@@ -239,22 +265,23 @@
 
   Interest command(commandName);
   manager.onFibRequest(command);
+
+  BOOST_REQUIRE(didCallbackFire());
 }
 
-BOOST_AUTO_TEST_CASE(UnknownFaceId)
+BOOST_FIXTURE_TEST_CASE(UnknownFaceId, FibManagerFixture)
 {
-  FibManagerFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(make_shared<InternalFace>());
   Fib fib;
   FibManager manager(fib,
                      bind(&FibManagerFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                      face);
 
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 400, "MALFORMED");
+    bind(&FibManagerFixture::validateControlResponse, this, _1, 400, "Malformed command");
 
   ndn::FibManagementOptions options;
   options.setName("/hello");
@@ -270,22 +297,22 @@
   Interest command(commandName);
   manager.onFibRequest(command);
 
+  BOOST_REQUIRE(didCallbackFire());
   BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", 0, 101) == false);
 }
 
-BOOST_AUTO_TEST_CASE(AddNextHopVerbInitialAdd)
+BOOST_FIXTURE_TEST_CASE(AddNextHopVerbInitialAdd, FibManagerFixture)
 {
-  FibManagerFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(make_shared<InternalFace>());
   Fib fib;
   FibManager manager(fib,
                      bind(&FibManagerFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 200, "OK");
+    bind(&FibManagerFixture::validateControlResponse, this, _1, 200, "OK");
 
   ndn::FibManagementOptions options;
   options.setName("/hello");
@@ -301,23 +328,23 @@
   Interest command(commandName);
   manager.onFibRequest(command);
 
+  BOOST_REQUIRE(didCallbackFire());
   BOOST_REQUIRE(addedNextHopWithCost(fib, "/hello", 0, 101));
 }
 
-BOOST_AUTO_TEST_CASE(AddNextHopVerbAddToExisting)
+BOOST_FIXTURE_TEST_CASE(AddNextHopVerbAddToExisting, FibManagerFixture)
 {
-  FibManagerFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(make_shared<InternalFace>());
   Fib fib;
   FibManager manager(fib,
                      bind(&FibManagerFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 200, "OK");
+    bind(&FibManagerFixture::validateControlResponse, this, _1, 200, "OK");
 
   // Add faces with cost == FaceID for the name /hello
   // This test assumes:
@@ -338,10 +365,12 @@
 
       Interest command(commandName);
       manager.onFibRequest(command);
+      BOOST_REQUIRE(didCallbackFire());
+      resetCallbackFired();
 
       shared_ptr<fib::Entry> entry = fib.findLongestPrefixMatch("/hello");
 
-      if (entry)
+      if (static_cast<bool>(entry))
         {
           const fib::NextHopList& hops = entry->getNextHops();
           for (int j = 1; j <= i; j++)
@@ -356,19 +385,19 @@
     }
 }
 
-BOOST_AUTO_TEST_CASE(AddNextHopVerbUpdateFaceCost)
+BOOST_FIXTURE_TEST_CASE(AddNextHopVerbUpdateFaceCost, FibManagerFixture)
 {
   FibManagerFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(make_shared<InternalFace>());
   Fib fib;
   FibManager manager(fib,
                      bind(&FibManagerFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
   face->onReceiveData +=
-    bind(&validateControlResponse, _1, 200, "OK");
+    bind(&FibManagerFixture::validateControlResponse, this, _1, 200, "OK");
 
   ndn::FibManagementOptions options;
   options.setName("/hello");
@@ -385,6 +414,8 @@
 
     Interest command(commandName);
     manager.onFibRequest(command);
+    BOOST_REQUIRE(didCallbackFire());
+    resetCallbackFired();
   }
 
   {
@@ -398,6 +429,7 @@
 
     Interest command(commandName);
     manager.onFibRequest(command);
+    BOOST_REQUIRE(didCallbackFire());
   }
 
   shared_ptr<fib::Entry> entry = fib.findLongestPrefixMatch("/hello");
@@ -405,7 +437,7 @@
   // Add faces with cost == FaceID for the name /hello
   // This test assumes:
   //   FaceIDs are -1 because we don't add them to a forwarder
-  if (entry)
+  if (static_cast<bool>(entry))
     {
       const fib::NextHopList& hops = entry->getNextHops();
       BOOST_REQUIRE(hops.size() == 1);
diff --git a/tests/mgmt/internal-face.cpp b/tests/mgmt/internal-face.cpp
index 1c6f70d..5953fc6 100644
--- a/tests/mgmt/internal-face.cpp
+++ b/tests/mgmt/internal-face.cpp
@@ -23,6 +23,13 @@
 {
 public:
 
+  InternalFaceFixture()
+    : m_onInterestFired(false),
+      m_noOnInterestFired(false)
+  {
+
+  }
+
   shared_ptr<Face>
   getFace(FaceId id)
   {
@@ -34,132 +41,98 @@
   }
 
   void
+  validateOnInterestCallback(const Name& name, const Interest& interest)
+  {
+    m_onInterestFired = true;
+  }
+
+  void
+  validateNoOnInterestCallback(const Name& name, const Interest& interest)
+  {
+    m_noOnInterestFired = true;
+  }
+
+  void
   addFace(shared_ptr<Face> face)
   {
     m_faces.push_back(face);
   }
 
+  bool
+  didOnInterestFire()
+  {
+    return m_onInterestFired;
+  }
+
+  bool
+  didNoOnInterestFire()
+  {
+    return m_noOnInterestFired;
+  }
+
+  void
+  resetOnInterestFired()
+  {
+    m_onInterestFired = false;
+  }
+
+  void
+  resetNoOnInterestFired()
+  {
+    m_noOnInterestFired = false;
+  }
+
 private:
   std::vector<shared_ptr<Face> > m_faces;
+  bool m_onInterestFired;
+  bool m_noOnInterestFired;
 };
 
 BOOST_AUTO_TEST_SUITE(MgmtInternalFace)
 
 void
-validateControlResponse(const Data& response,
-                        uint32_t expectedCode,
-                        const std::string& expectedText)
+validatePutData(bool& called, const Name& expectedName, const Data& data)
 {
-  Block controlRaw = response.getContent().blockFromValue();
-
-  ndn::ControlResponse control;
-  control.wireDecode(controlRaw);
-
-  NFD_LOG_DEBUG("received control response"
-                << " Name: " << response.getName()
-                << " code: " << control.getCode()
-                << " text: " << control.getText());
-
-  BOOST_REQUIRE(control.getCode() == expectedCode);
-  BOOST_REQUIRE(control.getText() == expectedText);
+  called = true;
+  BOOST_CHECK_EQUAL(expectedName, data.getName());
 }
 
-BOOST_AUTO_TEST_CASE(ValidAddNextHop)
+BOOST_FIXTURE_TEST_CASE(PutData, InternalFaceFixture)
 {
-  InternalFaceFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(new InternalFace);
   Fib fib;
   FibManager manager(fib,
                      bind(&InternalFaceFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
 
-  face->onReceiveData +=
-    bind(&validateControlResponse, _1, 200, "OK");
+  bool didPutData = false;
+  Name dataName("/hello");
+  face->onReceiveData += bind(&validatePutData, boost::ref(didPutData), dataName, _1);
 
-  ndn::FibManagementOptions options;
-  options.setName("/hello");
-  options.setFaceId(1);
-  options.setCost(1);
+  Data testData(dataName);
+  face->sign(testData);
+  face->put(testData);
 
-  Block encodedOptions(options.wireEncode());
-
-  Name commandName("/localhost/nfd/fib");
-  commandName.append("add-nexthop");
-  commandName.append(encodedOptions);
-
-  Interest command(commandName);
-  face->sendInterest(command);
-
-  shared_ptr<fib::Entry> entry = fib.findLongestPrefixMatch("/hello");
-
-  if (entry)
-    {
-      const fib::NextHopList& hops = entry->getNextHops();
-      BOOST_REQUIRE(hops.size() == 1);
-      BOOST_CHECK(hops[0].getCost() == 1);
-    }
-  else
-    {
-      BOOST_FAIL("Failed to find expected fib entry");
-    }
+  BOOST_REQUIRE(didPutData);
 }
 
-BOOST_AUTO_TEST_CASE(InvalidPrefixRegistration)
+BOOST_FIXTURE_TEST_CASE(SendInterestHitEnd, InternalFaceFixture)
 {
-  InternalFaceFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(new InternalFace);
   Fib fib;
   FibManager manager(fib,
                      bind(&InternalFaceFixture::getFace,
-                          &fixture, _1),
-                     face);
-
-  face->onReceiveData +=
-    bind(&validateControlResponse, _1, 404, "MALFORMED");
-
-  Interest nonRegInterest("/hello");
-  face->sendInterest(nonRegInterest);
-
-  shared_ptr<fib::Entry> entry = fib.findLongestPrefixMatch("/hello");
-
-  if (entry)
-    {
-      const fib::NextHopList& hops = entry->getNextHops();
-      BOOST_REQUIRE(hops.size() == 0);
-    }
-}
-
-void
-validateOnInterestCallback(const Name& name, const Interest& interest)
-{
-  NFD_LOG_DEBUG("Reached correct callback");
-}
-
-void
-validateNoOnInterestCallback(const Name& name, const Interest& interest)
-{
-  BOOST_FAIL("Reached wrong callback");
-}
-
-BOOST_AUTO_TEST_CASE(SendInterestHitEnd)
-{
-  InternalFaceFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
-
-  shared_ptr<InternalFace> face(new InternalFace);
-  Fib fib;
-  FibManager manager(fib,
-                     bind(&InternalFaceFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
 
   face->setInterestFilter("/localhost/nfd/fib",
-                          &validateOnInterestCallback);
+                          bind(&InternalFaceFixture::validateOnInterestCallback,
+                               this, _1, _2));
 
   // generate command whose name is canonically
   // ordered after /localhost/nfd/fib so that
@@ -168,24 +141,27 @@
   Name commandName("/localhost/nfd/fib/end");
   Interest command(commandName);
   face->sendInterest(command);
+
+  BOOST_REQUIRE(didOnInterestFire());
+  BOOST_REQUIRE(didNoOnInterestFire() == false);
 }
 
 
 
-BOOST_AUTO_TEST_CASE(SendInterestHitBegin)
+BOOST_FIXTURE_TEST_CASE(SendInterestHitBegin, InternalFaceFixture)
 {
-  InternalFaceFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(new InternalFace);
   Fib fib;
   FibManager manager(fib,
                      bind(&InternalFaceFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
 
   face->setInterestFilter("/localhost/nfd/fib",
-                          &validateNoOnInterestCallback);
+                          bind(&InternalFaceFixture::validateNoOnInterestCallback,
+                               this, _1, _2));
 
   // generate command whose name is canonically
   // ordered before /localhost/nfd/fib so that
@@ -194,30 +170,34 @@
   Name commandName("/localhost/nfd");
   Interest command(commandName);
   face->sendInterest(command);
+
+  BOOST_REQUIRE(didNoOnInterestFire() == false);
 }
 
 
 
-BOOST_AUTO_TEST_CASE(SendInterestHitExact)
+BOOST_FIXTURE_TEST_CASE(SendInterestHitExact, InternalFaceFixture)
 {
-  InternalFaceFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(new InternalFace);
   Fib fib;
   FibManager manager(fib,
                      bind(&InternalFaceFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
 
   face->setInterestFilter("/localhost/nfd/eib",
-                          &validateNoOnInterestCallback);
+                          bind(&InternalFaceFixture::validateNoOnInterestCallback,
+                               this, _1, _2));
 
   face->setInterestFilter("/localhost/nfd/fib",
-                          &validateOnInterestCallback);
+                          bind(&InternalFaceFixture::validateOnInterestCallback,
+                           this, _1, _2));
 
   face->setInterestFilter("/localhost/nfd/gib",
-                          &validateNoOnInterestCallback);
+                          bind(&InternalFaceFixture::validateNoOnInterestCallback,
+                               this, _1, _2));
 
   // generate command whose name exactly matches
   // /localhost/nfd/fib
@@ -225,27 +205,31 @@
   Name commandName("/localhost/nfd/fib");
   Interest command(commandName);
   face->sendInterest(command);
+
+  BOOST_REQUIRE(didOnInterestFire());
+  BOOST_REQUIRE(didNoOnInterestFire() == false);
 }
 
 
 
-BOOST_AUTO_TEST_CASE(SendInterestHitPrevious)
+BOOST_FIXTURE_TEST_CASE(SendInterestHitPrevious, InternalFaceFixture)
 {
-  InternalFaceFixture fixture;
-  fixture.addFace(make_shared<DummyFace>());
+  addFace(make_shared<DummyFace>());
 
   shared_ptr<InternalFace> face(new InternalFace);
   Fib fib;
   FibManager manager(fib,
                      bind(&InternalFaceFixture::getFace,
-                          &fixture, _1),
+                          this, _1),
                           face);
 
   face->setInterestFilter("/localhost/nfd/fib",
-                          &validateOnInterestCallback);
+                          bind(&InternalFaceFixture::validateOnInterestCallback,
+                               this, _1, _2));
 
   face->setInterestFilter("/localhost/nfd/fib/zzzzzzzzzzzzz/",
-                          &validateNoOnInterestCallback);
+                          bind(&InternalFaceFixture::validateNoOnInterestCallback,
+                               this, _1, _2));
 
   // generate command whose name exactly matches
   // an Interest filter
@@ -253,6 +237,9 @@
   Name commandName("/localhost/nfd/fib/previous");
   Interest command(commandName);
   face->sendInterest(command);
+
+  BOOST_REQUIRE(didOnInterestFire());
+  BOOST_REQUIRE(didNoOnInterestFire() == false);
 }
 
 BOOST_AUTO_TEST_SUITE_END()