[BREAKING CHANGE] Implement remaining torrent-get fields #92
Conversation
BREAKING CHANGE: The existing Torrent `_date` fields are now parsed into
`DateTime<Utc>` from `i64`. Invalid or missing `_date` fields are parsed
into `DateTime::UNIX_EPOCH`.
The affected `Torrent` fields are:
* activity_date
* added_date
* done_date
* edit_date
-----
This commit implements the remaining `torrent-get` fields through
transmission version `4.1.0` (rpc-version-semver: `5.4.0`, rpc-version:
`18`). Unless I missed something, the only missing fields should now be
`files.beginPiece` and `files.endPiece`.
* The `pieces` implementation adds a dependency on `base64`
(https://docs.rs/base64/).
* Exposes `IdleMode` and `RatioMode` enums which are the same but are
defined as separate enums in transmission itself
(libtransmission/transmission.h: `tr_idlelimit` and `tr_ratiolimit`).
I've opted to mirror that here instead of combining them.
* The unit tests are effectively tests against transmission rpc response
deserialization. Notably, they do not test against a malformed
response.
* The unit tests for `availability` and `trackerList` are `#[ignore]`d
because I don't have a way to get valid test data for them.
src/types/response.rs
Outdated
| pub files: Option<Vec<File>>, | ||
| /// for each file in files, whether or not they will be downloaded (0 or 1) | ||
| pub wanted: Option<Vec<i8>>, | ||
| pub wanted: Option<Vec<i8>>, // TODO: Deserialize from bool -> i8 (or u8 maybe?) to account for |
There was a problem hiding this comment.
Would be nice if you could do the i8 -> bool conversion
There was a problem hiding this comment.
The rpc spec (scroll up a bit) says:
wanted: An array oftr_torrentFileCount()0/1, 1 (true) if the corresponding file is to be downloaded. (Source:tr_file_view)Note: For backwards compatibility, in
4.x.x, wanted is serialized as an array of0or1that should be treated as booleans. This will be fixed in5.0.0to return an array of booleans.
I'm not sure how to implement deserialization from two different types. I'll look into it.
edit: Done, but adds a dependency on serde_json (which was just a dev-dependency before).
| pub activity_date: Option<i64>, | ||
| pub added_date: Option<i64>, | ||
| #[serde(deserialize_with = "from_ts_option", default)] | ||
| pub activity_date: Option<DateTime<Utc>>, |
There was a problem hiding this comment.
No, it converts any timestamp <= 0 to Some(_).
In fn from_ts_option (src/types/response.rs):
if ts <= 0 {
return Ok(Some(DateTime::UNIX_EPOCH));
}My undestanding is that the Torrent struct deserializes any field not requested to None, so anything that is requested (and exists in the response) should be deserialized to Some(_).
Should it convert to None if -1? What about 0?
From what I recall when writing this, transmission 3.00 treats unset/invalid timestamp fields as 0 and -1 interchangeably (manualAnnounceTime was the only field I found that returned a -1 whereas the others were 0 when unset/invalid).
If these cases should be None, users then cannot differentiate between a Torrent that was requested with an unset/invalid timestamp field (like TorrentGetField::ManualAnnounceTime for a torrent that has never been manually announced) and one where that field was not requested, does that matter?
If 0 should also be None, then the unlikely case where a Torrent's timestamp field is actually 0 (UNIX_EPOCH) is unaccounted for.
|
Much appreciated for the PR @lv10wizard. Do you think you could address my questions? Otherwise, I'm more than happy to accept the change and release it ASAP |
It's possible the type needs to be larger (i32 or maybe i64 even).
BREAKING CHANGE: Torrent.wanted type change from `Vec<i8>` to `Wanted` which is a simple wrapper around `Vec<bool>`. The `Wanted` type, like `Pieces` implements `Deref` resolving to `&Vec<_>`. * Adds a dependency on `serde_json` (which was solely a dev-dependency prior to this commit).
|
I've fixed I just happened to catch that this field can be negative in the rpcs docs. I may have wrongly typed some of these other fields implemented in this PR that I can't test. |
Users probably won't need to use or reference these types explicitly but exporting them makes the documentation link to their definitions which can be useful.
Adds `begin_piece` and `end_piece` to the Torrent files' subtype `File`. They are wrapped in `Option` because the fields are not present in transmission versions less than `4.1.0`. This should bring `torrent-get` up to the currently defined rpc spec (transmission 4.1.0, rpc-version: 18).
This splits out the types that users probably won't need to explicitly interact with into a separate export block 1) for documentation and 2) as a visual distinction.
* Adds feature flag documentation
|
Hey @lv10wizard, thanks for your effort, and sorry for the long delays in my answers. The PR looks great, but the linter is not very happy. Please format your changes, and CI should pass. |
No problem. There's no rush. |
Hi,
I've implemented (in a breaking way) the remaining
torrent-getfields specified in the rpc through to transmission version4.1.0(rpc-version-semver5.4.0, rpc-version18), though I've missed(edit:files.beginPieceandfiles.endPiecefiles.beginPieceandfiles.endPieceare implemented in 3e0d96f).This should resolve the less useful
torrent-gethalf of #81 (sequentialDownload)Breaking Change
Existing
Torrent.*_datefields are no longer deserialized asi64but ratherDateTime<Utc>. These fields are:activity_dateadded_datedone_dateedit_dateInvalid
torrent-gettimestamp rpc responses (which seem to be either0or-1) are parsed intoDateTime::UNIX_EPOCH. For example, this can occur forstart_datefor a torrent that has never been started ordone_dateif a torrent has yet to complete.I can change these back and deserialize the new timestamp fields as
i64if you prefer, but I think it better if we parse these into a relevant type especially sinceTrackerStat.*_timefields already do this.Maybe some form of deprecation would be better?
Other Notes
piecesimplementation adds a dependency on base64IdleModeandRatioModeenums are identical which mirrors transmission's implementation of the corresponding types (tr_idlelimitandtr_ratiolimit)availabilityandtrackerListare ignored/skipped because I don't have any valid data to test against.