fix: send missing multipart form data fields in create endpoints#16
Conversation
TorrentsClient.CreateTorrentAsync and AsyncCreateTorrentAsync were not sending AddOnlyIfCached. UsenetClient.CreateUsenetDownloadAsync and AsyncCreateUsenetDownloadAsync were not sending AsQueued or AddOnlyIfCached. All four fields are defined on the request models but were silently dropped when building the multipart content. Adds 4 unit tests to verify these fields are included. Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/7ba06d25-78c9-4732-a6c2-e40dd7145ac6 Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
Cover both true/false values for AddOnlyIfCached and AsQueued fields as suggested by code review. Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/7ba06d25-78c9-4732-a6c2-e40dd7145ac6 Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes missing multipart form-data fields so TorBox “create” endpoints actually receive add_only_if_cached (torrents + usenet) and as_queued (usenet), aligning the request models with what gets sent over the wire.
Changes:
- Add
add_only_if_cachedto multipart content inTorrentsClient.CreateTorrentAsyncandTorrentsClient.AsyncCreateTorrentAsync. - Add
as_queuedandadd_only_if_cachedto multipart content inUsenetClient.CreateUsenetDownloadAsyncandUsenetClient.AsyncCreateUsenetDownloadAsync. - Add unit tests asserting the new multipart fields are present.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/TorBoxSDK/Main/Torrents/TorrentsClient.cs |
Sends add_only_if_cached in multipart for both torrent create endpoints. |
src/TorBoxSDK/Main/Usenet/UsenetClient.cs |
Sends as_queued + add_only_if_cached in multipart for both usenet create endpoints. |
tests/TorboxSDK.UnitTests/Main/Torrents/TorrentsClientTests.cs |
Adds coverage for add_only_if_cached being included in multipart requests. |
tests/TorboxSDK.UnitTests/Main/Usenet/UsenetClientTests.cs |
Adds coverage for as_queued/add_only_if_cached being included in multipart requests. |
| Assert.Contains("as_queued", handler.LastRequestContent); | ||
| Assert.Contains("add_only_if_cached", handler.LastRequestContent); |
There was a problem hiding this comment.
These assertions only check that the multipart body contains the field names, but don't verify that the serialized values match the asQueued / addOnlyIfCached inputs. Adding value assertions (ideally tied to each field name) would better protect against regressions like hardcoding the value or swapping the two fields.
| Assert.Contains("as_queued", handler.LastRequestContent); | |
| Assert.Contains("add_only_if_cached", handler.LastRequestContent); | |
| string multipartContent = handler.LastRequestContent; | |
| System.Text.RegularExpressions.Match asQueuedMatch = System.Text.RegularExpressions.Regex.Match( | |
| multipartContent, | |
| "name=\"as_queued\"\\r\\n\\r\\n(?<value>[^\\r\\n]+)"); | |
| System.Text.RegularExpressions.Match addOnlyIfCachedMatch = System.Text.RegularExpressions.Regex.Match( | |
| multipartContent, | |
| "name=\"add_only_if_cached\"\\r\\n\\r\\n(?<value>[^\\r\\n]+)"); | |
| Assert.True(asQueuedMatch.Success); | |
| Assert.True(addOnlyIfCachedMatch.Success); | |
| Assert.Equal(asQueued.ToString().ToLowerInvariant(), asQueuedMatch.Groups["value"].Value.ToLowerInvariant()); | |
| Assert.Equal(addOnlyIfCached.ToString().ToLowerInvariant(), addOnlyIfCachedMatch.Groups["value"].Value.ToLowerInvariant()); |
| Assert.Contains("as_queued", handler.LastRequestContent); | ||
| Assert.Contains("add_only_if_cached", handler.LastRequestContent); |
There was a problem hiding this comment.
These assertions only check that the multipart body contains the field names, but don't verify that the serialized values match the asQueued / addOnlyIfCached inputs. Adding value assertions (ideally tied to each field name) would better protect against regressions like hardcoding the value or swapping the two fields.
| Assert.Contains("as_queued", handler.LastRequestContent); | |
| Assert.Contains("add_only_if_cached", handler.LastRequestContent); | |
| string expectedAsQueuedContent = $"name=\"as_queued\"\r\n\r\n{asQueued.ToString().ToLowerInvariant()}"; | |
| string expectedAddOnlyIfCachedContent = $"name=\"add_only_if_cached\"\r\n\r\n{addOnlyIfCached.ToString().ToLowerInvariant()}"; | |
| Assert.Contains(expectedAsQueuedContent, handler.LastRequestContent); | |
| Assert.Contains(expectedAddOnlyIfCachedContent, handler.LastRequestContent); |
| // --- AsyncCreateTorrentAsync --- | ||
|
|
||
| [Theory] | ||
| [InlineData(true)] | ||
| [InlineData(false)] | ||
| public async Task CreateTorrentAsync_WithAddOnlyIfCached_IncludesInMultipartContent(bool addOnlyIfCached) | ||
| { |
There was a problem hiding this comment.
This new CreateTorrentAsync_* test is placed under the // --- AsyncCreateTorrentAsync --- section header, which makes the file harder to navigate (the other CreateTorrentAsync_* tests are earlier in the file). Consider moving this test next to the existing CreateTorrentAsync tests and/or introducing a // --- CreateTorrentAsync --- section header so the grouping matches the method under test.
|
@copilot apply changes based on the comments in this thread |
- Move CreateTorrentAsync_WithAddOnlyIfCached test next to other CreateTorrentAsync tests (was under AsyncCreateTorrentAsync section) - Verify actual multipart field values (name + value) instead of just checking field names are present Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/d2932ac3-0ee5-49bf-a81f-2af39c1fcdf8 Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
Applied all three review comments in
|
TorrentsClient.CreateTorrentAsync— add missingadd_only_if_cachedfield to multipart contentTorrentsClient.AsyncCreateTorrentAsync— add missingadd_only_if_cachedfield to multipart contentUsenetClient.CreateUsenetDownloadAsync— add missingas_queuedandadd_only_if_cachedfields to multipart contentUsenetClient.AsyncCreateUsenetDownloadAsync— add missingas_queuedandadd_only_if_cachedfields to multipart contentCreateTorrentAsync_WithAddOnlyIfCachedtest next to otherCreateTorrentAsynctestsCreateUsenetDownloadAsyncAsyncCreateUsenetDownloadAsync