From ed1ccb69682e6c0216a30ba65eee50bc31192103 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Mon, 12 Feb 2024 17:56:40 +0100 Subject: [PATCH] enhancement(sharing): allow unmounting a share --- .../pkg/service/v0/api_drives_drive_item.go | 20 +++++++-- .../service/v0/api_drives_drive_item_test.go | 42 ++++++++++++++++--- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/services/graph/pkg/service/v0/api_drives_drive_item.go b/services/graph/pkg/service/v0/api_drives_drive_item.go index 1c799eeecc4..5f6f1b63312 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item.go +++ b/services/graph/pkg/service/v0/api_drives_drive_item.go @@ -14,6 +14,7 @@ import ( "google.golang.org/protobuf/types/known/fieldmaskpb" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3/shareid" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/owncloud/ocis/v2/ocis-pkg/log" @@ -56,6 +57,14 @@ func (s DrivesDriveItemService) UnmountShare(ctx context.Context, resourceID sto return err } + // HACK: This is a hack we should not rely on a specific format of the item id, but current the real + // resource id of the shared resource is buried in the opaque id of the share's resourceid + // Also this relies on specific behaviour of the jsoncs3 share manager + storageId, spaceId, opaqueId := shareid.Decode(resourceID.GetOpaqueId()) + resourceID.StorageId = storageId + resourceID.SpaceId = spaceId + resourceID.OpaqueId = opaqueId + receivedSharesResponse, err := gatewayClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ Filters: []*collaboration.Filter{ { @@ -86,17 +95,20 @@ func (s DrivesDriveItemService) UnmountShare(ctx context.Context, resourceID sto UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{_fieldMaskPathState}}, } - updateReceivedShareResponse, err := gatewayClient.UpdateReceivedShare(ctx, updateReceivedShareRequest) + _, err := gatewayClient.UpdateReceivedShare(ctx, updateReceivedShareRequest) if err != nil { errs = append(errs, err) continue } + } - // fixMe: send to nirvana, wait for toDriverItem func - _ = updateReceivedShareResponse + // We call it a success if all shares could successfully be rejected, otherwise + // we return an error + if len(errs) != 0 { + return errors.Join(errs...) } - return errors.Join(errs...) + return nil } // MountShare mounts a share diff --git a/services/graph/pkg/service/v0/api_drives_drive_item_test.go b/services/graph/pkg/service/v0/api_drives_drive_item_test.go index 9d5fa90e5bd..80a59b1c5b1 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item_test.go +++ b/services/graph/pkg/service/v0/api_drives_drive_item_test.go @@ -340,10 +340,15 @@ var _ = Describe("DrivesDriveItemService", func() { }) It("uses the correct filters to get the shares", func() { - expectedResourceID := storageprovider.ResourceId{ + driveItemResourceID := storageprovider.ResourceId{ StorageId: "1", - OpaqueId: "2", - SpaceId: "3", + SpaceId: "2", + OpaqueId: "3:4:5", + } + expectedResourceID := storageprovider.ResourceId{ + StorageId: "3", + SpaceId: "4", + OpaqueId: "5", } gatewayClient. On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything). @@ -373,13 +378,13 @@ var _ = Describe("DrivesDriveItemService", func() { return nil, nil }) - err := drivesDriveItemService.UnmountShare(context.Background(), expectedResourceID) + err := drivesDriveItemService.UnmountShare(context.Background(), driveItemResourceID) Expect(err).ToNot(HaveOccurred()) }) }) Describe("gateway client share update", func() { - It("updates the share state to be accepted", func() { + It("updates the share state to be rejected", func() { expectedShareID := collaborationv1beta1.ShareId{ OpaqueId: "1$2!3", } @@ -411,8 +416,33 @@ var _ = Describe("DrivesDriveItemService", func() { err := drivesDriveItemService.UnmountShare(context.Background(), storageprovider.ResourceId{}) Expect(err).ToNot(HaveOccurred()) }) + It("succeeds when all shares could be rejected", func() { + gatewayClient. + On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything). + Return(func(ctx context.Context, in *collaborationv1beta1.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) { + return &collaborationv1beta1.ListReceivedSharesResponse{ + Shares: []*collaborationv1beta1.ReceivedShare{ + {}, + {}, + {}, + }, + }, nil + }) + + var calls int + gatewayClient. + On("UpdateReceivedShare", mock.Anything, mock.Anything, mock.Anything). + Return(func(ctx context.Context, in *collaborationv1beta1.UpdateReceivedShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) { + calls++ + return &collaborationv1beta1.UpdateReceivedShareResponse{}, nil + }) + + err := drivesDriveItemService.UnmountShare(context.Background(), storageprovider.ResourceId{}) + Expect(calls).To(Equal(3)) + Expect(err).ToNot(HaveOccurred()) + }) - It("bubbles errors and continues", func() { + It("bubbles errors when any share fails rejecting", func() { gatewayClient. On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything). Return(func(ctx context.Context, in *collaborationv1beta1.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) {