fix: [#1502] bug in total number of downloads for all torrents metric

Relates to: 34c159a

A couple of days ago, I made a change in [this commit](34c159a).

I changed the `Swarm::meets_retaining_policy` method from:

```
/// Returns true if the torrents meets the retention policy, meaning that
/// it should be kept in the tracker.
pub fn meets_retaining_policy(&self, policy: &TrackerPolicy) -> bool {
    if policy.persistent_torrent_completed_stat && self.metadata().downloaded > 0 {
        return true;
    }

    if policy.remove_peerless_torrents && self.is_empty() {
        return false;
    }

    true
}
```

To:

```
pub fn meets_retaining_policy(&self, policy: &TrackerPolicy) -> bool {
    !(policy.remove_peerless_torrents && self.is_empty())
}
```

I thought this code was not needed:

```rust
if policy.persistent_torrent_completed_stat && self.metadata().downloaded > 0 {
    return true;
}
```

However, it's needed.

One of the metrics returned by the tracker API is the **total number of downloads for all torrents**.

```json
{
  "torrents": 320961,
  "seeders": 189885,
  "completed": 975119, <- this
  "leechers": 231044,
  ...
}
```

That metric is always stored in memory but can optionally persist into the database.

It's important to highlight that the metric represents:

- The total number of downloads for **ALL** torrents ever, when the metric is persisted.
- The total number of downloads for **ALL** torrents since the tracker started, when the metric is not persisted.

It could be mixed up with another internal metric (not exposed via the API), which is the same counter but only for ONE swarm (one torrent).

- The total number of downloads for **ONE** concrete torrent ever, when the metric is persisted.
- The total number of downloads for **ONE** concrete torrent since the tracker started, when the metric is not persisted.

The bug affects the first metric. The exposed via the API.

The problem is that this feature conflicts with removing the peerless torrents.

When removing the peerless torrents config option is enabled, the counter is lost unless it is persisted. Becuase the counter values are stored in the "Swarm" together with the list of peers.

If statistics persistence is enabled, that's not a problem. When the torrent is removed from the tracker (from the swarms or swarm collection), the counter is initialised again if the torrent is added. In other words, if a new peer starts the swarm again, the number of downloads is loaded from the database.

However, that works for the counter of each torrent (swarm) but not for the overall counter (the sum of downloads for all torrents). That metric is not stored anywhere. It's calculated on demand by iterating all the swarms and summing up the total for each torrent, giving the total amount of downloads for **ALL** torrents.

When the torrent is removed, the downloads for that torrent don't count in the total. That is the reason we have to keep the torrent (swarm) in memory, even if it does not have any peer (and it should be removed according to the other config flag).

The removed line:

```rust
if policy.persistent_torrent_completed_stat && self.metadata().downloaded > 0 {
    return true;
}
```

does that.

**When the stats persistence is disabled**, that's one way to store the value.

Alternatively, we could add another cache for the data and never remove that value.

The current solution has a problem: It can make the tracker consume a lot of memory because peerless torrents are not removed in practice (even if it's configured to be).

**When the stats persistence is enabled,** we can simply return the value from the database.

**NOTICE:** that the value is used in the scrape response, so it might be convenient to have a cache in memory anyway.

- [x] Revert the change to fix the bug asap.
- [x] Write a unit test. This behaviour was not covered by any test (or documented).
- [ ] Add an in-memory cache value in `Swarms` type to store the total for all torrents, regardless of which are the current active swarms.

Author: josecelano

packages/torrent-repository/src/swarm.rs

@@ -191,10 +191,20 @@ impl Swarm {
     }
 
     /// Returns true if the swarm meets the retention policy, meaning that
-    /// it should be kept in the tracker.
+    /// it should be kept in the list of swarms.
     #[must_use]
     pub fn meets_retaining_policy(&self, policy: &TrackerPolicy) -> bool {
-        !(policy.remove_peerless_torrents && self.is_empty())
+        !self.should_be_removed(policy)
+    }
+
+    fn should_be_removed(&self, policy: &TrackerPolicy) -> bool {
+        // If the policy is to remove peerless torrents and the swarm is empty (no peers),
+        (policy.remove_peerless_torrents && self.is_empty())
+            // but not when the policy is to persist torrent stats and the 
+            // torrent has been downloaded at least once.
+            // (because the only way to store the counter is to keep the swarm in memory.
+            // See https://github.com/torrust/torrust-tracker/issues/1502)
+            && !(policy.persistent_torrent_completed_stat && self.metadata().downloaded > 0)
     }
 }
 
@@ -205,7 +215,6 @@ mod tests {
     use std::sync::Arc;
 
     use aquatic_udp_protocol::PeerId;
-    use torrust_tracker_configuration::TrackerPolicy;
     use torrust_tracker_primitives::peer::fixture::PeerBuilder;
     use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
     use torrust_tracker_primitives::DurationSinceUnixEpoch;
@@ -376,28 +385,101 @@ mod tests {
         assert_eq!(swarm.len(), 1);
     }
 
-    #[test]
-    fn it_should_be_kept_when_empty_if_the_tracker_policy_is_not_to_remove_peerless_torrents() {
-        let empty_swarm = Swarm::default();
+    mod for_retaining_policy {
 
-        let policy = TrackerPolicy {
-            remove_peerless_torrents: false,
-            ..Default::default()
-        };
+        use torrust_tracker_configuration::TrackerPolicy;
+        use torrust_tracker_primitives::peer::fixture::PeerBuilder;
 
-        assert!(empty_swarm.meets_retaining_policy(&policy));
-    }
+        use crate::Swarm;
 
-    #[test]
-    fn it_should_be_removed_when_empty_if_the_tracker_policy_is_to_remove_peerless_torrents() {
-        let empty_swarm = Swarm::default();
+        fn empty_swarm() -> Swarm {
+            Swarm::default()
+        }
 
-        let policy = TrackerPolicy {
-            remove_peerless_torrents: true,
-            ..Default::default()
-        };
+        fn not_empty_swarm() -> Swarm {
+            let mut swarm = Swarm::default();
+            swarm.upsert_peer(PeerBuilder::default().build().into(), &mut false);
+            swarm
+        }
+
+        fn not_empty_swarm_with_downloads() -> Swarm {
+            let mut swarm = Swarm::default();
+
+            let mut peer = PeerBuilder::leecher().build();
+            let mut downloads_increased = false;
+
+            swarm.upsert_peer(peer.into(), &mut downloads_increased);
+
+            peer.event = aquatic_udp_protocol::AnnounceEvent::Completed;
+
+            swarm.upsert_peer(peer.into(), &mut downloads_increased);
+
+            assert!(swarm.metadata().downloads() > 0);
+
+            swarm
+        }
+
+        fn remove_peerless_torrents_policy() -> TrackerPolicy {
+            TrackerPolicy {
+                remove_peerless_torrents: true,
+                ..Default::default()
+            }
+        }
+
+        fn don_not_remove_peerless_torrents_policy() -> TrackerPolicy {
+            TrackerPolicy {
+                remove_peerless_torrents: false,
+                ..Default::default()
+            }
+        }
 
-        assert!(!empty_swarm.meets_retaining_policy(&policy));
+        mod when_removing_peerless_torrents_is_enabled {
+
+            use torrust_tracker_configuration::TrackerPolicy;
+
+            use crate::swarm::tests::for_retaining_policy::{
+                empty_swarm, not_empty_swarm, not_empty_swarm_with_downloads, remove_peerless_torrents_policy,
+            };
+
+            #[test]
+            fn it_should_be_removed_if_the_swarm_is_empty() {
+                assert!(empty_swarm().should_be_removed(&remove_peerless_torrents_policy()));
+            }
+
+            #[test]
+            fn it_should_not_be_removed_is_the_swarm_is_not_empty() {
+                assert!(!not_empty_swarm().should_be_removed(&remove_peerless_torrents_policy()));
+            }
+
+            #[test]
+            fn it_should_not_be_removed_even_if_the_swarm_is_empty_if_we_need_to_track_stats_for_downloads_and_there_has_been_downloads(
+            ) {
+                let policy = TrackerPolicy {
+                    remove_peerless_torrents: true,
+                    persistent_torrent_completed_stat: true,
+                    ..Default::default()
+                };
+
+                assert!(!not_empty_swarm_with_downloads().should_be_removed(&policy));
+            }
+        }
+
+        mod when_removing_peerless_torrents_is_disabled {
+
+            use crate::swarm::tests::for_retaining_policy::{
+                don_not_remove_peerless_torrents_policy, empty_swarm, not_empty_swarm,
+            };
+
+            #[test]
+            fn it_should_not_be_removed_even_if_the_swarm_is_empty() {
+                assert!(!empty_swarm().should_be_removed(&don_not_remove_peerless_torrents_policy()));
+            }
+
+            #[test]
+            fn it_should_not_be_removed_is_the_swarm_is_not_empty() {
+                assert!(!not_empty_swarm().should_be_removed(&don_not_remove_peerless_torrents_policy()));
+            }
+        }
     }
 
     #[test]