feat: add typed torrent sources#250
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR introduces a typed ChangesTyped torrent source
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Engine
participant TorrentSource
participant reqwest
Caller->>Engine: add_torrent(TorrentSource::...)
Engine->>TorrentSource: into_metainfo()
alt Remote URL
TorrentSource->>TorrentSource: validate http/https scheme
TorrentSource->>reqwest: get(url)
reqwest-->>TorrentSource: bytes
end
TorrentSource-->>Engine: MetaInfo or EngineError
Engine-->>Caller: Torrent or EngineError
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
crates/libtortillas/src/engine/mod.rs (2)
48-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the stable facade export explicit.
pub use source::*makes future public items insource.rspart of the engine facade unintentionally. Re-export only the intended stable API item.Proposed change
-pub use source::*; +pub use source::TorrentSource;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/engine/mod.rs` at line 48, The engine facade is re-exporting everything from source with pub use source::*, which can accidentally expose future internals; update mod.rs to re-export only the intended stable API item from the source module, using the relevant source symbol explicitly so the public surface stays controlled.
295-332: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a facade-level magnet success test.
The engine facade tests cover file-path success and typed remote mismatch, but not the documented magnet path through
Engine::add_torrent.Proposed test addition
use crate::{ engine::{Engine, TorrentSource}, errors::EngineError, - testing::{BIG_BUCK_BUNNY_INFO_HASH, BIG_BUCK_BUNNY_TORRENT_FILE, torrent_fixture_path}, + testing::{ + BIG_BUCK_BUNNY_INFO_HASH, BIG_BUCK_BUNNY_MAGNET, BIG_BUCK_BUNNY_TORRENT_FILE, + torrent_fixture_path, + }, }; @@ async fn engine_when_torrent_source_is_file_path_then_adds_torrent() { let engine = Engine::builder().autostart(false).build(); let source = TorrentSource::torrent_file_path(torrent_fixture_path(BIG_BUCK_BUNNY_TORRENT_FILE)); @@ assert_eq!(export.torrents.len(), 1); } + + #[tokio::test] + async fn engine_when_torrent_source_is_magnet_uri_then_adds_torrent() { + let engine = Engine::builder().autostart(false).build(); + let source = TorrentSource::magnet(BIG_BUCK_BUNNY_MAGNET); + + let torrent = engine.add_torrent(source).await.unwrap(); + let export = engine.export().await.unwrap(); + + assert_eq!(torrent.info_hash().to_hex(), BIG_BUCK_BUNNY_INFO_HASH); + assert_eq!(export.torrents.len(), 1); + } #[tokio::test] async fn engine_when_torrent_source_type_does_not_match_then_returns_typed_error() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/engine/mod.rs` around lines 295 - 332, Add a facade-level success test for the magnet path in the tests module under Engine. The current Engine::add_torrent coverage only verifies file-path success and a remote-source type mismatch, so add a new async tokio test that builds Engine with autostart(false), creates a TorrentSource via the magnet constructor, calls add_torrent, and asserts the returned torrent has the expected info hash and appears in Engine::export. Use the existing Engine, TorrentSource, and BIG_BUCK_BUNNY_INFO_HASH symbols to keep the new test aligned with the current facade-level coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/libtortillas/src/engine/mod.rs`:
- Line 248: The remote metainfo fetch path used by add_torrent is unbounded
because source.into_metainfo() can reach metainfo_from_remote_url, which still
relies on an un-timed reqwest::get call. Update the metainfo fetch flow in the
engine/mod.rs path so the request is executed with the existing client timeout
configuration, and make metainfo_from_remote_url use a timeout-aware client or
request builder instead of bare reqwest::get. Keep the fix localized around
source.into_metainfo(), metainfo_from_remote_url, and the add_torrent call chain
so stalled endpoints fail promptly instead of hanging indefinitely.
In `@crates/libtortillas/src/engine/source.rs`:
- Around line 95-98: The MagnetUri parsing in source handling is still mapping
parse failures to MetaInfoDeserializeError, which loses the typed magnet error
path. Update the error mapping in the MagnetUri::parse branch inside the source
parsing flow to return the dedicated typed magnet error variant instead of
MetaInfoDeserializeError, while keeping the existing logging of the malformed
URI and parse error. Make sure the fix is applied in the code path around
MagnetUri::parse and the EngineError handling used by the source parser.
- Around line 201-214: The current test only covers the invalid-scheme failure
path for TorrentSource::remote_torrent_url and does not verify the happy path.
Add a new async test alongside
torrent_source_when_remote_url_is_not_http_then_returns_typed_error that spins
up a local HTTP/HTTPS server serving a real .torrent file, constructs a
remote_torrent_url from that URL, and asserts into_metainfo() returns success.
Keep the existing invalid-scheme test intact and reuse the TorrentSource and
into_metainfo symbols so the new coverage clearly validates the remote download
path.
- Around line 118-123: The remote torrent fetch in `Source::download` currently
uses `reqwest::get(...).bytes()`, which buffers the entire response without
enforcing status checks, timeouts, or a maximum size. Update this path to use a
`reqwest::Client`-based request with a timeout, verify the response is
successful before reading, and stream/read the body with a hard size cap before
converting it into `torrent_file_bytes`. Keep the fix localized around the
existing `reqwest::get`, `EngineError::MetaInfoFetchError`, and
`torrent_file_bytes` handling.
---
Nitpick comments:
In `@crates/libtortillas/src/engine/mod.rs`:
- Line 48: The engine facade is re-exporting everything from source with pub use
source::*, which can accidentally expose future internals; update mod.rs to
re-export only the intended stable API item from the source module, using the
relevant source symbol explicitly so the public surface stays controlled.
- Around line 295-332: Add a facade-level success test for the magnet path in
the tests module under Engine. The current Engine::add_torrent coverage only
verifies file-path success and a remote-source type mismatch, so add a new async
tokio test that builds Engine with autostart(false), creates a TorrentSource via
the magnet constructor, calls add_torrent, and asserts the returned torrent has
the expected info hash and appears in Engine::export. Use the existing Engine,
TorrentSource, and BIG_BUCK_BUNNY_INFO_HASH symbols to keep the new test aligned
with the current facade-level coverage.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 691e3510-a7c0-4055-8536-1b42f76565e7
📒 Files selected for processing (3)
crates/libtortillas/src/engine/mod.rscrates/libtortillas/src/engine/source.rscrates/libtortillas/src/errors.rs
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
1 participant
Summary
TorrentSourcevariants for magnet URIs, local torrent paths, loaded torrent bytes, and remote torrent URLs.Engine::add_torrentto accept typed sources instead of guessing from arbitrary strings.Closes #224
Part of #221
Testing
cargo nextest run -p libtortillas torrent_sourcecargo nextest run -p libtortillas engine_when_torrent_sourcecargo nextest run --workspacecargo test --doc -p libtortillascargo clippy --workspace --all-targets -- -D warningsIgnored network tests were listed with
cargo nextest list --workspace --run-ignored ignored-only; they were not run because this PR only changes typed source validation and local metainfo parsing, while the live tracker/peer behavior remains delegated to existing network code.Summary by CodeRabbit
New Features
Bug Fixes
Documentation