From 9b91508c388c26b22587de44db46ba023a1ec027 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sun, 1 Feb 2015 14:30:43 +0000 Subject: [PATCH] clean up session_interface and tracker_manager a bit. work on making tracker_manager more testable --- include/libtorrent/aux_/session_impl.hpp | 4 +- include/libtorrent/aux_/session_interface.hpp | 53 +++++++++++-------- .../libtorrent/http_tracker_connection.hpp | 4 -- include/libtorrent/tracker_manager.hpp | 30 ++++++++--- src/http_tracker_connection.cpp | 21 +++----- src/session_impl.cpp | 21 +++++--- src/torrent.cpp | 12 +++-- src/tracker_manager.cpp | 33 +++++++----- src/udp_tracker_connection.cpp | 4 +- 9 files changed, 109 insertions(+), 73 deletions(-) diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 9c5df4f7c..8505b761e 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/include/libtorrent/aux_/session_impl.hpp @@ -367,10 +367,8 @@ namespace libtorrent void set_port_filter(port_filter const& f); port_filter const& get_port_filter() const; - // TODO: move the login info into the tracker_request object void queue_tracker_request(tracker_request& req - , std::string login, boost::weak_ptr c - , boost::uint32_t key); + , boost::weak_ptr c); // ==== peer class operations ==== diff --git a/include/libtorrent/aux_/session_interface.hpp b/include/libtorrent/aux_/session_interface.hpp index 9dda85664..5da3edeab 100644 --- a/include/libtorrent/aux_/session_interface.hpp +++ b/include/libtorrent/aux_/session_interface.hpp @@ -97,9 +97,35 @@ namespace libtorrent namespace libtorrent { namespace aux { - // TOOD: make this interface a lot smaller +#if defined TORRENT_LOGGING || TORRENT_USE_ASSERTS + // This is the basic logging and debug interface offered by the session. + // a release build with logging disabled (which is the default) will + // not have this class at all + struct session_logger + { +#if defined TORRENT_LOGGING + virtual void session_log(char const* fmt, ...) const = 0; + virtual void session_vlog(char const* fmt, va_list& va) const = 0; + virtual void log_all_torrents(peer_connection* p) = 0; +#endif + +#if TORRENT_USE_ASSERTS + virtual bool is_single_thread() const = 0; + virtual bool has_peer(peer_connection const* p) const = 0; + virtual bool any_torrent_has_peer(peer_connection const* p) const = 0; + virtual bool is_posting_torrent_updates() const = 0; +#endif + }; +#endif // TORRENT_LOGGING || TORRENT_USE_ASSERTS + + // TOOD: 2 make this interface a lot smaller. It could be split up into + // several smaller interfaces. Each subsystem could then limit the size + // of the mock object to test it. struct session_interface : buffer_allocator_interface +#if defined TORRENT_LOGGING || TORRENT_USE_ASSERTS + , session_logger +#endif { // TODO: 2 the IP voting mechanism should be factored out // to its own class, not part of the session @@ -125,6 +151,8 @@ namespace libtorrent { namespace aux typedef boost::function const&)> callback_t; + + // TODO: 2 remove this. There's already get_resolver() virtual void async_resolve(std::string const& host, int flags , callback_t const& h) = 0; @@ -179,6 +207,8 @@ namespace libtorrent { namespace aux virtual boost::uint16_t listen_port() const = 0; virtual boost::uint16_t ssl_listen_port() const = 0; + // TODO: 2 factor out the thread pool for socket jobs into a separate + // class // used to (potentially) issue socket write calls onto multiple threads virtual void post_socket_job(socket_job& j) = 0; @@ -215,8 +245,7 @@ namespace libtorrent { namespace aux virtual session_settings const& settings() const = 0; virtual void queue_tracker_request(tracker_request& req - , std::string login, boost::weak_ptr c - , boost::uint32_t key) = 0; + , boost::weak_ptr c) = 0; // peer-classes virtual void set_peer_classes(peer_class_set* s, address const& a, int st) = 0; @@ -291,28 +320,10 @@ namespace libtorrent { namespace aux virtual void prioritize_dht(boost::weak_ptr t) = 0; #endif -#if TORRENT_USE_ASSERTS - virtual bool is_single_thread() const = 0; - virtual bool has_peer(peer_connection const* p) const = 0; - virtual bool any_torrent_has_peer(peer_connection const* p) const = 0; - virtual bool is_posting_torrent_updates() const = 0; -#endif - #ifdef TORRENT_REQUEST_LOGGING virtual FILE* get_request_log() = 0; #endif -#if defined TORRENT_LOGGING - virtual void session_log(char const* fmt, ...) const = 0; - virtual void session_vlog(char const* fmt, va_list& va) const = 0; - virtual void log_all_torrents(peer_connection* p) = 0; -#endif - -#ifdef TORRENT_BUFFER_STATS - virtual void log_buffer_usage() = 0; - virtual std::ofstream& buffer_usage_logger() = 0; -#endif - virtual counters& stats_counters() = 0; virtual void received_buffer(int size) = 0; virtual void sent_buffer(int size) = 0; diff --git a/include/libtorrent/http_tracker_connection.hpp b/include/libtorrent/http_tracker_connection.hpp index f0516db9c..d23bf085f 100644 --- a/include/libtorrent/http_tracker_connection.hpp +++ b/include/libtorrent/http_tracker_connection.hpp @@ -71,10 +71,6 @@ namespace libtorrent , tracker_manager& man , tracker_request const& req , boost::weak_ptr c - , std::string const& password = "" -#if TORRENT_USE_I2P - , i2p_connection* i2p_conn = 0 -#endif ); void start(); diff --git a/include/libtorrent/tracker_manager.hpp b/include/libtorrent/tracker_manager.hpp index e92e21f37..9f6f41775 100644 --- a/include/libtorrent/tracker_manager.hpp +++ b/include/libtorrent/tracker_manager.hpp @@ -62,7 +62,6 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/deadline_timer.hpp" #include "libtorrent/union_endpoint.hpp" #include "libtorrent/udp_socket.hpp" // for udp_socket_observer -#include "libtorrent/session_settings.hpp" #ifdef TORRENT_USE_OPENSSL #include #endif @@ -78,7 +77,10 @@ namespace libtorrent class udp_socket; struct resolver_interface; struct counters; - namespace aux { struct session_impl; struct session_settings; } +#if TORRENT_USE_I2P + class i2p_connection; +#endif + namespace aux { struct session_logger; struct session_settings; } // returns -1 if gzip header is invalid or the header size in bytes TORRENT_EXTRA_EXPORT int gzip_header(const char* buf, int size); @@ -100,6 +102,9 @@ namespace libtorrent , apply_ip_filter(true) #ifdef TORRENT_USE_OPENSSL , ssl_ctx(0) +#endif +#if TORRENT_USE_I2P + , i2pconn(0) #endif {} @@ -120,6 +125,7 @@ namespace libtorrent std::string url; std::string trackerid; + std::string auth; boost::int64_t downloaded; boost::int64_t uploaded; @@ -144,6 +150,9 @@ namespace libtorrent bool apply_ip_filter; #ifdef TORRENT_USE_OPENSSL boost::asio::ssl::context* ssl_ctx; +#endif +#if TORRENT_USE_I2P + i2p_connection* i2pconn; #endif }; @@ -322,13 +331,20 @@ namespace libtorrent { public: - tracker_manager(aux::session_impl& ses); + tracker_manager(udp_socket& sock + , counters& stats_counters + , resolver_interface& resolver + , struct ip_filter& ipf + , aux::session_settings const& sett +#if defined TORRENT_LOGGING || TORRENT_USE_ASSERTS + , aux::session_logger& ses +#endif + ); ~tracker_manager(); void queue_request( io_service& ios , tracker_request r - , std::string const& auth , boost::weak_ptr c = boost::weak_ptr()); void abort_all_requests(bool all = false); @@ -371,13 +387,15 @@ namespace libtorrent typedef std::vector > http_conns_t; http_conns_t m_http_conns; - aux::session_impl& m_ses; - struct ip_filter const& m_ip_filter; class udp_socket& m_udp_socket; resolver_interface& m_host_resolver; aux::session_settings const& m_settings; counters& m_stats_counters; +#if defined TORRENT_LOGGING || TORRENT_USE_ASSERTS + aux::session_logger& m_ses; +#endif + bool m_abort; }; } diff --git a/src/http_tracker_connection.cpp b/src/http_tracker_connection.cpp index 87cc9dc28..ae41dcc7e 100644 --- a/src/http_tracker_connection.cpp +++ b/src/http_tracker_connection.cpp @@ -75,22 +75,15 @@ namespace libtorrent io_service& ios , tracker_manager& man , tracker_request const& req - , boost::weak_ptr c - , std::string const& auth -#if TORRENT_USE_I2P - , i2p_connection* i2p_conn -#endif - ) + , boost::weak_ptr c) : tracker_connection(man, req, ios, c) , m_man(man) -#if TORRENT_USE_I2P - , m_i2p_conn(i2p_conn) -#endif {} void http_tracker_connection::start() { - // TODO: 0 support authentication (i.e. user name and password) in the URL + // TODO: 3 take tracker_req().auth into account. Use it when making the + // request std::string url = tracker_req().url; if (tracker_req().kind == tracker_request::scrape_request) @@ -175,11 +168,11 @@ namespace libtorrent } #if TORRENT_USE_I2P - if (i2p) + if (i2p && tracker_req().i2pconn) { url += "&ip="; - url += escape_string(m_i2p_conn->local_endpoint().c_str() - , m_i2p_conn->local_endpoint().size()); + url += escape_string(tracker_req().i2pconn->local_endpoint().c_str() + , tracker_req().i2pconn->local_endpoint().size()); url += ".i2p"; } else @@ -233,7 +226,7 @@ namespace libtorrent ? resolver_interface::prefer_cache : resolver_interface::abort_on_shutdown #if TORRENT_USE_I2P - , m_i2p_conn + , tracker_req().i2pconn #endif ); diff --git a/src/session_impl.cpp b/src/session_impl.cpp index bbbb744b4..49718e2df 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -356,7 +356,12 @@ namespace aux { #else , m_upload_rate(peer_connection::upload_channel) #endif - , m_tracker_manager(*this) + , m_tracker_manager(m_udp_socket, m_stats_counters, m_host_resolver + , m_ip_filter, m_settings +#if defined TORRENT_LOGGING || TORRENT_USE_ASSERTS + , *this +#endif + ) , m_num_save_resume(0) , m_work(io_service::work(m_io_service)) , m_max_queue_pos(-1) @@ -1191,22 +1196,22 @@ namespace aux { } void session_impl::queue_tracker_request(tracker_request& req - , std::string login, boost::weak_ptr c, boost::uint32_t key) + , boost::weak_ptr c) { req.listen_port = listen_port(); - if (m_key) - req.key = m_key; - else - req.key = key; + if (m_key) req.key = m_key; #ifdef TORRENT_USE_OPENSSL // SSL torrents use the SSL listen port if (req.ssl_ctx) req.listen_port = ssl_listen_port(); req.ssl_ctx = &m_ssl_ctx; #endif +#if TORRENT_USE_I2P + req.i2pconn = &m_i2p_conn; +#endif + if (is_any(req.bind_ip)) req.bind_ip = m_listen_interface.address(); - m_tracker_manager.queue_request(get_io_service(), req - , login, c); + m_tracker_manager.queue_request(get_io_service(), req, c); } void session_impl::set_peer_class(int cid, peer_class_info const& pci) diff --git a/src/torrent.cpp b/src/torrent.cpp index 7c73f1c49..8d1d45571 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -3024,6 +3024,10 @@ namespace libtorrent continue; } } + + req.auth = tracker_login(); + req.key = tracker_key(); + #if defined TORRENT_LOGGING debug_log("==> TRACKER REQUEST \"%s\" event: %s abort: %d" , req.url.c_str() @@ -3034,12 +3038,12 @@ namespace libtorrent if (m_abort) { boost::shared_ptr tl(new aux::tracker_logger(m_ses)); - m_ses.queue_tracker_request(req, tracker_login(), tl, tracker_key()); + m_ses.queue_tracker_request(req, tl); } else #endif { - m_ses.queue_tracker_request(req, tracker_login(), shared_from_this(), tracker_key()); + m_ses.queue_tracker_request(req, shared_from_this()); } ae.updating = true; @@ -3077,7 +3081,9 @@ namespace libtorrent req.info_hash = m_torrent_file->info_hash(); req.kind = tracker_request::scrape_request; req.url = m_trackers[i].url; - m_ses.queue_tracker_request(req, tracker_login(), shared_from_this(), tracker_key()); + req.auth = tracker_login(); + req.key = tracker_key(); + m_ses.queue_tracker_request(req, shared_from_this()); } void torrent::tracker_warning(tracker_request const& req, std::string const& msg) diff --git a/src/tracker_manager.cpp b/src/tracker_manager.cpp index 8651e6bea..6f40a3a76 100644 --- a/src/tracker_manager.cpp +++ b/src/tracker_manager.cpp @@ -193,13 +193,25 @@ namespace libtorrent m_man.remove_request(this); } - tracker_manager::tracker_manager(aux::session_impl& ses) - : m_ses(ses) - , m_ip_filter(ses.m_ip_filter) - , m_udp_socket(ses.m_udp_socket) - , m_host_resolver(ses.m_host_resolver) - , m_settings(ses.settings()) - , m_stats_counters(ses.m_stats_counters) + // TODO: 2 some of these arguments could probably be moved to the + // tracker request itself. like the ip_filter and settings + tracker_manager::tracker_manager(class udp_socket& sock + , counters& stats_counters + , resolver_interface& resolver + , struct ip_filter& ipf + , aux::session_settings const& sett +#if defined TORRENT_LOGGING || TORRENT_USE_ASSERTS + , aux::session_logger& ses +#endif + ) + : m_ip_filter(ipf) + , m_udp_socket(sock) + , m_host_resolver(resolver) + , m_settings(sett) + , m_stats_counters(stats_counters) +#if defined TORRENT_LOGGING || TORRENT_USE_ASSERTS + , m_ses(ses) +#endif , m_abort(false) {} @@ -256,7 +268,6 @@ namespace libtorrent void tracker_manager::queue_request( io_service& ios , tracker_request req - , std::string const& auth , boost::weak_ptr c) { mutex_t::scoped_lock l(m_mutex); @@ -280,11 +291,7 @@ namespace libtorrent { boost::shared_ptr con = boost::make_shared( - boost::ref(ios), boost::ref(*this), boost::cref(req), c, auth -#if TORRENT_USE_I2P - , &m_ses.m_i2p_conn -#endif - ); + boost::ref(ios), boost::ref(*this), boost::cref(req), c); m_http_conns.push_back(con); con->start(); return; diff --git a/src/udp_tracker_connection.cpp b/src/udp_tracker_connection.cpp index 57983eb8a..b0cef064b 100644 --- a/src/udp_tracker_connection.cpp +++ b/src/udp_tracker_connection.cpp @@ -83,6 +83,7 @@ namespace libtorrent void udp_tracker_connection::start() { + // TODO: 2 support authentication here. tracker_req().auth std::string hostname; std::string protocol; int port; @@ -738,7 +739,8 @@ namespace libtorrent std::string request_string; error_code ec; using boost::tuples::ignore; - boost::tie(ignore, ignore, ignore, ignore, request_string) = parse_url_components(req.url, ec); + boost::tie(ignore, ignore, ignore, ignore, request_string) + = parse_url_components(req.url, ec); if (ec) request_string.clear(); if (!request_string.empty())