clean up session_interface and tracker_manager a bit. work on making tracker_manager more testable

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<request_callback> c
+				, boost::weak_ptr<request_callback> c);
-				, boost::uint32_t key);
 
 			// ==== peer class operations ====
 

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<void(error_code const&, std::vector<address> 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<request_callback> c
+			, boost::weak_ptr<request_callback> c) = 0;
-			, boost::uint32_t key) = 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<torrent> 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;

include/libtorrent/http_tracker_connection.hpp

@@ -71,10 +71,6 @@ namespace libtorrent
 			, tracker_manager& man
 			, tracker_request const& req
 			, boost::weak_ptr<request_callback> c
-			, std::string const& password = ""
-#if TORRENT_USE_I2P
-			, i2p_connection* i2p_conn = 0
-#endif
 			);
 
 		void start();

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 <boost/asio/ssl/context.hpp>
 #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<request_callback> c
 				= boost::weak_ptr<request_callback>());
 		void abort_all_requests(bool all = false);
@@ -371,13 +387,15 @@ namespace libtorrent
 		typedef std::vector<boost::shared_ptr<http_tracker_connection> > 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;
 	};
 }

src/http_tracker_connection.cpp

@@ -75,22 +75,15 @@ namespace libtorrent
 		io_service& ios
 		, tracker_manager& man
 		, tracker_request const& req
-		, boost::weak_ptr<request_callback> c
+		, boost::weak_ptr<request_callback> c)
-		, std::string const& auth
-#if TORRENT_USE_I2P
-		, i2p_connection* i2p_conn
-#endif
-		)
 		: 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()
+				url += escape_string(tracker_req().i2pconn->local_endpoint().c_str()
-					, m_i2p_conn->local_endpoint().size());
+					, 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
 			);
 

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<request_callback> c, boost::uint32_t key)
+		, boost::weak_ptr<request_callback> c)
 	{
 		req.listen_port = listen_port();
-		if (m_key)
+		if (m_key) req.key = m_key;
-			req.key = m_key;
-		else
-			req.key = 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
+		m_tracker_manager.queue_request(get_io_service(), req, c);
-			, login, c);
 	}
 
 	void session_impl::set_peer_class(int cid, peer_class_info const& pci)

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<aux::tracker_logger> 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)

src/tracker_manager.cpp

@@ -193,13 +193,25 @@ namespace libtorrent
 		m_man.remove_request(this);
 	}
 
-	tracker_manager::tracker_manager(aux::session_impl& ses)
+	// TODO: 2 some of these arguments could probably be moved to the
-		: m_ses(ses)
+	// tracker request itself. like the ip_filter and settings
-		, m_ip_filter(ses.m_ip_filter)
+	tracker_manager::tracker_manager(class udp_socket& sock
-		, m_udp_socket(ses.m_udp_socket)
+		, counters& stats_counters
-		, m_host_resolver(ses.m_host_resolver)
+		, resolver_interface& resolver
-		, m_settings(ses.settings())
+		, struct ip_filter& ipf
-		, m_stats_counters(ses.m_stats_counters)
+		, 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<request_callback> c)
 	{
 		mutex_t::scoped_lock l(m_mutex);
@@ -280,11 +291,7 @@ namespace libtorrent
 		{
 			boost::shared_ptr<http_tracker_connection> con
 				= boost::make_shared<http_tracker_connection>(
-				boost::ref(ios), boost::ref(*this), boost::cref(req), c, auth
+				boost::ref(ios), boost::ref(*this), boost::cref(req), c);
-#if TORRENT_USE_I2P
-				, &m_ses.m_i2p_conn
-#endif
-				);
 			m_http_conns.push_back(con);
 			con->start();
 			return;

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())