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.

Sign up for GitHub

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

Jump to bottom

Restore broken behavior: after uploading a new torrent its tracker stats must be updated #87

Merged
merged 2 commits into from
Nov 30, 2022
Merged

Restore broken behavior: after uploading a new torrent its tracker stats must be updated #87

merged 2 commits into from
Nov 30, 2022

Conversation

josecelano
Copy link
Member

@josecelano josecelano commented Nov 29, 2022 •
edited
Loading

When a new torrent is uploaded, we must update the tracker torrent stats in the torrust_torrent_tracker_stats table.

It worked from the UI but not for the DB migration script because the frontend makes a request to get the torrent info after the upload, and that endpoint updates the torrent tracker stats.

It must update the stats even if that endpoint is not called after uploading a new torrent.

Tasks

  • After uploading a new torrent, update its tracker stats.
  • Change SQL query for torrent list result if we allow torrents without stats.

Potencial refactors

  • Extract TrackerApiClient from TrackerService
  • Rename TrackerService::get_torrent_info to TrackerService::update_torrent_tracker_stats

@josecelano josecelano linked an issue Nov 29, 2022 that may be closed by this pull request
Endpoint to get a list of torrents does not return all torrents #84
Closed
@josecelano josecelano mentioned this pull request Nov 29, 2022
Closed
@josecelano josecelano marked this pull request as ready for review November 29, 2022 19:05
@josecelano josecelano changed the title Restore broken behavior: after uploading a the new torrent torrent tracker stats must updated Restore broken behavior: after uploading a new torrent its tracker stats must updated Nov 29, 2022
@josecelano josecelano changed the title Restore broken behavior: after uploading a new torrent its tracker stats must updated Restore broken behavior: after uploading a new torrent its tracker stats must be updated Nov 29, 2022
@josecelano josecelano marked this pull request as draft November 29, 2022 19:17
da2ce7
da2ce7 approved these changes Nov 29, 2022
View reviewed changes
Copy link
Contributor

@da2ce7 da2ce7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 6000f2e

@da2ce7 da2ce7 added the Needs Rebase Base Branch has Incompatibilities label Nov 29, 2022
@josecelano
fix: [#84] restore behavior. Update torrent stats after upload
c291557 
When a new torrent is uploaded we have to update the tracker torrent
stats in the `torrust_torrent_tracker_stats` table.

It was from the UI but not for the DB migration script becuase the
frontend makes a request to get the torrent info after the upload and
that endpint updates the torrent tracket stats.

It must update the stats even if that endpoint is not called after
uploading a new torrent.
@josecelano josecelano force-pushed the issue-84-restore-torrent-stats-behavior branch from 6000f2e to c291557 Compare November 30, 2022 08:39
@josecelano josecelano removed the Needs Rebase Base Branch has Incompatibilities label Nov 30, 2022
@josecelano josecelano mentioned this pull request Nov 30, 2022
Merged
39 tasks
@josecelano
fix: [#84] SQL query to get list of torrents without stats
3c47ffc 
The query was not returning all the torrents regardless whether they
have traacker stats or not.
@josecelano josecelano marked this pull request as ready for review November 30, 2022 12:01
da2ce7
da2ce7 approved these changes Nov 30, 2022
View reviewed changes
Copy link
Contributor

@da2ce7 da2ce7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 3c47ffc

@da2ce7 da2ce7 merged commit 3e7feb1 into develop Nov 30, 2022
@josecelano josecelano deleted the issue-84-restore-torrent-stats-behavior branch November 30, 2022 13:59
josecelano added a commit that referenced this pull request Nov 30, 2022
@josecelano
Merge #77: Command to migrate DB data from v1.0.0 to v2.0.0
34cc8f5 
5a7d875 feat: [#56] console command to import tracker stats for all torrents (Jose Celano)
19d054e refactor: [#56] rename test mods to follow prod mods (Jose Celano)
e8d984d refactor: [#56] rename destiny DB to target DB (Jose Celano)
b400962 fix: format (Jose Celano)
8b761c8 feat: [#56] keep category id in DB migration script (Jose Celano)
b29d4d7 fix: [#56] db migration for imported users (Jose Celano)
38fee53 test: [#56] new test for password verification (Jose Celano)
b9a8bf9 fix: [#56] remove comment (Jose Celano)
e1790f6 refactor: [#56] extract mods in upgrader (Jose Celano)
e23d948 refactor: remove duplication in tests (Jose Celano)
ee01e7b test: [#56] for torrent files table in upgrader (new case) (Jose Celano)
afffaef tests: [#56] for torrents files table in upgrader (Jose Celano)
82b84a3 refactor: [#56] extract test configuration (Jose Celano)
750969d refactor: [#56] rename methods (Jose Celano)
0063289 tests: [#56] for torrents info and announce urls tables in upgrader (Jose Celano)
cd95987 refactor: [#56] rename mod and variables (Jose Celano)
f0f581f tests: [#56] for torrents table in upgrader (Jose Celano)
eef980c tests: [#56] for tracker keys table in upgrader (Jose Celano)
8d74e66 tests: [#56] for users profile and auth tables in upgrader (Jose Celano)
0a58b6c fix: [#56] bio and avatar is user profile should be NULL for imported users (Jose Celano)
5d0def2 refactor: [#56] tests for upgrader (Jose Celano)
f993107 tests: [#56] for users table in upgrader (Jose Celano)
6188b10 refactor: extract mod sqlite_v1_0_0 in tests (Jose Celano)
44927e5 test: [#56] WIP. scaffolding to test upgrader command (Jose Celano)
7f0a7ea fix: open source db in read-only mode in upgarder (Jose Celano)
217fae2 feat: [#56] take source DB in upgrader command from args (Jose Celano)
aabc3ef feat: the upgrader command takes args (Jose Celano)
693994f feat: add new dependency text_colorizer (Jose Celano)
f620e05 fix: [#56] announce list has precedence over announce (Jose Celano)
309e141 fix: take torrent private flag from torrent file (Jose Celano)
72dc139 refactor: reformat sql queries (Jose Celano)
6bb4c53 refactor: extract struct TorrentRecordV2 (Jose Celano)
b9bf405 feat: [#56] improve command output (Jose Celano)
7152654 refactor: [#56] rename structs for DB records (Jose Celano)
99edf52 feat: imported users have importation date instead of registrataion date (Jose Celano)
21174d4 feat: [#56] trasnfer torrents (4/4 tables) from v1.0.0 to v2.0.0 (Jose Celano)
8bdf32f feat: [#56] trasnfer torrents (3/4 tables) from v1.0.0 to v2.0.0 (Jose Celano)
3fea6ea feat: [#56] trasnfer torrents (2/4 tables) from v1.0.0 to v2.0.0 (Jose Celano)
03e4bef feat: [#56] remove unused scripts and write basic upgrage guide (Jose Celano)
0b3aefa feat: [#56] transfer torrents (1/4 tables) from v1.0.0 to v2.0.0 (Jose Celano)
8d26faa fix: [#78] parsing keys from tracker (Jose Celano)
35f1e37 fix: [#56} default user registration date with time (Jose Celano)
dd949fa feat: [#56] transfer tracker keys from v1.0.0 to v2.0.0 (Jose Celano)
d9b4e87 feat: [#56] transfer user password from v1.0.0 to v2.0.0 (Jose Celano)
01921ed fix: [#56] triggering recompilation on migration changes (Jose Celano)
cf09283 docs: [#56] update README for integration tests (Jose Celano)
d1059f5 feat: [#56] trasnfer user data from v1.0.0 to v2.0.0 (Jose Celano)
d590972 refactor: [#56] move upgrader from main upgrade mod to specific version upgrader mod (Jose Celano)
996c7d1 refactor: [#56] rename command al dirs (Jose Celano)
b92fb08 feat: [#56] transfer categories from db v1.0.0 to v2.0.0 (Jose Celano)
7513df0 refactor: add scaffolding for database migration command (Jose Celano)
5d6dec0 refactor: allow adding more binaries (Jose Celano)
c3414da feat: add target dir to .gitignore (Jose Celano)

Pull request description:

  Depends on: #87

  I've created the basic scaffolding of the command. My plan is:

  - Migrate all the table records.
  - Insert torrents in DB from `uploads` dir.

  Potential problems are:

  - AUTOINCREMENTS
  - Foreign keys

  **UPDATE 2022-11-30**

  ### Tasks

  - [x] Transfer `torrent_categories`
  - [x] Transfer `torrent_user`
  - [x] Allow login for the transferred users using the Pbkdf2 for hashing the password
  - [x] Use date and time in `today_iso8601` function instead of only the date
  - [x] Transfer `torrent_tracker_keys`
  - [x] Remove unused content from dir `upgrades`. We do not need the MySQL stuff for this migration. There was no MySQL support for version `v1.0.0`
  - [x] Transfer `torrent_torrents`. See below the subtasks for this task
  - [x] Transfer `torrent_torrent_files`. It was not used in `v1.0.0`
  - [x] Make `registration_date` optional and leave it empty for imported users
  - [x] Add a new DB field `imported_date` to the `torrust_users` table with the importation date for user records imported with this command
  - [x] Refactor
  - [x] Add automated tests
  - [x] Intensive testing
  - [x] Write documentation explaining how to upgrade
  - [x] Rename `destiny` DB to `target` DB.

  ### Subtasks from issue #84

  This rework is needed because @ldpr found a bug in the current version that affects the upgrader. We have to fix that bug and update the "upgrader".
  - [x] Fix issue #84
  - [x] Rework: remove the `torrust_torrent_tracker_stats::tracker_url` column. **DISCARDED**.
  - [x] Populate table `torrust_torrent_tracker_stats` with one record per torrent.

  ### Transfer torrents subtasks

  - [x] Fill table `torrust_torrents`
  - [x] Fill table `torrust_torrent_files`
  - [x] Fill table `torrust_torrent_announce_urls`
  - [x] Fill table `torrust_torrent_info`

  ### Testing subtasks

  Assertions for integration test:

  - [x] `torrust_users` table
  - [x] `torrust_user_authentication` table
  - [x] `torrust_user_profiles` table
  - [x] `torrust_tracker_keys` table
  - [x] `torrust_torrents` table
  - [x] `torrust_torrent_files` table
  - [x] `torrust_torrent_info` table
  - [x] `torrust_torrent_announce_urls` table

  Some cases to test:

  - [x] Torrent with `pieces`.

  Using a single tracker URL or multiple URLs:

  - [x] Torrent with `announce` (single tracker url).
  - [x] Torrent with `announce_list` (multiple tracker URLs).

  Containing one file or multiple files:

  - [x] Torrent with one file.
  - [x] Torrent with multiple files.

  Add tests for the application behaviour changed:

  - [x] User can use have the password hashed with "argon2id" or a "pbkdf2-sha256". Add a test for the "verify_password" function.

  Things I'm not going to test:

  - [ ] Torrent with MD5 checksum?. I do not know how to include this MD5 checksum in the torrent file.
  - [ ] Torrent with `root_hash` ([BEP-0030](https://www.bittorrent.org/beps/bep_0030.html)).
  - [ ] The user registration date can be null for users that have been imported. We do not test it for MySQL since the upgrader is only for SQLite.

ACKs for top commit:
  josecelano:
    ACK 5a7d875
  da2ce7:
    ACK 5a7d875

Tree-SHA512: 0f79cb36bc8fac5f0166fa55d6addbbb1a4096c2f2e0e1d898b5b694bf16877cdc572a9336a69d0afd7da8c075edc21d49f6a5d640b187163269e59fe8f37138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@da2ce7 da2ce7 da2ce7 approved these changes

@mickvandijke mickvandijke Awaiting requested review from mickvandijke

Assignees
No one assigned
Labels
None yet
Projects
Torrust Solution
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Endpoint to get a list of torrents does not return all torrents
2 participants
@josecelano @da2ce7