commit 3206565bac1de454bfff515c03743ad896a1a9f2
author Davide Pesavento <davide.pesavento@lip6.fr> Sun Jan 15 01:52:21 2017 -0500
committer Davide Pesavento <davide.pesavento@lip6.fr> Sun Jan 15 15:04:19 2017 -0500
tree dda9edbd33545cd740fa22eda61667eb2c23a309
parent 32dab97321d954800f70e4a695e326c998d37a93

face: revert unintended behavior change in Transport::setPersistency

Prior to commit 32dab97321d954800f70e4a695e326c998d37a93, persistency
transitions from NONE were not logged and before/afterChangePersistency
was not called. Restore that behavior.

This commit also improves test coverage of the persistency-related
functions in the various Transport subclasses.

Change-Id: Ide77c67ea277fca3d1cad5ea131ae0fa259db75c
Refs: #3232

daemon/face/transport.cpp

diff --git a/daemon/face/transport.cpp b/daemon/face/transport.cpp
index 81f6edf..9c054fa 100644
--- a/daemon/face/transport.cpp
+++ b/daemon/face/transport.cpp
@@ -68,9 +68,7 @@
 {
 }
 
-Transport::~Transport()
-{
-}
+Transport::~Transport() = default;
 
 void
 Transport::setFaceAndLinkService(Face& face, LinkService& service)
@@ -91,7 +89,7 @@
 
   this->setState(TransportState::CLOSING);
   this->doClose();
-  // warning: don't access any fields after this:
+  // warning: don't access any members after this:
   // the Transport may be deallocated if doClose changes state to CLOSED
 }
 
@@ -158,11 +156,13 @@
     return;
   }
 
-  NFD_LOG_FACE_INFO("setPersistency " << m_persistency << " -> " << newPersistency);
-
   auto oldPersistency = m_persistency;
   m_persistency = newPersistency;
-  this->afterChangePersistency(oldPersistency);
+
+  if (oldPersistency != ndn::nfd::FACE_PERSISTENCY_NONE) {
+    NFD_LOG_FACE_INFO("setPersistency " << oldPersistency << " -> " << newPersistency);
+    this->afterChangePersistency(oldPersistency);
+  }
 }
 
 void
@@ -198,7 +198,7 @@
   }
 
   if (!isValid) {
-    throw std::runtime_error("invalid state transition");
+    BOOST_THROW_EXCEPTION(std::runtime_error("invalid state transition"));
   }
 
   NFD_LOG_FACE_INFO("setState " << m_state << " -> " << newState);
@@ -206,7 +206,7 @@
   TransportState oldState = m_state;
   m_state = newState;
   afterStateChange(oldState, newState);
-  // warning: don't access any fields after this:
+  // warning: don't access any members after this:
   // the Transport may be deallocated in the signal handler if newState is CLOSED
 }
 

daemon/face/transport.hpp

diff --git a/daemon/face/transport.hpp b/daemon/face/transport.hpp
index 3db4714..ff5363f 100644
--- a/daemon/face/transport.hpp
+++ b/daemon/face/transport.hpp
@@ -221,16 +221,13 @@
   ndn::nfd::FacePersistency
   getPersistency() const;
 
-  /** \brief check whether the intended change from the current persistency to \p newPersistency
-   *  can be performed
+  /** \brief check whether the face persistency can be changed to \p newPersistency
    *
-   *  This function serves as an external API, and invokes the internal function
-   *  canChangePersistencyToImpl to perform further checks if \p newPersistency differs from
-   *  the current persistency.
+   *  This function serves as the external API, and invokes the protected function
+   *  canChangePersistencyToImpl to perform further checks if \p newPersistency differs
+   *  from the current persistency.
    *
-   *  \pre getPersistency() != NONE
-   *
-   *  \return true if the intended change can be performed, otherwise false
+   *  \return true if the change can be performed, false otherwise
    */
   bool
   canChangePersistencyTo(ndn::nfd::FacePersistency newPersistency) const;
@@ -238,7 +235,7 @@
   /** \brief changes face persistency setting
    */
   void
-  setPersistency(ndn::nfd::FacePersistency persistency);
+  setPersistency(ndn::nfd::FacePersistency newPersistency);
 
   /** \return whether face is point-to-point or multi-access
    */