fw: reorder function params to make the strategy API more uniform

Also add a non-const overload of Face::getCounters to avoid const_cast

Refs: #5173
Change-Id: Iff0bfbdedb90e68a373090cf3f247d9a7501f58d
diff --git a/daemon/fw/strategy.hpp b/daemon/fw/strategy.hpp
index 3c6536e..93a154b 100644
--- a/daemon/fw/strategy.hpp
+++ b/daemon/fw/strategy.hpp
@@ -32,7 +32,8 @@
 namespace nfd {
 namespace fw {
 
-/** \brief Represents a forwarding strategy
+/**
+ * \brief Represents a forwarding strategy
  */
 class Strategy : noncopyable
 {
@@ -115,169 +116,193 @@
   }
 
 public: // triggers
-  /** \brief Trigger after Interest is received
+  /**
+   * \brief Trigger after an Interest is received.
    *
-   *  The Interest:
+   * The Interest:
    *  - has not exceeded HopLimit
    *  - does not violate Scope
    *  - is not looped
    *  - cannot be satisfied by ContentStore
    *  - is under a namespace managed by this strategy
    *
-   *  The PIT entry is set to expire after InterestLifetime has elapsed at each downstream.
+   * The PIT entry is set to expire after InterestLifetime has elapsed at each downstream.
    *
-   *  The strategy should decide whether and where to forward this Interest.
+   * The strategy should decide whether and where to forward this Interest.
    *  - If the strategy decides to forward this Interest,
-   *    invoke \c sendInterest for each upstream, either now or shortly after via a scheduler event,
-   *    but before PIT entry expires.
-   *    Optionally, the strategy can invoke \c setExpiryTimer to adjust how long it would wait for a response.
-   *  - If the strategy has already forwarded this Interest previously and decides to continue waiting,
-   *    do nothing.
-   *    Optionally, the strategy can invoke \c setExpiryTimer to adjust how long it would wait for a response.
+   *    invoke sendInterest() for each upstream, either now or shortly after via a scheduler event,
+   *    but before the PIT entry expires.
+   *    Optionally, the strategy can invoke setExpiryTimer() to adjust how long it would wait for a response.
+   *  - If the strategy has already forwarded this Interest previously and decides to continue
+   *    waiting, do nothing.
+   *    Optionally, the strategy can invoke setExpiryTimer() to adjust how long it would wait for a response.
    *  - If the strategy concludes that this Interest cannot be satisfied,
-   *    invoke \c rejectPendingInterest to erase the PIT entry.
+   *    invoke rejectPendingInterest() to erase the PIT entry.
    *
-   *  \warning The strategy must not retain shared_ptr<pit::Entry>, otherwise undefined behavior
-   *           may occur. However, the strategy is allowed to store weak_ptr<pit::Entry>.
+   * \warning The strategy must not retain a copy of the \p pitEntry shared_ptr after this function
+   *          returns, otherwise undefined behavior may occur. However, the strategy is allowed to
+   *          construct and keep a weak_ptr to \p pitEntry.
    */
   virtual void
-  afterReceiveInterest(const FaceEndpoint& ingress, const Interest& interest,
+  afterReceiveInterest(const Interest& interest, const FaceEndpoint& ingress,
                        const shared_ptr<pit::Entry>& pitEntry) = 0;
 
-  /** \brief Trigger before PIT entry is satisfied
+  /**
+   * \brief Trigger after a matching Data is found in the Content Store.
    *
-   *  This trigger is invoked when an incoming Data satisfies more than one PIT entry.
-   *  The strategy can collect measurements information, but cannot manipulate Data forwarding.
-   *  When an incoming Data satisfies only one PIT entry, \c afterReceiveData is invoked instead
-   *  and given full control over Data forwarding. If a strategy does not override \c afterReceiveData,
-   *  the default implementation invokes \c beforeSatisfyInterest.
+   * In the base class, this method sends \p data to \p ingress.
    *
-   *  Normally, PIT entries would be erased after receiving the first matching Data.
-   *  If the strategy wishes to collect responses from additional upstream nodes,
-   *  it should invoke \c setExpiryTimer within this function to prolong the PIT entry lifetime.
-   *  If a Data arrives from another upstream during the extended PIT entry lifetime, this trigger will be invoked again.
-   *  At that time, this function must invoke \c setExpiryTimer again to continue collecting more responses.
-   *
-   *  In this base class this method does nothing.
-   *
-   *  \warning The strategy must not retain shared_ptr<pit::Entry>, otherwise undefined behavior
-   *           may occur. However, the strategy is allowed to store weak_ptr<pit::Entry>.
+   * \warning The strategy must not retain a copy of the \p pitEntry shared_ptr after this function
+   *          returns, otherwise undefined behavior may occur. However, the strategy is allowed to
+   *          construct and keep a weak_ptr to \p pitEntry.
    */
   virtual void
-  beforeSatisfyInterest(const shared_ptr<pit::Entry>& pitEntry,
-                        const FaceEndpoint& ingress, const Data& data);
+  afterContentStoreHit(const Data& data, const FaceEndpoint& ingress,
+                       const shared_ptr<pit::Entry>& pitEntry);
 
-  /** \brief Trigger after a Data is matched in CS
+  /**
+   * \brief Trigger before a PIT entry is satisfied.
    *
-   *  In the base class this method sends \p data to \p ingress
+   * This trigger is invoked when an incoming Data satisfies more than one PIT entry.
+   * The strategy can collect measurements information, but cannot manipulate Data forwarding.
+   * When an incoming Data satisfies only one PIT entry, afterReceiveData() is invoked instead
+   * and given full control over Data forwarding. If a strategy does not override afterReceiveData(),
+   * the default implementation invokes beforeSatisfyInterest().
+   *
+   * Normally, PIT entries are erased after receiving the first matching Data.
+   * If the strategy wishes to collect responses from additional upstream nodes,
+   * it should invoke setExpiryTimer() within this function to prolong the PIT entry lifetime.
+   * If a Data arrives from another upstream during the extended PIT entry lifetime, this trigger
+   * will be invoked again. At that time, the strategy must invoke setExpiryTimer() again to
+   * continue collecting more responses.
+   *
+   * In the base class, this method does nothing.
+   *
+   * \warning The strategy must not retain a copy of the \p pitEntry shared_ptr after this function
+   *          returns, otherwise undefined behavior may occur. However, the strategy is allowed to
+   *          construct and keep a weak_ptr to \p pitEntry.
    */
   virtual void
-  afterContentStoreHit(const shared_ptr<pit::Entry>& pitEntry,
-                       const FaceEndpoint& ingress, const Data& data);
+  beforeSatisfyInterest(const Data& data, const FaceEndpoint& ingress,
+                        const shared_ptr<pit::Entry>& pitEntry);
 
-  /** \brief Trigger after Data is received
+  /**
+   * \brief Trigger after Data is received.
    *
-   *  This trigger is invoked when an incoming Data satisfies exactly one PIT entry,
-   *  and gives the strategy full control over Data forwarding.
+   * This trigger is invoked when an incoming Data satisfies exactly one PIT entry,
+   * and gives the strategy full control over Data forwarding.
    *
-   *  When this trigger is invoked:
+   * When this trigger is invoked:
    *  - The Data has been verified to satisfy the PIT entry.
    *  - The PIT entry expiry timer is set to now
    *
-   *  Within this function:
-   *  - A strategy should return Data to downstream nodes via \c sendData or \c sendDataToAll.
+   * Inside this function:
+   *  - A strategy should return Data to downstream nodes via sendData() or sendDataToAll().
    *  - A strategy can modify the Data as long as it still satisfies the PIT entry, such as
    *    adding or removing congestion marks.
-   *  - A strategy can delay Data forwarding by prolonging the PIT entry lifetime via \c setExpiryTimer,
-   *    and forward Data before the PIT entry is erased.
+   *  - A strategy can delay Data forwarding by prolonging the PIT entry lifetime via setExpiryTimer(),
+   *    and later forward the Data before the PIT entry is erased.
    *  - A strategy can collect measurements about the upstream.
    *  - A strategy can collect responses from additional upstream nodes by prolonging the PIT entry
-   *    lifetime via \c setExpiryTimer every time a Data is received. Note that only one Data should
+   *    lifetime via setExpiryTimer() every time a Data is received. Note that only one Data should
    *    be returned to each downstream node.
    *
-   *  In the base class this method invokes \c beforeSatisfyInterest trigger and then returns
-   *  the Data to downstream faces via \c sendDataToAll.
+   * In the base class, this method invokes beforeSatisfyInterest() and then returns the Data
+   * to all downstream faces via sendDataToAll().
+   *
+   * \warning The strategy must not retain a copy of the \p pitEntry shared_ptr after this function
+   *          returns, otherwise undefined behavior may occur. However, the strategy is allowed to
+   *          construct and keep a weak_ptr to \p pitEntry.
    */
   virtual void
-  afterReceiveData(const shared_ptr<pit::Entry>& pitEntry,
-                   const FaceEndpoint& ingress, const Data& data);
-
-  /** \brief Trigger after Nack is received
-   *
-   *  This trigger is invoked when an incoming Nack is received in response to
-   *  an forwarded Interest.
-   *  The Nack has been confirmed to be a response to the last Interest forwarded
-   *  to that upstream, i.e. the PIT out-record exists and has a matching Nonce.
-   *  The NackHeader has been recorded in the PIT out-record.
-   *
-   *  If the PIT entry is not yet satisfied, its expiry timer remains unchanged.
-   *  Otherwise, the PIT entry normally would expire immediately after this function returns.
-   *
-   *  If the strategy wishes to collect responses from additional upstream nodes,
-   *  it should invoke \c setExpiryTimer within this function to retain the PIT entry.
-   *  If a Nack arrives from another upstream during the extended PIT entry lifetime, this trigger will be invoked again.
-   *  At that time, this function must invoke \c setExpiryTimer again to continue collecting more responses.
-   *
-   *  In the base class this method does nothing.
-   *
-   *  \warning The strategy must not retain shared_ptr<pit::Entry>, otherwise undefined behavior
-   *           may occur. However, the strategy is allowed to store weak_ptr<pit::Entry>.
-   */
-  virtual void
-  afterReceiveNack(const FaceEndpoint& ingress, const lp::Nack& nack,
+  afterReceiveData(const Data& data, const FaceEndpoint& ingress,
                    const shared_ptr<pit::Entry>& pitEntry);
 
-  /** \brief Trigger after Interest dropped (e.g., for exceeding allowed retransmissions)
+  /**
+   * \brief Trigger after a Nack is received.
    *
-   *  In the base class this method does nothing.
+   * This trigger is invoked when an incoming Nack is received in response to
+   * an forwarded Interest.
+   * The Nack has been confirmed to be a response to the last Interest forwarded
+   * to that upstream, i.e. the PIT out-record exists and has a matching Nonce.
+   * The NackHeader has been recorded in the PIT out-record.
+   *
+   * If the PIT entry is not yet satisfied, its expiry timer remains unchanged.
+   * Otherwise, the PIT entry will normally expire immediately after this function returns.
+   *
+   * If the strategy wishes to collect responses from additional upstream nodes,
+   * it should invoke setExpiryTimer() within this function to prolong the PIT entry lifetime.
+   * If a Nack arrives from another upstream during the extended PIT entry lifetime, this trigger
+   * will be invoked again. At that time, the strategy must invoke setExpiryTimer() again to
+   * continue collecting more responses.
+   *
+   * In the base class, this method does nothing.
+   *
+   * \warning The strategy must not retain a copy of the \p pitEntry shared_ptr after this function
+   *          returns, otherwise undefined behavior may occur. However, the strategy is allowed to
+   *          construct and keep a weak_ptr to \p pitEntry.
    */
   virtual void
-  onDroppedInterest(const Face& egress, const Interest& interest);
+  afterReceiveNack(const lp::Nack& nack, const FaceEndpoint& ingress,
+                   const shared_ptr<pit::Entry>& pitEntry);
 
-  /** \brief Trigger after new nexthop is added
+  /**
+   * \brief Trigger after an Interest is dropped (e.g., for exceeding allowed retransmissions).
    *
-   *  The strategy should decide whether to send the buffered Interests to the new nexthop.
-   *  In the base class, this method does nothing.
+   * In the base class, this method does nothing.
+   */
+  virtual void
+  onDroppedInterest(const Interest& interest, Face& egress);
+
+  /**
+   * \brief Trigger after a new nexthop is added.
+   *
+   * The strategy should decide whether to send the buffered Interests to the new nexthop.
+   *
+   * In the base class, this method does nothing.
    */
   virtual void
   afterNewNextHop(const fib::NextHop& nextHop, const shared_ptr<pit::Entry>& pitEntry);
 
 protected: // actions
-  /** \brief Send an Interest packet.
-   *  \param pitEntry the PIT entry
-   *  \param egress face through which to send out the Interest
-   *  \param interest the Interest packet
-   *  \return A pointer to the out-record created or nullptr if the Interest was dropped
+  /**
+   * \brief Send an Interest packet.
+   * \param interest the Interest packet
+   * \param egress face through which to send out the Interest
+   * \param pitEntry the PIT entry
+   * \return A pointer to the out-record created or nullptr if the Interest was dropped
    */
   NFD_VIRTUAL_WITH_TESTS pit::OutRecord*
-  sendInterest(const shared_ptr<pit::Entry>& pitEntry, Face& egress,
-               const Interest& interest);
+  sendInterest(const Interest& interest, Face& egress, const shared_ptr<pit::Entry>& pitEntry);
 
-  /** \brief Send a Data packet.
-   *  \param pitEntry the PIT entry
-   *  \param data the Data packet
-   *  \param egress face through which to send out the Data
-   *  \return Whether the Data was sent (true) or dropped (false)
+  /**
+   * \brief Send a Data packet.
+   * \param data the Data packet
+   * \param egress face through which to send out the Data
+   * \param pitEntry the PIT entry
+   * \return Whether the Data was sent (true) or dropped (false)
    */
   NFD_VIRTUAL_WITH_TESTS bool
-  sendData(const shared_ptr<pit::Entry>& pitEntry, const Data& data, Face& egress);
+  sendData(const Data& data, Face& egress, const shared_ptr<pit::Entry>& pitEntry);
 
-  /** \brief Send a Data packet to all matched and qualified faces.
+  /**
+   * \brief Send a Data packet to all matched and qualified faces.
    *
-   *  A matched face is qualified if it is ad-hoc or it is NOT \p inFace.
+   * A matched face qualifies if it is ad-hoc OR it is NOT \p inFace.
    *
-   *  \param pitEntry the PIT entry
-   *  \param inFace face on which the Data arrived
-   *  \param data the Data packet
+   * \param data the Data packet
+   * \param pitEntry the PIT entry
+   * \param inFace face on which the Data arrived
    */
   NFD_VIRTUAL_WITH_TESTS void
-  sendDataToAll(const shared_ptr<pit::Entry>& pitEntry, const Face& inFace, const Data& data);
+  sendDataToAll(const Data& data, const shared_ptr<pit::Entry>& pitEntry, const Face& inFace);
 
-  /** \brief Schedule the PIT entry for immediate deletion.
+  /**
+   * \brief Schedule the PIT entry for immediate deletion.
    *
-   *  This helper function sets the PIT entry expiry time to zero.
-   *  The strategy should invoke this function when it concludes that the Interest cannot
-   *  be forwarded and it does not want to wait for responses from existing upstream nodes.
+   * This helper function sets the PIT entry expiry time to zero.
+   * The strategy should invoke this function when it concludes that the Interest cannot
+   * be forwarded and it does not want to wait for responses from existing upstream nodes.
    */
   NFD_VIRTUAL_WITH_TESTS void
   rejectPendingInterest(const shared_ptr<pit::Entry>& pitEntry)
@@ -285,33 +310,35 @@
     this->setExpiryTimer(pitEntry, 0_ms);
   }
 
-  /** \brief Send a Nack packet.
+  /**
+   * \brief Send a Nack packet.
    *
-   *  The egress face must have a PIT in-record, otherwise this method has no effect.
+   * The egress face must have a PIT in-record, otherwise this method has no effect.
    *
-   *  \param pitEntry the PIT entry
-   *  \param egress face through which to send out the Nack
-   *  \param header the Nack header
-   *  \return Whether the Nack was sent (true) or dropped (false)
+   * \param header the Nack header
+   * \param egress face through which to send out the Nack
+   * \param pitEntry the PIT entry
+   * \return Whether the Nack was sent (true) or dropped (false)
    */
   NFD_VIRTUAL_WITH_TESTS bool
-  sendNack(const shared_ptr<pit::Entry>& pitEntry, Face& egress,
-           const lp::NackHeader& header)
+  sendNack(const lp::NackHeader& header, Face& egress, const shared_ptr<pit::Entry>& pitEntry)
   {
-    return m_forwarder.onOutgoingNack(pitEntry, egress, header);
+    return m_forwarder.onOutgoingNack(header, egress, pitEntry);
   }
 
-  /** \brief Send Nack to every face that has an in-record, except those in \p exceptFaces
-   *  \param pitEntry the PIT entry
-   *  \param header the Nack header
-   *  \param exceptFaces list of faces that should be excluded from sending Nacks
-   *  \note This is not an action, but a helper that invokes the sendNack() action.
+  /**
+   * \brief Send Nack to every face that has an in-record, except those in \p exceptFaces
+   * \param header the Nack header
+   * \param pitEntry the PIT entry
+   * \param exceptFaces list of faces that should be excluded from sending Nacks
+   * \note This is not an action, but a helper that invokes the sendNack() action.
    */
   void
-  sendNacks(const shared_ptr<pit::Entry>& pitEntry, const lp::NackHeader& header,
+  sendNacks(const lp::NackHeader& header, const shared_ptr<pit::Entry>& pitEntry,
             std::initializer_list<const Face*> exceptFaces = {});
 
-  /** \brief Schedule the PIT entry to be erased after \p duration.
+  /**
+   * \brief Schedule the PIT entry to be erased after \p duration.
    */
   void
   setExpiryTimer(const shared_ptr<pit::Entry>& pitEntry, time::milliseconds duration)
@@ -320,7 +347,8 @@
   }
 
 protected: // accessors
-  /** \brief Performs a FIB lookup, considering Link object if present.
+  /**
+   * \brief Performs a FIB lookup, considering Link object if present.
    */
   const fib::Entry&
   lookupFib(const pit::Entry& pitEntry) const;
@@ -381,8 +409,8 @@
   }
 
 private: // registry
-  typedef std::function<unique_ptr<Strategy>(Forwarder& forwarder, const Name& strategyName)> CreateFunc;
-  typedef std::map<Name, CreateFunc> Registry; // indexed by strategy name
+  using CreateFunc = std::function<unique_ptr<Strategy>(Forwarder&, const Name& /*strategyName*/)>;
+  using Registry = std::map<Name, CreateFunc>; // indexed by strategy name
 
   static Registry&
   getRegistry();
@@ -396,13 +424,7 @@
 
 private: // instance fields
   Name m_name;
-
-  /** \brief Reference to the forwarder
-   *
-   *  Triggers can access forwarder indirectly via actions.
-   */
   Forwarder& m_forwarder;
-
   MeasurementsAccessor m_measurements;
 };