From 7ff6756ac541edfc3fbd6f4e50d199142dd5bce4 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 13 Nov 2021 19:09:33 -0600 Subject: [PATCH] refactor: tr session.blocklist (#2147) * refactor: tr_session.blocklist --- libtransmission/rpcimpl.cc | 30 ++++++++---------- libtransmission/session.cc | 45 +++++++++++++-------------- libtransmission/session.h | 32 ++++++++++++++++--- tests/libtransmission/session-test.cc | 30 ++++++++++++++++++ 4 files changed, 93 insertions(+), 44 deletions(-) diff --git a/libtransmission/rpcimpl.cc b/libtransmission/rpcimpl.cc index 8edceeeab..47ca0cd25 100644 --- a/libtransmission/rpcimpl.cc +++ b/libtransmission/rpcimpl.cc @@ -1604,7 +1604,7 @@ static char const* blocklistUpdate( tr_variant* /*args_out*/, struct tr_rpc_idle_data* idle_data) { - tr_webRun(session, session->blocklist_url, gotNewBlocklist, idle_data); + tr_webRun(session, session->blocklistUrl().c_str(), gotNewBlocklist, idle_data); return nullptr; } @@ -1871,7 +1871,6 @@ static char const* sessionSet( auto boolVal = bool{}; auto d = double{}; auto i = int64_t{}; - char const* str = nullptr; auto sv = std::string_view{}; if (tr_variantDictFindInt(args_in, TR_KEY_cache_size_mb, &i)) @@ -1916,12 +1915,12 @@ static char const* sessionSet( if (tr_variantDictFindBool(args_in, TR_KEY_blocklist_enabled, &boolVal)) { - tr_blocklistSetEnabled(session, boolVal); + session->useBlocklist(boolVal); } - if (tr_variantDictFindStr(args_in, TR_KEY_blocklist_url, &str, nullptr)) + if (tr_variantDictFindStrView(args_in, TR_KEY_blocklist_url, &sv)) { - tr_blocklistSetURL(session, str); + session->setBlocklistUrl(sv); } if (!std::empty(download_dir)) @@ -2209,11 +2208,11 @@ static void addSessionField(tr_session* s, tr_variant* d, tr_quark key) break; case TR_KEY_blocklist_enabled: - tr_variantDictAddBool(d, key, tr_blocklistIsEnabled(s)); + tr_variantDictAddBool(d, key, s->useBlocklist()); break; case TR_KEY_blocklist_url: - tr_variantDictAddStr(d, key, tr_blocklistGetURL(s)); + tr_variantDictAddStr(d, key, s->blocklistUrl()); break; case TR_KEY_cache_size_mb: @@ -2229,7 +2228,7 @@ static void addSessionField(tr_session* s, tr_variant* d, tr_quark key) break; case TR_KEY_download_dir: - tr_variantDictAddStr(d, key, tr_sessionGetDownloadDir(s)); + tr_variantDictAddStr(d, key, s->downloadDir()); break; case TR_KEY_download_dir_free_space: @@ -2253,11 +2252,11 @@ static void addSessionField(tr_session* s, tr_variant* d, tr_quark key) break; case TR_KEY_incomplete_dir: - tr_variantDictAddStr(d, key, tr_sessionGetIncompleteDir(s)); + tr_variantDictAddStr(d, key, s->incompleteDir()); break; case TR_KEY_incomplete_dir_enabled: - tr_variantDictAddBool(d, key, tr_sessionIsIncompleteDirEnabled(s)); + tr_variantDictAddBool(d, key, s->useIncompleteDir()); break; case TR_KEY_pex_enabled: @@ -2441,8 +2440,9 @@ static char const* freeSpace( tr_variant* args_out, tr_rpc_idle_data* /*idle_data*/) { - char const* path = nullptr; - if (!tr_variantDictFindStr(args_in, TR_KEY_path, &path, nullptr)) + auto path = std::string_view{}; + + if (!tr_variantDictFindStrView(args_in, TR_KEY_path, &path)) { return "directory path argument is missing"; } @@ -2460,11 +2460,7 @@ static char const* freeSpace( errno = old_errno; /* response */ - if (path != nullptr) - { - tr_variantDictAddStr(args_out, TR_KEY_path, path); - } - + tr_variantDictAddStr(args_out, TR_KEY_path, path); tr_variantDictAddInt(args_out, TR_KEY_size_bytes, dir_space.free); tr_variantDictAddInt(args_out, TR_KEY_total_size, dir_space.total); return err; diff --git a/libtransmission/session.cc b/libtransmission/session.cc index a2745730d..bcb7f2908 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -337,7 +337,7 @@ void tr_sessionGetDefaultSettings(tr_variant* d) tr_variantDictReserve(d, 69); tr_variantDictAddBool(d, TR_KEY_blocklist_enabled, false); - tr_variantDictAddStr(d, TR_KEY_blocklist_url, "http://www.example.com/blocklist"); + tr_variantDictAddStr(d, TR_KEY_blocklist_url, "http://www.example.com/blocklist"sv); tr_variantDictAddInt(d, TR_KEY_cache_size_mb, DefaultCacheSizeMB); tr_variantDictAddBool(d, TR_KEY_dht_enabled, true); tr_variantDictAddBool(d, TR_KEY_utp_enabled, true); @@ -412,8 +412,8 @@ void tr_sessionGetSettings(tr_session* s, tr_variant* d) TR_ASSERT(tr_variantIsDict(d)); tr_variantDictReserve(d, 68); - tr_variantDictAddBool(d, TR_KEY_blocklist_enabled, tr_blocklistIsEnabled(s)); - tr_variantDictAddStr(d, TR_KEY_blocklist_url, tr_blocklistGetURL(s)); + tr_variantDictAddBool(d, TR_KEY_blocklist_enabled, s->useBlocklist()); + tr_variantDictAddStr(d, TR_KEY_blocklist_url, s->blocklistUrl()); tr_variantDictAddInt(d, TR_KEY_cache_size_mb, tr_sessionGetCacheLimit_MB(s)); tr_variantDictAddBool(d, TR_KEY_dht_enabled, s->isDHTEnabled); tr_variantDictAddBool(d, TR_KEY_utp_enabled, s->isUTPEnabled); @@ -867,12 +867,12 @@ static void sessionSetImpl(void* vdata) if (tr_variantDictFindBool(settings, TR_KEY_blocklist_enabled, &boolVal)) { - tr_blocklistSetEnabled(session, boolVal); + session->useBlocklist(boolVal); } - if (tr_variantDictFindStr(settings, TR_KEY_blocklist_url, &strVal, nullptr)) + if (tr_variantDictFindStrView(settings, TR_KEY_blocklist_url, &sv)) { - tr_blocklistSetURL(session, strVal); + session->setBlocklistUrl(sv); } if (tr_variantDictFindBool(settings, TR_KEY_start_added_torrents, &boolVal)) @@ -2053,7 +2053,6 @@ void tr_sessionClose(tr_session* session) tr_free(session->configDir); tr_free(session->resumeDir); tr_free(session->torrentDir); - tr_free(session->blocklist_url); tr_free(session->peer_congestion_algorithm); delete session; } @@ -2343,7 +2342,7 @@ static bool tr_stringEndsWith(char const* strval, char const* end) static void loadBlocklists(tr_session* session) { auto loadme = std::unordered_set{}; - auto const isEnabled = session->isBlocklistEnabled; + auto const isEnabled = session->useBlocklist(); /* walk the blocklist directory... */ auto const dirname = tr_strvPath(session->configDir, "blocklists"sv); @@ -2460,20 +2459,24 @@ bool tr_blocklistIsEnabled(tr_session const* session) { TR_ASSERT(tr_isSession(session)); - return session->isBlocklistEnabled; + return session->useBlocklist(); +} + +void tr_session::useBlocklist(bool enabled) +{ + this->blocklist_enabled_ = enabled; + + std::for_each( + std::begin(blocklists), + std::end(blocklists), + [enabled](auto* blocklist) { tr_blocklistFileSetEnabled(blocklist, enabled); }); } void tr_blocklistSetEnabled(tr_session* session, bool enabled) { TR_ASSERT(tr_isSession(session)); - session->isBlocklistEnabled = enabled; - - auto& src = session->blocklists; - std::for_each( - std::begin(src), - std::end(src), - [enabled](auto* blocklist) { tr_blocklistFileSetEnabled(blocklist, enabled); }); + session->useBlocklist(enabled); } bool tr_blocklistExists(tr_session const* session) @@ -2498,7 +2501,7 @@ int tr_blocklistSetContent(tr_session* session, char const* contentFilename) if (it == std::end(src)) { auto path = tr_strvJoin(session->configDir, "blocklists"sv, name); - b = tr_blocklistFileNew(path.c_str(), session->isBlocklistEnabled); + b = tr_blocklistFileNew(path.c_str(), session->useBlocklist()); src.push_back(b); } else @@ -2523,16 +2526,12 @@ bool tr_sessionIsAddressBlocked(tr_session const* session, tr_address const* add void tr_blocklistSetURL(tr_session* session, char const* url) { - if (session->blocklist_url != url) - { - tr_free(session->blocklist_url); - session->blocklist_url = tr_strdup(url); - } + session->setBlocklistUrl(url ? url : ""); } char const* tr_blocklistGetURL(tr_session const* session) { - return session->blocklist_url; + return session->blocklistUrl().c_str(); } /*** diff --git a/libtransmission/session.h b/libtransmission/session.h index dd74631e0..987a1d7a7 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -131,6 +131,8 @@ struct CaseInsensitiveStringCompare // case-insensitive string compare struct tr_session { public: + // download dir + std::string const& downloadDir() const { return download_dir_; @@ -141,6 +143,8 @@ public: download_dir_ = dir; } + // incomplete dir + std::string const& incompleteDir() const { return incomplete_dir_; @@ -161,6 +165,8 @@ public: incomplete_dir_enabled_ = enabled; } + // scripts + void useScript(TrScript i, bool enabled) { scripts_enabled_[i] = enabled; @@ -181,13 +187,31 @@ public: return scripts_[i]; } + // blocklist + + bool useBlocklist() const + { + return blocklist_enabled_; + } + + void useBlocklist(bool enabled); + + std::string const& blocklistUrl() const + { + return blocklist_url_; + } + + void setBlocklistUrl(std::string_view url) + { + blocklist_url_ = url; + } + public: bool isPortRandom; bool isPexEnabled; bool isDHTEnabled; bool isUTPEnabled; bool isLPDEnabled; - bool isBlocklistEnabled; bool isPrefetchEnabled; bool isClosing; bool isClosed; @@ -268,8 +292,6 @@ public: char* resumeDir; char* torrentDir; - char* blocklist_url; - std::list blocklists; struct tr_peerMgr* peerMgr; struct tr_shared* shared; @@ -307,10 +329,12 @@ public: private: std::array scripts_; - std::string incomplete_dir_; + std::string blocklist_url_; std::string download_dir_; + std::string incomplete_dir_; std::array scripts_enabled_; + bool blocklist_enabled_ = false; bool incomplete_dir_enabled_ = false; }; diff --git a/tests/libtransmission/session-test.cc b/tests/libtransmission/session-test.cc index f5da5e3ec..9426ccb93 100644 --- a/tests/libtransmission/session-test.cc +++ b/tests/libtransmission/session-test.cc @@ -110,6 +110,36 @@ TEST_F(SessionTest, properties) EXPECT_EQ(value, session->useIncompleteDir()); EXPECT_EQ(value, tr_sessionIsIncompleteDirEnabled(session)); } + + // blocklist url + + for (auto const& value : { "foo"sv, "bar"sv, ""sv }) + { + session->setBlocklistUrl(value); + EXPECT_EQ(value, session->blocklistUrl()); + EXPECT_EQ(value, tr_blocklistGetURL(session)); + + tr_blocklistSetURL(session, std::string(value).c_str()); + EXPECT_EQ(value, session->blocklistUrl()); + EXPECT_EQ(value, tr_blocklistGetURL(session)); + } + + tr_blocklistSetURL(session, nullptr); + EXPECT_EQ(""sv, session->blocklistUrl()); + EXPECT_EQ(""sv, tr_blocklistGetURL(session)); + + // blocklist enabled + + for (auto const value : { true, false }) + { + session->useBlocklist(value); + EXPECT_EQ(value, session->useBlocklist()); + EXPECT_EQ(value, tr_blocklistIsEnabled(session)); + + tr_sessionSetIncompleteDirEnabled(session, value); + EXPECT_EQ(value, session->useBlocklist()); + EXPECT_EQ(value, tr_blocklistIsEnabled(session)); + } } TEST_F(SessionTest, peerId)