Endpoint to get a list of torrents does not return all torrents

[josecelano]

Endpoint: http://0.0.0.0:3000/torrents?page_size=20&page=0&sort=UploadedDesc&categories=&search=

Expected response:

{
  "data": {
    "total": 2,
    "results": [
      {
        "torrent_id": 1,
        "uploader": "user01",
        "info_hash": "9e0217d0fa71c87332cd8bf9dbeabcb2c2cf3c4d",
        "title": "A Mandelbrot Set 2048x2048px picture",
        "description": "A beautiful Mandelbrot Set picture in black and white. \n - Hybrid torrent V1 and V2. \n - Single-file torrent. \n - Public. \n - More than one tracker URL. \n ",
        "category_id": 10,
        "date_uploaded": "2022-11-04 07:19:18",
        "file_size": 9219566,
        "seeders": 0,
        "leechers": 0
      },
      {
        "torrent_id": 2,
        "uploader": "user01",
        "info_hash": "0902d375f18ec020f0cc68ed4810023032ba81cb",
        "title": "Two Mandelbrot Set 2048x2048px pictures",
        "description": "Two beautiful Mandelbrot Set pictures in black and white. \n - Hybrid torrent V1 and V2. \n - Multiple-files torrent. \n - Private. \n - Only one tracker URL. \n ",
        "category_id": 10,
        "date_uploaded": "2022-11-04 07:19:18",
        "file_size": 9219566,
        "seeders": 0,
        "leechers": 0
      }
    ]
  }
}

Actual response:

{
  "data": {
    "total": 1,
    "results": [
      {
        "torrent_id": 1,
        "uploader": "user01",
        "info_hash": "9e0217d0fa71c87332cd8bf9dbeabcb2c2cf3c4d",
        "title": "A Mandelbrot Set 2048x2048px picture",
        "description": "A beautiful Mandelbrot Set picture in black and white. \n - Hybrid torrent V1 and V2. \n - Single-file torrent. \n - Public. \n - More than one tracker URL. \n ",
        "category_id": 10,
        "date_uploaded": "2022-11-04 07:19:18",
        "file_size": 9219566,
        "seeders": 0,
        "leechers": 0
      }
    ]
  }
}

How to reproduce

1. Run this backend test: cargo test upgrades_data_from_version_v1_0_0_to_v2_0_0 -- --nocapture. It's still only available on PR Command to migrate DB data from v1.0.0 to v2.0.0 #77 77. That will produce a tests DB in thetests/upgrades/from_v1_0_0_to_v2_0_0/output dir.
2. Run the tracker
3. Run the backend using the previously generated database.
4. Run the frontend
5. Load this URL: http://localhost:8080/torrents/trending. You will see only one torrent A Mandelbrot Set 2048x2048px picture.
6. Click on the torrent name to go to the torrent details page and back to the previous list page. You will now see two torrents.
7. If you go to the DB and delete all records in the torrust_toprrent_tracker_stats, you will see only one result again.

Conclusion

There must be a problem with the SQL sentence. Maybe there is an inner join that uses that table.
The torrent that does not appear is private if that helps find the issue.

Attachments

The bug. THe list with only one torrent.

The torrent detail page.

The correct list page with two torrents.

The stats table with one record. ONce you have that record, the list page is right.

[josecelano]

It seems that the current behaviour is:

• When a user loads a torrent detail page (the frontend app makes a request to the API endpoint to get info about a torrent). And the API updates the stats table here.
• The application also updates all torrents automatically every hour here.
• The stats are not updated when a new torrent is uploaded.
• There torrent 2 does not appear in the results until there is at least one record in the stats table. It does not have to be the record for torrent 2.

Even if that's the standard behaviour, I think both torrents should appear on the list regardless of whether there are stats for them or not. Is that right @WarmBeer ? Maybe you did not want to show them if we do not have stats.

On the other hand, I would not update the stats when the /api/torrent/:info_hash is called. I think we should not change the state with a GET request unless we want, for example, to increase a request counter. I think it should be enough with the cronjob. We could also update the stats when the torrent is uploaded. @WarmBeer you told me the stats are also updated when you upload a new torrent, but I do not see that in the code. Could you confirm it and point me to the code where that is done?

[mickvandijke]

Hey @josecelano ,

Thank you for reporting and describing this bug!

To respond to your email:

1.- I think it makes sense but the result in the list page should not depend on having those records. I mean the index should show both torrents even if we do not have stats yet for that torrent.

2.- I do not see that behavior you mentioned:

default empty stats record should be inserted for every newly created torrent.

I only see the cron job and what I mentioned on the issue (stats record is created when you go to the torrents details page).

This is important because if you add a stats record when the torrent is uploaded then I have to fix the "upgrader" script to fill that table too.

We can continue the discussion on the PR and new issue.

I realize what went wrong now. The torrent stats used to be stored in a single record. With V3 however, I wanted to show seeders and leechers info per tracker. So somewhere along the way I have removed the insertion of default tracker stats on torrent upload and forgot to implement the new format. I should have written more tests to catch this :). I guess I never noticed it because you get redirected to the torrent details page after uploading and this will trigger a stats update.

Now to fix this, my first instinct would be to aggregate all the different tracker stats and add the seeders and leechers stats together. This might however result in an inflated seeders/leechers count. Because seeders and leechers can announce to multiple trackers and this will result in double counted stats. So to have more "accurate" stats, I think we should just grab the tracker stats of the Torrust tracker linked to the backend. What do you think?

[josecelano]

Hey @josecelano ,

Thank you for reporting and describing this bug!

To respond to your email:

1.- I think it makes sense but the result in the list page should not depend on having those records. I mean the index should show both torrents even if we do not have stats yet for that torrent.

2.- I do not see that behavior you mentioned:

default empty stats record should be inserted for every newly created torrent.

I only see the cron job and what I mentioned on the issue (stats record is created when you go to the torrents details page).

This is important because if you add a stats record when the torrent is uploaded then I have to fix the "upgrader" script to fill that table too.

We can continue the discussion on the PR and new issue.

I realize what went wrong now. The torrent stats used to be stored in a single record. With V3 however, I wanted to show seeders and leechers info per tracker. So somewhere along the way I have removed the insertion of default tracker stats on torrent upload and forgot to implement the new format. I should have written more tests to catch this :). I guess I never noticed it because you get redirected to the torrent details page after uploading and this will trigger a stats update.

Now to fix this, my first instinct would be to aggregate all the different tracker stats and add the seeders and leechers stats together. This might however result in an inflated seeders/leechers count. Because seeders and leechers can announce to multiple trackers and this will result in double counted stats. So to have more "accurate" stats, I think we should just grab the tracker stats of the Torrust tracker linked to the backend. What do you think?

hey @WarmBeer in fact today I was wondering how you know you do not have duplicate seeders.

I think you should restore the current behavior first and then open a new issue if we want to change the behavior. As far as I know, the split statistics require at least two things:

• Make requests to the other trackers to know how many seeders leechers they have.
• Change the UI in the frontend.

If that's right, it requires a big effort. I would:

• Show only the stats for the tracker the index is linked to (and maybe we can change the UI if that's not obvious).
• Rework the missing part. Create the stats record for the torrent when a new torrent is uploaded.
• Create a new migration to remove the tracker_url column from the torrust_torrent_tracker_stats.
• Upgrade the upgrader. We have to remove the tracker_url column but I suppose we also have to pre-fill the stats table because torrents have been already uploaded in version 1.

I think we can first fix this issue before merging the PR for the "upgrader" and then fix and merge the "upgrader"

Does that plan sounds good to you @da2ce7 @WarmBeer?

[mickvandijke]

Hey @josecelano ,

Your plan sounds good! Let's first restore the original behaviour with only one stats entry per torrent that only contains stats from the officially linked Torrust tracker.

[josecelano]

Hey @josecelano ,

Your plan sounds good! Let's first restore the original behaviour with only one stats entry per torrent that only contains stats from the officially linked Torrust tracker.

hey @WarmBeer, I've finished the first task. See the PR.

I've only updated the stats after the upload. That restores the previous behavior.

After thinking about this issue again, I think we could keep the column tracker_url for a different reason. We can keep it as a log of where we get the data from. It's always from the linked tracker its URL (udo) could change. That makes things a lot easier because this task would be finished and the only thing I have to do is fill the table in the "upgrader" script.

We can remove the column later if we think it's not useful at all.

On the other hand, I would like to refactor some things and add tests, but it would take me a couple of days. I think it's better if we merge this patch, fix the "upgrader" and merge it, and them we decide if we want to implement those refactors or tests. For the time being, I've tested the changed manually for both cases:

• After upload
• With the cronjob