Grant id in mountpoint (#2664)

* receive also the grantID in mountpoints * fix shares * add changelog * linter * change the earliest share implementation * we could have shares with no metadata * add spaceAlias to mountpoints * move Metadata to the share pkg
cs3org · Mar 29, 2022 · 7810180 · 7810180
1 parent f9fa084
commit 7810180
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 53 deletions.
diff --git a/changelog/unreleased/sharing-fix-add-grantID.md b/changelog/unreleased/sharing-fix-add-grantID.md
@@ -0,0 +1,5 @@
+Enhancement: Add grantID to mountpoint
+
+We distinguish between the mountpoint of a share and the grant where the original file is located on the storage.
+
+https://github.com/cs3org/reva/pull/2664
diff --git a/internal/grpc/services/gateway/gateway.go b/internal/grpc/services/gateway/gateway.go
@@ -24,7 +24,6 @@ import (
 	"strings"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
-
 	"github.com/cs3org/reva/v2/pkg/errtypes"
 	"github.com/cs3org/reva/v2/pkg/rgrpc"
 	"github.com/cs3org/reva/v2/pkg/sharedconf"

diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go
@@ -32,6 +32,7 @@ import (
 	"github.com/cs3org/reva/v2/pkg/errtypes"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/status"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/v2/pkg/share"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/grants"
 	rtrace "github.com/cs3org/reva/v2/pkg/trace"
 	"github.com/pkg/errors"
@@ -166,7 +167,7 @@ func (s *svc) ListReceivedShares(ctx context.Context, req *collaboration.ListRec
 	// 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))
+	shareMetaData := make(map[string]share.Metadata, 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 {
@@ -182,10 +183,10 @@ func (s *svc) ListReceivedShares(ctx context.Context, req *collaboration.ListRec
 				Msg("ListRecievedShares: failed to stat the resource")
 			continue
 		}
-		shareEtags[rs.Share.Id.OpaqueId] = sRes.Info.Etag
+		shareMetaData[rs.Share.Id.OpaqueId] = share.Metadata{ETag: sRes.Info.Etag, Mtime: sRes.Info.Mtime}
 	}
 
-	marshalled, err := json.Marshal(shareEtags)
+	marshalled, err := json.Marshal(shareMetaData)
 	if err != nil {
 		logger.Error().
 			Err(err).
@@ -197,7 +198,7 @@ func (s *svc) ListReceivedShares(ctx context.Context, req *collaboration.ListRec
 				Map: map[string]*typesv1beta1.OpaqueEntry{},
 			}
 		}
-		opaque.Map["etags"] = &typesv1beta1.OpaqueEntry{
+		opaque.Map["shareMetadata"] = &typesv1beta1.OpaqueEntry{
 			Decoder: "json",
 			Value:   marshalled,
 		}

diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go
@@ -25,6 +25,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/cs3org/reva/v2/pkg/share"
 	"google.golang.org/grpc"
 	codes "google.golang.org/grpc/codes"
 	gstatus "google.golang.org/grpc/status"
@@ -359,24 +360,13 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 	}
 
 	var receivedShares []*collaboration.ReceivedShare
-	var shareEtags map[string]string
+	var shareMd map[string]share.Metadata
+	var err error
 	if fetchShares {
-		lsRes, err := s.gateway.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{
-			// FIXME filter by received shares for resource id - listing all shares is tooo expensive!
-		})
+		receivedShares, shareMd, err = s.fetchShares(ctx)
 		if err != nil {
 			return nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest")
 		}
-		if lsRes.Status.Code != rpc.Code_CODE_OK {
-			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{
@@ -390,39 +380,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
-					}
-				}
-
+				earliestShare, atLeastOneAccepted := findEarliestShare(receivedShares, shareMd)
 				var opaque *typesv1beta1.Opaque
 				if earliestShare != nil {
-					if etag, ok := shareEtags[earliestShare.Id.OpaqueId]; ok {
-						opaque = utils.AppendPlainToOpaque(opaque, "etag", etag)
+					if md, ok := shareMd[earliestShare.Id.OpaqueId]; ok {
+						opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag)
 					}
 				}
-				space := &provider.StorageSpace{
-					Opaque: opaque,
-					Id: &provider.StorageSpaceId{
-						OpaqueId: virtualRootID.StorageId + "!" + virtualRootID.OpaqueId,
-					},
-					SpaceType: "virtual",
-					//Owner:     &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient
-					// the sharesstorageprovider keeps track of mount points
-					Root: virtualRootID,
-					Name: "Shares Jail",
+				// only display the shares jail if we have accepted shares
+				if atLeastOneAccepted {
+					opaque = utils.AppendPlainToOpaque(opaque, "spaceAlias", "virtual/shares")
+					space := &provider.StorageSpace{
+						Opaque: opaque,
+						Id: &provider.StorageSpaceId{
+							OpaqueId: virtualRootID.StorageId + "!" + virtualRootID.OpaqueId,
+						},
+						SpaceType: "virtual",
+						//Owner:     &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient
+						// the sharesstorageprovider keeps track of mount points
+						Root: virtualRootID,
+						Name: "Shares Jail",
+					}
+					res.StorageSpaces = append(res.StorageSpaces, space)
 				}
-				res.StorageSpaces = append(res.StorageSpaces, space)
 			}
 		case "grant":
 			for _, receivedShare := range receivedShares {
@@ -433,8 +413,8 @@ 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)
+				if md, ok := shareMd[receivedShare.Share.Id.OpaqueId]; ok {
+					opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag)
 				}
 				// we know a grant for this resource
 				space := &provider.StorageSpace{
@@ -474,9 +454,15 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 					}
 				}
 				var opaque *typesv1beta1.Opaque
-				if etag, ok := shareEtags[receivedShare.Share.Id.OpaqueId]; ok {
-					opaque = utils.AppendPlainToOpaque(opaque, "etag", etag)
+				if md, ok := shareMd[receivedShare.Share.Id.OpaqueId]; ok {
+					opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag)
+				}
+				// add the resourceID for the grant
+				if receivedShare.Share.ResourceId != nil {
+					opaque = utils.AppendPlainToOpaque(opaque, "grantStorageID", receivedShare.Share.ResourceId.StorageId)
+					opaque = utils.AppendPlainToOpaque(opaque, "grantOpaqueID", receivedShare.Share.ResourceId.OpaqueId)
 				}
+
 				space := &provider.StorageSpace{
 					Opaque: opaque,
 					Id: &provider.StorageSpaceId{
@@ -492,6 +478,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 				// for now use the name from the share to override the name determined by stat
 				if receivedShare.MountPoint != nil {
 					space.Name = receivedShare.MountPoint.Path
+					space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "spaceAlias", space.SpaceType+"/"+strings.ReplaceAll(strings.ToLower(space.Name), " ", "-"))
 				}
 
 				// what if we don't have a name?
@@ -670,7 +657,11 @@ func (s *service) Unlock(ctx context.Context, req *provider.UnlockRequest) (*pro
 
 func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) {
 	if isVirtualRoot(req.Ref.ResourceId) && (req.Ref.Path == "" || req.Ref.Path == ".") {
-		// The root is empty, it is filled by mountpoints
+		receivedShares, shareMd, err := s.fetchShares(ctx)
+		if err != nil {
+			return nil, err
+		}
+		earliestShare, _ := findEarliestShare(receivedShares, shareMd)
 		return &provider.StatResponse{
 			Status: status.NewOK(ctx),
 			Info: &provider.ResourceInfo{
@@ -687,13 +678,14 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide
 					OpaqueId:  utils.ShareStorageProviderID,
 				},
 				Type:          provider.ResourceType_RESOURCE_TYPE_CONTAINER,
-				Mtime:         &typesv1beta1.Timestamp{},
+				Mtime:         shareMd[earliestShare.Id.OpaqueId].Mtime,
 				Path:          "/",
 				MimeType:      "httpd/unix-directory",
 				Size:          0,
 				PermissionSet: &provider.ResourcePermissions{
 					// TODO
 				},
+				Etag: shareMd[earliestShare.Id.OpaqueId].ETag,
 			},
 		}, nil
 	}
@@ -939,3 +931,59 @@ func (s *service) rejectReceivedShare(ctx context.Context, receivedShare *collab
 
 	return errtypes.NewErrtypeFromStatus(res.Status)
 }
+
+func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedShare, map[string]share.Metadata, error) {
+	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 {
+		return nil, nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest")
+	}
+	if lsRes.Status.Code != rpc.Code_CODE_OK {
+		return nil, nil, fmt.Errorf("sharesstorageprovider: error calling ListReceivedSharesRequest")
+	}
+	receivedShares := lsRes.Shares
+
+	var shareMd map[string]share.Metadata
+	if lsRes.Opaque != nil {
+		if entry, ok := lsRes.Opaque.Map["shareMetadata"]; ok {
+			// If we can't get the etags thats fine, just continue.
+			_ = json.Unmarshal(entry.Value, &shareMd)
+		}
+	}
+	return receivedShares, shareMd, nil
+}
+
+func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareMd map[string]share.Metadata) (earliestShare *collaboration.Share, atLeastOneAccepted bool) {
+	for _, rs := range receivedShares {
+		var hasCurrentMd bool
+		var hasEarliestMd bool
+
+		current := rs.Share
+		if rs.State == collaboration.ShareState_SHARE_STATE_ACCEPTED {
+			atLeastOneAccepted = true
+		}
+
+		// We cannot assume that every share has metadata
+		if current.Id != nil {
+			_, hasCurrentMd = shareMd[current.Id.OpaqueId]
+		}
+		if earliestShare != nil && earliestShare.Id != nil {
+			_, hasEarliestMd = shareMd[earliestShare.Id.OpaqueId]
+		}
+
+		switch {
+		case earliestShare == nil:
+			earliestShare = current
+		// ignore if one of the shares has no metadata
+		case !hasEarliestMd || !hasCurrentMd:
+			continue
+		case shareMd[current.Id.OpaqueId].Mtime.Seconds > shareMd[earliestShare.Id.OpaqueId].Mtime.Seconds:
+			earliestShare = current
+		case shareMd[current.Id.OpaqueId].Mtime.Seconds == shareMd[earliestShare.Id.OpaqueId].Mtime.Seconds &&
+			shareMd[current.Id.OpaqueId].Mtime.Nanos > shareMd[earliestShare.Id.OpaqueId].Mtime.Nanos:
+			earliestShare = current
+		}
+	}
+	return earliestShare, atLeastOneAccepted
+}
diff --git a/pkg/share/share.go b/pkg/share/share.go
@@ -24,6 +24,7 @@ import (
 	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/conversions"
 	"github.com/cs3org/reva/v2/pkg/utils"
 	"google.golang.org/genproto/protobuf/field_mask"
@@ -37,6 +38,12 @@ const (
 
 //go:generate mockery -name Manager
 
+// Metadata contains Metadata for a share
+type Metadata struct {
+	ETag  string
+	Mtime *types.Timestamp
+}
+
 // Manager is the interface that manipulates shares.
 type Manager interface {
 	// Create a new share in fn with the given acl.