-
Notifications
You must be signed in to change notification settings - Fork 48
Description
I think the core::Tracker
is the most "important type" in the whole repo.
pub struct Tracker {
/// The tracker configuration.
config: Core,
/// A database driver implementation: [`Sqlite3`](crate::core::databases::sqlite)
/// or [`MySQL`](crate::core::databases::mysql)
database: Arc<Box<dyn Database>>,
/// Tracker users' keys. Only for private trackers.
keys: tokio::sync::RwLock<std::collections::HashMap<Key, auth::PeerKey>>,
/// The list of allowed torrents. Only for listed trackers.
whitelist: tokio::sync::RwLock<std::collections::HashSet<InfoHash>>,
/// The in-memory torrents repository.
torrents: Arc<Torrents>,
/// Service to send stats events.
stats_event_sender: Option<Box<dyn statistics::event::sender::Sender>>,
/// The in-memory stats repo.
stats_repository: statistics::repository::Repository,
}
In my opinion it's too big. It has too many responsibilities. It's coupled with everything and it makes hard to write unit tests. In many places the Tracker
is injected as a dependency and it makes impossible to unit test anything that depends on the Tracker
.
The tracker has at least these responsibilities:
- Tracker (core tracker functionalities: handle announce and scrape)
- Authentication (only for HTTP trackers. It handles the keys)
- Whitelist
- Statistics
I called them contexts
and I organized methods in contexts long time ago.
I think we should consider refactor to improve further the sustainability and testability of the code.
I think we can start by extracting new services inside the tracker:
- Authentication
- Whitelist
- Statistics
Moving their data structures and methods. And after that, we can move the new services out of the tracker. That will require to inject them directly where they are currently used via the tracker. for example Axum handlers.
We could start with the most obvious ones Whitelist
and Statistics
.
Extracting the services will be beneficial for the core module even if we keep those services behind the tracker facade. In fact we could also extract a core tracker (context tracker) and rename the current one to TrackerFacade
or something like that.
What do you think @da2ce7?
Sub-issues
- Overhaul core Tracker: extract whitelist #1182
- Overhaul core Tracker: extract stats #1184
- Overhaul core Tracker: extract IoC Container #1187
- Overhaul core Tracker: extract whitelist authorization #1189
- Overhaul core Tracker: extract authentication #1191
- Overhaul core Tracker: extract torrents context (part 1) #1201
- Overhaul core Tracker: extract torrents context (part 2) #1203
- Overhaul core Tracker: extract scrape handler #1205
- Overhaul core Tracker: extract announce handler #1207
- Overhaul core Tracker: refactor authentication module #1195
- Overhaul core Tracker: refactor tests (remove duplicate code) #1198
- Overhaul core Tracker: remove deprecated
core::Tracker
#1209 - Overhaul core Tracker: refactor tests #1211
- Overhaul core Tracker: remove complexity (type_complexity) #1215
- Overhaul core Tracker: create dependency containers for UDP tracker, HTTP tracker and Tracker API #1217
- Overhaul core Tracker: reorganize code #1221
- Overhaul core Tracker: extract workspace package for the tracker core lib #1223 *(1)
- Overhaul core Tracker: refactor statistics module #1228
- Overhaul core Tracker: add units tests to the new
tracker-core
package #1226 - Overhaul core Tracker: review public methods for
tracker-core
package #1258 *(2) - Overhaul core Tracker: review
tracker-core
package documentation #1261 - Overhaul core Tracker: add integration test #1266
- Overhaul core Tracker: review whitelist functionality #1268
- Overhaul core Tracker: review whitelist functionality (part 2) #1270
- Overhaul core Tracker: move authentication to
http-tracker-core
andudp-tracker-core
#1275 - Overhaul core Tracker: review announce handler (peer mutability) #1276
[ ]Overhaul core Tracker: publish the new crate[ ] Overhaul core Tracker: write a blog post about the new crate
*1 = Database migration are only used inside the core tracker module. If we extract the core lib package we need to move the DB migrations together with the new crate.
*2 = We only need to publicly expose types and functions in the tracker lib crate that are used in the main repo. At the beginning we can start with a restrictive approach for the new package, exposing only what we are using.