From 321c692ff4f91392b79043ec4862e03284c56603 Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Wed, 31 Jan 2024 16:13:26 +0100 Subject: [PATCH] test: add more tests for the DrivesDriveItemService implementation (cherry picked from commit d48da968af116102b21f1f4397d3f589adbc8ce1) --- services/graph/.mockery.yaml | 19 +- services/graph/mocks/gateway_selector.go | 104 ++++++ .../pkg/service/v0/api_drives_drive_item.go | 9 +- .../service/v0/api_drives_drive_item_test.go | 308 +++++++++++++++++- 4 files changed, 430 insertions(+), 10 deletions(-) create mode 100644 services/graph/mocks/gateway_selector.go diff --git a/services/graph/.mockery.yaml b/services/graph/.mockery.yaml index d3c9a20aa..a4a3d774b 100644 --- a/services/graph/.mockery.yaml +++ b/services/graph/.mockery.yaml @@ -5,13 +5,20 @@ outpkg: "mocks" packages: github.com/owncloud/ocis/v2/services/graph/pkg/service/v0: config: - dir: "mocks" + dir: "mocks" interfaces: - DrivesDriveItemProvider: - HTTPClient: - Permissions: - Publisher: - RoleService: + DrivesDriveItemProvider: + HTTPClient: + Permissions: + Publisher: + RoleService: + github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool: + config: + dir: "mocks" + interfaces: + Selectable: + config: + filename: "gateway_selector.go" github.com/owncloud/ocis/v2/services/graph/pkg/identity: config: dir: "pkg/identity/mocks" diff --git a/services/graph/mocks/gateway_selector.go b/services/graph/mocks/gateway_selector.go new file mode 100644 index 000000000..aa66401b1 --- /dev/null +++ b/services/graph/mocks/gateway_selector.go @@ -0,0 +1,104 @@ +// Code generated by mockery v2.40.1. DO NOT EDIT. + +package mocks + +import ( + pool "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" + mock "github.com/stretchr/testify/mock" +) + +// Selectable is an autogenerated mock type for the Selectable type +type Selectable[T interface{}] struct { + mock.Mock +} + +type Selectable_Expecter[T interface{}] struct { + mock *mock.Mock +} + +func (_m *Selectable[T]) EXPECT() *Selectable_Expecter[T] { + return &Selectable_Expecter[T]{mock: &_m.Mock} +} + +// Next provides a mock function with given fields: opts +func (_m *Selectable[T]) Next(opts ...pool.Option) (T, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + if len(ret) == 0 { + panic("no return value specified for Next") + } + + var r0 T + var r1 error + if rf, ok := ret.Get(0).(func(...pool.Option) (T, error)); ok { + return rf(opts...) + } + if rf, ok := ret.Get(0).(func(...pool.Option) T); ok { + r0 = rf(opts...) + } else { + r0 = ret.Get(0).(T) + } + + if rf, ok := ret.Get(1).(func(...pool.Option) error); ok { + r1 = rf(opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Selectable_Next_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Next' +type Selectable_Next_Call[T interface{}] struct { + *mock.Call +} + +// Next is a helper method to define mock.On call +// - opts ...pool.Option +func (_e *Selectable_Expecter[T]) Next(opts ...interface{}) *Selectable_Next_Call[T] { + return &Selectable_Next_Call[T]{Call: _e.mock.On("Next", + append([]interface{}{}, opts...)...)} +} + +func (_c *Selectable_Next_Call[T]) Run(run func(opts ...pool.Option)) *Selectable_Next_Call[T] { + _c.Call.Run(func(args mock.Arguments) { + variadicArgs := make([]pool.Option, len(args)-0) + for i, a := range args[0:] { + if a != nil { + variadicArgs[i] = a.(pool.Option) + } + } + run(variadicArgs...) + }) + return _c +} + +func (_c *Selectable_Next_Call[T]) Return(_a0 T, _a1 error) *Selectable_Next_Call[T] { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Selectable_Next_Call[T]) RunAndReturn(run func(...pool.Option) (T, error)) *Selectable_Next_Call[T] { + _c.Call.Return(run) + return _c +} + +// NewSelectable creates a new instance of Selectable. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewSelectable[T interface{}](t interface { + mock.TestingT + Cleanup(func()) +}) *Selectable[T] { + mock := &Selectable[T]{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} 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 77cd66ac6..6f077153a 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item.go +++ b/services/graph/pkg/service/v0/api_drives_drive_item.go @@ -8,13 +8,14 @@ import ( gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" - "github.com/cs3org/reva/v2/pkg/storagespace" - "github.com/cs3org/reva/v2/pkg/utils" "github.com/go-chi/render" libregraph "github.com/owncloud/libre-graph-api-go" "google.golang.org/protobuf/types/known/fieldmaskpb" + "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/v2/pkg/storagespace" + "github.com/cs3org/reva/v2/pkg/utils" + "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" ) @@ -199,6 +200,8 @@ func (api DrivesDriveItemApi) DeleteDriveItem(w http.ResponseWriter, r *http.Req return } + // fixMe: check if itemID is a share jail? + if err := api.drivesDriveItemService.UnmountShare(ctx, itemID); err != nil { msg := "unmounting share failed" api.logger.Debug().Err(err).Msg(msg) 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 132f7b954..e06d39362 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 @@ -5,23 +5,329 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + collaborationv1beta1 "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/go-chi/chi/v5" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/stretchr/testify/mock" "github.com/tidwall/gjson" + "google.golang.org/grpc" + "github.com/cs3org/reva/v2/pkg/storagespace" + cs3mocks "github.com/cs3org/reva/v2/tests/cs3mocks/mocks" "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/graph/mocks" svc "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0" ) +var _ = Describe("DrivesDriveItemService", func() { + var ( + drivesDriveItemService svc.DrivesDriveItemService + gatewayClient *cs3mocks.GatewayAPIClient + gatewaySelector *mocks.Selectable[gateway.GatewayAPIClient] + ) + + BeforeEach(func() { + logger := log.NewLogger() + gatewayClient = cs3mocks.NewGatewayAPIClient(GinkgoT()) + + gatewaySelector = mocks.NewSelectable[gateway.GatewayAPIClient](GinkgoT()) + gatewaySelector.On("Next").Return(gatewayClient, nil) + + service, err := svc.NewDrivesDriveItemService(logger, gatewaySelector) + Expect(err).ToNot(HaveOccurred()) + drivesDriveItemService = service + }) + + Describe("UnmountShare", func() { + It("handles gateway selector related errors", func() { + gatewaySelector.ExpectedCalls = nil + + expectedError := errors.New("obtaining next gatewayClient failed") + gatewaySelector.On("Next").Return(gatewayClient, expectedError) + + _, err := drivesDriveItemService.MountShare(context.Background(), storageprovider.ResourceId{}, "") + Expect(err).To(MatchError(expectedError)) + }) + + Describe("gateway client share listing", func() { + It("handles share listing errors", func() { + expectedError := errors.New("listing shares failed") + gatewayClient. + On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything). + Return(&collaborationv1beta1.ListReceivedSharesResponse{}, expectedError) + + _, err := drivesDriveItemService.MountShare(context.Background(), storageprovider.ResourceId{}, "") + Expect(err).To(MatchError(expectedError)) + }) + + It("uses the correct filters to get the shares", func() { + expectedResourceID := storageprovider.ResourceId{ + StorageId: "1", + OpaqueId: "2", + SpaceId: "3", + } + gatewayClient. + On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything). + Return(func(ctx context.Context, in *collaborationv1beta1.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) { + Expect(in.Filters).To(HaveLen(3)) + + var shareStates []collaborationv1beta1.ShareState + var resourceIDs []*storageprovider.ResourceId + + for _, filter := range in.Filters { + switch filter.Term.(type) { + case *collaborationv1beta1.Filter_State: + shareStates = append(shareStates, filter.GetState()) + case *collaborationv1beta1.Filter_ResourceId: + resourceIDs = append(resourceIDs, filter.GetResourceId()) + } + } + + Expect(shareStates).To(HaveLen(2)) + Expect(shareStates).To(ContainElements( + collaborationv1beta1.ShareState_SHARE_STATE_PENDING, + collaborationv1beta1.ShareState_SHARE_STATE_REJECTED, + )) + + Expect(resourceIDs).To(HaveLen(1)) + Expect(resourceIDs[0]).To(Equal(&expectedResourceID)) + + return nil, nil + }) + + _, err := drivesDriveItemService.MountShare(context.Background(), expectedResourceID, "") + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Describe("gateway client share update", func() { + It("updates the share state to be accepted", func() { + expectedShareID := collaborationv1beta1.ShareId{ + OpaqueId: "1$2!3", + } + + 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{ + { + State: collaborationv1beta1.ShareState_SHARE_STATE_PENDING, + Share: &collaborationv1beta1.Share{ + Id: &expectedShareID, + }, + }, + }, + }, nil + }) + + gatewayClient. + On("UpdateReceivedShare", mock.Anything, mock.Anything, mock.Anything). + Return(func(ctx context.Context, in *collaborationv1beta1.UpdateReceivedShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) { + Expect(in.GetUpdateMask().GetPaths()).To(Equal([]string{"state"})) + Expect(in.GetShare().GetState()).To(Equal(collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED)) + Expect(in.GetShare().GetShare().GetId().GetOpaqueId()).To(Equal(expectedShareID.GetOpaqueId())) + return &collaborationv1beta1.UpdateReceivedShareResponse{}, nil + }) + + _, err := drivesDriveItemService.MountShare(context.Background(), storageprovider.ResourceId{}, "") + Expect(err).ToNot(HaveOccurred()) + }) + + It("updates the mountPoint", 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 + }) + + gatewayClient. + On("UpdateReceivedShare", mock.Anything, mock.Anything, mock.Anything). + Return(func(ctx context.Context, in *collaborationv1beta1.UpdateReceivedShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) { + Expect(in.GetUpdateMask().GetPaths()).To(HaveLen(2)) + Expect(in.GetUpdateMask().GetPaths()).To(ContainElements("mount_point")) + Expect(in.GetShare().GetMountPoint().GetPath()).To(Equal("./new name")) + return &collaborationv1beta1.UpdateReceivedShareResponse{}, nil + }) + + _, err := drivesDriveItemService.MountShare(context.Background(), storageprovider.ResourceId{}, "new name") + Expect(err).ToNot(HaveOccurred()) + }) + + It("bubbles errors and continues", 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++ + Expect(calls).To(BeNumerically("<=", 3)) + + if calls <= 2 { + return nil, fmt.Errorf("error %d", calls) + } + + return &collaborationv1beta1.UpdateReceivedShareResponse{}, nil + }) + + _, err := drivesDriveItemService.MountShare(context.Background(), storageprovider.ResourceId{}, "new name") + Expect(fmt.Sprint(err)).To(Equal("error 1\nerror 2")) + }) + }) + }) + + Describe("UnmountShare", func() { + It("handles gateway selector related errors", func() { + gatewaySelector.ExpectedCalls = nil + + expectedError := errors.New("obtaining next gatewayClient failed") + gatewaySelector.On("Next").Return(gatewayClient, expectedError) + + err := drivesDriveItemService.UnmountShare(context.Background(), storageprovider.ResourceId{}) + Expect(err).To(MatchError(expectedError)) + }) + + Describe("gateway client share listing", func() { + It("handles share listing errors", func() { + expectedError := errors.New("listing shares failed") + gatewayClient. + On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything). + Return(&collaborationv1beta1.ListReceivedSharesResponse{}, expectedError) + + err := drivesDriveItemService.UnmountShare(context.Background(), storageprovider.ResourceId{}) + Expect(err).To(MatchError(expectedError)) + }) + + It("uses the correct filters to get the shares", func() { + expectedResourceID := storageprovider.ResourceId{ + StorageId: "1", + OpaqueId: "2", + SpaceId: "3", + } + gatewayClient. + On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything). + Return(func(ctx context.Context, in *collaborationv1beta1.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) { + Expect(in.Filters).To(HaveLen(2)) + + var shareStates []collaborationv1beta1.ShareState + var resourceIDs []*storageprovider.ResourceId + + for _, filter := range in.Filters { + switch filter.Term.(type) { + case *collaborationv1beta1.Filter_State: + shareStates = append(shareStates, filter.GetState()) + case *collaborationv1beta1.Filter_ResourceId: + resourceIDs = append(resourceIDs, filter.GetResourceId()) + } + } + + Expect(shareStates).To(HaveLen(1)) + Expect(shareStates).To(ContainElements( + collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED, + )) + + Expect(resourceIDs).To(HaveLen(1)) + Expect(resourceIDs[0]).To(Equal(&expectedResourceID)) + + return nil, nil + }) + + err := drivesDriveItemService.UnmountShare(context.Background(), expectedResourceID) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Describe("gateway client share update", func() { + It("updates the share state to be accepted", func() { + expectedShareID := collaborationv1beta1.ShareId{ + OpaqueId: "1$2!3", + } + + 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{ + { + State: collaborationv1beta1.ShareState_SHARE_STATE_PENDING, + Share: &collaborationv1beta1.Share{ + Id: &expectedShareID, + }, + }, + }, + }, nil + }) + + gatewayClient. + On("UpdateReceivedShare", mock.Anything, mock.Anything, mock.Anything). + Return(func(ctx context.Context, in *collaborationv1beta1.UpdateReceivedShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) { + Expect(in.GetUpdateMask().GetPaths()).To(Equal([]string{"state"})) + Expect(in.GetShare().GetState()).To(Equal(collaborationv1beta1.ShareState_SHARE_STATE_REJECTED)) + Expect(in.GetShare().GetShare().GetId().GetOpaqueId()).To(Equal(expectedShareID.GetOpaqueId())) + return &collaborationv1beta1.UpdateReceivedShareResponse{}, nil + }) + + err := drivesDriveItemService.UnmountShare(context.Background(), storageprovider.ResourceId{}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("bubbles errors and continues", 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++ + Expect(calls).To(BeNumerically("<=", 3)) + + if calls <= 2 { + return nil, fmt.Errorf("error %d", calls) + } + + return &collaborationv1beta1.UpdateReceivedShareResponse{}, nil + }) + + err := drivesDriveItemService.UnmountShare(context.Background(), storageprovider.ResourceId{}) + Expect(fmt.Sprint(err)).To(Equal("error 1\nerror 2")) + }) + }) + }) +}) + var _ = Describe("DrivesDriveItemApi", func() { var ( mockProvider *mocks.DrivesDriveItemProvider