Skip to content

Overhaul core Tracker: review whitelist functionality (part 2) #1270

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Closed
6 tasks done
Tracked by #1181
josecelano opened this issue Feb 14, 2025 · 0 comments · Fixed by #1274
Closed
6 tasks done
Tracked by #1181

Overhaul core Tracker: review whitelist functionality (part 2) #1270

josecelano opened this issue Feb 14, 2025 · 0 comments · Fixed by #1274
Assignees
@josecelano
Labels
- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat

Comments

@josecelano
Copy link
Member

josecelano commented Feb 14, 2025 •
edited
Loading

Parent issue: #1261
Relates to:

This is second part of the implementation of this issue.

I decided to merge the PR for the issue #1268 before continuing working of the refactor.

Sub-tasks

  • In the tracker-core announce handler return a Result<AnnounceData, AnnounceError> when the torrent is not included in the whitelist.
  • In the tracker-core scrape handler return a Result<ScrapeData, ScrapeError> so we are able to return errors in the future without breaking the public API.
  • Move packages::udp_tracker_core::peer_builder::from_request to new package udp-protocol
  • Inline invoke function in handlers when possible.
  • Move tests for the handlers (announce and scrape) from HTTP server to http_tracker_core. *1
  • Move tests for the handlers (announce and scrape) from UDP server to udp_tracker_core. *1

NOTE*1: instead of moving the tests I will create new ones for the http_tracker_core after refactoring all packages. The tests contain logic from the delivery layer that it will not be moved to the http_tracker_core so It will be harder to refactor those tests than created new one for the logic that we will finally move to the http_tracker_core.
@josecelano josecelano added - Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat labels Feb 14, 2025
@josecelano josecelano self-assigned this Feb 14, 2025
This was referenced Feb 14, 2025
Merged
Closed
josecelano added a commit that referenced this issue Feb 14, 2025
@josecelano
Merge #1269: Overhaul core Tracker: review whitelist functionality
ec07b78 
eca5c59 refactor: [#1268] move scrape logic from udp server to udp_tracker_core package (Jose Celano)
c0fc390 refactor: [#1268] move announce logic from udp server to udp_tracker_core package (Jose Celano)
37a142e refactor: [#1268] move scrape logic from axum to http_tracker_core package (Jose Celano)
74815ab refactor: [#1268] move announce logic from axum to http_tracker_core package (Jose Celano)
e48aaf5 [#1268] move udp services to udp_tracker_core package (Jose Celano)
73753e3 [#1268] move http services to http_tracker_core package (Jose Celano)
dec742e refactor: [#1268] extract servers::udp::services::scrape service (Jose Celano)
3c07b26 refactor: [#1268] extract servers::udp::services::announce service (Jose Celano)
81825c9 refactor: [#1268] separate UDP handlers into diferent modules (Jose Celano)

Pull request description:

  Overhaul core Tracker: review whitelist functionality.

  ### Sub-tasks

  - [x] Introduce submodules for handlers in UDP: `servers::udp::handlers::{announce, scrape}`.
  - [x] Create the missing services (app layer) in the UDP tracker. There is no intermediary level between handlers and the core tracker. It will moved to its own package `udp-tracker-core` later.
  - [x] Move the service `services::announce::invoke()` in the HTTP tracker to the `http-tracker-core` package.
  - [x] Move the service `services::announce::invoke()` in the UDP tracker to the `udp-tracker-core` package.
  - [x] Move logic from the handler (in the framework level - delivery layer) to the application service in the `http-tracker-core` package.
    - [x] For the `announce` request
    - [x] For the `scrape` request
  - [x] Move logic from the handler (controller level - delivery layer) to the application service in the `udp-tracker-core` package.
    - [x] For the `announce` request
    - [x] For the `scrape` request
  - [ ] ~~Add version module also for the UDP tracker. I don't see any reason to use `v1` in the http tracker but not in the UDP tracker.~~ I will leave this until we introduce a new major version.

  ### Sub-tasks for a new PR

  I've left these tasks for a new [issue](#1270). This PR is just moving things and the new tasks imply changing function signatures.

  - [ ] In the tracker-core announce handler return a `Result<AnnounceData, AnnounceError>` when the torrent is not included in the whitelist.
  - [ ] In the tracker-core scrape handler return a `Result<ScrapeData, ScrapeError>` so we are able to return errors in the future without breaking the public API.

ACKs for top commit:
  josecelano:
    ACK eca5c59

Tree-SHA512: d3ee37ffa806e8a86813fe564e3840fab7bfc44d2072f85bc2eba84ac3402c95c0f6a5beb2725071cb0498415f55915431b656e98c71b3a6bf469de961c37f03
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 17, 2025
@josecelano
refactor: [torrust#1270] return error in tracker core announce handler
e40777f 
when the tracker is running in `listed` mode and the client is trying to
announce a torrents whose infohash is not whitelisted.
@josecelano josecelano linked a pull request Feb 17, 2025 that will close this issue
Overhaul core Tracker: review whitelist functionality (part 2) #1274
Merged
4 tasks
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 17, 2025
@josecelano
refactor: [torrust#1270] return errorin core announce and scrape handler
da1353b 
- In the announce handler, it returns an error when the tracker is
  running in `listed` mode and the infohash is not whitelisted. This was
done only in the delivery layers but not in the domain.
- In the scrape handler, it does not return any errors for now, but It
  will allow us in the future to return errors whithout making breaking
changes.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 17, 2025
@josecelano
refactor: [torrust#1270] extract fn to new package udp-protocol
dbee7ad
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 17, 2025
@josecelano
refactor: [torrust#1270] inline http tracker announce invoke fn
096d503
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 17, 2025
@josecelano
refactor: [torrust#1270] inline http tracker scrape invoke fn
6651343
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 17, 2025
@josecelano
refactor: [torrust#1270] inline udp tracker announce invoke fn
af28429
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 17, 2025
@josecelano
refactor: [torrust#1270] inline udp tracker scrape invoke fn
d49aebd
@josecelano josecelano mentioned this issue Feb 17, 2025
Closed
josecelano added a commit that referenced this issue Feb 17, 2025
@josecelano
Merge #1274: Overhaul core Tracker: review whitelist functionality (p…
467d50a 
…art 2)

d0e6936 refactor: simplify loop with map (Jose Celano)
d49aebd refactor: [#1270] inline udp tracker scrape invoke fn (Jose Celano)
af28429 refactor: [#1270] inline udp tracker announce invoke fn (Jose Celano)
6651343  refactor: [#1270] inline http tracker scrape invoke fn (Jose Celano)
096d503 refactor: [#1270] inline http tracker announce invoke fn (Jose Celano)
dbee7ad refactor: [#1270] extract fn to new package udp-protocol (Jose Celano)
da1353b refactor: [#1270] return errorin core announce and scrape handler (Jose Celano)

Pull request description:

  Overhaul core Tracker: review whitelist functionality (part 2).

  - [x] Return an error in the announce handler if the tracker is running in `listed` mode and the infohash is not whitelisted. This is already implemented in the higher delivery layer but it should be also validated in the tracker core (domain layer).
  - [x] Change scrape handler signature to return an error even if it's not needed now. That will allow us to return an error in the future whiteout breaking changes.
  - [x] Move `packages::udp_tracker_core::peer_builder::from_request` to new package `udp-protocol`
  - [x] Inline `invoke` function in handlers (`http-tracker-core` and `udp-tracker-core`) when possible.

ACKs for top commit:
  josecelano:
    ACK d0e6936

Tree-SHA512: b4e9356291b35b1cc6a20a5b28a6c0b9136baafbb9bc71638bd83932709970f3ee8bee91a81cc09a775244fd3e774131d238cf4f52920bcc141fed4161b86b03
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@josecelano josecelano

Labels
- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Overhaul core Tracker: review whitelist functionality (part 2) josecelano/torrust-tracker
1 participant
@josecelano