From 41f7ec94073364e5793ecd5e45084b34d34ee177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 Jun 2022 14:07:31 +0000 Subject: [PATCH 1/9] use shares providerid to ease routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../owncloud/ocs/handlers/apps/sharing/shares/shares.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index 5eb73bfb5a..3fc1936c77 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -864,7 +864,7 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { // first stat mount point, but the shares storage provider only handles accepted shares so we send the try to make the requests for only those if rs.State == collaboration.ShareState_SHARE_STATE_ACCEPTED { mountID := &provider.ResourceId{ - StorageId: utils.ShareStorageProviderID, + StorageId: storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageProviderID), OpaqueId: rs.Share.Id.OpaqueId, } info, status, err = h.getResourceInfoByID(ctx, client, mountID) From 2049489dbd36bd776c36d7e02f48e70df53624d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 Jun 2022 14:08:47 +0000 Subject: [PATCH 2/9] introduce ShareStorageSpaceID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/utils/utils.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index f40825359f..d20ce429e5 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -50,8 +50,10 @@ var ( // globals is not encouraged, and this is a workaround until the PR is out of a draft state. GlobalRegistry registry.Registry = memory.New(map[string]interface{}{}) - // ShareStorageProviderID is the id used by the sharestorageprovider + // ShareStorageProviderID is the provider id used by the sharestorageprovider ShareStorageProviderID = "a0ca6a90-a365-4782-871e-d44447bbc668" + // ShareStorageSpaceID is the space id used by the sharestorageprovider share jail space + ShareStorageSpaceID = "a0ca6a90-a365-4782-871e-d44447bbc668" // PublicStorageProviderID is the id used by the sharestorageprovider PublicStorageProviderID = "7993447f-687f-490d-875c-ac95e89a62a4" From e4abdec108c7668088a6b6ac380ff51749ecf919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 Jun 2022 14:09:24 +0000 Subject: [PATCH 3/9] make sharesstorageprovider return storageid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../sharesstorageprovider/sharesstorageprovider.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index b15c38e084..b56a99c45a 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -368,7 +368,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora switch k { case "virtual": virtualRootID := &provider.ResourceId{ - StorageId: utils.ShareStorageProviderID, + StorageId: storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageProviderID), OpaqueId: utils.ShareStorageProviderID, } if spaceID == nil || isShareJailRoot(spaceID) { @@ -387,7 +387,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora space := &provider.StorageSpace{ Opaque: opaque, Id: &provider.StorageSpaceId{ - OpaqueId: virtualRootID.StorageId + "!" + virtualRootID.OpaqueId, + OpaqueId: storagespace.FormatResourceID(*virtualRootID), }, SpaceType: "virtual", //Owner: &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient @@ -415,7 +415,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora space := &provider.StorageSpace{ Opaque: opaque, Id: &provider.StorageSpaceId{ - OpaqueId: root.StorageId + "!" + root.OpaqueId, + OpaqueId: storagespace.FormatResourceID(*root), }, SpaceType: "grant", Owner: &userv1beta1.User{Id: receivedShare.Share.Owner}, @@ -433,7 +433,6 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora root := &provider.ResourceId{ StorageId: utils.ShareStorageProviderID, OpaqueId: receivedShare.Share.Id.OpaqueId, - //OpaqueId: utils.ShareStorageProviderID, } // do we filter by id if spaceID != nil { @@ -461,10 +460,15 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora opaque = utils.AppendPlainToOpaque(opaque, "grantOpaqueID", receivedShare.Share.ResourceId.OpaqueId) } + // prefix storageid if we are responsible + if root.StorageId == utils.ShareStorageSpaceID { + root.StorageId = storagespace.FormatStorageID(utils.ShareStorageProviderID, root.StorageId) + } + space := &provider.StorageSpace{ Opaque: opaque, Id: &provider.StorageSpaceId{ - OpaqueId: root.StorageId + "!" + root.OpaqueId, + OpaqueId: storagespace.FormatResourceID(*root), }, SpaceType: "mountpoint", Owner: &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient From 018c6cfcd87d305daca7f0578327a5dd89323a50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 Jun 2022 14:10:21 +0000 Subject: [PATCH 4/9] allow empty providerid when checking share jail root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../services/owncloud/ocdav/propfind/propfind.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 78dc3f8c32..52a95dbb73 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -1354,9 +1354,14 @@ func (pn *Props) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { } } +// isVirtualRootResourceID returns true if the id points to the share jail root. The providerid is optional for legacy ids func isVirtualRootResourceID(id *provider.ResourceId) bool { - return utils.ResourceIDEqual(id, &provider.ResourceId{ - StorageId: utils.ShareStorageProviderID, - OpaqueId: utils.ShareStorageProviderID, - }) + switch { + case id == nil: + return false + case id.OpaqueId != utils.ShareStorageProviderID: + return false + } + providerID, spaceID := storagespace.SplitStorageID(id.StorageId) + return spaceID == utils.ShareStorageProviderID && (providerID == "" || providerID == utils.ShareStorageProviderID) } From 347f9f4e6101fb89cc22bfea6cf9520cf105e8cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 Jun 2022 14:11:15 +0000 Subject: [PATCH 5/9] add todo for cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/grpc/services/gateway/storageprovidercache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/grpc/services/gateway/storageprovidercache.go b/internal/grpc/services/gateway/storageprovidercache.go index efa4487d2a..916135446b 100644 --- a/internal/grpc/services/gateway/storageprovidercache.go +++ b/internal/grpc/services/gateway/storageprovidercache.go @@ -200,7 +200,7 @@ func (c *cachedRegistryClient) ListStorageProviders(ctx context.Context, in *reg return resp, nil case storageID == "": return resp, nil - case storageID == utils.ShareStorageProviderID: + case storageID == utils.ShareStorageProviderID: // TODO do we need to compare providerid and spaceid separately? return resp, nil default: return resp, pushToCache(cache, key, resp) From 89e70a53a47d8aed32120cf05f1dd9710901030d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 Jun 2022 14:25:25 +0000 Subject: [PATCH 6/9] skip storageproviders that are not configured for the requested space type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/registry/spaces/spaces.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index bfa17d1cba..77fa56d4ad 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -321,9 +321,28 @@ func (r *registry) buildFilters(filterMap map[string]string) []*providerpb.ListS func (r *registry) findProvidersForFilter(ctx context.Context, filters []*providerpb.ListStorageSpacesRequest_Filter, unrestricted bool, _ string) []*registrypb.ProviderInfo { + var requestedSpaceType string + for _, f := range filters { + if f.Type == providerpb.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE { + requestedSpaceType = f.GetSpaceType() + } + } + currentUser := ctxpkg.ContextMustGetUser(ctx) providerInfos := []*registrypb.ProviderInfo{} for address, provider := range r.c.Providers { + if requestedSpaceType != "" { + // check if we can skip this provider altogether + found := false + for spaceType := range provider.Spaces { + if spaceType == requestedSpaceType { + found = true + } + } + if !found { + continue + } + } p := ®istrypb.ProviderInfo{ Address: address, } From d03fce0ef49187bb196f39af88c752676abfb22a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 Jun 2022 14:27:46 +0000 Subject: [PATCH 7/9] add changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/leverage-sharesstorage-storageid.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/leverage-sharesstorage-storageid.md diff --git a/changelog/unreleased/leverage-sharesstorage-storageid.md b/changelog/unreleased/leverage-sharesstorage-storageid.md new file mode 100644 index 0000000000..6183683b0e --- /dev/null +++ b/changelog/unreleased/leverage-sharesstorage-storageid.md @@ -0,0 +1,6 @@ +Enhancement: Leverage shares space storageid and type when listing shares + +The list shares call now also fills the storageid to allow the space registry to directly route requests to the correct storageprovider. The spaces registry will now also skip storageproviders that are not configured for a requested type, causing type 'personal' requests to skip the sharestorageprovider. + + +https://github.com/cs3org/reva/pull/2975 From 063d4dac619b69b72b9830011e9a3f2d40eb92e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 16 Jun 2022 09:38:21 +0000 Subject: [PATCH 8/9] use ShareStorageSpaceID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../services/sharesstorageprovider/sharesstorageprovider.go | 2 +- .../owncloud/ocs/handlers/apps/sharing/shares/shares.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index b56a99c45a..7ae2f34792 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -368,7 +368,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora switch k { case "virtual": virtualRootID := &provider.ResourceId{ - StorageId: storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageProviderID), + StorageId: storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageSpaceID), OpaqueId: utils.ShareStorageProviderID, } if spaceID == nil || isShareJailRoot(spaceID) { diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index 3fc1936c77..f66c22c3b9 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -864,7 +864,7 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { // first stat mount point, but the shares storage provider only handles accepted shares so we send the try to make the requests for only those if rs.State == collaboration.ShareState_SHARE_STATE_ACCEPTED { mountID := &provider.ResourceId{ - StorageId: storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageProviderID), + StorageId: storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageSpaceID), OpaqueId: rs.Share.Id.OpaqueId, } info, status, err = h.getResourceInfoByID(ctx, client, mountID) From 11a4c9643f5a765c9ad58b88200c49da622e6777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 16 Jun 2022 10:03:00 +0000 Subject: [PATCH 9/9] make codacy happy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/leverage-sharesstorage-storageid.md | 1 - 1 file changed, 1 deletion(-) diff --git a/changelog/unreleased/leverage-sharesstorage-storageid.md b/changelog/unreleased/leverage-sharesstorage-storageid.md index 6183683b0e..53e0aa1273 100644 --- a/changelog/unreleased/leverage-sharesstorage-storageid.md +++ b/changelog/unreleased/leverage-sharesstorage-storageid.md @@ -2,5 +2,4 @@ Enhancement: Leverage shares space storageid and type when listing shares The list shares call now also fills the storageid to allow the space registry to directly route requests to the correct storageprovider. The spaces registry will now also skip storageproviders that are not configured for a requested type, causing type 'personal' requests to skip the sharestorageprovider. - https://github.com/cs3org/reva/pull/2975