Refactor sharing

changelog/unreleased/update-share-response.md

@@ -0,0 +1,5 @@
+Enhancement: Add share to update response
+
+After accepting or rejecting a share the API includes the updated share in the response.
+
+https://github.com/cs3org/reva/pull/1685

internal/grpc/services/gateway/usershareprovider.go

@@ -196,23 +196,9 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq
 	// TODO(labkode): if both commits are enabled they could be done concurrently.
 
 	if s.c.CommitShareToStorageGrant {
-		getShareReq := &collaboration.GetShareRequest{
-			Ref: req.Ref,
-		}
-		getShareRes, err := c.GetShare(ctx, getShareReq)
-		if err != nil {
-			return nil, errors.Wrap(err, "gateway: error calling GetShare")
-		}
-
-		if getShareRes.Status.Code != rpc.Code_CODE_OK {
-			return &collaboration.UpdateShareResponse{
-				Status: status.NewInternal(ctx, status.NewErrorFromCode(getShareRes.Status.Code, "gateway"),
-					"error getting share when committing to the share"),
-			}, nil
-		}
-		updateGrantStatus, err := s.updateGrant(ctx, getShareRes.GetShare().GetResourceId(),
-			getShareRes.GetShare().GetGrantee(),
-			getShareRes.GetShare().GetPermissions().GetPermissions())
+		updateGrantStatus, err := s.updateGrant(ctx, res.GetShare().GetResourceId(),
+			res.GetShare().GetGrantee(),
+			res.GetShare().GetPermissions().GetPermissions())
 
 		if err != nil {
 			return nil, errors.Wrap(err, "gateway: error calling updateGrant")
@@ -221,6 +207,7 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq
 		if updateGrantStatus.Code != rpc.Code_CODE_OK {
 			return &collaboration.UpdateShareResponse{
 				Status: updateGrantStatus,
+				Share:  res.Share,
 			}, nil
 		}
 	}
@@ -309,34 +296,17 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update
 	// share display name and storage filename.
 	if req.Field.GetState() != collaboration.ShareState_SHARE_STATE_INVALID {
 		if req.Field.GetState() == collaboration.ShareState_SHARE_STATE_ACCEPTED {
-			getShareReq := &collaboration.GetReceivedShareRequest{Ref: req.Ref}
-			getShareRes, err := s.GetReceivedShare(ctx, getShareReq)
-			if err != nil {
-				log.Err(err).Msg("gateway: error calling GetReceivedShare")
-				return &collaboration.UpdateReceivedShareResponse{
-					Status: &rpc.Status{
-						Code: rpc.Code_CODE_INTERNAL,
-					},
-				}, nil
-			}
-
-			if getShareRes.Status.Code != rpc.Code_CODE_OK {
-				log.Error().Msg("gateway: error calling GetReceivedShare")
-				return &collaboration.UpdateReceivedShareResponse{
-					Status: &rpc.Status{
-						Code: rpc.Code_CODE_INTERNAL,
-					},
-				}, nil
-			}
-
-			share := getShareRes.Share
+			share := res.Share
 			if share == nil {
 				panic("gateway: error updating a received share: the share is nil")
 			}
-			createRefStatus, err := s.createReference(ctx, share.Share.ResourceId)
-			return &collaboration.UpdateReceivedShareResponse{
-				Status: createRefStatus,
-			}, err
+			createRefStatus := s.createReference(ctx, share.Share.ResourceId)
+			rsp := &collaboration.UpdateReceivedShareResponse{Status: createRefStatus}
+
+			if createRefStatus.Code == rpc.Code_CODE_OK {
+				rsp.Share = share
+			}
+			return rsp, nil
 		} else if req.Field.GetState() == collaboration.ShareState_SHARE_STATE_REJECTED {
 			// Nothing more to do, return the original result
 			return res, nil
@@ -350,17 +320,17 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update
 	}, nil
 }
 
-func (s *svc) createReference(ctx context.Context, resourceID *provider.ResourceId) (*rpc.Status, error) {
+func (s *svc) createReference(ctx context.Context, resourceID *provider.ResourceId) *rpc.Status {
 
 	log := appctx.GetLogger(ctx)
 
 	// get the metadata about the share
 	c, err := s.findByID(ctx, resourceID)
 	if err != nil {
 		if _, ok := err.(errtypes.IsNotFound); ok {
-			return status.NewNotFound(ctx, "storage provider not found"), nil
+			return status.NewNotFound(ctx, "storage provider not found")
 		}
-		return status.NewInternal(ctx, err, "error finding storage provider"), nil
+		return status.NewInternal(ctx, err, "error finding storage provider")
 	}
 
 	statReq := &provider.StatRequest{
@@ -373,19 +343,19 @@ func (s *svc) createReference(ctx context.Context, resourceID *provider.Resource
 
 	statRes, err := c.Stat(ctx, statReq)
 	if err != nil {
-		return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+resourceID.String()), nil
+		return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+resourceID.String())
 	}
 
 	if statRes.Status.Code != rpc.Code_CODE_OK {
 		err := status.NewErrorFromCode(statRes.Status.GetCode(), "gateway")
 		log.Err(err).Msg("gateway: Stat failed on the share resource id: " + resourceID.String())
-		return status.NewInternal(ctx, err, "error updating received share"), nil
+		return status.NewInternal(ctx, err, "error updating received share")
 	}
 
 	homeRes, err := s.GetHome(ctx, &provider.GetHomeRequest{})
 	if err != nil {
 		err := errors.Wrap(err, "gateway: error calling GetHome")
-		return status.NewInternal(ctx, err, "error updating received share"), nil
+		return status.NewInternal(ctx, err, "error updating received share")
 	}
 
 	// reference path is the home path + some name
@@ -410,25 +380,25 @@ func (s *svc) createReference(ctx context.Context, resourceID *provider.Resource
 	c, err = s.findByPath(ctx, refPath)
 	if err != nil {
 		if _, ok := err.(errtypes.IsNotFound); ok {
-			return status.NewNotFound(ctx, "storage provider not found"), nil
+			return status.NewNotFound(ctx, "storage provider not found")
 		}
-		return status.NewInternal(ctx, err, "error finding storage provider"), nil
+		return status.NewInternal(ctx, err, "error finding storage provider")
 	}
 
 	createRefRes, err := c.CreateReference(ctx, createRefReq)
 	if err != nil {
 		log.Err(err).Msg("gateway: error calling GetHome")
 		return &rpc.Status{
 			Code: rpc.Code_CODE_INTERNAL,
-		}, nil
+		}
 	}
 
 	if createRefRes.Status.Code != rpc.Code_CODE_OK {
 		err := status.NewErrorFromCode(createRefRes.Status.GetCode(), "gateway")
-		return status.NewInternal(ctx, err, "error updating received share"), nil
+		return status.NewInternal(ctx, err, "error updating received share")
 	}
 
-	return status.NewOK(ctx), nil
+	return status.NewOK(ctx)
 }
 
 func (s *svc) addGrant(ctx context.Context, id *provider.ResourceId, g *provider.Grantee, p *provider.ResourcePermissions) (*rpc.Status, error) {

internal/grpc/services/usershareprovider/usershareprovider.go

@@ -172,7 +172,7 @@ func (s *service) ListShares(ctx context.Context, req *collaboration.ListSharesR
 }
 
 func (s *service) UpdateShare(ctx context.Context, req *collaboration.UpdateShareRequest) (*collaboration.UpdateShareResponse, error) {
-	_, err := s.sm.UpdateShare(ctx, req.Ref, req.Field.GetPermissions()) // TODO(labkode): check what to update
+	share, err := s.sm.UpdateShare(ctx, req.Ref, req.Field.GetPermissions()) // TODO(labkode): check what to update
 	if err != nil {
 		return &collaboration.UpdateShareResponse{
 			Status: status.NewInternal(ctx, err, "error updating share"),
@@ -181,6 +181,7 @@ func (s *service) UpdateShare(ctx context.Context, req *collaboration.UpdateShar
 
 	res := &collaboration.UpdateShareResponse{
 		Status: status.NewOK(ctx),
+		Share:  share,
 	}
 	return res, nil
 }
@@ -219,7 +220,7 @@ func (s *service) GetReceivedShare(ctx context.Context, req *collaboration.GetRe
 }
 
 func (s *service) UpdateReceivedShare(ctx context.Context, req *collaboration.UpdateReceivedShareRequest) (*collaboration.UpdateReceivedShareResponse, error) {
-	_, err := s.sm.UpdateReceivedShare(ctx, req.Ref, req.Field) // TODO(labkode): check what to update
+	share, err := s.sm.UpdateReceivedShare(ctx, req.Ref, req.Field) // TODO(labkode): check what to update
 	if err != nil {
 		return &collaboration.UpdateReceivedShareResponse{
 			Status: status.NewInternal(ctx, err, "error updating received share"),
@@ -228,6 +229,7 @@ func (s *service) UpdateReceivedShare(ctx context.Context, req *collaboration.Up
 
 	res := &collaboration.UpdateReceivedShareResponse{
 		Status: status.NewOK(ctx),
+		Share:  share,
 	}
 	return res, nil
 }

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/group.go

@@ -80,30 +80,5 @@ func (h *Handler) createGroupShare(w http.ResponseWriter, r *http.Request, statI
 		},
 	}
 
-	createShareResponse, err := c.CreateShare(ctx, createShareReq)
-	if err != nil {
-		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error sending a grpc create share request", err)
-		return
-	}
-	if createShareResponse.Status.Code != rpc.Code_CODE_OK {
-		if createShareResponse.Status.Code == rpc.Code_CODE_NOT_FOUND {
-			response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "not found", nil)
-			return
-		}
-		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc create share request failed", err)
-		return
-	}
-	s, err := conversions.CS3Share2ShareData(ctx, createShareResponse.Share)
-	if err != nil {
-		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err)
-		return
-	}
-	err = h.addFileInfo(ctx, s, statInfo)
-	if err != nil {
-		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error adding fileinfo to share", err)
-		return
-	}
-	h.mapUserIds(ctx, c, s)
-
-	response.WriteOCSSuccess(w, r, s)
+	h.createCs3Share(ctx, w, r, c, createShareReq, statInfo)
 }

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go

@@ -20,31 +20,37 @@ package shares
 
 import (
 	"net/http"
+	"path"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
+	"github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions"
 	"github.com/cs3org/reva/internal/http/services/owncloud/ocs/response"
+	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
 	"github.com/pkg/errors"
 )
 
 func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, shareID string, rejectShare bool) {
 	ctx := r.Context()
+	logger := appctx.GetLogger(ctx)
 
-	uClient, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+	client, err := pool.GetGatewayServiceClient(h.gatewayAddr)
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err)
 		return
 	}
 
-	shareRequest := &collaboration.UpdateReceivedShareRequest{
-		Ref: &collaboration.ShareReference{
-			Spec: &collaboration.ShareReference_Id{
-				Id: &collaboration.ShareId{
-					OpaqueId: shareID,
-				},
+	ref := &collaboration.ShareReference{
+		Spec: &collaboration.ShareReference_Id{
+			Id: &collaboration.ShareId{
+				OpaqueId: shareID,
 			},
 		},
+	}
+
+	shareRequest := &collaboration.UpdateReceivedShareRequest{
+		Ref: ref,
 		Field: &collaboration.UpdateReceivedShareRequest_UpdateField{
 			Field: &collaboration.UpdateReceivedShareRequest_UpdateField_State{
 				State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
@@ -59,7 +65,7 @@ func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, sh
 		}
 	}
 
-	shareRes, err := uClient.UpdateReceivedShare(ctx, shareRequest)
+	shareRes, err := client.UpdateReceivedShare(ctx, shareRequest)
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc update received share request failed", err)
 		return
@@ -73,4 +79,31 @@ func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, sh
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc update received share request failed", errors.Errorf("code: %d, message: %s", shareRes.Status.Code, shareRes.Status.Message))
 		return
 	}
+
+	rs := shareRes.GetShare()
+
+	info, status, err := h.getResourceInfoByID(ctx, client, rs.Share.ResourceId)
+	if err != nil || status.Code != rpc.Code_CODE_OK {
+		h.logProblems(status, err, "could not stat, skipping")
+	}
+
+	data, err := conversions.CS3Share2ShareData(r.Context(), rs.Share)
+	if err != nil {
+		logger.Debug().Interface("share", rs.Share).Interface("shareData", data).Err(err).Msg("could not CS3Share2ShareData, skipping")
+	}
+
+	data.State = mapState(rs.GetState())
+
+	if err := h.addFileInfo(ctx, data, info); err != nil {
+		logger.Debug().Interface("received_share", rs).Interface("info", info).Interface("shareData", data).Err(err).Msg("could not add file info, skipping")
+	}
+	h.mapUserIds(r.Context(), client, data)
+
+	if data.State == ocsStateAccepted {
+		// Needed because received shares can be jailed in a folder in the users home
+		data.FileTarget = path.Join(h.sharePrefix, path.Base(info.Path))
+		data.Path = path.Join(h.sharePrefix, path.Base(info.Path))
+	}
+
+	response.WriteOCSSuccess(w, r, data)
 }

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
-	"time"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
@@ -143,7 +142,7 @@ func (h *Handler) listPublicShares(r *http.Request, filters []*link.ListPublicSh
 	ocsDataPayload := make([]*conversions.ShareData, 0)
 	// TODO(refs) why is this guard needed? Are we moving towards a gateway only for service discovery? without a gateway this is dead code.
 	if h.gatewayAddr != "" {
-		c, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+		client, err := pool.GetGatewayServiceClient(h.gatewayAddr)
 		if err != nil {
 			return ocsDataPayload, nil, err
 		}
@@ -152,38 +151,19 @@ func (h *Handler) listPublicShares(r *http.Request, filters []*link.ListPublicSh
 			Filters: filters,
 		}
 
-		res, err := c.ListPublicShares(ctx, &req)
+		res, err := client.ListPublicShares(ctx, &req)
 		if err != nil {
 			return ocsDataPayload, nil, err
 		}
 		if res.Status.Code != rpc.Code_CODE_OK {
 			return ocsDataPayload, res.Status, nil
 		}
 
-		var info *provider.ResourceInfo
 		for _, share := range res.GetShare() {
-			key := wrapResourceID(share.ResourceId)
-			if infoIf, err := h.resourceInfoCache.Get(key); h.resourceInfoCacheTTL > 0 && err == nil {
-				log.Debug().Msgf("cache hit for resource %+v", share.ResourceId)
-				info = infoIf.(*provider.ResourceInfo)
-			} else {
-				statRequest := &provider.StatRequest{
-					Ref: &provider.Reference{
-						Spec: &provider.Reference_Id{
-							Id: share.ResourceId,
-						},
-					},
-				}
-
-				statResponse, err := c.Stat(ctx, statRequest)
-				if err != nil || res.Status.Code != rpc.Code_CODE_OK {
-					log.Debug().Interface("share", share).Interface("response", statResponse).Err(err).Msg("could not stat share, skipping")
-					continue
-				}
-				info = statResponse.Info
-				if h.resourceInfoCacheTTL > 0 {
-					_ = h.resourceInfoCache.SetWithExpire(key, info, time.Second*h.resourceInfoCacheTTL)
-				}
+			info, status, err := h.getResourceInfoByID(ctx, client, share.ResourceId)
+			if err != nil || status.Code != rpc.Code_CODE_OK {
+				log.Debug().Interface("share", share).Interface("status", status).Err(err).Msg("could not stat share, skipping")
+				continue
 			}
 
 			sData := conversions.PublicShare2ShareData(share, r, h.publicURL)
@@ -194,7 +174,7 @@ func (h *Handler) listPublicShares(r *http.Request, filters []*link.ListPublicSh
 				log.Debug().Interface("share", share).Interface("info", info).Err(err).Msg("could not add file info, skipping")
 				continue
 			}
-			h.mapUserIds(ctx, c, sData)
+			h.mapUserIds(ctx, client, sData)
 
 			log.Debug().Interface("share", share).Interface("info", info).Interface("shareData", share).Msg("mapped")