face: insert pending Interest records for Interests from forwarder

refs #4228

Change-Id: Id039f62496c6289aec8a36964b25c1f25c52eda8
diff --git a/src/detail/face-impl.hpp b/src/detail/face-impl.hpp
index 09c481a..53eb4c0 100644
--- a/src/detail/face-impl.hpp
+++ b/src/detail/face-impl.hpp
@@ -90,16 +90,17 @@
   {
     this->ensureConnected(true);
 
+    const Interest& interest2 = *interest;
     auto entry = m_pendingInterestTable.insert(make_shared<PendingInterest>(
-      interest, afterSatisfied, afterNacked, afterTimeout, ref(m_scheduler))).first;
+      std::move(interest), afterSatisfied, afterNacked, afterTimeout, ref(m_scheduler))).first;
     (*entry)->setDeleter([this, entry] { m_pendingInterestTable.erase(entry); });
 
     lp::Packet lpPacket;
-    addFieldFromTag<lp::NextHopFaceIdField, lp::NextHopFaceIdTag>(lpPacket, *interest);
-    addFieldFromTag<lp::CongestionMarkField, lp::CongestionMarkTag>(lpPacket, *interest);
+    addFieldFromTag<lp::NextHopFaceIdField, lp::NextHopFaceIdTag>(lpPacket, interest2);
+    addFieldFromTag<lp::CongestionMarkField, lp::CongestionMarkTag>(lpPacket, interest2);
 
-    m_face.m_transport->send(finishEncoding(std::move(lpPacket), interest->wireEncode(),
-                                            'I', interest->getName()));
+    m_face.m_transport->send(finishEncoding(std::move(lpPacket), interest2.wireEncode(),
+                                            'I', interest2.getName()));
   }
 
   void
@@ -114,37 +115,62 @@
     m_pendingInterestTable.clear();
   }
 
-  void
+  /** @return whether the Data should be sent to the forwarder, if it does not come from the forwarder
+   */
+  bool
   satisfyPendingInterests(const Data& data)
   {
+    bool hasAppMatch = false, hasForwarderMatch = false;
     for (auto entry = m_pendingInterestTable.begin(); entry != m_pendingInterestTable.end(); ) {
-      if ((*entry)->getInterest()->matchesData(data)) {
-        shared_ptr<PendingInterest> matchedEntry = *entry;
-        NDN_LOG_DEBUG("   satisfying " << *matchedEntry->getInterest());
-        entry = m_pendingInterestTable.erase(entry);
+      if (!(*entry)->getInterest()->matchesData(data)) {
+        ++entry;
+        continue;
+      }
+
+      shared_ptr<PendingInterest> matchedEntry = *entry;
+      NDN_LOG_DEBUG("   satisfying " << *matchedEntry->getInterest() <<
+                    " from " << matchedEntry->getOrigin());
+      entry = m_pendingInterestTable.erase(entry);
+
+      if (matchedEntry->getOrigin() == PendingInterestOrigin::APP) {
+        hasAppMatch = true;
         matchedEntry->invokeDataCallback(data);
       }
       else {
-        ++entry;
+        hasForwarderMatch = true;
       }
     }
+    // if Data matches no pending Interest record, it is sent to the forwarder as unsolicited Data
+    return hasForwarderMatch || !hasAppMatch;
   }
 
-  void
+  /** @return whether the Nack should be sent to the forwarder, if it does not come from the forwarder
+   */
+  bool
   nackPendingInterests(const lp::Nack& nack)
   {
+    bool shouldSendToForwarder = false;
     for (auto entry = m_pendingInterestTable.begin(); entry != m_pendingInterestTable.end(); ) {
-      const Interest& pendingInterest = *(*entry)->getInterest();
-      if (nack.getInterest().matchesInterest(pendingInterest)) {
-        shared_ptr<PendingInterest> matchedEntry = *entry;
-        NDN_LOG_DEBUG("   nacking " << *matchedEntry->getInterest());
-        entry = m_pendingInterestTable.erase(entry);
+      if (!nack.getInterest().matchesInterest(*(*entry)->getInterest())) {
+        ++entry;
+        continue;
+      }
+
+      shared_ptr<PendingInterest> matchedEntry = *entry;
+      NDN_LOG_DEBUG("   nacking " << *matchedEntry->getInterest() <<
+                    " from " << matchedEntry->getOrigin());
+      entry = m_pendingInterestTable.erase(entry);
+
+      // TODO #4228 record Nack on PendingInterest record, and send Nack only if all InterestFilters have Nacked
+
+      if (matchedEntry->getOrigin() == PendingInterestOrigin::APP) {
         matchedEntry->invokeNackCallback(nack);
       }
       else {
-        ++entry;
+        shouldSendToForwarder = true;
       }
     }
+    return shouldSendToForwarder;
   }
 
 public: // producer
@@ -168,12 +194,18 @@
   }
 
   void
-  processInterestFilters(const Interest& interest)
+  processIncomingInterest(shared_ptr<const Interest> interest)
   {
+    const Interest& interest2 = *interest;
+    auto entry = m_pendingInterestTable.insert(make_shared<PendingInterest>(
+      std::move(interest), ref(m_scheduler))).first;
+    (*entry)->setDeleter([this, entry] { m_pendingInterestTable.erase(entry); });
+
     for (const auto& filter : m_interestFilterTable) {
-      if (filter->doesMatch(interest.getName())) {
+      if (filter->doesMatch(interest2.getName())) {
         NDN_LOG_DEBUG("   matches " << filter->getFilter());
-        filter->invokeInterestCallback(interest);
+        filter->invokeInterestCallback(interest2);
+        // TODO #4228 record number of matched InterestFilters on PendingInterest record
       }
     }
   }
@@ -181,6 +213,11 @@
   void
   asyncPutData(const Data& data)
   {
+    bool shouldSendToForwarder = satisfyPendingInterests(data);
+    if (!shouldSendToForwarder) {
+      return;
+    }
+
     this->ensureConnected(true);
 
     lp::Packet lpPacket;
@@ -194,6 +231,11 @@
   void
   asyncPutNack(const lp::Nack& nack)
   {
+    bool shouldSendToForwarder = nackPendingInterests(nack);
+    if (!shouldSendToForwarder) {
+      return;
+    }
+
     this->ensureConnected(true);
 
     lp::Packet lpPacket;
diff --git a/src/detail/pending-interest.hpp b/src/detail/pending-interest.hpp
index 735db80..38d7f70 100644
--- a/src/detail/pending-interest.hpp
+++ b/src/detail/pending-interest.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2016 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -22,23 +22,45 @@
 #ifndef NDN_DETAIL_PENDING_INTEREST_HPP
 #define NDN_DETAIL_PENDING_INTEREST_HPP
 
-#include "../interest.hpp"
 #include "../data.hpp"
+#include "../interest.hpp"
 #include "../lp/nack.hpp"
 #include "../util/scheduler-scoped-event-id.hpp"
 
 namespace ndn {
 
 /**
- * @brief stores a pending Interest and associated callbacks
+ * @brief Indicates where a pending Interest came from
+ */
+enum class PendingInterestOrigin
+{
+  APP, ///< Interest was received from this app via Face::expressInterest API
+  FORWARDER ///< Interest was received from the forwarder via Transport
+};
+
+std::ostream&
+operator<<(std::ostream& os, PendingInterestOrigin origin)
+{
+  switch (origin) {
+    case PendingInterestOrigin::APP:
+      return os << "app";
+    case PendingInterestOrigin::FORWARDER:
+      return os << "forwarder";
+  }
+  BOOST_ASSERT(false);
+  return os;
+}
+
+/**
+ * @brief Stores a pending Interest and associated callbacks
  */
 class PendingInterest : noncopyable
 {
 public:
   /**
-   * @brief Construct a pending Interest record
+   * @brief Construct a pending Interest record for an Interest from Face::expressInterest
    *
-   * The timeout is set based on the current time and the Interest lifetime.
+   * The timeout is set based on the current time and InterestLifetime.
    * This class will invoke the timeout callback unless the record is deleted before timeout.
    *
    * @param interest the Interest
@@ -52,21 +74,32 @@
                   const NackCallback& nackCallback,
                   const TimeoutCallback& timeoutCallback,
                   Scheduler& scheduler)
-    : m_interest(interest)
+    : m_interest(std::move(interest))
+    , m_origin(PendingInterestOrigin::APP)
     , m_dataCallback(dataCallback)
     , m_nackCallback(nackCallback)
     , m_timeoutCallback(timeoutCallback)
     , m_timeoutEvent(scheduler)
   {
-    m_timeoutEvent =
-      scheduler.scheduleEvent(m_interest->getInterestLifetime() > time::milliseconds::zero() ?
-                              m_interest->getInterestLifetime() :
-                              DEFAULT_INTEREST_LIFETIME,
-                              [=] { this->invokeTimeoutCallback(); });
+    scheduleTimeoutEvent(scheduler);
   }
 
   /**
-   * @return the Interest
+   * @brief Construct a pending Interest record for an Interest from NFD
+   *
+   * @param interest the Interest
+   * @param scheduler Scheduler for scheduling the timeout event
+   */
+  PendingInterest(shared_ptr<const Interest> interest, Scheduler& scheduler)
+    : m_interest(std::move(interest))
+    , m_origin(PendingInterestOrigin::FORWARDER)
+    , m_timeoutEvent(scheduler)
+  {
+    scheduleTimeoutEvent(scheduler);
+  }
+
+  /**
+   * @brief Get the Interest
    */
   shared_ptr<const Interest>
   getInterest() const
@@ -74,8 +107,14 @@
     return m_interest;
   }
 
+  PendingInterestOrigin
+  getOrigin() const
+  {
+    return m_origin;
+  }
+
   /**
-   * @brief invokes the Data callback
+   * @brief Invoke the Data callback
    * @note This method does nothing if the Data callback is empty
    */
   void
@@ -87,7 +126,7 @@
   }
 
   /**
-   * @brief invokes the Nack callback
+   * @brief Invoke the Nack callback
    * @note This method does nothing if the Nack callback is empty
    */
   void
@@ -108,8 +147,15 @@
   }
 
 private:
+  void
+  scheduleTimeoutEvent(Scheduler& scheduler)
+  {
+    m_timeoutEvent = scheduler.scheduleEvent(m_interest->getInterestLifetime(),
+                                             [=] { this->invokeTimeoutCallback(); });
+  }
+
   /**
-   * @brief invokes the timeout callback (if non-empty) and the deleter
+   * @brief Invoke the timeout callback (if non-empty) and the deleter
    */
   void
   invokeTimeoutCallback()
@@ -124,6 +170,7 @@
 
 private:
   shared_ptr<const Interest> m_interest;
+  PendingInterestOrigin m_origin;
   DataCallback m_dataCallback;
   NackCallback m_nackCallback;
   TimeoutCallback m_timeoutCallback;
diff --git a/src/face.cpp b/src/face.cpp
index 8580720..92c46ab 100644
--- a/src/face.cpp
+++ b/src/face.cpp
@@ -402,7 +402,7 @@
       else {
         extractLpLocalFields(*interest, lpPacket);
         NDN_LOG_DEBUG(">I " << *interest);
-        m_impl->processInterestFilters(*interest);
+        m_impl->processIncomingInterest(std::move(interest));
       }
       break;
     }
diff --git a/tests/unit-tests/face.t.cpp b/tests/unit-tests/face.t.cpp
index 8657613..e5f1d47 100644
--- a/tests/unit-tests/face.t.cpp
+++ b/tests/unit-tests/face.t.cpp
@@ -1,5 +1,5 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
  * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -329,18 +329,26 @@
 {
   BOOST_CHECK_EQUAL(face.sentNacks.size(), 0);
 
-  face.put(makeNack(Interest("/Hello/World", time::milliseconds(50)), lp::NackReason::NO_ROUTE));
-
+  face.put(makeNack("/unsolicited", 18645250, lp::NackReason::NO_ROUTE));
   advanceClocks(time::milliseconds(10));
-  BOOST_CHECK_EQUAL(face.sentNacks.size(), 1);
+  BOOST_CHECK_EQUAL(face.sentNacks.size(), 0); // unsolicited Nack would not be sent
 
-  auto nack = makeNack(Interest("/another/prefix", time::milliseconds(50)), lp::NackReason::NO_ROUTE);
+  face.receive(*makeInterest("/Hello/World", 14247162));
+  face.receive(*makeInterest("/another/prefix", 92203002));
+  advanceClocks(time::milliseconds(10));
+
+  face.put(makeNack("/Hello/World", 14247162, lp::NackReason::DUPLICATE));
+  advanceClocks(time::milliseconds(10));
+  BOOST_REQUIRE_EQUAL(face.sentNacks.size(), 1);
+  BOOST_CHECK_EQUAL(face.sentNacks[0].getReason(), lp::NackReason::DUPLICATE);
+  BOOST_CHECK(face.sentNacks[0].getTag<lp::CongestionMarkTag>() == nullptr);
+
+  auto nack = makeNack("/another/prefix", 92203002, lp::NackReason::NO_ROUTE);
   nack.setTag(make_shared<lp::CongestionMarkTag>(1));
   face.put(nack);
-
   advanceClocks(time::milliseconds(10));
   BOOST_REQUIRE_EQUAL(face.sentNacks.size(), 2);
-  BOOST_CHECK(face.sentNacks[0].getTag<lp::CongestionMarkTag>() == nullptr);
+  BOOST_CHECK_EQUAL(face.sentNacks[1].getReason(), lp::NackReason::NO_ROUTE);
   BOOST_CHECK(face.sentNacks[1].getTag<lp::CongestionMarkTag>() != nullptr);
 }