diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go index b19b58170b..ca6c28df5d 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go @@ -36,6 +36,7 @@ import ( "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/response" "github.com/cs3org/reva/v2/pkg/appctx" "github.com/cs3org/reva/v2/pkg/conversions" + "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/utils" "github.com/go-chi/chi/v5" "github.com/pkg/errors" @@ -63,65 +64,110 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) { return } - rs, ocsResponse := getReceivedShareFromID(ctx, client, shareID) + receivedShare, ocsResponse := getReceivedShareFromID(ctx, client, shareID) if ocsResponse != nil { response.WriteOCSResponse(w, r, *ocsResponse, nil) return } - rs.State = collaboration.ShareState_SHARE_STATE_ACCEPTED - - // now we also need to determine the mount point by evaluating all other mounted shares - - sharedResource, ocsResponse := getSharedResource(ctx, client, rs.Share.ResourceId) + sharedResource, ocsResponse := getSharedResource(ctx, client, receivedShare.Share.ResourceId) if ocsResponse != nil { response.WriteOCSResponse(w, r, *ocsResponse, nil) return } - lrs, ocsResponse := getSharesList(ctx, client) - if ocsResponse != nil { - response.WriteOCSResponse(w, r, *ocsResponse, nil) + mount, unmountedShares, err := GetMountpointAndUnmountedShares(ctx, client, sharedResource.Info) + if err != nil { + response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "could not determine mountpoint", err) + return + } + + // first update the requested share + receivedShare.State = collaboration.ShareState_SHARE_STATE_ACCEPTED + // we need to add a path to the share + receivedShare.MountPoint = &provider.Reference{ + Path: mount, + } + + updateMask := &fieldmaskpb.FieldMask{Paths: []string{"state", "mount_point"}} + data, meta, err := h.updateReceivedShare(r.Context(), receivedShare, updateMask) + if err != nil { + // we log an error for affected shares, for the actual share we return an error + response.WriteOCSData(w, r, meta, data, err) return } + response.WriteOCSSuccess(w, r, []*conversions.ShareData{data}) + + // then update other unmounted shares to the same resource + for _, rs := range unmountedShares { + if rs.GetShare().GetId().GetOpaqueId() == shareID { + // we already updated this one + continue + } + + rs.State = collaboration.ShareState_SHARE_STATE_ACCEPTED + // set the same mountpoint as for the requested received share + rs.MountPoint = &provider.Reference{ + Path: mount, + } + + _, _, err := h.updateReceivedShare(r.Context(), rs, updateMask) + if err != nil { + // we log an error for affected shares, the actual share was successful + appctx.GetLogger(ctx).Error().Err(err).Str("received_share", shareID).Str("affected_share", rs.GetShare().GetId().GetOpaqueId()).Msg("could not update affected received share") + } + } +} + +// GetMountpointAndUnmountedShares returns a new or existing mountpoint for the given info and produces a list of unmounted received shares for the same resource +func GetMountpointAndUnmountedShares(ctx context.Context, gwc gateway.GatewayAPIClient, info *provider.ResourceInfo) (string, []*collaboration.ReceivedShare, error) { + unmountedShares := []*collaboration.ReceivedShare{} + receivedShares, err := listReceivedShares(ctx, gwc) + if err != nil { + return "", unmountedShares, err + } // we need to sort the received shares by mount point in order to make things easier to evaluate. - base := path.Base(sharedResource.GetInfo().GetPath()) // TODO should use name - mount := base - var mountedShares []*collaboration.ReceivedShare - sharesToAccept := map[string]bool{shareID: true} - for _, s := range lrs.Shares { - if utils.ResourceIDEqual(s.Share.ResourceId, rs.Share.GetResourceId()) { + mount := filepath.Clean(info.Name) + existingMountpoint := "" + mountedShares := make([]*collaboration.ReceivedShare, 0, len(receivedShares)) + for _, s := range receivedShares { + if utils.ResourceIDEqual(s.Share.ResourceId, info.GetId()) { if s.State == collaboration.ShareState_SHARE_STATE_ACCEPTED { - mount = s.MountPoint.Path + // a share to the resource already exists and is mounted, remember the mount point + _, err := utils.GetResourceByID(ctx, s.Share.ResourceId, gwc) + if err == nil { + existingMountpoint = s.MountPoint.Path + } } else { - sharesToAccept[s.Share.Id.OpaqueId] = true - } - } else { - if s.State == collaboration.ShareState_SHARE_STATE_ACCEPTED { - mountedShares = append(mountedShares, s) + // a share to the resource already exists but is not mounted, collect the unmounted share + unmountedShares = append(unmountedShares, s) } } + + if s.State == collaboration.ShareState_SHARE_STATE_ACCEPTED { + mountedShares = append(mountedShares, s) + } } - compareMountPoint := func(i, j int) bool { + sort.Slice(mountedShares, func(i, j int) bool { return mountedShares[i].MountPoint.Path > mountedShares[j].MountPoint.Path + }) + + if existingMountpoint != "" { + // we want to reuse the same mountpoint for all unmounted shares to the same resource + return existingMountpoint, unmountedShares, nil } - sort.Slice(mountedShares, compareMountPoint) - // now we have a list of shares, we want to iterate over all of them and check for name collisions + // we have a list of shares, we want to iterate over all of them and check for name collisions for i, ms := range mountedShares { if ms.MountPoint.Path == mount { // does the shared resource still exist? - res, err := client.Stat(ctx, &provider.StatRequest{ - Ref: &provider.Reference{ - ResourceId: ms.Share.ResourceId, - }, - }) - if err == nil && res.Status.Code == rpc.Code_CODE_OK { + _, err := utils.GetResourceByID(ctx, ms.Share.ResourceId, gwc) + if err == nil { // The mount point really already exists, we need to insert a number into the filename - ext := filepath.Ext(base) - name := strings.TrimSuffix(base, ext) + ext := filepath.Ext(mount) + name := strings.TrimSuffix(mount, ext) // be smart about .tar.(gz|bz) files if strings.HasSuffix(name, ".tar") { name = strings.TrimSuffix(name, ".tar") @@ -133,30 +179,7 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) { // TODO we could delete shares here if the stat returns code NOT FOUND ... but listening for file deletes would be better } } - - // we need to add a path to the share - rs.MountPoint = &provider.Reference{ - Path: mount, - } - updateMask := &fieldmaskpb.FieldMask{Paths: []string{"state", "mount_point"}} - - for id := range sharesToAccept { - data, meta, err := h.updateReceivedShare(r.Context(), rs, updateMask) - if err != nil { - // we log an error for affected shares, for the actual share we return an error - if id == shareID { - response.WriteOCSData(w, r, meta, nil, err) - continue - } else { - appctx.GetLogger(ctx).Error().Err(err).Str("received_share", shareID).Str("affected_share", id).Msg("could not update affected received share") - } - } else { - // only render the data for the changed share - if id == shareID { - response.WriteOCSSuccess(w, r, []*conversions.ShareData{data}) - } - } - } + return mount, unmountedShares, nil } // RejectReceivedShare handles DELETE Requests on /apps/files_sharing/api/v1/shares/{shareid} @@ -388,23 +411,17 @@ func getSharedResource(ctx context.Context, client gateway.GatewayAPIClient, res return res, nil } -// getSharedResource gets the list of all shares for the current user. -func getSharesList(ctx context.Context, client gateway.GatewayAPIClient) (*collaboration.ListReceivedSharesResponse, *response.Response) { - shares, err := client.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) +// listReceivedShares list all received shares for the current user. +func listReceivedShares(ctx context.Context, client gateway.GatewayAPIClient) ([]*collaboration.ReceivedShare, error) { + res, err := client.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) if err != nil { - e := errors.Wrap(err, "error getting shares list") - return nil, arbitraryOcsResponse(response.MetaNotFound.StatusCode, e.Error()) + return nil, errtypes.InternalError("grpc list received shares request failed") } - if shares.Status.Code != rpc.Code_CODE_OK { - if shares.Status.Code == rpc.Code_CODE_NOT_FOUND { - e := fmt.Errorf("not found") - return nil, arbitraryOcsResponse(response.MetaNotFound.StatusCode, e.Error()) - } - e := fmt.Errorf(shares.GetStatus().GetMessage()) - return nil, arbitraryOcsResponse(response.MetaServerError.StatusCode, e.Error()) + if err := errtypes.NewErrtypeFromStatus(res.Status); err != nil { + return nil, err } - return shares, nil + return res.Shares, nil } // arbitraryOcsResponse abstracts the boilerplate that is creating a response.Response struct. diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go index b6736b3043..404f094b21 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go @@ -116,18 +116,38 @@ var _ = Describe("The ocs API", func() { ) BeforeEach(func() { - client.On("GetReceivedShare", mock.Anything, mock.Anything).Return(&collaboration.GetReceivedShareResponse{ + client.On("GetReceivedShare", mock.Anything, mock.MatchedBy(func(req *collaboration.GetReceivedShareRequest) bool { + return req.Ref.GetId().GetOpaqueId() == "1" + })).Return(&collaboration.GetReceivedShareResponse{ Status: status.NewOK(context.Background()), Share: &collaboration.ReceivedShare{ Share: share, }, }, nil) + client.On("GetReceivedShare", mock.Anything, mock.MatchedBy(func(req *collaboration.GetReceivedShareRequest) bool { + return req.Ref.GetId().GetOpaqueId() == "2" + })).Return(&collaboration.GetReceivedShareResponse{ + Status: status.NewOK(context.Background()), + Share: &collaboration.ReceivedShare{ + Share: share2, + }, + }, nil) + client.On("GetReceivedShare", mock.Anything, mock.MatchedBy(func(req *collaboration.GetReceivedShareRequest) bool { + return req.Ref.GetId().GetOpaqueId() == "3" + })).Return(&collaboration.GetReceivedShareResponse{ + Status: status.NewOK(context.Background()), + Share: &collaboration.ReceivedShare{ + Share: share3, + }, + }, nil) - client.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *provider.StatRequest) bool { + return req.GetRef().ResourceId.OpaqueId == resID.OpaqueId + })).Return(&provider.StatResponse{ Status: status.NewOK(context.Background()), Info: &provider.ResourceInfo{ Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, - Path: "/share1", + Name: "share1", Id: resID, Owner: alice.Id, PermissionSet: &provider.ResourcePermissions{ @@ -137,6 +157,22 @@ var _ = Describe("The ocs API", func() { }, }, nil) + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *provider.StatRequest) bool { + return req.GetRef().ResourceId.OpaqueId == otherResID.OpaqueId + })).Return(&provider.StatResponse{ + Status: status.NewOK(context.Background()), + Info: &provider.ResourceInfo{ + Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, + Path: "/share2", + Id: otherResID, + Owner: alice.Id, + PermissionSet: &provider.ResourcePermissions{ + Stat: true, + }, + Size: 10, + }, + }, nil) + client.On("ListContainer", mock.Anything, mock.Anything).Return(&provider.ListContainerResponse{ Status: status.NewOK(context.Background()), Infos: []*provider.ResourceInfo{ diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index 9e8cc7ee0c..f75a30b340 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -398,12 +398,12 @@ func (h *Handler) updateExistingShareMountpoints(ctx context.Context, shareType } granteeCtx = metadata.AppendToOutgoingContext(granteeCtx, ctxpkg.TokenHeader, authRes.Token) - lrs, ocsResponse := getSharesList(granteeCtx, client) - if ocsResponse != nil { - return ocsResponse.OCS.Meta.StatusCode, ocsResponse.OCS.Meta.Message, nil + receivedShares, err := listReceivedShares(granteeCtx, client) + if err != nil { + return response.MetaServerError.StatusCode, "could not list shares", nil } - for _, s := range lrs.Shares { + for _, s := range receivedShares { if s.GetShare().GetId() != share.Id && s.State == collaboration.ShareState_SHARE_STATE_ACCEPTED && utils.ResourceIDEqual(s.Share.ResourceId, info.GetId()) { updateRequest := &collaboration.UpdateReceivedShareRequest{ Share: &collaboration.ReceivedShare{