feat: [OCISDEV-533] Separate the storage-users and graph to handle vault storage by 2403905 · Pull Request #559 · owncloud/reva

2403905 · 2026-03-12T10:25:21Z

Separate the storage-users and graph to handle vault storage
Related to owncloud/ocis#12108

Copilot

Pull request overview

This PR introduces storage-id–aware routing and event handling to better support a dedicated “vault” storage provider, while also simplifying/removing some gateway caching layers.

Changes:

Add storage_id propagation/filtering in gateway + spaces registry, and introduce a vault storage provider ID constant.
Make decomposedfs async event consumption configurable via events.consumer_group, and ignore postprocessing events that target a different storage.
Remove the gateway “create home / create personal space” caching types and client wrappers, and refactor some client acquisition to use pool directly.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/utils/utils.go	Adds `VaultStorageProviderID` constant.
pkg/storage/utils/decomposedfs/options/options.go	Adds `events.consumer_group` option with default `dcfs`.
pkg/storage/utils/decomposedfs/decomposedfs.go	Uses configurable consumer group and filters postprocessing events by storage id.
pkg/storage/registry/spaces/spaces.go	Adds vault filtering and threads `storage_id` into provider selection/filtering.
pkg/storage/cache/cache.go	Removes CreateHome/CreatePersonalSpace cache interfaces and registries.
pkg/storage/cache/createhome.go	Deletes legacy create-home cache implementation.
pkg/storage/cache/createpersonalspace.go	Deletes legacy create-personal-space cache implementation.
pkg/events/postprocessing.go	Adds `ResourceID` to postprocessing events.
internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go	Simplifies provider client acquisition via `pool`.
internal/grpc/services/storageprovider/storageprovider.go	Ensures storage id is added to `RootInfo.Id` in `Stat` responses.
internal/grpc/services/gateway/storageprovidercache.go	Updates cache keying to include `storage_id` (though wrapper usage changed elsewhere).
internal/grpc/services/gateway/storageprovider.go	Propagates `storage_id` and switches to direct `pool` clients (removing cached wrappers).
internal/grpc/services/gateway/gateway.go	Removes create-personal-space cache config/field wiring.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

This PR introduces storage-id–aware routing and event handling to support a dedicated “vault” storage alongside existing storage providers (related to owncloud/ocis#12108). It propagates a storage_id signal through gateway/registry flows and adds event-consumer separation to avoid cross-storage interference.

Changes:

Add a vault storage provider ID constant and propagate storage_id through gateway requests.
Make decomposedfs async postprocessing consumer group configurable and ignore postprocessing events from other storages.
Update spaces registry/provider discovery and gateway provider-cache keying to consider storage_id.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
pkg/utils/utils.go	Adds a Vault storage provider ID constant.
pkg/storage/utils/decomposedfs/options/options.go	Adds `events.consumer_group` config + defaulting.
pkg/storage/utils/decomposedfs/decomposedfs.go	Uses configurable consumer group; filters postprocessing events by storage.
pkg/storage/registry/spaces/spaces.go	Filters vault spaces when `storage_id` absent; threads `storage_id` into provider selection.
pkg/events/postprocessing.go	Extends postprocessing events with `ResourceID`.
internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go	Simplifies provider client acquisition (direct pool call).
internal/grpc/services/storageprovider/storageprovider.go	Ensures storage provider IDs are filled in additional metadata IDs.
internal/grpc/services/gateway/storageprovidercache.go	Makes provider-cache key storage-aware via `storage_id`.
internal/grpc/services/gateway/storageprovider.go	Passes `storage_id` through CreateHome/CreateStorageSpace and ListStorageSpaces.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-16T09:27:55Z

+	case errtypes.PermissionDenied:
+		if strings.Contains(err.Error(), "MFA required") {
+			rw.Header().Set("X-Ocis-Mfa-Required", "true")
+			rw.WriteHeader(http.StatusForbidden)
+		} else {
+			rw.WriteHeader(http.StatusNotFound)
+		}


Detecting the MFA condition via strings.Contains(err.Error(), "MFA required") is brittle (message wording changes will silently break the behavior). Consider using a dedicated error type/sentinel (e.g., errors.Is), or checking a structured status/detail that the MFA interceptor can set, so this remains stable across refactors/localization.

jvillafanez · 2026-04-16T11:31:06Z

+		w.WriteHeader(http.StatusConflict)
+		b, err := errors.Marshal(http.StatusBadRequest, "destination is not allowed", "", "")


I think we should use the same error code, and StatusConflict doesn't seem a good one to use here. If there is anything else in place (HandleWebdavError might do things), we probably need some comment explaining why the errors are different.

I followed the same pattern as in the Copy and Move functions, most of the errors are handled the same way.

I think it's better to at least reference the original code so we know this piece has been copied, and where it has been copied from.
I'm pretty sure that later I'll see this code and think that it's a bug and make the change. The comment will help to double-check, and it will also help to know that there are multiple places with the same problem (if it's a problem)

…ult storage update ListStorageProviders cache update findProvidersForFilter fix the finishUpload event restict webdav copy/move from the vault feat: provide the mfa to webdav and grpc feat: refactoring: provide the mfa to webdav and grpc draft: mfa collaboration remove redundant code updated the storage registry condition. added the tests

2403905 changed the title ~~feat: Sepatate the storage-users and graph to handle vault storage~~ feat: Separate the storage-users and graph to handle vault storage Mar 12, 2026

2403905 force-pushed the feat/OCISDEV-533-graph branch 2 times, most recently from b6bc6fa to b4cd50a Compare March 13, 2026 11:49

2403905 mentioned this pull request Mar 13, 2026

[full-ci] feat: ocisdev-533 graph owncloud/ocis#12108

Open

2403905 force-pushed the feat/OCISDEV-533-graph branch from b1ac85b to a145e18 Compare March 16, 2026 14:08

2403905 marked this pull request as ready for review March 16, 2026 14:32

2403905 requested review from Copilot, jvillafanez, kobergj and mklos-kw March 16, 2026 14:33

Copilot started reviewing on behalf of 2403905 March 16, 2026 14:33 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

Comment thread pkg/storage/registry/spaces/spaces.go

Comment thread pkg/storage/utils/decomposedfs/decomposedfs.go

Comment thread pkg/storage/utils/decomposedfs/decomposedfs.go

Comment thread internal/grpc/services/gateway/storageprovider.go

2403905 force-pushed the feat/OCISDEV-533-graph branch 3 times, most recently from 38c8b71 to 1130aeb Compare March 24, 2026 15:30

2403905 requested a review from Copilot March 24, 2026 15:37

Copilot started reviewing on behalf of 2403905 March 24, 2026 15:37 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

2403905 force-pushed the feat/OCISDEV-533-graph branch from cc61754 to a90b037 Compare April 7, 2026 15:24

2403905 force-pushed the feat/OCISDEV-533-graph branch from 01eba14 to 1b5ed3c Compare April 14, 2026 16:29

kobergj previously approved these changes Apr 15, 2026

View reviewed changes

2403905 dismissed kobergj’s stale review via 3f827ba April 16, 2026 09:06

2403905 requested a review from Copilot April 16, 2026 09:07

Copilot started reviewing on behalf of 2403905 April 16, 2026 09:12 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

jvillafanez reviewed Apr 16, 2026

View reviewed changes

jvillafanez mentioned this pull request Apr 30, 2026

feat: metadata propagation through events #582

Open

2403905 force-pushed the feat/OCISDEV-533-graph branch 2 times, most recently from f808909 to 6f22580 Compare May 4, 2026 16:57

2403905 requested a review from kobergj May 5, 2026 07:32

kobergj previously approved these changes May 5, 2026

View reviewed changes

2403905 changed the title ~~feat: Separate the storage-users and graph to handle vault storage~~ feat: [OCISDEV-533] Separate the storage-users and graph to handle vault storage May 5, 2026

2403905 dismissed kobergj’s stale review via ef396cf May 6, 2026 06:49

2403905 force-pushed the feat/OCISDEV-533-graph branch from 6f22580 to ef396cf Compare May 6, 2026 06:49

2403905 force-pushed the feat/OCISDEV-533-graph branch from ef396cf to b350cd1 Compare May 6, 2026 06:51

kobergj approved these changes May 6, 2026

View reviewed changes

2403905 merged commit 621bc04 into main May 6, 2026
13 checks passed

2403905 deleted the feat/OCISDEV-533-graph branch May 6, 2026 07:12

		w.WriteHeader(http.StatusConflict)
		b, err := errors.Marshal(http.StatusBadRequest, "destination is not allowed", "", "")

Conversation

2403905 commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jvillafanez Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

2403905 Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

jvillafanez Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

2403905 commented Mar 12, 2026 •

edited

Loading