refactor peerConnectTimeout to CachePeer::connectTimeout (#664)

Resolve a TODO in the code.

diff --git a/src/CachePeer.cc b/src/CachePeer.cc
index fd1a4face5d6da89ccae9df2c2f5a48c3ea97bb9..3409f249057861a071872ed05e762b367b8c2578 100644 (file)
--- a/src/CachePeer.cc
+++ b/src/CachePeer.cc
@@ -13,6 +13,7 @@
 #include "NeighborTypeDomainList.h"
 #include "pconn.h"
 #include "PeerPoolMgr.h"
+#include "SquidConfig.h"
 
 CBDATA_CLASS_INIT(CachePeer);
 
@@ -46,3 +47,11 @@ CachePeer::~CachePeer()
     xfree(domain);
 }
 
+time_t
+CachePeer::connectTimeout() const
+{
+    if (connect_timeout_raw > 0)
+        return connect_timeout_raw;
+    return Config.Timeout.peer_connect;
+}
+

diff --git a/src/CachePeer.h b/src/CachePeer.h
index c7af56e593c1b5e1db1dc7f42bcb48999cf2b47b..b86b18f962122aef373cf7a73edb161a17fd6eae 100644 (file)
--- a/src/CachePeer.h
+++ b/src/CachePeer.h
@@ -32,6 +32,9 @@ public:
     CachePeer() = default;
     ~CachePeer();
 
+    /// \returns the effective connect timeout for the given peer
+    time_t connectTimeout() const;
+
     u_int index = 0;
     char *name = nullptr;
     char *host = nullptr;
@@ -169,7 +172,7 @@ public:
     } sourcehash;
 
     char *login = nullptr;        /* Proxy authorization */
-    time_t connect_timeout_raw = 0; ///< connect_timeout; use peerConnectTimeout() instead!
+    time_t connect_timeout_raw = 0; ///< connect_timeout; use connectTimeout() instead!
     int connect_fail_limit = 0;
     int max_conn = 0;
 
@@ -193,4 +196,3 @@ public:
 };
 
 #endif /* SQUID_CACHEPEER_H_ */
-

diff --git a/src/Makefile.am b/src/Makefile.am
index 508657e2d0e9b8437701eb85d4d7a3cc1cfa4cda..89748f926319786ebf83a66ce6765635f00118c5 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1107,6 +1107,7 @@ check_PROGRAMS += tests/testRock
 tests_testRock_SOURCES = \
        AccessLogEntry.cc \
        AccessLogEntry.h \
+       tests/stub_CachePeer.cc \
        cbdata.cc \
        CacheDigest.h \
        CollapsedForwarding.h \
@@ -1279,6 +1280,7 @@ tests_testUfs_SOURCES = \
        AccessLogEntry.h \
        tests/testUfs.cc \
        tests/testUfs.h \
+       tests/stub_CachePeer.cc \
        tests/stub_cache_manager.cc \
        tests/stub_client_db.cc \
        tests/stub_CollapsedForwarding.cc \
@@ -1622,6 +1624,7 @@ tests_testDiskIO_SOURCES = \
        AccessLogEntry.h \
        CacheDigest.h \
        tests/stub_CacheDigest.cc \
+       tests/stub_CachePeer.cc \
        cbdata.cc \
        client_db.h \
        ClientInfo.h \

diff --git a/src/PeerPoolMgr.cc b/src/PeerPoolMgr.cc
index d69c65bcb5ca6db80d8f6362c97a09b0672f245f..4d5728f4d793422d49a1c14f4a5317b05eb8daf2 100644 (file)
--- a/src/PeerPoolMgr.cc
+++ b/src/PeerPoolMgr.cc
@@ -120,7 +120,7 @@ PeerPoolMgr::handleOpenedConnection(const CommConnectCbParams &params)
         securer = asyncCall(48, 4, "PeerPoolMgr::handleSecuredPeer",
                             MyAnswerDialer(this, &PeerPoolMgr::handleSecuredPeer));
 
-        const int peerTimeout = peerConnectTimeout(peer);
+        const auto peerTimeout = peer->connectTimeout();
         const int timeUsed = squid_curtime - params.conn->startTime();
         // Use positive timeout when less than one second is left for conn.
         const int timeLeft = positiveTimeout(peerTimeout - timeUsed);
@@ -225,7 +225,7 @@ PeerPoolMgr::openNewConnection()
     getOutgoingAddress(request.getRaw(), conn);
     GetMarkingsToServer(request.getRaw(), *conn);
 
-    const int ctimeout = peerConnectTimeout(peer);
+    const auto ctimeout = peer->connectTimeout();
     typedef CommCbMemFunT<PeerPoolMgr, CommConnectCbParams> Dialer;
     opener = JobCallback(48, 5, Dialer, this, PeerPoolMgr::handleOpenedConnection);
     Comm::ConnOpener *cs = new Comm::ConnOpener(conn, opener, ctimeout);

diff --git a/src/comm/Connection.cc b/src/comm/Connection.cc
index 301888e54c0d7b8cef70c3d828c3ed893fa382af..c074b39c88e4e63fd36251696418266bc3a09bdb 100644 (file)
--- a/src/comm/Connection.cc
+++ b/src/comm/Connection.cc
@@ -145,7 +145,7 @@ Comm::Connection::connectTimeout(const time_t fwdStart) const
 {
     // a connection opening timeout (ignoring forwarding time limits for now)
     const CachePeer *peer = getPeer();
-    const time_t ctimeout = peer ? peerConnectTimeout(peer) : Config.Timeout.connect;
+    const auto ctimeout = peer ? peer->connectTimeout() : Config.Timeout.connect;
 
     // time we have left to finish the whole forwarding process
     const time_t fwdTimeLeft = FwdState::ForwardTimeout(fwdStart);

diff --git a/src/neighbors.cc b/src/neighbors.cc
index 474c23edf4d7a909a9324ee3edf10c9f4973219c..aa4b7d782782bb6b3af31373dc73a125be28963b 100644 (file)
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -1183,14 +1183,6 @@ neighborUp(const CachePeer * p)
     return 1;
 }
 
-/// \returns the effective connect timeout for this peer
-time_t
-peerConnectTimeout(const CachePeer *peer)
-{
-    return peer->connect_timeout_raw > 0 ?
-           peer->connect_timeout_raw : Config.Timeout.peer_connect;
-}
-
 time_t
 positiveTimeout(const time_t timeout)
 {
@@ -1340,7 +1332,7 @@ peerProbeConnect(CachePeer *p, const bool reprobeIfBusy)
     }
     p->reprobe = false;
 
-    const time_t ctimeout = peerConnectTimeout(p);
+    const auto ctimeout = p->connectTimeout();
     /* for each IP address of this CachePeer. find one that we can connect to and probe it. */
     for (int i = 0; i < p->n_addresses; ++i) {
         Comm::ConnectionPointer conn = new Comm::Connection;

diff --git a/src/neighbors.h b/src/neighbors.h
index 5fccc4880f58ded9dbeac19da12187e85ac71822..34e9f6ac9d1c829642725e815363dfd4cf7b6082 100644 (file)
--- a/src/neighbors.h
+++ b/src/neighbors.h
@@ -61,10 +61,6 @@ void peerConnectSucceded(CachePeer *);
 void dump_peer_options(StoreEntry *, CachePeer *);
 int peerHTTPOkay(const CachePeer *, PeerSelector *);
 
-// TODO: Consider moving this method to CachePeer class.
-/// \returns the effective connect timeout for the given peer
-time_t peerConnectTimeout(const CachePeer *peer);
-
 /// \returns max(1, timeout)
 time_t positiveTimeout(const time_t timeout);
 

diff --git a/src/tests/stub_CachePeer.cc b/src/tests/stub_CachePeer.cc
new file mode 100644 (file)
index 0000000..bd351dd
--- /dev/null
+++ b/src/tests/stub_CachePeer.cc
@@ -0,0 +1,16 @@
+/*
+ * Copyright (C) 1996-2020 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#include "squid.h"
+
+#define STUB_API "CachePeer.cc"
+#include "tests/STUB.h"
+
+#include "CachePeer.h"
+
+time_t CachePeer::connectTimeout() const STUB_RETVAL(0)

diff --git a/src/tests/stub_neighbors.cc b/src/tests/stub_neighbors.cc
index d5af2003289ec632ea8ece40574af215fc60da7b..bf7b78186232908e3566737b43cf11762b77965a 100644 (file)
--- a/src/tests/stub_neighbors.cc
+++ b/src/tests/stub_neighbors.cc
@@ -17,8 +17,6 @@
 void
 peerConnClosed(CachePeer *p) STUB
 
-time_t
-peerConnectTimeout(const CachePeer *peer) STUB_RETVAL(0)
 time_t
 FwdState::ForwardTimeout(const time_t) STUB_RETVAL(0)
 bool