From 634002fedca6707e28a6644a447729b5969c46ed Mon Sep 17 00:00:00 2001 From: David Christofas Date: Tue, 8 Mar 2022 16:06:21 +0100 Subject: [PATCH] include spaces etags in virtual spaces --- changelog/unreleased/etags-virtual-spaces.md | 5 ++ .../services/gateway/usershareprovider.go | 46 ++++++++++++++++- .../mocks/GatewayClient.go | 33 ++++++++++++ .../sharesstorageprovider.go | 51 +++++++++++++++++-- 4 files changed, 129 insertions(+), 6 deletions(-) create mode 100644 changelog/unreleased/etags-virtual-spaces.md diff --git a/changelog/unreleased/etags-virtual-spaces.md b/changelog/unreleased/etags-virtual-spaces.md new file mode 100644 index 0000000000..d77be2a97c --- /dev/null +++ b/changelog/unreleased/etags-virtual-spaces.md @@ -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 diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 4a07923b71..88f2ab7cd4 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -20,6 +20,7 @@ package gateway import ( "context" + "encoding/json" "path" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -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{ @@ -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? + // 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 } diff --git a/internal/grpc/services/sharesstorageprovider/mocks/GatewayClient.go b/internal/grpc/services/sharesstorageprovider/mocks/GatewayClient.go index 5538440905..580bf5655d 100644 --- a/internal/grpc/services/sharesstorageprovider/mocks/GatewayClient.go +++ b/internal/grpc/services/sharesstorageprovider/mocks/GatewayClient.go @@ -23,7 +23,10 @@ package mocks import ( context "context" + collaborationv1beta1 "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" + gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + grpc "google.golang.org/grpc" mock "github.com/stretchr/testify/mock" @@ -246,6 +249,36 @@ func (_m *GatewayClient) ListFileVersions(ctx context.Context, req *providerv1be return r0, r1 } +// ListReceivedShares provides a mock function with given fields: ctx, req, opts +func (_m *GatewayClient) ListReceivedShares(ctx context.Context, req *collaborationv1beta1.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, req) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 *collaborationv1beta1.ListReceivedSharesResponse + if rf, ok := ret.Get(0).(func(context.Context, *collaborationv1beta1.ListReceivedSharesRequest, ...grpc.CallOption) *collaborationv1beta1.ListReceivedSharesResponse); ok { + r0 = rf(ctx, req, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*collaborationv1beta1.ListReceivedSharesResponse) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *collaborationv1beta1.ListReceivedSharesRequest, ...grpc.CallOption) error); ok { + r1 = rf(ctx, req, opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // Move provides a mock function with given fields: ctx, in, opts func (_m *GatewayClient) Move(ctx context.Context, in *providerv1beta1.MoveRequest, opts ...grpc.CallOption) (*providerv1beta1.MoveResponse, error) { _va := make([]interface{}, len(opts)) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 4df9bfdda0..4164784c3f 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -20,6 +20,7 @@ package sharesstorageprovider import ( "context" + "encoding/json" "fmt" "path/filepath" "strings" @@ -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 @@ -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. @@ -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 @@ -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{ // FIXME filter by received shares for resource id - listing all shares is tooo expensive! }) if err != nil { @@ -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 { @@ -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, }, @@ -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, }, @@ -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 { + opaque = utils.AppendPlainToOpaque(opaque, "etag", etag) + } space := &provider.StorageSpace{ + Opaque: opaque, Id: &provider.StorageSpaceId{ OpaqueId: root.StorageId + "!" + root.OpaqueId, },