diff --git a/libtransmission/dns-ev.h b/libtransmission/dns-ev.h index 041e84b0d..29275104f 100644 --- a/libtransmission/dns-ev.h +++ b/libtransmission/dns-ev.h @@ -86,22 +86,24 @@ public: } } + std::optional> cached(std::string_view address, Hints hints = {}) const override + { + if (auto const* entry = cached(makeKey(address, hints)); entry != nullptr) + { + return std::make_pair(reinterpret_cast(&entry->ss_), entry->sslen_); + } + + return {}; + } + Tag lookup(std::string_view address, Callback&& callback, Hints hints = {}) override { - auto const key = std::make_pair(tr_strlower(address), hints); - auto const now = time_func_(); + auto const key = makeKey(address, hints); - if (auto iter = cache_.find(key); iter != std::end(cache_)) + if (auto const* entry = cached(key); entry) { - auto const& entry = iter->second; - - if (entry.expires_at_ > now) - { - callback(reinterpret_cast(&entry.ss_), entry.sslen_); - return {}; - } - - cache_.erase(iter); // expired + callback(reinterpret_cast(&entry->ss_), entry->sslen_, entry->expires_at_); + return {}; } auto& request = requests_[key]; @@ -131,7 +133,7 @@ public: continue; } - iter->callback_(nullptr, 0); + iter->callback_(nullptr, 0, 0); request.callbacks.erase(iter); @@ -148,6 +150,28 @@ public: } private: + [[nodiscard]] static Key makeKey(std::string_view address, Hints hints) + { + return Key{ tr_strlower(address), hints }; + } + + [[nodiscard]] CacheEntry const* cached(Key const& key) const + { + if (auto iter = cache_.find(key); iter != std::end(cache_)) + { + auto const& entry = iter->second; + + if (auto const now = time_func_(); entry.expires_at_ > now) + { + return &entry; + } + + cache_.erase(iter); // expired + } + + return nullptr; + } + static void evcallback(int /*result*/, struct evutil_addrinfo* res, void* varg) { auto* const arg = static_cast(varg); @@ -168,7 +192,10 @@ private: { for (auto& callback : request_entry.mapped().callbacks) { - callback.callback_(reinterpret_cast(&cache_entry.ss_), cache_entry.sslen_); + callback.callback_( + reinterpret_cast(&cache_entry.ss_), + cache_entry.sslen_, + cache_entry.expires_at_); } } } @@ -176,7 +203,7 @@ private: TimeFunc const time_func_; static time_t constexpr CacheTtlSecs = 3600U; std::unique_ptr const evdns_base_; - std::map cache_; + mutable std::map cache_; std::map requests_; unsigned int next_tag_ = 1; }; diff --git a/libtransmission/dns.h b/libtransmission/dns.h index 33f49e053..5398025c0 100644 --- a/libtransmission/dns.h +++ b/libtransmission/dns.h @@ -20,7 +20,7 @@ class Dns public: virtual ~Dns() = default; - using Callback = std::function; + using Callback = std::function; using Tag = unsigned int; class Hints @@ -60,6 +60,10 @@ public: } }; + [[nodiscard]] virtual std::optional> cached( + std::string_view address, + Hints hints = {}) const = 0; + virtual Tag lookup(std::string_view address, Callback&& callback, Hints hints = {}) = 0; virtual void cancel(Tag) = 0; diff --git a/tests/libtransmission/dns-test.cc b/tests/libtransmission/dns-test.cc index 965c2bc9a..fe5894715 100644 --- a/tests/libtransmission/dns-test.cc +++ b/tests/libtransmission/dns-test.cc @@ -41,7 +41,10 @@ protected: ::testing::Test::TearDown(); } - bool waitFor(struct event_base* evb, std::function const& test, std::chrono::milliseconds msec = DefaultTimeout) + static bool waitFor( + struct event_base* evb, + std::function const& test, + std::chrono::milliseconds msec = DefaultTimeout) { auto const deadline = std::chrono::steady_clock::now() + msec; @@ -72,10 +75,11 @@ TEST_F(EvDnsTest, canLookup) dns.lookup( "example.com", - [&done](struct sockaddr const* ai, socklen_t ailen) + [&done](struct sockaddr const* ai, socklen_t ailen, time_t expires_at) { EXPECT_NE(nullptr, ai); EXPECT_GT(ailen, 0); + EXPECT_GT(expires_at, tr_time()); done = true; }); @@ -90,19 +94,21 @@ TEST_F(EvDnsTest, canRequestWhilePending) dns.lookup( "example.com", - [&n_done](struct sockaddr const* ai, socklen_t ailen) + [&n_done](struct sockaddr const* ai, socklen_t ailen, time_t expires_at) { EXPECT_NE(nullptr, ai); EXPECT_GT(ailen, 0); + EXPECT_GT(expires_at, tr_time()); ++n_done; }); dns.lookup( "example.com", - [&n_done](struct sockaddr const* ai, socklen_t ailen) + [&n_done](struct sockaddr const* ai, socklen_t ailen, time_t expires_at) { EXPECT_NE(nullptr, ai); EXPECT_GT(ailen, 0); + EXPECT_GT(expires_at, tr_time()); ++n_done; }); @@ -115,26 +121,29 @@ TEST_F(EvDnsTest, canCancel) { auto dns = EvDns{ event_base_, tr_time }; auto n_done = size_t{ 0 }; + static auto constexpr Name = "example.com"sv; auto tag = dns.lookup( - "example.com", - [&n_done](struct sockaddr const* ai, socklen_t ailen) + Name, + [&n_done](struct sockaddr const* ai, socklen_t ailen, time_t expires_at) { ++n_done; // we cancelled this req, so `ai` and `ailen` should be zeroed out EXPECT_EQ(nullptr, ai); EXPECT_EQ(0, ailen); + EXPECT_EQ(0, expires_at); }); dns.lookup( - "example.com", - [&n_done](struct sockaddr const* ai, socklen_t ailen) + Name, + [&n_done](struct sockaddr const* ai, socklen_t ailen, time_t expires_at) { ++n_done; // this one did _not_ get cancelled so it should be OK EXPECT_NE(nullptr, ai); EXPECT_GT(ailen, 0); + EXPECT_GT(expires_at, tr_time()); }); dns.cancel(tag); @@ -147,15 +156,17 @@ TEST_F(EvDnsTest, canCancel) TEST_F(EvDnsTest, doesCacheEntries) { auto dns = EvDns{ event_base_, tr_time }; + static auto constexpr Name = "example.com"sv; struct sockaddr const* ai_addr = nullptr; dns.lookup( - "example.com", - [&ai_addr](struct sockaddr const* ai, socklen_t ailen) + Name, + [&ai_addr](struct sockaddr const* ai, socklen_t ailen, time_t expires_at) { EXPECT_NE(nullptr, ai); EXPECT_GT(ailen, 0); + EXPECT_GT(expires_at, tr_time()); ai_addr = ai; }); @@ -165,17 +176,24 @@ TEST_F(EvDnsTest, doesCacheEntries) auto second_callback_called = false; dns.lookup( - "example.com", - [&ai_addr, &second_callback_called](struct sockaddr const* ai, socklen_t ailen) + Name, + [&ai_addr, &second_callback_called](struct sockaddr const* ai, socklen_t ailen, time_t expires_at) { EXPECT_NE(nullptr, ai); EXPECT_GT(ailen, 0); EXPECT_EQ(ai_addr, ai); + EXPECT_GT(expires_at, tr_time()); second_callback_called = true; }); // since it's cached, the callback should have been invoked // without waiting for the event loop EXPECT_TRUE(second_callback_called); + + // confirm that `cached()` returns the cached value immediately + auto res = dns.cached(Name); + EXPECT_TRUE(res); + EXPECT_EQ(ai_addr, res->first); + EXPECT_GT(res->second, 0); } } // namespace libtransmission::test diff --git a/tests/libtransmission/net-test.cc b/tests/libtransmission/net-test.cc index 3920c8cd0..448e2acdf 100644 --- a/tests/libtransmission/net-test.cc +++ b/tests/libtransmission/net-test.cc @@ -14,19 +14,19 @@ using namespace std::literals; TEST_F(NetTest, conversionsIPv4) { - auto constexpr port = tr_port::fromHost(80); - auto constexpr addrstr = "127.0.0.1"sv; + auto constexpr Port = tr_port::fromHost(80); + auto constexpr AddrStr = "127.0.0.1"sv; - auto addr = tr_address::fromString(addrstr); + auto addr = tr_address::fromString(AddrStr); EXPECT_TRUE(addr); - EXPECT_EQ(addrstr, addr->readable()); + EXPECT_EQ(AddrStr, addr->readable()); - auto [ss, sslen] = addr->toSockaddr(port); + auto [ss, sslen] = addr->toSockaddr(Port); EXPECT_EQ(AF_INET, ss.ss_family); - EXPECT_EQ(port.network(), reinterpret_cast(&ss)->sin_port); + EXPECT_EQ(Port.network(), reinterpret_cast(&ss)->sin_port); auto addrport = tr_address::fromSockaddr(reinterpret_cast(&ss)); EXPECT_TRUE(addrport); EXPECT_EQ(addr, addrport->first); - EXPECT_EQ(port, addrport->second); + EXPECT_EQ(Port, addrport->second); }