Skip to content

Commit 643a10d

Browse files
trishortsclaude
andcommitted
feat(usefulproteomicsdatabases): thread CancellationToken through GetProjectFilesAsync
Address the actionable items from the PR #1088 review (nbollis): - GetProjectFilesAsync accepts an optional CancellationToken, checks it per page, and passes it to GetAsync / ReadAsStringAsync so a long pagination sequence can be cancelled cooperatively. - seal PrideArchiveClient (it owns a single HttpClient and is not a base type), which is the intended resolution of the non-virtual-Dispose note. - Add <inheritdoc/> to Dispose(). - Tests: double Dispose is idempotent; an injected HttpClient survives the client's Dispose and stays usable; an injected HttpClient with no BaseAddress gets the PRIDE default; pageSize validation now covers a negative value; MaxPages cap is exercised at edge values 0 and 1. The remaining review findings (Newtonsoft coupling, IHttpClientFactory, IJsonSerializer/IPrideArchiveClient abstractions, thread-safe dispose, User-Agent) were declined as inconsistent with mzLib conventions and resolved on the PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016y1PW7eLKHQMHw7Ymp9p9p
1 parent a4a629a commit 643a10d

2 files changed

Lines changed: 55 additions & 7 deletions

File tree

mzLib/Test/DatabaseTests/PrideArchiveClientTests.cs

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,11 +202,12 @@ public void GetProjectFilesAsync_BlankAccession_ThrowsArgumentException()
202202
Assert.That(async () => await client.GetProjectFilesAsync(" "), Throws.ArgumentException);
203203
}
204204

205-
[Test]
206-
public void GetProjectFilesAsync_NonPositivePageSize_ThrowsArgumentOutOfRange()
205+
[TestCase(0)]
206+
[TestCase(-1)]
207+
public void GetProjectFilesAsync_NonPositivePageSize_ThrowsArgumentOutOfRange(int pageSize)
207208
{
208209
using var client = new PrideArchiveClient(new HttpClient(new StubHandler(_ => JsonResponse("[]"))));
209-
Assert.That(async () => await client.GetProjectFilesAsync("PXD012345", 0), Throws.InstanceOf<ArgumentOutOfRangeException>());
210+
Assert.That(async () => await client.GetProjectFilesAsync("PXD012345", pageSize), Throws.InstanceOf<ArgumentOutOfRangeException>());
210211
}
211212

212213
[Test]
@@ -231,6 +232,47 @@ public void DefaultConstructor_And_Dispose_DoNotThrow()
231232
var client = new PrideArchiveClient();
232233
Assert.That(() => client.Dispose(), Throws.Nothing);
233234
}
235+
236+
[Test]
237+
public void Dispose_CalledTwice_DoesNotThrow()
238+
{
239+
var client = new PrideArchiveClient(new HttpClient(new StubHandler(_ => JsonResponse("[]"))));
240+
client.Dispose();
241+
Assert.That(() => client.Dispose(), Throws.Nothing); // idempotent
242+
}
243+
244+
[Test]
245+
public void Dispose_InjectedHttpClient_IsNotDisposed_AndRemainsUsable()
246+
{
247+
var handler = new StubHandler(_ => JsonResponse("[]"));
248+
var httpClient = new HttpClient(handler) { BaseAddress = new Uri(PrideArchiveClient.DefaultBaseAddress) };
249+
var client = new PrideArchiveClient(httpClient); // caller retains ownership of httpClient
250+
251+
client.Dispose();
252+
253+
// the injected HttpClient must survive the client's Dispose (a disposed client would throw here)
254+
Assert.That(async () => await httpClient.GetAsync("projects/x/files"), Throws.Nothing);
255+
httpClient.Dispose();
256+
}
257+
258+
[Test]
259+
public void Constructor_InjectedHttpClientWithoutBaseAddress_SetsPrideDefault()
260+
{
261+
var httpClient = new HttpClient(new StubHandler(_ => JsonResponse("[]"))); // no BaseAddress
262+
using var client = new PrideArchiveClient(httpClient);
263+
Assert.That(httpClient.BaseAddress, Is.EqualTo(new Uri(PrideArchiveClient.DefaultBaseAddress)));
264+
}
265+
266+
[TestCase(0)]
267+
[TestCase(1)]
268+
public void GetProjectFilesAsync_ServerIgnoresPaging_LowMaxPages_Throws(int maxPages)
269+
{
270+
// every page is full and carries no total_records header, so only the MaxPages cap can stop it
271+
var handler = new StubHandler(_ => JsonResponse(Array(FileJson("a"), FileJson("b"))));
272+
using var client = new PrideArchiveClient(new HttpClient(handler)) { MaxPages = maxPages };
273+
Assert.That(async () => await client.GetProjectFilesAsync("PXD012345", pageSize: 2),
274+
Throws.InstanceOf<HttpRequestException>());
275+
}
234276
}
235277

236278
/// <summary>

mzLib/UsefulProteomicsDatabases/PrideArchiveClient.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Net.Http;
4+
using System.Threading;
45
using System.Threading.Tasks;
56
using Newtonsoft.Json;
67

@@ -16,7 +17,7 @@ namespace UsefulProteomicsDatabases
1617
/// per unit of work and dispose it. A constructor overload accepts an <see cref="HttpClient"/> to
1718
/// support testing and custom configuration.
1819
/// </remarks>
19-
public class PrideArchiveClient : IDisposable
20+
public sealed class PrideArchiveClient : IDisposable
2021
{
2122
/// <summary>The base address of the PRIDE Archive REST API (v3).</summary>
2223
public const string DefaultBaseAddress = "https://www.ebi.ac.uk/pride/ws/archive/v3/";
@@ -65,14 +66,17 @@ private PrideArchiveClient(HttpClient httpClient, bool ownsHttpClient)
6566
/// </summary>
6667
/// <param name="accession">The PRIDE project accession, e.g. "PXD012345".</param>
6768
/// <param name="pageSize">Files requested per page (default 100). The full manifest is returned regardless.</param>
69+
/// <param name="cancellationToken">Cancels the (possibly multi-page) fetch.</param>
6870
/// <returns>
6971
/// The project's files. Empty if the project has no files or the accession is unknown (PRIDE
7072
/// returns an empty result for an unknown accession). Never null.
7173
/// </returns>
7274
/// <exception cref="ArgumentException">The accession is null, empty, or whitespace.</exception>
7375
/// <exception cref="ArgumentOutOfRangeException">The page size is not positive.</exception>
7476
/// <exception cref="HttpRequestException">The API returned a non-success status code.</exception>
75-
public async Task<List<PrideArchiveFile>> GetProjectFilesAsync(string accession, int pageSize = 100)
77+
/// <exception cref="OperationCanceledException">The operation was cancelled via <paramref name="cancellationToken"/>.</exception>
78+
public async Task<List<PrideArchiveFile>> GetProjectFilesAsync(string accession, int pageSize = 100,
79+
CancellationToken cancellationToken = default)
7680
{
7781
if (string.IsNullOrWhiteSpace(accession))
7882
throw new ArgumentException("A PRIDE project accession is required.", nameof(accession));
@@ -84,14 +88,15 @@ public async Task<List<PrideArchiveFile>> GetProjectFilesAsync(string accession,
8488

8589
while (true)
8690
{
91+
cancellationToken.ThrowIfCancellationRequested();
8792
string requestUri = $"projects/{Uri.EscapeDataString(accession)}/files?pageSize={pageSize}&page={page}";
88-
using HttpResponseMessage response = await _httpClient.GetAsync(requestUri).ConfigureAwait(false);
93+
using HttpResponseMessage response = await _httpClient.GetAsync(requestUri, cancellationToken).ConfigureAwait(false);
8994

9095
if (!response.IsSuccessStatusCode)
9196
throw new HttpRequestException(
9297
$"PRIDE Archive request failed with status {(int)response.StatusCode} {response.ReasonPhrase} for '{requestUri}'.");
9398

94-
string content = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
99+
string content = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
95100
List<PrideArchiveFile> pageFiles =
96101
JsonConvert.DeserializeObject<List<PrideArchiveFile>>(content, JsonSettings) ?? new List<PrideArchiveFile>();
97102

@@ -131,6 +136,7 @@ private static bool TryGetTotalRecords(HttpResponseMessage response, out long to
131136
return false;
132137
}
133138

139+
/// <inheritdoc/>
134140
public void Dispose()
135141
{
136142
if (_disposed)

0 commit comments

Comments
 (0)