Skip to content

Commit

Permalink
include spaces etags in virtual spaces
Browse files Browse the repository at this point in the history
  • Loading branch information
David Christofas committed Mar 9, 2022
1 parent e4509f1 commit 634002f
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 6 deletions.
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?
// 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{
// 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 {
opaque = utils.AppendPlainToOpaque(opaque, "etag", etag)
}
space := &provider.StorageSpace{
Opaque: opaque,
Id: &provider.StorageSpaceId{
OpaqueId: root.StorageId + "!" + root.OpaqueId,
},
Expand Down

0 comments on commit 634002f

Please sign in to comment.