Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include spaces etags in virtual spaces #2624

Merged
merged 1 commit into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/etags-virtual-spaces.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Enhancement: Add etags to virtual spaces

The shares storage provider didn't include the etag in virtual spaces like the shares jail or mountpoints.

https://github.com/cs3org/reva/pull/2624
46 changes: 45 additions & 1 deletion internal/grpc/services/gateway/usershareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package gateway

import (
"context"
"encoding/json"
"path"

rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
Expand Down Expand Up @@ -146,9 +147,10 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq
// received shares. The display name of the shares should be the a friendly name, like the basename
// of the original file.
func (s *svc) ListReceivedShares(ctx context.Context, req *collaboration.ListReceivedSharesRequest) (*collaboration.ListReceivedSharesResponse, error) {
logger := appctx.GetLogger(ctx)
c, err := pool.GetUserShareProviderClient(s.c.UserShareProviderEndpoint)
if err != nil {
appctx.GetLogger(ctx).
logger.Error().
Err(err).
Msg("ListReceivedShares: failed to get user share provider")
return &collaboration.ListReceivedSharesResponse{
Expand All @@ -160,6 +162,48 @@ func (s *svc) ListReceivedShares(ctx context.Context, req *collaboration.ListRec
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling ListReceivedShares")
}

// TODO: This is a hack for now.
// Can we do that cleaner somehow?
micbar marked this conversation as resolved.
Show resolved Hide resolved
// The `ListStorageSpaces` method in sharesstorageprovider/sharesstorageprovider.go needs the etags.
shareEtags := make(map[string]string, len(res.Shares))
for _, rs := range res.Shares {
sRes, err := s.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: rs.Share.ResourceId}})
if err != nil {
logger.Error().
Err(err).
Interface("resourceID", rs.Share.ResourceId).
Msg("ListRecievedShares: failed to stat the resource")
continue
}
if sRes.Status.Code != rpc.Code_CODE_OK {
logger.Error().
Interface("resourceID", rs.Share.ResourceId).
Msg("ListRecievedShares: failed to stat the resource")
continue
}
shareEtags[rs.Share.Id.OpaqueId] = sRes.Info.Etag
}

marshalled, err := json.Marshal(shareEtags)
if err != nil {
logger.Error().
Err(err).
Msg("ListRecievedShares: failed marshal share etags")
} else {
opaque := res.Opaque
if opaque == nil {
opaque = &typesv1beta1.Opaque{
Map: map[string]*typesv1beta1.OpaqueEntry{},
}
}
opaque.Map["etags"] = &typesv1beta1.OpaqueEntry{
Decoder: "json",
Value: marshalled,
}
res.Opaque = opaque
}

return res, nil
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package sharesstorageprovider

import (
"context"
"encoding/json"
"fmt"
"path/filepath"
"strings"
Expand Down Expand Up @@ -62,6 +63,7 @@ type GatewayClient interface {
InitiateFileUpload(ctx context.Context, req *provider.InitiateFileUploadRequest, opts ...grpc.CallOption) (*gateway.InitiateFileUploadResponse, error)
SetArbitraryMetadata(ctx context.Context, req *provider.SetArbitraryMetadataRequest, opts ...grpc.CallOption) (*provider.SetArbitraryMetadataResponse, error)
UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArbitraryMetadataRequest, opts ...grpc.CallOption) (*provider.UnsetArbitraryMetadataResponse, error)
ListReceivedShares(ctx context.Context, req *collaboration.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaboration.ListReceivedSharesResponse, error)
}

// SharesProviderClient provides methods for listing and modifying received shares
Expand Down Expand Up @@ -300,7 +302,7 @@ func (s *service) CreateStorageSpace(ctx context.Context, req *provider.CreateSt
return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented")
}

// ListStorageSpaces ruturns a list storage spaces with type "share" the current user has acces to.
// ListStorageSpaces returns a list storage spaces with type "share" the current user has acces to.
// Do owners of shares see type "shared"? Do they see andyhing? They need to if the want a fast lookup of shared with others
// -> but then a storage sprovider has to do everything? not everything but permissions (= shares) related operations, yes
// The root node of every storag space is the (spaceid, nodeid) of the shared node.
Expand All @@ -313,9 +315,6 @@ func (s *service) CreateStorageSpace(ctx context.Context, req *provider.CreateSt
func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSpacesRequest) (*provider.ListStorageSpacesResponse, error) {
spaceTypes := map[string]struct{}{}
var exists = struct{}{}
res := &provider.ListStorageSpacesResponse{
Status: status.NewOK(ctx),
}
var fetchShares bool
appendTypes := []string{}
var spaceID *provider.ResourceId
Expand Down Expand Up @@ -360,8 +359,9 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
}

var receivedShares []*collaboration.ReceivedShare
var shareEtags map[string]string
if fetchShares {
lsRes, err := s.sharesProviderClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{
lsRes, err := s.gateway.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{
kobergj marked this conversation as resolved.
Show resolved Hide resolved
// FIXME filter by received shares for resource id - listing all shares is tooo expensive!
})
if err != nil {
Expand All @@ -371,6 +371,16 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
return nil, fmt.Errorf("sharesstorageprovider: error calling ListReceivedSharesRequest")
}
receivedShares = lsRes.Shares
if lsRes.Opaque != nil {
if entry, ok := lsRes.Opaque.Map["etags"]; ok {
// If we can't get the etags thats fine, just continue.
_ = json.Unmarshal(entry.Value, &shareEtags)
}
}
}

res := &provider.ListStorageSpacesResponse{
Status: status.NewOK(ctx),
}
for k := range spaceTypes {
switch k {
Expand All @@ -380,8 +390,29 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
OpaqueId: utils.ShareStorageProviderID,
}
if spaceID == nil || utils.ResourceIDEqual(virtualRootID, spaceID) {
var earliestShare *collaboration.Share
// Lookup the last changed received share and use its etag for the share jail.
for _, rs := range receivedShares {
current := rs.Share
switch {
case earliestShare == nil:
earliestShare = current
case current.Mtime.Seconds > earliestShare.Mtime.Seconds:
earliestShare = current
case current.Mtime.Seconds == earliestShare.Mtime.Seconds &&
current.Mtime.Nanos > earliestShare.Mtime.Nanos:
earliestShare = current
}
}

var opaque *typesv1beta1.Opaque
if earliestShare != nil {
if etag, ok := shareEtags[earliestShare.Id.OpaqueId]; ok {
opaque = utils.AppendPlainToOpaque(opaque, "etag", etag)
}
}
space := &provider.StorageSpace{
Opaque: opaque,
Id: &provider.StorageSpaceId{
OpaqueId: virtualRootID.StorageId + "!" + virtualRootID.OpaqueId,
},
Expand All @@ -401,8 +432,13 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
// none of our business
continue
}
var opaque *typesv1beta1.Opaque
if etag, ok := shareEtags[receivedShare.Share.Id.OpaqueId]; ok {
opaque = utils.AppendPlainToOpaque(opaque, "etag", etag)
}
// we know a grant for this resource
space := &provider.StorageSpace{
Opaque: opaque,
Id: &provider.StorageSpaceId{
OpaqueId: root.StorageId + "!" + root.OpaqueId,
},
Expand Down Expand Up @@ -437,7 +473,12 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
continue
}
}
var opaque *typesv1beta1.Opaque
if etag, ok := shareEtags[receivedShare.Share.Id.OpaqueId]; ok {
C0rby marked this conversation as resolved.
Show resolved Hide resolved
C0rby marked this conversation as resolved.
Show resolved Hide resolved
opaque = utils.AppendPlainToOpaque(opaque, "etag", etag)
}
space := &provider.StorageSpace{
Opaque: opaque,
Id: &provider.StorageSpaceId{
OpaqueId: root.StorageId + "!" + root.OpaqueId,
},
Expand Down