Skip to content

[docs-only] fix: add ResourceID field to UploadReady event with file's node ID#12060

Merged
mmattel merged 2 commits intoowncloud:masterfrom
paul43210:fix/uploadready-add-resourceid
Mar 14, 2026
Merged

[docs-only] fix: add ResourceID field to UploadReady event with file's node ID#12060
mmattel merged 2 commits intoowncloud:masterfrom
paul43210:fix/uploadready-add-resourceid

Conversation

@paul43210
Copy link
Copy Markdown
Contributor

Summary

The UploadReady event's FileRef.ResourceId.OpaqueId is set to the space root ID (required for CS3 gateway path resolution). NATS consumers that need the file's actual node ID for Graph API URLs or metadata writes get the space root instead.

Problem

FileRef uses OpaqueId = session.SpaceID() because NodeFromResource() uses OpaqueId as the anchor for WalkPath(Path). This is correct for CS3 gateway resolution, but means FileRef.ResourceId cannot be used as a file identifier.

A previous fix attempt (PR #12057, closed) changed OpaqueId to session.NodeID(), which broke:

  • CS3 gateway resolution (WalkPath from a file node fails with CODE_NOT_FOUND)
  • Removing Path to work around that broke external consumers that need the file path

Fix

Add a separate ResourceID field to the UploadReady event, following the same pattern already used by BytesReceived:

ResourceID: &provider.ResourceId{
    StorageId: session.ProviderID(),
    SpaceId:   session.SpaceID(),
    OpaqueId:  session.NodeID(),  // actual file node ID
},

FileRef is unchanged — CS3 gateway resolution and Path continue to work. JSON deserialization silently ignores unknown fields, so existing consumers are unaffected.

Changes

File Change
vendor/.../events/postprocessing.go Add ResourceID *provider.ResourceId to UploadReady struct
vendor/.../decomposedfs/decomposedfs.go Populate ResourceID with session.NodeID()
vendor/.../posix/tree/assimilation.go Populate ResourceID from existing ref.ResourceId
changelog/unreleased/fix-uploadready-resource-id.md Changelog entry

Test plan

  • Upload a file and verify UploadReady NATS event contains both FileRef (with Path) and ResourceID (with correct OpaqueId)
  • Verify activitylog, clientlog, search continue to work (they use FileRef)
  • Verify external consumer can read ResourceID.OpaqueId as the file's node ID

Upstream reva PR: owncloud/reva#547
Related: #12056

🤖 Generated with Claude Code

@sonarqubecloud
Copy link
Copy Markdown

@2403905
Copy link
Copy Markdown
Contributor

2403905 commented Feb 27, 2026

@paul43210 Thank you for your PR.
I read this one and the related tickets. I can't figure out which exact bug this PR should fix.
Could you point me to the consumer that is missing the resource ID?
What I see from the code is that the consummers useFileRef.
activitylog
clientlog
search

@paul43210
Copy link
Copy Markdown
Contributor Author

Thanks for the review @2403905!

You're right that the built-in oCIS consumers (activitylog, clientlog, search) all use FileRef and work fine — they resolve the file via FileRef.Path relative to the space root.

The consumer that needs the actual file node ID is an external NATS event consumer — a face detection service that subscribes to UploadReady events to process uploaded photos. It needs the file's OpaqueId (node ID) to construct Graph API URLs like:

/graph/v1.0/drives/{space_id}/items/{opaque_id}

Currently FileRef.ResourceId.OpaqueId is the space root ID (identical to SpaceId), so those URLs resolve to the space root instead of the file. The service can't use the CS3 gateway — it only has WebDAV/Graph API access.

The BytesReceived event already has this same pattern — a separate ResourceID field with the actual file node ID alongside the gateway-resolvable reference. This PR applies the same approach to UploadReady.

That said, if an external consumer use case isn't compelling enough for this change, I understand. Happy to discuss alternative approaches.

@mmattel mmattel requested review from 2403905 and mklos-kw March 4, 2026 12:19
@2403905
Copy link
Copy Markdown
Contributor

2403905 commented Mar 4, 2026

Hi @paul43210. Sorry for the delay.
The OCIS doesn't share the ResourceID because the clients can do the extra webDav PROPFIND call using the relative path to get the file id and other info. The changes are looks reasonable to avoid the extra call for each file in case the client wants uses id.

Copy link
Copy Markdown
Contributor

@2403905 2403905 left a comment

Choose a reason for hiding this comment

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

The reva changes should go first

@2403905
Copy link
Copy Markdown
Contributor

2403905 commented Mar 4, 2026

Related PR owncloud/reva#547

@2403905
Copy link
Copy Markdown
Contributor

2403905 commented Mar 6, 2026

@paul43210 I merged the reva #12087 to ocis. It includes the related pr #547
What you need to do to have this pr merged:

  • remove the vendor directory and keep only changelog/unreleased/fix-uploadready-resource-id.md
  • Add the link https://github.com/owncloud/reva/pull/547 to the changelog/unreleased/fix-uploadready-resource-id.md tail
  • rebase git pull --rebase origin master and push
    Thaks.

@paul43210 paul43210 force-pushed the fix/uploadready-add-resourceid branch from f4fb364 to d754c5b Compare March 6, 2026 23:11
@mmattel mmattel force-pushed the fix/uploadready-add-resourceid branch 2 times, most recently from c843e17 to 1ee038e Compare March 10, 2026 08:21
@mmattel mmattel enabled auto-merge March 10, 2026 08:56
@mmattel mmattel force-pushed the fix/uploadready-add-resourceid branch from 1ee038e to 39a4c07 Compare March 11, 2026 08:08
@mmattel mmattel changed the title fix: add ResourceID field to UploadReady event with file's node ID [docs-only] fix: add ResourceID field to UploadReady event with file's node ID Mar 11, 2026
@paul43210
Copy link
Copy Markdown
Contributor Author

@2403905 Thanks for the guidance! I've rebased onto master, squashed into a single commit, and added the reva PR #547 link to the changelog.

Re: removing the vendor directory — I checked and the reva bump in PR #12087 only added the new ResourceID field. The FileRef fixes (changing OpaqueId from SpaceID()NodeID() and removing Path) are still not in the vendored reva on master. So I've kept those two vendor changes as they're needed for the FileRef fix described in the original issue.

The PR now contains just:

  • changelog/unreleased/fix-uploadready-resource-id.md (with reva Searching sharee with displayname #547 link added)
  • vendor/.../decomposedfs.go — FileRef OpaqueId fix
  • vendor/.../upload.go — FileRef OpaqueId + Path removal

@mmattel mmattel disabled auto-merge March 12, 2026 06:37
@2403905
Copy link
Copy Markdown
Contributor

2403905 commented Mar 12, 2026

Hello @paul43210 I see the upload.go changes in a closed pr owncloud/reva#546
If you want to add it to the reva, please make a new pr. Then I'll bump the reva in an ocis

@paul43210
Copy link
Copy Markdown
Contributor Author

@2403905 Done — opened reva PR #560 with the FileRef fix and it's been merged. Whenever you bump reva in oCIS, I'll rebase this PR. Thanks!

paul43210 and others added 2 commits March 14, 2026 09:52
The UploadReady event's FileRef.ResourceId.OpaqueId is set to the space
root ID (required for CS3 gateway path resolution via WalkPath). This
means consumers that need the file's actual node ID for Graph API URLs
get the space root instead.

Add a separate ResourceID field (following the BytesReceived pattern)
that carries the file's actual resource identifier with the correct
OpaqueId set to session.NodeID().

Upstream: https://github.com/owncloud/reva/pull/XXXXX

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mmattel mmattel force-pushed the fix/uploadready-add-resourceid branch from da743c4 to 4a20047 Compare March 14, 2026 08:52
@mmattel mmattel merged commit eb9fae4 into owncloud:master Mar 14, 2026
2 checks passed
ownclouders pushed a commit that referenced this pull request Mar 14, 2026
[docs-only] fix: add ResourceID field to UploadReady event with file's node ID
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