Skip to content

feat: download PRIDE Archive files over HTTPS + manifest helpers (stacked on #1088)#1093

Open
trishorts wants to merge 5 commits into
smith-chem-wisc:masterfrom
trishorts:pride-download
Open

feat: download PRIDE Archive files over HTTPS + manifest helpers (stacked on #1088)#1093
trishorts wants to merge 5 commits into
smith-chem-wisc:masterfrom
trishorts:pride-download

Conversation

@trishorts

Copy link
Copy Markdown
Contributor

Summary

Adds the "get the data" capability to the PRIDE Archive client: download a project's files to disk. Builds directly on the file-manifest MVP in #1088.

⚠️ Stacked on #1088. This branch is based on pride-client (#1088), which is not yet on master, so the diff below temporarily contains both commits. Review only the top commitfeat(usefulproteomicsdatabases): download PRIDE files over HTTPS + manifest helpers. Once #1088 merges, this diff self-reduces to the download changes alone.

The design decision: no FTP subsystem

PRIDE advertises each file's locations only as FTP (PRIDE:0000469) and Aspera (PRIDE:0000468) — there is no HTTPS location field (verified live against PXD012345 and PXD000001). The naive path would be a System.Net.FtpWebRequest downloader, but that API is deprecated and mzLib has no FTP code today.

Instead: PRIDE's FTP host ftp.pride.ebi.ac.uk also serves the identical path over HTTPS (verified: HEAD200, Content-Length, Accept-Ranges: bytes). So the client scheme-upgrades ftp://https:// and downloads through the existing reused HttpClient. The result:

  • ✅ works on real PRIDE data (which is FTP/Aspera-only),
  • no new dependency, no deprecated FTP code,
  • ✅ reuses the client's Koina-style HttpClient (success-checked, streamed).

API

  • PrideArchiveClient.DownloadFileAsync(file, destinationDirectory, overwrite = true, ct) — downloads one file, returns the written path. Streams to a .partial sibling and moves it into place only on success (no truncated files on interruption). overwrite = false skips an existing file (cheap resume).
  • PrideArchiveClient.DownloadProjectFilesAsync(accession, destinationDirectory, filter = null, ...) — manifest → optional filter → download each.
  • PrideArchiveExtensions (new): TryGetHttpsDownloadUrl / GetHttpsDownloadUrl (explicit HTTPS → FTP-upgraded-to-HTTPS → NotSupportedException for Aspera-only), plus WhereCategory / WhereExtension / TotalSizeBytes manifest helpers.

Tests

  • 17 offline tests via a stub HttpMessageHandler: FTP→HTTPS resolution, explicit-HTTPS preference, Aspera-only rejection, category/extension filters, size aggregation, overwrite policy, non-success (+ no leftover .partial), and filtered project download.
  • Live canary PrideArchiveDownloadLiveTests under [Category("ExternalService")] / [Category("Pride")], routed through ExternalServiceTestHelper so an EBI outage skips while a contract break fails. Downloads the smallest file of a real project end-to-end.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.24771% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.47%. Comparing base (77fb7db) to head (3cc184c).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...sefulProteomicsDatabases/PrideArchiveExtensions.cs 94.91% 0 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1093      +/-   ##
==========================================
+ Coverage   91.46%   91.47%   +0.01%     
==========================================
  Files         427      428       +1     
  Lines       52777    52886     +109     
  Branches     6325     6356      +31     
==========================================
+ Hits        48273    48379     +106     
  Misses       3268     3268              
- Partials     1236     1239       +3     
Files with missing lines Coverage Δ
...ib/UsefulProteomicsDatabases/PrideArchiveClient.cs 100.00% <100.00%> (ø)
...sefulProteomicsDatabases/PrideArchiveExtensions.cs 94.91% <94.91%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

trishorts added a commit to trishorts/mzLib that referenced this pull request Jul 10, 2026
…ownload

Address the 9 findings from the PR smith-chem-wisc#1093 review:

- DownloadFileAsync reduces the server-supplied FileName to its leaf
  (Path.GetFileName) and throws if it differs, so a "../" or rooted name
  can no longer escape the destination directory via Path.Combine.
- Restrict the ftp:// -> https:// scheme-upgrade to ftp.pride.ebi.ac.uk,
  and select the FTP location by a usable ftp:// value so a malformed
  accession-tagged entry can't shadow a valid one elsewhere in the list.
- WhereExtension throws when every supplied extension is blank instead of
  silently matching zero files.
- Add offline tests: path-traversal rejection, non-PRIDE-host and
  shadowing resolution, blank-extension guard, null-filter and zero-match
  project download, filtered-download content assertion, pre-cancelled and
  mid-stream-fault partial cleanup, and extensionless/double-extension/
  null-category helper edge cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016y1PW7eLKHQMHw7Ymp9p9p
trishorts added a commit to trishorts/mzLib that referenced this pull request Jul 10, 2026
…ownload

Address the 9 findings from the PR smith-chem-wisc#1093 review:

- DownloadFileAsync reduces the server-supplied FileName to its leaf
  (Path.GetFileName) and throws if it differs, so a "../" or rooted name
  can no longer escape the destination directory via Path.Combine.
- Restrict the ftp:// -> https:// scheme-upgrade to ftp.pride.ebi.ac.uk,
  and select the FTP location by a usable ftp:// value so a malformed
  accession-tagged entry can't shadow a valid one elsewhere in the list.
- WhereExtension throws when every supplied extension is blank instead of
  silently matching zero files.
- Add offline tests: path-traversal rejection, non-PRIDE-host and
  shadowing resolution, blank-extension guard, null-filter and zero-match
  project download, filtered-download content assertion, pre-cancelled and
  mid-stream-fault partial cleanup, and extensionless/double-extension/
  null-category helper edge cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016y1PW7eLKHQMHw7Ymp9p9p
trishorts and others added 3 commits July 10, 2026 12:35
…nifest helpers

Adds capability #1 (get the data) to the PRIDE Archive client, building on the
merged manifest MVP (smith-chem-wisc#1088).

PRIDE advertises files only as FTP/Aspera locations, but its FTP host
(ftp.pride.ebi.ac.uk) serves the identical path over HTTPS. So rather than
introduce a deprecated System.Net.FtpWebRequest subsystem (mzLib has no FTP code),
the download scheme-upgrades an ftp:// location to https:// and streams it through
PrideArchiveClient's reused HttpClient (EnsureSuccessStatusCode, streamed to a
".partial" file then moved into place). No new dependency; works on real PRIDE data.

- PrideArchiveClient.DownloadFileAsync(file, dir, overwrite = true, ct): returns the
  written path; overwrite=false skips an existing file (cheap resume).
- PrideArchiveClient.DownloadProjectFilesAsync(accession, dir, filter, ...): manifest
  -> filter -> download; threads the CancellationToken through the manifest fetch.
- PrideArchiveExtensions (new): TryGetHttpsDownloadUrl / GetHttpsDownloadUrl (HTTPS
  preferred, else FTP upgraded, else NotSupportedException for Aspera-only), plus
  WhereCategory / WhereExtension / TotalSizeBytes manifest helpers.
- Tests: 17 offline (stub HttpMessageHandler) covering resolution, filters, overwrite
  policy, non-success, Aspera-only, and filtered project download; live canary under
  [Category("ExternalService")]/[Category("Pride")], skip-on-outage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ownload

Address the 9 findings from the PR smith-chem-wisc#1093 review:

- DownloadFileAsync reduces the server-supplied FileName to its leaf
  (Path.GetFileName) and throws if it differs, so a "../" or rooted name
  can no longer escape the destination directory via Path.Combine.
- Restrict the ftp:// -> https:// scheme-upgrade to ftp.pride.ebi.ac.uk,
  and select the FTP location by a usable ftp:// value so a malformed
  accession-tagged entry can't shadow a valid one elsewhere in the list.
- WhereExtension throws when every supplied extension is blank instead of
  silently matching zero files.
- Add offline tests: path-traversal rejection, non-PRIDE-host and
  shadowing resolution, blank-extension guard, null-filter and zero-match
  project download, filtered-download content assertion, pre-cancelled and
  mid-stream-fault partial cleanup, and extensionless/double-extension/
  null-category helper edge cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016y1PW7eLKHQMHw7Ymp9p9p
… filter branches

Extend the PRIDE manifest-helper tests to exercise the remaining
partial branches flagged by patch coverage:

- WhereExtension over a null sequence throws ArgumentNullException.
- WhereCategory / WhereExtension skip a null file element in the
  sequence without a NullReferenceException.
- TryGetHttpsDownloadUrl skips a null location entry.

Lifts PrideArchiveExtensions.cs patch coverage to 100% (overall 98.65%).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016y1PW7eLKHQMHw7Ymp9p9p
trishorts and others added 2 commits July 11, 2026 07:53
…me skip

Address the four remaining findings from the PR smith-chem-wisc#1093 re-review:

- DownloadFileAsync now also rejects a bare "." or ".." FileName.
  Path.GetFileName returns these unchanged, so they slipped past the
  leaf-name guard despite the documented "no '..'" contract.
- Move the overwrite==false && File.Exists short-circuit ahead of
  GetHttpsDownloadUrl and Directory.CreateDirectory, so a resume
  (overwrite: false) skips an already-present file that has no HTTPS
  location (e.g. Aspera-only) instead of throwing, and does no needless
  URL resolution or filesystem work.
- Fix the path-traversal test to assert against the real escape target
  derived from the fileName parameter (parent dir for "../evil.raw", a
  nested subdir for "sub/dir/file.raw") rather than a fixed
  _tempDir/evil.raw that neither case ever occupies; add "." and ".." cases.
- Exercise TryGetHttpsDownloadUrl(out ...) in the explicit-HTTPS-preference
  test, and add a resume-skip test for an already-present Aspera-only file.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@trishorts trishorts requested review from nbollis and pcruzparri July 13, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants