Skip to content

Commit

Permalink
Use manager to list shares
Browse files Browse the repository at this point in the history 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
  • Loading branch information
@butonic
butonic committed Nov 25, 2024
1 parent 8ffaada commit 638da1a
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 69 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/usershareprovider-list-with-manager.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: Use manager to list shares

When updating a received share the usershareprovider now uses the share manager directly to list received shares instead of going through the gateway again.

https://github.com/cs3org/reva/pull/4971
47 changes: 25 additions & 22 deletions internal/grpc/services/usershareprovider/usershareprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,12 +520,7 @@ func (s *service) UpdateReceivedShare(ctx context.Context, req *collaboration.Up
isMountPointSet := slices.Contains(req.GetUpdateMask().GetPaths(), _fieldMaskPathMountPoint) && req.GetShare().GetMountPoint().GetPath() != ""
// we calculate a valid mountpoint only if the share should be accepted and the mount point is not set explicitly
if isStateTransitionShareAccepted && !isMountPointSet {
gatewayClient, err := s.gatewaySelector.Next()
if err != nil {
return nil, err
}

s, err := setReceivedShareMountPoint(ctx, gatewayClient, req)
s, err := s.setReceivedShareMountPoint(ctx, req)
switch {
case err != nil:
fallthrough
Expand Down Expand Up @@ -556,7 +551,11 @@ func (s *service) UpdateReceivedShare(ctx context.Context, req *collaboration.Up
}
}

func setReceivedShareMountPoint(ctx context.Context, gwc gateway.GatewayAPIClient, req *collaboration.UpdateReceivedShareRequest) (*rpc.Status, error) {
func (s *service) setReceivedShareMountPoint(ctx context.Context, req *collaboration.UpdateReceivedShareRequest) (*rpc.Status, error) {
gwc, err := s.gatewaySelector.Next()
if err != nil {
return nil, err
}
receivedShare, err := gwc.GetReceivedShare(ctx, &collaboration.GetReceivedShareRequest{
Ref: &collaboration.ShareReference{
Spec: &collaboration.ShareReference_Id{
Expand All @@ -575,6 +574,10 @@ func setReceivedShareMountPoint(ctx context.Context, gwc gateway.GatewayAPIClien
return status.NewOK(ctx), nil
}

gwc, err = s.gatewaySelector.Next()
if err != nil {
return nil, err
}
resourceStat, err := gwc.Stat(ctx, &provider.StatRequest{
Ref: &provider.Reference{
ResourceId: receivedShare.GetShare().GetShare().GetResourceId(),
Expand All @@ -593,7 +596,7 @@ func setReceivedShareMountPoint(ctx context.Context, gwc gateway.GatewayAPIClien
_ = utils.ReadJSONFromOpaque(req.Opaque, "userid", &userID)

// check if the requested mount point is available and if not, find a suitable one
availableMountpoint, _, err := GetMountpointAndUnmountedShares(ctx, gwc,
availableMountpoint, _, err := GetMountpointAndUnmountedShares(ctx, s.sm, s.gatewaySelector,
resourceStat.GetInfo().GetId(),
resourceStat.GetInfo().GetName(),
userID,
Expand All @@ -615,34 +618,30 @@ func setReceivedShareMountPoint(ctx context.Context, gwc gateway.GatewayAPIClien
}

// 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, id *provider.ResourceId, name string, userId *userpb.UserId) (string, []*collaboration.ReceivedShare, error) {
listReceivedSharesReq := &collaboration.ListReceivedSharesRequest{}
if userId != nil {
listReceivedSharesReq.Opaque = utils.AppendJSONToOpaque(nil, "userid", userId)
}
func GetMountpointAndUnmountedShares(ctx context.Context, sm share.Manager, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], id *provider.ResourceId, name string, userId *userpb.UserId) (string, []*collaboration.ReceivedShare, error) {

listReceivedSharesRes, err := gwc.ListReceivedShares(ctx, listReceivedSharesReq)
receivedShares, err := sm.ListReceivedShares(ctx, []*collaboration.Filter{}, userId)
if err != nil {
return "", nil, errtypes.InternalError("grpc list received shares request failed")
}

if err := errtypes.NewErrtypeFromStatus(listReceivedSharesRes.GetStatus()); err != nil {
return "", nil, err
}

unmountedShares := []*collaboration.ReceivedShare{}
base := filepath.Clean(name)
mount := base
existingMountpoint := ""
mountedShares := make([]string, 0, len(listReceivedSharesRes.GetShares()))
mountedShares := make([]string, 0, len(receivedShares))
var pathExists bool

for _, s := range listReceivedSharesRes.GetShares() {
for _, s := range receivedShares {
resourceIDEqual := utils.ResourceIDEqual(s.GetShare().GetResourceId(), id)

if resourceIDEqual && s.State == collaboration.ShareState_SHARE_STATE_ACCEPTED {
// a share to the resource already exists and is mounted, remembers the mount point
_, err := utils.GetResourceByID(ctx, s.GetShare().GetResourceId(), gwc)
gwc, err := gatewaySelector.Next()
if err != nil {
return "", nil, err
}
_, err = utils.GetResourceByID(ctx, s.GetShare().GetResourceId(), gwc)
if err == nil {
existingMountpoint = s.GetMountPoint().GetPath()
}
Expand All @@ -658,7 +657,11 @@ func GetMountpointAndUnmountedShares(ctx context.Context, gwc gateway.GatewayAPI
mountedShares = append(mountedShares, s.GetMountPoint().GetPath())
if s.GetMountPoint().GetPath() == mount {
// does the shared resource still exist?
_, err := utils.GetResourceByID(ctx, s.GetShare().GetResourceId(), gwc)
gwc, err := gatewaySelector.Next()
if err != nil {
return "", nil, err
}
_, err = utils.GetResourceByID(ctx, s.GetShare().GetResourceId(), gwc)
if err == nil {
pathExists = true
}
Expand Down
119 changes: 72 additions & 47 deletions internal/grpc/services/usershareprovider/usershareprovider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,26 +582,59 @@ var _ = Describe("user share provider service", func() {
})

var _ = Describe("helpers", func() {
var (
ctx context.Context
manager *mocks.Manager
gatewayClient *cs3mocks.GatewayAPIClient
gatewaySelector pool.Selectable[gateway.GatewayAPIClient]
statResourceResponse *providerpb.StatResponse
)
BeforeEach(func() {
manager = &mocks.Manager{}

registry.Register("mockManager", func(m map[string]interface{}) (share.Manager, error) {
return manager, nil
})
manager.On("UpdateShare", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&collaborationpb.Share{}, nil)

gatewayClient = &cs3mocks.GatewayAPIClient{}
pool.RemoveSelector("GatewaySelector" + "any")
gatewaySelector = pool.GetSelector[gateway.GatewayAPIClient](
"GatewaySelector",
"any",
func(cc grpc.ClientConnInterface) gateway.GatewayAPIClient {
return gatewayClient
},
)
statResourceResponse = &providerpb.StatResponse{
Status: status.NewOK(ctx),
Info: &providerpb.ResourceInfo{
PermissionSet: &providerpb.ResourcePermissions{},
},
}
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResourceResponse, nil)

})

type GetMountpointAndUnmountedSharesArgs struct {
withName string
withResourceId *providerpb.ResourceId
listReceivedSharesResponse *collaborationpb.ListReceivedSharesResponse
listReceivedSharesError error
expectedName string
withName string
withResourceId *providerpb.ResourceId
receivedShares []*collaborationpb.ReceivedShare
receivedSharesError error
expectedName string
}
DescribeTable("GetMountpointAndUnmountedShares",
func(args GetMountpointAndUnmountedSharesArgs) {
gatewayClient := cs3mocks.NewGatewayAPIClient(GinkgoT())

gatewayClient.EXPECT().
ListReceivedShares(mock.Anything, mock.Anything).
RunAndReturn(func(ctx context.Context, request *collaborationpb.ListReceivedSharesRequest, option ...grpc.CallOption) (*collaborationpb.ListReceivedSharesResponse, error) {
return args.listReceivedSharesResponse, args.listReceivedSharesError
manager.EXPECT().
ListReceivedShares(mock.Anything, mock.Anything, mock.Anything).
RunAndReturn(func(ctx context.Context, filter []*collaborationpb.Filter, userid *userpb.UserId) ([]*collaborationpb.ReceivedShare, error) {
return args.receivedShares, args.receivedSharesError
})

statCallCount := 0

for _, s := range args.listReceivedSharesResponse.GetShares() {
for _, s := range args.receivedShares {
if s.GetState() != collaborationpb.ShareState_SHARE_STATE_ACCEPTED {
continue
}
Expand All @@ -627,9 +660,9 @@ var _ = Describe("helpers", func() {
}).Times(statCallCount)
}

availableMountpoint, _, err := usershareprovider.GetMountpointAndUnmountedShares(context.Background(), gatewayClient, args.withResourceId, args.withName, nil)
availableMountpoint, _, err := usershareprovider.GetMountpointAndUnmountedShares(context.Background(), manager, gatewaySelector, args.withResourceId, args.withName, nil)

if args.listReceivedSharesError != nil {
if args.receivedSharesError != nil {
Expect(err).To(HaveOccurred(), "expected error, got none")
return
}
Expand All @@ -641,17 +674,15 @@ var _ = Describe("helpers", func() {
Entry(
"listing received shares errors",
GetMountpointAndUnmountedSharesArgs{
listReceivedSharesError: errors.New("some error"),
receivedSharesError: errors.New("some error"),
},
),
Entry(
"returns the given name if no shares are found",
GetMountpointAndUnmountedSharesArgs{
withName: "name1",
listReceivedSharesResponse: &collaborationpb.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
},
expectedName: "name1",
withName: "name1",
receivedShares: []*collaborationpb.ReceivedShare{},
expectedName: "name1",
},
),
Entry(
Expand All @@ -663,20 +694,17 @@ var _ = Describe("helpers", func() {
OpaqueId: "2",
SpaceId: "3",
},
listReceivedSharesResponse: &collaborationpb.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaborationpb.ReceivedShare{
{
State: collaborationpb.ShareState_SHARE_STATE_ACCEPTED,
MountPoint: &providerpb.Reference{
Path: "path",
},
Share: &collaborationpb.Share{
ResourceId: &providerpb.ResourceId{
StorageId: "1",
OpaqueId: "2",
SpaceId: "3",
},
receivedShares: []*collaborationpb.ReceivedShare{
{
State: collaborationpb.ShareState_SHARE_STATE_ACCEPTED,
MountPoint: &providerpb.Reference{
Path: "path",
},
Share: &collaborationpb.Share{
ResourceId: &providerpb.ResourceId{
StorageId: "1",
OpaqueId: "2",
SpaceId: "3",
},
},
},
Expand All @@ -688,20 +716,17 @@ var _ = Describe("helpers", func() {
"enumerates the name if a share with the same path already exists",
GetMountpointAndUnmountedSharesArgs{
withName: "some name",
listReceivedSharesResponse: &collaborationpb.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaborationpb.ReceivedShare{
{
State: collaborationpb.ShareState_SHARE_STATE_ACCEPTED,
MountPoint: &providerpb.Reference{
Path: "some name",
},
Share: &collaborationpb.Share{
ResourceId: &providerpb.ResourceId{
StorageId: "1",
OpaqueId: "2",
SpaceId: "3",
},
receivedShares: []*collaborationpb.ReceivedShare{
{
State: collaborationpb.ShareState_SHARE_STATE_ACCEPTED,
MountPoint: &providerpb.Reference{
Path: "some name",
},
Share: &collaborationpb.Share{
ResourceId: &providerpb.ResourceId{
StorageId: "1",
OpaqueId: "2",
SpaceId: "3",
},
},
},
Expand Down

0 comments on commit 638da1a

Please sign in to comment.