cs+fw: omitted FreshnessPeriod implies always stale

refs #3944

Change-Id: I018242b3ba5850e8a126f956af4150b512406efd
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index 41ab2e2..3263b43 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -300,7 +300,7 @@
 
 void
 Forwarder::onInterestFinalize(const shared_ptr<pit::Entry>& pitEntry, bool isSatisfied,
-                              time::milliseconds dataFreshnessPeriod)
+                              ndn::optional<time::milliseconds> dataFreshnessPeriod)
 {
   NFD_LOG_DEBUG("onInterestFinalize interest=" << pitEntry->getName() <<
                 (isSatisfied ? " satisfied" : " unsatisfied"));
@@ -552,7 +552,7 @@
 
 void
 Forwarder::setStragglerTimer(const shared_ptr<pit::Entry>& pitEntry, bool isSatisfied,
-                             time::milliseconds dataFreshnessPeriod)
+                             ndn::optional<time::milliseconds> dataFreshnessPeriod)
 {
   time::nanoseconds stragglerTime = time::milliseconds(100);
 
@@ -577,18 +577,15 @@
 
 void
 Forwarder::insertDeadNonceList(pit::Entry& pitEntry, bool isSatisfied,
-                               time::milliseconds dataFreshnessPeriod, Face* upstream)
+                               ndn::optional<time::milliseconds> dataFreshnessPeriod, Face* upstream)
 {
   // need Dead Nonce List insert?
-  bool needDnl = false;
+  bool needDnl = true;
   if (isSatisfied) {
-    bool hasFreshnessPeriod = dataFreshnessPeriod >= time::milliseconds::zero();
-    // Data never becomes stale if it doesn't have FreshnessPeriod field
+    BOOST_ASSERT(dataFreshnessPeriod);
+    BOOST_ASSERT(*dataFreshnessPeriod >= time::milliseconds::zero());
     needDnl = static_cast<bool>(pitEntry.getInterest().getMustBeFresh()) &&
-              (hasFreshnessPeriod && dataFreshnessPeriod < m_deadNonceList.getLifetime());
-  }
-  else {
-    needDnl = true;
+              *dataFreshnessPeriod < m_deadNonceList.getLifetime();
   }
 
   if (!needDnl) {
diff --git a/daemon/fw/forwarder.hpp b/daemon/fw/forwarder.hpp
index fca81eb..b0cc156 100644
--- a/daemon/fw/forwarder.hpp
+++ b/daemon/fw/forwarder.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2016,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -216,7 +216,7 @@
    */
   VIRTUAL_WITH_TESTS void
   onInterestFinalize(const shared_ptr<pit::Entry>& pitEntry, bool isSatisfied,
-                     time::milliseconds dataFreshnessPeriod = time::milliseconds(-1));
+                     ndn::optional<time::milliseconds> dataFreshnessPeriod = ndn::nullopt);
 
   /** \brief incoming Data pipeline
    */
@@ -249,7 +249,7 @@
 
   VIRTUAL_WITH_TESTS void
   setStragglerTimer(const shared_ptr<pit::Entry>& pitEntry, bool isSatisfied,
-                    time::milliseconds dataFreshnessPeriod = time::milliseconds(-1));
+                    ndn::optional<time::milliseconds> dataFreshnessPeriod = ndn::nullopt);
 
   VIRTUAL_WITH_TESTS void
   cancelUnsatisfyAndStragglerTimer(pit::Entry& pitEntry);
@@ -260,7 +260,7 @@
    */
   VIRTUAL_WITH_TESTS void
   insertDeadNonceList(pit::Entry& pitEntry, bool isSatisfied,
-                      time::milliseconds dataFreshnessPeriod, Face* upstream);
+                      ndn::optional<time::milliseconds> dataFreshnessPeriod, Face* upstream);
 
   /** \brief call trigger (method) on the effective strategy of pitEntry
    */
diff --git a/daemon/table/cs-entry-impl.cpp b/daemon/table/cs-entry-impl.cpp
index c8bd357..255cfff 100644
--- a/daemon/table/cs-entry-impl.cpp
+++ b/daemon/table/cs-entry-impl.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2015,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -46,13 +46,6 @@
   return !this->hasData();
 }
 
-bool
-EntryImpl::canStale() const
-{
-  BOOST_ASSERT(!this->isQuery());
-  return this->getStaleTime() < time::steady_clock::TimePoint::max();
-}
-
 void
 EntryImpl::unsetUnsolicited()
 {
diff --git a/daemon/table/cs-entry-impl.hpp b/daemon/table/cs-entry-impl.hpp
index d061f88..262fcd1 100644
--- a/daemon/table/cs-entry-impl.hpp
+++ b/daemon/table/cs-entry-impl.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2015,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -52,11 +52,6 @@
    */
   EntryImpl(shared_ptr<const Data> data, bool isUnsolicited);
 
-  /** \return true if entry can become stale, false if entry is never stale
-   */
-  bool
-  canStale() const;
-
   void
   unsetUnsolicited();
 
diff --git a/daemon/table/cs-entry.cpp b/daemon/table/cs-entry.cpp
index d4f3921..4bf84d5 100644
--- a/daemon/table/cs-entry.cpp
+++ b/daemon/table/cs-entry.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2015,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -48,12 +48,7 @@
 Entry::updateStaleTime()
 {
   BOOST_ASSERT(this->hasData());
-  if (m_data->getFreshnessPeriod() >= time::milliseconds::zero()) {
-    m_staleTime = time::steady_clock::now() + time::milliseconds(m_data->getFreshnessPeriod());
-  }
-  else {
-    m_staleTime = time::steady_clock::TimePoint::max();
-  }
+  m_staleTime = time::steady_clock::now() + time::milliseconds(m_data->getFreshnessPeriod());
 }
 
 bool
diff --git a/daemon/table/cs-entry.hpp b/daemon/table/cs-entry.hpp
index 68289cf..48895b1 100644
--- a/daemon/table/cs-entry.hpp
+++ b/daemon/table/cs-entry.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2016,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -76,8 +76,8 @@
     return m_isUnsolicited;
   }
 
-  /** \return the absolute time when the stored Data becomes expired
-   *  \retval time::steady_clock::TimePoint::max() if the stored Data never expires
+  /** \return the absolute time when the stored Data becomes stale
+   *  \note if the returned TimePoint is in the past, the Data is stale
    *  \pre hasData()
    */
   const time::steady_clock::TimePoint&
diff --git a/daemon/table/cs-policy-priority-fifo.cpp b/daemon/table/cs-policy-priority-fifo.cpp
index a61f334..934d759 100644
--- a/daemon/table/cs-policy-priority-fifo.cpp
+++ b/daemon/table/cs-policy-priority-fifo.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2016,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -117,11 +117,8 @@
   }
   else {
     entryInfo->queueType = QUEUE_FIFO;
-
-    if (i->canStale()) {
-      entryInfo->moveStaleEventId = scheduler::schedule(i->getData().getFreshnessPeriod(),
-                                              bind(&PriorityFifoPolicy::moveToStaleQueue, this, i));
-    }
+    entryInfo->moveStaleEventId = scheduler::schedule(i->getData().getFreshnessPeriod(),
+                                                      bind(&PriorityFifoPolicy::moveToStaleQueue, this, i));
   }
 
   Queue& queue = m_queues[entryInfo->queueType];
diff --git a/tests/daemon/table/cs.t.cpp b/tests/daemon/table/cs.t.cpp
index ecbbf38..0e14b0f 100644
--- a/tests/daemon/table/cs.t.cpp
+++ b/tests/daemon/table/cs.t.cpp
@@ -47,7 +47,6 @@
   insert(uint32_t id, const Name& name, const std::function<void(Data&)>& modifyData = nullptr)
   {
     shared_ptr<Data> data = makeData(name);
-    data->setFreshnessPeriod(time::milliseconds(99999));
     data->setContent(reinterpret_cast<const uint8_t*>(&id), sizeof(id));
 
     if (modifyData != nullptr) {
@@ -253,15 +252,12 @@
   CHECK_CS_FIND(6);
 }
 
-BOOST_AUTO_TEST_CASE_EXPECTED_FAILURES(MustBeFresh, 1)
 BOOST_AUTO_TEST_CASE(MustBeFresh)
 {
-  ///\todo #3944 add /A/1 without FreshnessPeriod, which is same as FreshnessPeriod=0ms
+  insert(1, "/A/1"); // omitted FreshnessPeriod means FreshnessPeriod = 0 ms
   insert(2, "/A/2", [] (Data& data) { data.setFreshnessPeriod(time::seconds(0)); });
   insert(3, "/A/3", [] (Data& data) { data.setFreshnessPeriod(time::seconds(1)); });
   insert(4, "/A/4", [] (Data& data) { data.setFreshnessPeriod(time::seconds(3600)); });
-  insert(5, "/A/5"); // omitted FreshnessPeriod means infinite
-  ///\todo #3944 delete /A/5
 
   // lookup at exact same moment as insertion is not tested because this won't happen in reality
 
@@ -283,8 +279,7 @@
   this->advanceClocks(time::seconds(3500)); // @7002s
   startInterest("/A")
     .setMustBeFresh(true);
-  CHECK_CS_FIND(5); // expected failure https://redmine.named-data.net/issues/3944#note-2
-  ///\todo #3944 this should not find any Data
+  CHECK_CS_FIND(0);
 }
 
 BOOST_AUTO_TEST_CASE(DigestOrder)
diff --git a/tests/daemon/table/pit-entry.t.cpp b/tests/daemon/table/pit-entry.t.cpp
index fad1528..73cf7de 100644
--- a/tests/daemon/table/pit-entry.t.cpp
+++ b/tests/daemon/table/pit-entry.t.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2016,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -178,8 +178,6 @@
 BOOST_AUTO_TEST_CASE(Lifetime)
 {
   shared_ptr<Interest> interest = makeInterest("ndn:/7oIEurbgy6");
-  // library uses -1 to indicate unset lifetime
-  BOOST_ASSERT(interest->getInterestLifetime() < time::milliseconds::zero());
 
   shared_ptr<Face> face = make_shared<DummyFace>();
   Entry entry(*interest);
diff --git a/tests/daemon/table/pit.t.cpp b/tests/daemon/table/pit.t.cpp
index 8a784f6..a468ed0 100644
--- a/tests/daemon/table/pit.t.cpp
+++ b/tests/daemon/table/pit.t.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2016,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -78,6 +78,12 @@
   BOOST_CHECK_EQUAL(insertResult.second, true);
   BOOST_CHECK_EQUAL(pit.size(), 1);
 
+  // same as A
+  shared_ptr<Interest> interestA2 = make_shared<Interest>(*interestA);
+  insertResult = pit.insert(*interestA2);
+  BOOST_CHECK_EQUAL(insertResult.second, false); // sharing the same PIT entry
+  BOOST_CHECK_EQUAL(pit.size(), 1);
+
   // A+MinSuffixComponents
   shared_ptr<Interest> interestB = make_shared<Interest>(*interestA);
   interestB->setMinSuffixComponents(2);
@@ -120,47 +126,40 @@
   BOOST_CHECK_EQUAL(insertResult.second, true);
   BOOST_CHECK_EQUAL(pit.size(), 7);
 
-  // A+ChildSelector0
-  shared_ptr<Interest> interestH = make_shared<Interest>(*interestA);
-  interestH->setChildSelector(0);
-  insertResult = pit.insert(*interestH);
-  BOOST_CHECK_EQUAL(insertResult.second, true);
-  BOOST_CHECK_EQUAL(pit.size(), 8);
-
   // A+ChildSelector1
   shared_ptr<Interest> interestI = make_shared<Interest>(*interestA);
   interestI->setChildSelector(1);
   insertResult = pit.insert(*interestI);
   BOOST_CHECK_EQUAL(insertResult.second, true);
-  BOOST_CHECK_EQUAL(pit.size(), 9);
+  BOOST_CHECK_EQUAL(pit.size(), 8);
 
   // A+MustBeFresh
   shared_ptr<Interest> interestJ = make_shared<Interest>(*interestA);
   interestJ->setMustBeFresh(true);
   insertResult = pit.insert(*interestJ);
   BOOST_CHECK_EQUAL(insertResult.second, true);
-  BOOST_CHECK_EQUAL(pit.size(), 10);
+  BOOST_CHECK_EQUAL(pit.size(), 9);
 
   // A+InterestLifetime
   shared_ptr<Interest> interestK = make_shared<Interest>(*interestA);
   interestK->setInterestLifetime(time::milliseconds(1000));
   insertResult = pit.insert(*interestK);
-  BOOST_CHECK_EQUAL(insertResult.second, false);// only guiders differ
-  BOOST_CHECK_EQUAL(pit.size(), 10);
+  BOOST_CHECK_EQUAL(insertResult.second, false); // only guiders differ
+  BOOST_CHECK_EQUAL(pit.size(), 9);
 
   // A+Nonce
   shared_ptr<Interest> interestL = make_shared<Interest>(*interestA);
   interestL->setNonce(2192);
   insertResult = pit.insert(*interestL);
-  BOOST_CHECK_EQUAL(insertResult.second, false);// only guiders differ
-  BOOST_CHECK_EQUAL(pit.size(), 10);
+  BOOST_CHECK_EQUAL(insertResult.second, false); // only guiders differ
+  BOOST_CHECK_EQUAL(pit.size(), 9);
 
   // different Name+Exclude1
   shared_ptr<Interest> interestM = make_shared<Interest>(name2);
   interestM->setExclude(exclude1);
   insertResult = pit.insert(*interestM);
   BOOST_CHECK_EQUAL(insertResult.second, true);
-  BOOST_CHECK_EQUAL(pit.size(), 11);
+  BOOST_CHECK_EQUAL(pit.size(), 10);
 }
 
 BOOST_AUTO_TEST_CASE(Erase)