New app architecture

[josecelano]

Hi @da2ce7 @WarmBeer, I have created this diagram reflecting the current app architecture:

I want to use it to discuss some changes I want to propose.

But before proposing concrete changes, I will explain the current architecture and the drawbacks I think it has.

Current architecture

Some thing I thin we could improve are:

1. The tracker has too many responsibilities.

As you can see in the diagram, the Tracker has at least six responsibilities.

• Authentication
• Authorization
• Handle the whitelist
• Handle torrents/peers (core tracker responsibilities)
• Send events for statistics
• Store metrics

For the authentication, whitelist, and core tracker, it handles the persistence too (persistence).

And it's also responsible for constructing all those services (app container).

One advantage of this big class is you only need to pass it around to get access to all the services you need. For example, in the web handlers (Axum controllers), you only need one dependency: the tracker.

On the other hand, one of the drawbacks is that it's hard to test, or it makes it harder to test each responsibility independently.

For example, it would be much easier to test a "whitelist" service which is internally implemented with the RwLock<std::collections::HashSet<InfoHash>>. We could have two whitelist implementations the In-Memory implementation and the In-Database implementation.

Deletaging subtasks to other services would make the code easier to test, to change and less coupled.

2. Some tracker responsibilities do not belong to it.

The tracker is also responsible for handling the authentication keys. But authentication keys are used only by the HTTP tracker app. They do not make sense for the UDP tracker or the tracker API.

Short-term changes

I've started introducing new layers to the app architecture. For example, the HTTP tracker now has a service layer:

• HTTP::v1::services::announce::invoke
• HTTP::v1::services::announce::scrape

This service is responsible for calling the domain service Tracker::announce or Tracker::scrape and sending the statistics events. Events are sent at this level because events are specific for the UPD or HTTP tracker. Maybe in the future, we could have generic events with an attribute. Instead of:

pub enum Event {
    Tcp4Announce,
    Tcp4Scrape,
    Tcp6Announce,
    Tcp6Scrape,
    Udp4Connect,
    Udp4Announce,
    Udp4Scrape,
    Udp6Connect,
    Udp6Announce,
    Udp6Scrape,
}

We could have:

pub enum Event {
    Connect: { IpVersion , Protocol }
    Announce: { IpVersion , Protocol }
    Scrape: { IpVersion , Protocol }
}

pub enum IpVersion {
    Ipv4,
    Ipv6
}

pub enum Protocol {
    Tcp,
    Udp
}

Although I've introduced these new services, there is still a lot of code in the Axum handlers that could potentially be moved to the service because it does not depend on Axum, and it could be reused if we change Axum. For example:

The announce handler:

async fn handle_announce(
    tracker: &Arc<Tracker>,
    announce_request: &Announce,
    client_ip_sources: &ClientIpSources,
    maybe_key: Option<Key>,
) -> Result<AnnounceData, responses::error::Error> {
    // Authentication
    if tracker.requires_authentication() {
        match maybe_key {
            Some(key) => match tracker.authenticate(&key).await {
                Ok(_) => (),
                Err(error) => return Err(responses::error::Error::from(error)),
            },
            None => {
                return Err(responses::error::Error::from(auth::Error::MissingAuthKey {
                    location: Location::caller(),
                }))
            }
        }
    }

    // Authorization
    match tracker.authorize(&announce_request.info_hash).await {
        Ok(_) => (),
        Err(error) => return Err(responses::error::Error::from(error)),
    }

    let peer_ip = match peer_ip_resolver::invoke(tracker.config.on_reverse_proxy, client_ip_sources) {
        Ok(peer_ip) => peer_ip,
        Err(error) => return Err(responses::error::Error::from(error)),
    };

    let mut peer = peer_from_request(announce_request, &peer_ip);

    let announce_data = services::announce::invoke(tracker.clone(), announce_request.info_hash, &mut peer).await;

    Ok(announce_data)
}

As you can see, the handler does three actions:

• Authenticate
• Authorize
• Resolve the peer IP. It does not get the IP from the request, it makes the decision about what IP to use (the one in the connection or the one in the proxy header).

Those tasks are there because they were also there before migrating to Axum and because we return a custom response, and the HTTP response is something that should not be in the App service.

One of my proposals is to move that code inside the service. The service only returns an App error, and the handler captures the error and builds the right custom response.

The final handler would be like this:

async fn handle_announce(
    tracker: &Arc<Tracker>,
    announce_request: &Announce,
    client_ip_sources: &ClientIpSources,
    maybe_key: Option<Key>,
) -> Result<AnnounceData, responses::error::Error> {
    let mut peer = peer_from_request(announce_request, &peer_ip);

    match services::announce::invoke(tracker.clone(), announce_request.info_hash, &mut peer).await {
    Err(error: authentication::Error) => return Err(responses::error::Error::from(error)),
    Err(error: authorization::Error) => return Err(responses::error::Error::from(error)),
    Ok(announce_Data) => Ok(announce_data)
}

NOTE: I've also started using the authorisation term. Right now, the only authorization is the whitelist. Once you are authenticated, we check if the tracker is whitelisted and if the torrent is in the whitelist. Although now authorization means whitelist, I've introduced the generic authorization layer that happens to use a whitelist under the hood. In the future, we could define permissions for categories, for example.

Middle-term changes

In the middle-term we do two things:

• Extract new domain services from the tracker: whitelist, authorization
• Extract even the core tracker (the logic to handle torrents and peers) and keep the current tracker as an application container whose responsibility is building other services

Long-term changes

In the long term, we could discuss it again about persistence. Right now, we are handling both: in-memory objects and DB records and the same time, but they are independent. The tracker should work without persistence. For example, when we generate a new key, we keep it in memory, and we also store it in the DB. The DB is only used to recover backups. Somehow the source of truth is the memory, and the DB is just a backup that is synced in real-time.

I would introduce services that handle both in-memory collections and persistence. For example:

WhitelistService -uses-> InMemoryWhitelistRepository,DbWhitelistRepository

The WhitelistService uses the DbWhitelistRepository only if persistence is enabled.

[josecelano]

Regarding extracting more packages in the workspace the aquatic BitTorrent tracker could be a case to study. We could even check which packages we could have in common.