Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul core::Tracker #1181

Open
23 of 24 tasks
josecelano opened this issue Jan 11, 2025 · 1 comment
Open
23 of 24 tasks

Overhaul core::Tracker #1181

josecelano opened this issue Jan 11, 2025 · 1 comment
Assignees
Labels
- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat EPIC Contains several subissues

Comments

@josecelano
Copy link
Member

josecelano commented Jan 11, 2025

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 Trackeris 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

*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.

@josecelano
Copy link
Member Author

josecelano commented Jan 31, 2025

Hi @da2ce7, I've removed these two sub-tasks from this epic:

  • [ ]Overhaul core Tracker: publish the new crate
  • [ ] Overhaul core Tracker: write a blog post about the new crate

Because I think it's better to wait until we finish the "Refactor packages" issue. The public API will be more stable. In the mean time I can move things from one workspace package to another if needed and we are not in a hurry to publish those crates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat EPIC Contains several subissues
Projects
None yet
Development

No branches or pull requests

1 participant