Repository navigation
⚡ Optimize small file upload in dataroom (prevent too many API calls) - #67
Merged
Merged
Conversation
HanXHX
changed the title
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation is well covered and consistent, with only a minor documentation symbol correction remaining.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Optimizes small dataroom uploads by combining node, version, and chunk creation into one request.
Changes:
- Adds atomic uploads for files up to 8 MB.
- Integrates the path into CLI/MCP and WebDAV uploads.
- Adds route normalization, documentation, and tests.
| File | Description |
|---|---|
internal/service/dataroom.go |
Implements small-file uploads. |
internal/service/dataroom_test.go |
Tests service behavior. |
internal/service/chunks.go |
Extracts chunk encryption. |
internal/metrics/metrics.go |
Normalizes the new route. |
internal/metrics/metrics_test.go |
Tests route normalization. |
internal/api/dataroom.go |
Adds the multipart API operation. |
internal/api/dataroom_test.go |
Tests multipart requests. |
internal/api/client.go |
Generalizes multipart handling. |
doc/webdav.md |
Documents upload behavior. |
cmd/webdav.go |
Adds buffered small PUT handling. |
cmd/webdav_test.go |
Tests PUTs, ETags, and caching. |
cmd/webdav_metrics_test.go |
Updates metric test setup. |
CLAUDE.md |
Records the new upload path. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Writing a small file cost three sequential round trips, each waiting for
the previous one's ID: node, version, chunk (four on an overwrite whose
node was not cached: a 409, a listing to find the node, then version and
chunk). The API now creates the node, its version and its content in one
request: POST /dataroom/{id}/node/file (createDataroomFileNode, multipart),
with overwrite adding a version to a file already holding the name, and
the server discarding what a failed request created.
api.CreateDataroomFileNode sends it through a postMultipart that
PostMultipartChunk now shares. service.UploadSmallFile encrypts the name,
the MIME type and the single chunk (encryptChunk, split out of
UploadChunks so the crypto.encrypt span and metric stay), maps 410 (the
name held by a node pending deletion) to ErrNameBeingDeleted, which no
longer reads as a missing parent, and returns the node as a listing
reports it.
Every file up to service.UploadChunkSize, an empty one included, takes
it: dataroom cp and the MCP upload tool through uploadDataroomFile, and
WebDAV through smallWriteHandle, which keeps the body in memory, refuses
bytes beyond the declared size, sends it on Close, updates the cached
listing and drops the stale ones on a 404. The buffered PUT goes through
the same openUpload. Larger files keep node, version and chunks.
On the CSI trace of `mkdir -p a/b && echo x > a/b/f`, the two PUTs (the
empty one davfs2 sends on create, then the content) go from 4 API calls
to 2. Checked against the dev API: dataroom cp of small, empty and 9 MiB
files, an overwrite, and WebDAV PUTs with and without Content-Length,
each read back identical.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A PUT without Content-Length (chunked, as macOS Finder sends it) is buffered to a temp file, and writeFileHandle.Stat returned that file's os.FileInfo. The WebDAV handler Stats the file before Close and reads the ETag after it from that info: without a version ID it fell back to the ModTime+Size default, so the client's next conditional GET did not match and downloaded the file it had just uploaded. writeFileHandle now reports a webdavFileInfo whose size follows the writes, and Close copies into it the node and version the upload created. Checked against the dev API: a chunked PUT answers the version ID as its ETag, and a GET with it answers 304. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HanXHX
force-pushed
the
optim/dataroom_single_upload
branch
from
October 5, 2026 10:33
d82c5e2 to
fb11d05
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.