diff --git a/changelog/unreleased/leverage-sharesstorage-storageid.md b/changelog/unreleased/leverage-sharesstorage-storageid.md new file mode 100644 index 0000000000..53e0aa1273 --- /dev/null +++ b/changelog/unreleased/leverage-sharesstorage-storageid.md @@ -0,0 +1,5 @@ +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 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) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index b15c38e084..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: utils.ShareStorageProviderID, + StorageId: storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageSpaceID), 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 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) } 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..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: utils.ShareStorageProviderID, + StorageId: storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageSpaceID), OpaqueId: rs.Share.Id.OpaqueId, } info, status, err = h.getResourceInfoByID(ctx, client, mountID) 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, } 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"