Skip to content

NDI Cloud file CRC-32: lookupFiles, checksummed uploads, crc32 in file listings - #1025

Merged
stevevanhooser merged 4 commits into
mainfrom
claude/file-crc32-lookup
Oct 13, 2026
Merged

stevevanhooser merged 4 commits into
mainfrom
claude/file-crc32-lookup

Conversation

@stevevanhooser

@stevevanhooser stevevanhooser commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Client side of Waltham-Data-Science/ndi-cloud-node#162 (server PR: Waltham-Data-Science/ndi-cloud-node#163).

The goal is resumable direct-to-cloud imports. The cloud now records a CRC-32 for every uploaded file, so a client can tell whether the cloud copy matches a local file and re-upload only the files that don't.

Changes

  • ndi.fun.file.crc32(filePath) and ndi.fun.file.crc32ToBase64(value) compute a local file's CRC-32 in the cloud's encoding: base64 of the 4-byte big-endian CRC-32 (IEEE), the same value S3 uses for x-amz-checksum-crc32. The file is read in chunks through java.util.zip.CRC32.
  • ndi.cloud.api.files.getFileUploadURL(..., 'checksumCRC32', crc) signs the upload URL with the checksum, and putFiles(..., 'checksumCRC32', crc) sends the matching x-amz-checksum-crc32 header. S3 then rejects content that doesn't match, and the cloud records the value as the file's crc32. ndi.cloud.uploadSingleFile now does both by default ('checksum', true).
  • New ndi.cloud.api.files.lookupFiles(datasetId, uids) calls POST /datasets/:id/files/lookup. It returns files (uid, uploaded, size, crc32) and missing (the uids the dataset has no record of), and signs no URLs. Lists longer than the server's 5000-uid limit are split into several calls.
  • listFiles / listFilesAll rows now include crc32 ('' when none was recorded). Before this change the client copied out a fixed set of fields, so crc32 would have been dropped.
  • getSignedURLSet / getSignedURLSetAll now return fileInfo, an Nx1 struct array (uid, uploaded, size, crc32) for the page's files. The server sends it as an array, not a uid-keyed object, so jsondecode's renaming of keys that start with a digit doesn't affect it.
  • The developer manual (cloud.md) has a new lookupFiles row and a "File checksums" section.

Tests

  • tests/+ndi/+unittest/FileCrc32OfflineTest.m needs no network. It covers:
    • CRC values against zlib reference values, including bytes ≥128 and chunk boundaries
    • percent-encoding of checksumCRC32 in the upload URL, and the x-amz-checksum-crc32 header in putFiles
    • the lookupFiles request body (a single uid is still sent as a JSON array), the 5000-uid limit and an empty input
    • normalizing the lookup response (a null crc32 becomes '', missing becomes a string row)
    • signedURLSetPage handling of fileInfo
  • Live tests in FilesTest:
    • an upload with a checksum, then crc32 read back from getFileDetails, listFiles and lookupFiles, including a missing uid
    • an upload with a wrong checksum, which S3 should reject
    • uploadSingleFile recording the checksum
    • the server refusing more than 5000 uids or an invalid body
    • a lookup longer than 5000 uids being split into two calls
  • Live test in SignedURLSetTest: fileInfo matches the URL map, and crc32 is set for files uploaded through the bulk path.
  • The live tests fail (they do not skip) on an environment without the server change, so a missing or broken route is reported rather than filtered.
  • SignedURLSetTest.testCreateAndWaitForSignedURLSetJob likewise now fails, rather than skipping, when the signed-URL-set worker isn't configured (CONFIGURATION_ERROR / ENQUEUE_ERROR / JOB_CREATION_ERROR).
  • All of the new tests passed against dev, run locally in MATLAB.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SwsqFA9Zs1HGm6YM4g2XRV


Generated by Claude Code

…ds, crc32 in listings

Client side of Waltham-Data-Science/ndi-cloud-node#162.

- ndi.fun.file.crc32 / crc32ToBase64: a local file's CRC-32 as base64 of
  the 4-byte big-endian value (the S3 / NDI Cloud encoding), read in
  chunks via java.util.zip.CRC32.
- getFileUploadURL takes 'checksumCRC32'; ndi.cloud.uploadSingleFile
  sends it by default ('checksum', true), so S3 rejects a corrupted
  upload and the cloud records the file's crc32.
- ndi.cloud.api.files.lookupFiles(datasetId, uids): POST /files/lookup,
  returning files (uid, uploaded, size, crc32) and missing uids; splits
  lists over the server's 5000-uid limit.
- listFiles / listFilesAll rows carry crc32 ('' when none recorded).
- getSignedURLSet / getSignedURLSetAll return fileInfo (uid, uploaded,
  size, crc32) for the page's files.
- Tests: FileCrc32OfflineTest (no network) and live tests in FilesTest
  and SignedURLSetTest, which skip on an environment that predates the
  server change.

Authored without a MATLAB runtime; needs MATLAB to run the tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SwsqFA9Zs1HGm6YM4g2XRV
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

1 432 tests  +23   1 399 ✅ +23   6m 16s ⏱️ - 1m 49s
  210 suites + 1      33 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit f7955ea. ± Comparison against base commit 1dd3029.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.02532% with 60 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.32%. Comparing base (1dd3029) to head (f7955ea).

Files with missing lines Patch % Lines
...i/+cloud/+api/+implementation/+files/LookupFiles.m 45.16% 17 Missing ⚠️
src/ndi/+ndi/+cloud/+api/+files/lookupFiles.m 42.10% 11 Missing ⚠️
src/ndi/+ndi/+cloud/uploadSingleFile.m 0.00% 8 Missing ⚠️
...ndi/+cloud/+api/+implementation/+files/ListFiles.m 0.00% 5 Missing ⚠️
...+ndi/+cloud/+api/+implementation/+files/PutFiles.m 73.68% 5 Missing ⚠️
src/ndi/+ndi/+cloud/+api/+files/getFileUploadURL.m 0.00% 3 Missing ⚠️
src/ndi/+ndi/+cloud/+api/+files/putFiles.m 0.00% 3 Missing ⚠️
...oud/+api/+implementation/+files/GetFileUploadURL.m 83.33% 2 Missing ⚠️
...ud/+api/+implementation/+files/fileMetadataArray.m 90.00% 2 Missing ⚠️
...d/+api/+implementation/+files/GetSignedURLSetAll.m 66.66% 1 Missing ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1025      +/-   ##
==========================================
+ Coverage   30.17%   30.32%   +0.15%     
==========================================
  Files         870      876       +6     
  Lines       44076    44217     +141     
==========================================
+ Hits        13298    13410     +112     
- Misses      30778    30807      +29     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…162)

On dev, a checksum carried in the upload URL's query string was neither
validated nor stored by S3, so crc32 stayed empty and a mismatched
upload was accepted. The server now signs x-amz-checksum-crc32 as a
header (Waltham-Data-Science/ndi-cloud-node#163), so the PUT must send
it.

- putFiles / PutFiles take 'checksumCRC32' and send the header from both
  the curl and MATLAB HTTP paths. The value is checked to be a base64
  CRC-32 before it goes into the curl command.
- uploadSingleFile passes its checksum to putFiles.
- FilesTest passes the checksum to putFiles; offline tests cover the
  header and the malformed-value refusal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SwsqFA9Zs1HGm6YM4g2XRV
A skip hid whether the tests ran at all; an environment without the
lookup route or fileInfo now fails these tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SwsqFA9Zs1HGm6YM4g2XRV
@stevevanhooser
stevevanhooser merged commit 1426fcd into main Oct 13, 2026
12 checks passed
@stevevanhooser
stevevanhooser deleted the claude/file-crc32-lookup branch October 13, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants