Skip to content

feat(tus): return etag and permissions after the last TUS chunk#718

Merged
rhafer merged 1 commit into
opencloud-eu:mainfrom
michaelstingl:issue-2409-tus-etag-iface
Jul 6, 2026
Merged

feat(tus): return etag and permissions after the last TUS chunk#718
rhafer merged 1 commit into
opencloud-eu:mainfrom
michaelstingl:issue-2409-tus-etag-iface

Conversation

@michaelstingl

Copy link
Copy Markdown

Description

A chunked TUS upload's finalizing PATCH returned only OC-FileID, so a client needed a follow-up PROPFIND for the new etag and permissions. The creation-with-upload path already sets OC-ETag/ETag/OC-Perm in the ocdav gateway.

This registers tusd's PreFinishResponseCallback in the dataprovider and sets the same headers on the chunked finalize. A new optional storage.UploadSessionResolver resolves the finished upload to its ResourceInfo, read as the upload's executant:

type UploadSessionResolver interface {
    ResolveUpload(ctx context.Context, info tusd.FileInfo) (*provider.ResourceInfo, context.Context, error)
}

decomposedfs implements it from the upload's own session, so the datatx layer no longer reconstructs the executant or reads decomposedfs FileInfo keys. The session is rebuilt from the given FileInfo, not the store, because a synchronous upload has cleaned its session up by callback time. On any lookup failure the callback sets no headers and the client keeps its PROPFIND fallback.

The WebDAV-permission derivation moves into net.WebDAVPermissions, shared with the gateway so the two cannot drift; propfind keeps its share-types-aware variant.

Related Issue

Motivation and Context

The storage-interface variant of #666: it keeps the executant handling inside the storage driver instead of depending on decomposedfs FileInfo keys in the datatx layer.

How Has This Been Tested?

  • unit (ginkgo): tus_internal_test.go drives the callback against a fake resolver (etag, permissions, executant-context shared marker, no-op on stat error / nil resource / non-resolver driver); net/permissions_test.go covers the shared WebDAVPermissions helper
  • go build, gofmt, go vet, pinned golangci-lint v2.10.1: 0 issues
  • over HTTP, posix driver: a chunked upload's finalize PATCH carries OC-ETag/ETag (equal to a follow-up PROPFIND) and OC-Perm, OC-FileID unchanged; the creation-with-upload path is unchanged
  • the api-integration/behat TUS suite runs after a maintainer approves fork CI

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation added

🤖 drafted with Claude Code, reviewed before submitting.

The finalizing PATCH of a chunked TUS upload returned only the file id,
so a client had to issue a follow-up PROPFIND to learn the new etag and
permissions, an extra round-trip per upload. The creation-with-upload
path already returns them: the ocdav gateway stats the resource and sets
OC-ETag/ETag/OC-Perm.

This registers tusd's PreFinishResponseCallback in the dataprovider tus
handler and attaches the same headers to the chunked finalize. The
finalized resource is resolved by the storage driver through a new
UploadSessionResolver interface, so the generic datatx layer does not
reconstruct the upload's executant: decomposedfs resolves the finished
upload as its own executant (the data path's transfer token carries no
user identity) and returns the resource info. The session is rebuilt
from the in-hand tus info, so it still resolves after a synchronous
upload has cleaned up its session. The lookup is best-effort: on a stat
error, or a driver that does not implement the interface, it sets no
headers and leaves the PROPFIND fallback intact.

The WebDAV-permission derivation is shared with the gateway's
creation-with-upload path via net.WebDAVPermissions, so the two cannot
drift; the propfind handler keeps its own share-types-aware variant.

Fixes: opencloud-eu/opencloud#2409

@rhafer rhafer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you going to implement acceptance tests for this in OpenCloud?

@michaelstingl

Copy link
Copy Markdown
Author

Yes, I'll add acceptance tests for this in OpenCloud. 👍 I'll open a follow-up PR there once this one is merged. Thanks for the review! 🎉

@rhafer rhafer merged commit c895a8e into opencloud-eu:main Jul 6, 2026
18 of 19 checks passed
@openclouders openclouders mentioned this pull request Jul 6, 2026
1 task
michaelstingl added a commit to michaelstingl/opencloud-eu-opencloud that referenced this pull request Jul 6, 2026
…lizing TUS chunk

The chunked TUS finalize returns OC-ETag, ETag and OC-Perm since
opencloud-eu/reva#718, so a client no longer needs a follow-up PROPFIND
for the new etag and permissions. Add a scenario asserting these headers
on the finalizing chunk, reusing existing step definitions (the etag is a
dynamic quoted hash, hence the header-regex assertion).

Verified locally red->green: red on stock (headers absent), green on an
opencloud built on reva-with-opencloud-eu#718, on posix and decomposed.

opencloud-eu#2409
michaelstingl added a commit to michaelstingl/opencloud-eu-opencloud that referenced this pull request Jul 6, 2026
…lizing TUS chunk

The chunked TUS finalize returns OC-ETag, ETag and OC-Perm since
opencloud-eu/reva#718, so a client no longer
needs a follow-up PROPFIND for the new etag and permissions. Add a
scenario asserting these headers on the finalizing chunk, reusing
existing step definitions (the etag is a dynamic quoted hash, hence the
header-regex assertion).

Verified locally red->green: red on stock (headers absent), green on an
opencloud built on a reva carrying that change, on posix and decomposed.

opencloud-eu#2409
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