interest: reorganize code and test cases

* Sort functions to generally follow the order of fields in wire format.
* Merge similar constructors.
* Sort tests to follow the code order.
* Split InterestFilter to its own test suite.
* Add test coverage for Interest::getNonce() and
  Interest::refreshNonce().
* Move Link and SelectedDelegation tests to a sub test suite.
* Delete tests of malformed Link, which are already covered by
  Link test suite.

refs #4171

Change-Id: Ia7115f0be479e301673897115d112b0544a47f9e
diff --git a/src/interest.cpp b/src/interest.cpp
index 8959881..34ebd4a 100644
--- a/src/interest.cpp
+++ b/src/interest.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).
@@ -35,24 +35,14 @@
 static_assert(std::is_base_of<tlv::Error, Interest::Error>::value,
               "Interest::Error must inherit from tlv::Error");
 
-Interest::Interest()
-  : m_interestLifetime(DEFAULT_INTEREST_LIFETIME)
-  , m_selectedDelegationIndex(INVALID_SELECTED_DELEGATION_INDEX)
-{
-}
-
-Interest::Interest(const Name& name)
-  : m_name(name)
-  , m_interestLifetime(DEFAULT_INTEREST_LIFETIME)
-  , m_selectedDelegationIndex(INVALID_SELECTED_DELEGATION_INDEX)
-{
-}
-
-Interest::Interest(const Name& name, const time::milliseconds& interestLifetime)
+Interest::Interest(const Name& name, time::milliseconds interestLifetime)
   : m_name(name)
   , m_interestLifetime(interestLifetime)
   , m_selectedDelegationIndex(INVALID_SELECTED_DELEGATION_INDEX)
 {
+  if (interestLifetime < time::milliseconds::zero()) {
+    BOOST_THROW_EXCEPTION(std::invalid_argument("InterestLifetime must be >= 0"));
+  }
 }
 
 Interest::Interest(const Block& wire)
@@ -60,49 +50,148 @@
   wireDecode(wire);
 }
 
-uint32_t
-Interest::getNonce() const
-{
-  if (!m_nonce.hasWire())
-    const_cast<Interest*>(this)->setNonce(random::generateWord32());
+// ---- encode and decode ----
 
-  if (m_nonce.value_size() == sizeof(uint32_t))
-    return *reinterpret_cast<const uint32_t*>(m_nonce.value());
-  else {
-    // for compatibility reasons.  Should be removed eventually
-    return readNonNegativeInteger(m_nonce);
+template<encoding::Tag TAG>
+size_t
+Interest::wireEncode(EncodingImpl<TAG>& encoder) const
+{
+  size_t totalLength = 0;
+
+  // Interest ::= INTEREST-TYPE TLV-LENGTH
+  //                Name
+  //                Selectors?
+  //                Nonce
+  //                InterestLifetime?
+  //                Link?
+  //                SelectedDelegation?
+
+  // (reverse encoding)
+
+  // Link and SelectedDelegation
+  if (hasLink()) {
+    if (hasSelectedDelegation()) {
+      totalLength += prependNonNegativeIntegerBlock(encoder,
+                                                    tlv::SelectedDelegation,
+                                                    m_selectedDelegationIndex);
+    }
+    totalLength += encoder.prependBlock(m_link);
   }
+  else {
+    BOOST_ASSERT(!hasSelectedDelegation());
+  }
+
+  // InterestLifetime
+  if (getInterestLifetime() != DEFAULT_INTEREST_LIFETIME) {
+    totalLength += prependNonNegativeIntegerBlock(encoder,
+                                                  tlv::InterestLifetime,
+                                                  getInterestLifetime().count());
+  }
+
+  // Nonce
+  getNonce(); // to ensure that Nonce is properly set
+  totalLength += encoder.prependBlock(m_nonce);
+
+  // Selectors
+  if (hasSelectors()) {
+    totalLength += getSelectors().wireEncode(encoder);
+  }
+
+  // Name
+  totalLength += getName().wireEncode(encoder);
+
+  totalLength += encoder.prependVarNumber(totalLength);
+  totalLength += encoder.prependVarNumber(tlv::Interest);
+  return totalLength;
 }
 
-Interest&
-Interest::setNonce(uint32_t nonce)
+template size_t
+Interest::wireEncode<encoding::EncoderTag>(EncodingImpl<encoding::EncoderTag>& encoder) const;
+
+template size_t
+Interest::wireEncode<encoding::EstimatorTag>(EncodingImpl<encoding::EstimatorTag>& encoder) const;
+
+const Block&
+Interest::wireEncode() const
 {
-  if (m_wire.hasWire() && m_nonce.value_size() == sizeof(uint32_t)) {
-    std::memcpy(const_cast<uint8_t*>(m_nonce.value()), &nonce, sizeof(nonce));
-  }
-  else {
-    m_nonce = makeBinaryBlock(tlv::Nonce,
-                              reinterpret_cast<const uint8_t*>(&nonce),
-                              sizeof(nonce));
-    m_wire.reset();
-  }
-  return *this;
+  if (m_wire.hasWire())
+    return m_wire;
+
+  EncodingEstimator estimator;
+  size_t estimatedSize = wireEncode(estimator);
+
+  EncodingBuffer buffer(estimatedSize, 0);
+  wireEncode(buffer);
+
+  // to ensure that Nonce block points to the right memory location
+  const_cast<Interest*>(this)->wireDecode(buffer.block());
+
+  return m_wire;
 }
 
 void
-Interest::refreshNonce()
+Interest::wireDecode(const Block& wire)
 {
-  if (!hasNonce())
-    return;
+  m_wire = wire;
+  m_wire.parse();
 
-  uint32_t oldNonce = getNonce();
-  uint32_t newNonce = oldNonce;
-  while (newNonce == oldNonce)
-    newNonce = random::generateWord32();
+  if (m_wire.type() != tlv::Interest)
+    BOOST_THROW_EXCEPTION(Error("Unexpected TLV number when decoding Interest"));
 
-  setNonce(newNonce);
+  // Name
+  m_name.wireDecode(m_wire.get(tlv::Name));
+
+  // Selectors
+  Block::element_const_iterator val = m_wire.find(tlv::Selectors);
+  if (val != m_wire.elements_end()) {
+    m_selectors.wireDecode(*val);
+  }
+  else
+    m_selectors = Selectors();
+
+  // Nonce
+  m_nonce = m_wire.get(tlv::Nonce);
+
+  // InterestLifetime
+  val = m_wire.find(tlv::InterestLifetime);
+  if (val != m_wire.elements_end()) {
+    m_interestLifetime = time::milliseconds(readNonNegativeInteger(*val));
+  }
+  else {
+    m_interestLifetime = DEFAULT_INTEREST_LIFETIME;
+  }
+
+  // Link
+  m_linkCached.reset();
+  val = m_wire.find(tlv::Data);
+  if (val != m_wire.elements_end()) {
+    m_link = (*val);
+  }
+  else {
+    m_link = Block();
+  }
+
+  // SelectedDelegation
+  val = m_wire.find(tlv::SelectedDelegation);
+  if (val != m_wire.elements_end()) {
+    if (!this->hasLink()) {
+      BOOST_THROW_EXCEPTION(Error("Interest contains SelectedDelegation, but no LINK object"));
+    }
+    uint64_t selectedDelegation = readNonNegativeInteger(*val);
+    if (selectedDelegation < uint64_t(Link::countDelegationsFromWire(m_link))) {
+      m_selectedDelegationIndex = static_cast<size_t>(selectedDelegation);
+    }
+    else {
+      BOOST_THROW_EXCEPTION(Error("Invalid selected delegation index when decoding Interest"));
+    }
+  }
+  else {
+    m_selectedDelegationIndex = INVALID_SELECTED_DELEGATION_INDEX;
+  }
 }
 
+// ---- matching ----
+
 bool
 Interest::matchesName(const Name& name) const
 {
@@ -222,6 +311,51 @@
           this->getSelectors() == other.getSelectors());
 }
 
+// ---- field accessors ----
+
+uint32_t
+Interest::getNonce() const
+{
+  if (!m_nonce.hasWire())
+    const_cast<Interest*>(this)->setNonce(random::generateWord32());
+
+  if (m_nonce.value_size() == sizeof(uint32_t))
+    return *reinterpret_cast<const uint32_t*>(m_nonce.value());
+  else {
+    // for compatibility reasons.  Should be removed eventually
+    return readNonNegativeInteger(m_nonce);
+  }
+}
+
+Interest&
+Interest::setNonce(uint32_t nonce)
+{
+  if (m_wire.hasWire() && m_nonce.value_size() == sizeof(uint32_t)) {
+    std::memcpy(const_cast<uint8_t*>(m_nonce.value()), &nonce, sizeof(nonce));
+  }
+  else {
+    m_nonce = makeBinaryBlock(tlv::Nonce,
+                              reinterpret_cast<const uint8_t*>(&nonce),
+                              sizeof(nonce));
+    m_wire.reset();
+  }
+  return *this;
+}
+
+void
+Interest::refreshNonce()
+{
+  if (!hasNonce())
+    return;
+
+  uint32_t oldNonce = getNonce();
+  uint32_t newNonce = oldNonce;
+  while (newNonce == oldNonce)
+    newNonce = random::generateWord32();
+
+  setNonce(newNonce);
+}
+
 Interest&
 Interest::setInterestLifetime(time::milliseconds interestLifetime)
 {
@@ -233,151 +367,6 @@
   return *this;
 }
 
-template<encoding::Tag TAG>
-size_t
-Interest::wireEncode(EncodingImpl<TAG>& encoder) const
-{
-  size_t totalLength = 0;
-
-  // Interest ::= INTEREST-TYPE TLV-LENGTH
-  //                Name
-  //                Selectors?
-  //                Nonce
-  //                InterestLifetime?
-  //                Link?
-  //                SelectedDelegation?
-
-  // (reverse encoding)
-
-  if (hasLink()) {
-    if (hasSelectedDelegation()) {
-      totalLength += prependNonNegativeIntegerBlock(encoder,
-                                                    tlv::SelectedDelegation,
-                                                    m_selectedDelegationIndex);
-    }
-    totalLength += encoder.prependBlock(m_link);
-  }
-  else {
-    BOOST_ASSERT(!hasSelectedDelegation());
-  }
-
-  // InterestLifetime
-  if (getInterestLifetime() != DEFAULT_INTEREST_LIFETIME) {
-    totalLength += prependNonNegativeIntegerBlock(encoder,
-                                                  tlv::InterestLifetime,
-                                                  getInterestLifetime().count());
-  }
-
-  // Nonce
-  getNonce(); // to ensure that Nonce is properly set
-  totalLength += encoder.prependBlock(m_nonce);
-
-  // Selectors
-  if (hasSelectors()) {
-    totalLength += getSelectors().wireEncode(encoder);
-  }
-
-  // Name
-  totalLength += getName().wireEncode(encoder);
-
-  totalLength += encoder.prependVarNumber(totalLength);
-  totalLength += encoder.prependVarNumber(tlv::Interest);
-  return totalLength;
-}
-
-template size_t
-Interest::wireEncode<encoding::EncoderTag>(EncodingImpl<encoding::EncoderTag>& encoder) const;
-
-template size_t
-Interest::wireEncode<encoding::EstimatorTag>(EncodingImpl<encoding::EstimatorTag>& encoder) const;
-
-const Block&
-Interest::wireEncode() const
-{
-  if (m_wire.hasWire())
-    return m_wire;
-
-  EncodingEstimator estimator;
-  size_t estimatedSize = wireEncode(estimator);
-
-  EncodingBuffer buffer(estimatedSize, 0);
-  wireEncode(buffer);
-
-  // to ensure that Nonce block points to the right memory location
-  const_cast<Interest*>(this)->wireDecode(buffer.block());
-
-  return m_wire;
-}
-
-void
-Interest::wireDecode(const Block& wire)
-{
-  m_wire = wire;
-  m_wire.parse();
-
-  // Interest ::= INTEREST-TYPE TLV-LENGTH
-  //                Name
-  //                Selectors?
-  //                Nonce
-  //                InterestLifetime?
-  //                Link?
-  //                SelectedDelegation?
-
-  if (m_wire.type() != tlv::Interest)
-    BOOST_THROW_EXCEPTION(Error("Unexpected TLV number when decoding Interest"));
-
-  // Name
-  m_name.wireDecode(m_wire.get(tlv::Name));
-
-  // Selectors
-  Block::element_const_iterator val = m_wire.find(tlv::Selectors);
-  if (val != m_wire.elements_end()) {
-    m_selectors.wireDecode(*val);
-  }
-  else
-    m_selectors = Selectors();
-
-  // Nonce
-  m_nonce = m_wire.get(tlv::Nonce);
-
-  // InterestLifetime
-  val = m_wire.find(tlv::InterestLifetime);
-  if (val != m_wire.elements_end()) {
-    m_interestLifetime = time::milliseconds(readNonNegativeInteger(*val));
-  }
-  else {
-    m_interestLifetime = DEFAULT_INTEREST_LIFETIME;
-  }
-
-  // Link object
-  m_linkCached.reset();
-  val = m_wire.find(tlv::Data);
-  if (val != m_wire.elements_end()) {
-    m_link = (*val);
-  }
-  else {
-    m_link = Block();
-  }
-
-  // SelectedDelegation
-  val = m_wire.find(tlv::SelectedDelegation);
-  if (val != m_wire.elements_end()) {
-    if (!this->hasLink()) {
-      BOOST_THROW_EXCEPTION(Error("Interest contains SelectedDelegation, but no LINK object"));
-    }
-    uint64_t selectedDelegation = readNonNegativeInteger(*val);
-    if (selectedDelegation < uint64_t(Link::countDelegationsFromWire(m_link))) {
-      m_selectedDelegationIndex = static_cast<size_t>(selectedDelegation);
-    }
-    else {
-      BOOST_THROW_EXCEPTION(Error("Invalid selected delegation index when decoding Interest"));
-    }
-  }
-  else {
-    m_selectedDelegationIndex = INVALID_SELECTED_DELEGATION_INDEX;
-  }
-}
-
 bool
 Interest::hasLink() const
 {
@@ -462,6 +451,8 @@
   m_wire.reset();
 }
 
+// ---- operators ----
+
 std::ostream&
 operator<<(std::ostream& os, const Interest& interest)
 {
diff --git a/src/interest.hpp b/src/interest.hpp
index 78e6802..e9af4cf 100644
--- a/src/interest.hpp
+++ b/src/interest.hpp
@@ -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).
@@ -22,11 +22,11 @@
 #ifndef NDN_INTEREST_HPP
 #define NDN_INTEREST_HPP
 
+#include "link.hpp"
 #include "name.hpp"
 #include "selectors.hpp"
-#include "util/time.hpp"
 #include "tag-host.hpp"
-#include "link.hpp"
+#include "util/time.hpp"
 
 namespace ndn {
 
@@ -52,31 +52,17 @@
     }
   };
 
-  /** @brief Create a new Interest with an empty name (`ndn:/`)
-   *  @warning In certain contexts that use Interest::shared_from_this(), Interest must be created
-   *           using `make_shared`. Otherwise, .shared_from_this() will throw an exception.
-   */
-  Interest();
-
-  /** @brief Create a new Interest with the given name
-   *  @param name The name for the interest.
-   *  @note This constructor allows implicit conversion from Name.
-   *  @warning In certain contexts that use Interest::shared_from_this(), Interest must be created
-   *           using `make_shared`. Otherwise, .shared_from_this() will throw an exception.
-   */
-  Interest(const Name& name);
-
   /** @brief Create a new Interest with the given name and interest lifetime
-   *  @param name             The name for the interest.
-   *  @param interestLifetime The interest lifetime in time::milliseconds, or -1 for none.
+   *  @note This constructor allows implicit conversion from Name.
+   *  @throw std::invalid_argument InterestLifetime is negative
    *  @warning In certain contexts that use Interest::shared_from_this(), Interest must be created
-   *           using `make_shared`. Otherwise, .shared_from_this() will throw an exception.
+   *           using `make_shared`. Otherwise, .shared_from_this() will trigger undefined behavior.
    */
-  Interest(const Name& name, const time::milliseconds& interestLifetime);
+  Interest(const Name& name = Name(), time::milliseconds interestLifetime = DEFAULT_INTEREST_LIFETIME);
 
   /** @brief Create from wire encoding
    *  @warning In certain contexts that use Interest::shared_from_this(), Interest must be created
-   *           using `make_shared`. Otherwise, .shared_from_this() will throw an exception.
+   *           using `make_shared`. Otherwise, .shared_from_this() will trigger undefined behavior.
    */
   explicit
   Interest(const Block& wire);
@@ -118,77 +104,6 @@
   std::string
   toUri() const;
 
-public: // Link and forwarding hint
-  /**
-   * @brief Check whether the Interest contains a Link object
-   * @return True if there is a link object, otherwise false
-   */
-  bool
-  hasLink() const;
-
-  /**
-   * @brief Get the link object for this interest
-   * @return The link object if there is one contained in this interest
-   * @throws Interest::Error if there is no link object contained in the interest
-   * @throws tlv::Error if the incorporated link object is malformed
-   */
-  const Link&
-  getLink() const;
-
-  /**
-   * @brief Set the link object for this interest
-   * @param link The link object that will be included in this interest (in wire format)
-   * @post !hasSelectedDelegation()
-   */
-  void
-  setLink(const Block& link);
-
-  /**
-   * @brief Delete the link object for this interest
-   * @post !hasLink()
-   */
-  void
-  unsetLink();
-
-  /**
-   * @brief Check whether the Interest includes a selected delegation
-   * @return True if there is a selected delegation, otherwise false
-   */
-  bool
-  hasSelectedDelegation() const;
-
-  /**
-   * @brief Get the name of the selected delegation
-   * @return The name of the selected delegation
-   * @throw Error SelectedDelegation is not set.
-   */
-  Name
-  getSelectedDelegation() const;
-
-  /**
-   * @brief Set the selected delegation
-   * @param delegationName The name of the selected delegation
-   * @throw Error Link is not set.
-   * @throw std::invalid_argument @p delegationName does not exist in Link.
-   */
-  void
-  setSelectedDelegation(const Name& delegationName);
-
-  /**
-   * @brief Set the selected delegation
-   * @param delegationIndex The index of the selected delegation
-   * @throw Error Link is not set.
-   * @throw std::out_of_range @p delegationIndex is out of bound in Link.
-   */
-  void
-  setSelectedDelegation(size_t delegationIndex);
-
-   /**
-   * @brief Unset the selected delegation
-   */
-  void
-  unsetSelectedDelegation();
-
 public: // matching
   /** @brief Check if Interest, including selectors, matches the given @p name
    *  @param name The name to be matched. If this is a Data name, it shall contain the
@@ -219,7 +134,7 @@
   bool
   matchesInterest(const Interest& other) const;
 
-public: // Name and guiders
+public: // Name, Nonce, and Guiders
   const Name&
   getName() const
   {
@@ -234,19 +149,6 @@
     return *this;
   }
 
-  const time::milliseconds&
-  getInterestLifetime() const
-  {
-    return m_interestLifetime;
-  }
-
-  /**
-   * @brief Set Interest's lifetime
-   * @throw std::invalid_argument specified lifetime is < 0
-   */
-  Interest&
-  setInterestLifetime(time::milliseconds interestLifetime);
-
   /** @brief Check if Nonce set
    */
   bool
@@ -280,6 +182,19 @@
   void
   refreshNonce();
 
+  time::milliseconds
+  getInterestLifetime() const
+  {
+    return m_interestLifetime;
+  }
+
+  /**
+   * @brief Set Interest's lifetime
+   * @throw std::invalid_argument specified lifetime is < 0
+   */
+  Interest&
+  setInterestLifetime(time::milliseconds interestLifetime);
+
 public: // Selectors
   /**
    * @return true if Interest has any selector present
@@ -388,18 +303,76 @@
     return *this;
   }
 
-public: // EqualityComparable concept
+public: // Link and SelectedDelegation
+  /**
+   * @brief Check whether the Interest contains a Link object
+   * @return True if there is a link object, otherwise false
+   */
   bool
-  operator==(const Interest& other) const
-  {
-    return wireEncode() == other.wireEncode();
-  }
+  hasLink() const;
 
+  /**
+   * @brief Get the link object for this interest
+   * @return The link object if there is one contained in this interest
+   * @throws Interest::Error if there is no link object contained in the interest
+   * @throws tlv::Error if the incorporated link object is malformed
+   */
+  const Link&
+  getLink() const;
+
+  /**
+   * @brief Set the link object for this interest
+   * @param link The link object that will be included in this interest (in wire format)
+   * @post !hasSelectedDelegation()
+   */
+  void
+  setLink(const Block& link);
+
+  /**
+   * @brief Delete the link object for this interest
+   * @post !hasLink()
+   */
+  void
+  unsetLink();
+
+  /**
+   * @brief Check whether the Interest includes a selected delegation
+   * @return True if there is a selected delegation, otherwise false
+   */
   bool
-  operator!=(const Interest& other) const
-  {
-    return !(*this == other);
-  }
+  hasSelectedDelegation() const;
+
+  /**
+   * @brief Get the name of the selected delegation
+   * @return The name of the selected delegation
+   * @throw Error SelectedDelegation is not set.
+   */
+  Name
+  getSelectedDelegation() const;
+
+  /**
+   * @brief Set the selected delegation
+   * @param delegationName The name of the selected delegation
+   * @throw Error Link is not set.
+   * @throw std::invalid_argument @p delegationName does not exist in Link.
+   */
+  void
+  setSelectedDelegation(const Name& delegationName);
+
+  /**
+   * @brief Set the selected delegation
+   * @param delegationIndex The index of the selected delegation
+   * @throw Error Link is not set.
+   * @throw std::out_of_range @p delegationIndex is out of bound in Link.
+   */
+  void
+  setSelectedDelegation(size_t delegationIndex);
+
+  /**
+   * @brief Unset the selected delegation
+   */
+  void
+  unsetSelectedDelegation();
 
 private:
   Name m_name;
@@ -424,6 +397,18 @@
   return os.str();
 }
 
+inline bool
+operator==(const Interest& lhs, const Interest& rhs)
+{
+  return lhs.wireEncode() == rhs.wireEncode();
+}
+
+inline bool
+operator!=(const Interest& lhs, const Interest& rhs)
+{
+  return !(lhs == rhs);
+}
+
 } // namespace ndn
 
 #endif // NDN_INTEREST_HPP