mirror/opencloud
1
0
Fork 0
mirror of https://github.com/opencloud-eu/opencloud.git synced 2026-01-07 21:00:30 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix the status code for multiple mount and unmount share

Browse Source
This commit is contained in:
Roman Perekhod
2024-05-16 18:38:12 +02:00
parent e239d1e3ac
commit 8d30bd59f1
4 changed files with 140 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
changelog/unreleased/fix-share-mount-unmount.md Normal file
View File

@@ -0,0 +1,6 @@
Bugfix: Fix the status code for multiple mount and unmount share
We fixed the status code for multiple mount and unmount share.
https://github.com/owncloud/ocis/pull/9193
https://github.com/owncloud/ocis/issues/8876

73
services/graph/pkg/service/v0/api_drives_drive_item.go
View File

@@ -64,6 +64,12 @@ var (
// ErrNoShares is returned when no shares are found
ErrNoShares = errorcode.New(errorcode.ItemNotFound, "no shares found")
// ErrAlreadyMounted is returned when all shares are already mounted
ErrAlreadyMounted = errorcode.New(errorcode.NameAlreadyExists, "shares already mounted")
// ErrAlreadyUnmounted is returned when all shares are already unmounted
ErrAlreadyUnmounted = errorcode.New(errorcode.NameAlreadyExists, "shares already unmounted")
)
type (
@@ -214,17 +220,39 @@ func (s DrivesDriveItemService) UnmountShare(ctx context.Context, shareID *colla
return err
}
availableShares, err := s.GetSharesForResource(ctx, share.GetShare().GetResourceId(), []*collaboration.Filter{
shares, err := s.GetSharesForResource(ctx, share.GetShare().GetResourceId(), []*collaboration.Filter{
{
Type: collaboration.Filter_TYPE_STATE,
Term: &collaboration.Filter_State{
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
},
},
{
Type: collaboration.Filter_TYPE_STATE,
Term: &collaboration.Filter_State{
State: collaboration.ShareState_SHARE_STATE_REJECTED,
},
},
})
if err != nil {
return err
}
availableShares := make([]*collaboration.ReceivedShare, 0, 1)
rejectedShares := make([]*collaboration.ReceivedShare, 0, 1)
for _, v := range shares {
switch v.GetState() {
case collaboration.ShareState_SHARE_STATE_ACCEPTED:
availableShares = append(availableShares, v)
case collaboration.ShareState_SHARE_STATE_REJECTED:
rejectedShares = append(rejectedShares, v)
}
}
if len(availableShares) == 0 {
if len(rejectedShares) > 0 {
return ErrAlreadyUnmounted
}
return ErrNoShares
}
_, err = s.UpdateShares(ctx, availableShares, func(_ *collaboration.ReceivedShare, request *collaboration.UpdateReceivedShareRequest) {
request.Share.State = collaboration.ShareState_SHARE_STATE_REJECTED
@@ -246,24 +274,28 @@ func (s DrivesDriveItemService) MountShare(ctx context.Context, resourceID *stor
name = filepath.Clean(name)
}
availableShares, err := s.GetSharesForResource(ctx, resourceID, []*collaboration.Filter{
{
Type: collaboration.Filter_TYPE_STATE,
Term: &collaboration.Filter_State{
State: collaboration.ShareState_SHARE_STATE_PENDING,
},
},
{
Type: collaboration.Filter_TYPE_STATE,
Term: &collaboration.Filter_State{
State: collaboration.ShareState_SHARE_STATE_REJECTED,
},
},
})
shares, err := s.GetSharesForResource(ctx, resourceID, nil)
if err != nil {
return nil, err
}
availableShares := make([]*collaboration.ReceivedShare, 0, len(shares))
mountedShares := make([]*collaboration.ReceivedShare, 0, 1)
for _, v := range shares {
switch v.GetState() {
case collaboration.ShareState_SHARE_STATE_ACCEPTED:
mountedShares = append(mountedShares, v)
case collaboration.ShareState_SHARE_STATE_PENDING, collaboration.ShareState_SHARE_STATE_REJECTED:
availableShares = append(availableShares, v)
}
}
if len(availableShares) == 0 {
if len(mountedShares) > 0 {
return nil, ErrAlreadyMounted
}
return nil, ErrNoShares
}
updatedShares, err := s.UpdateShares(ctx, availableShares, func(share *collaboration.ReceivedShare, request *collaboration.UpdateReceivedShareRequest) {
request.Share.State = collaboration.ShareState_SHARE_STATE_ACCEPTED
request.UpdateMask.Paths = append(request.UpdateMask.Paths, _fieldMaskPathState)
@@ -328,8 +360,8 @@ func (api DrivesDriveItemApi) DeleteDriveItem(w http.ResponseWriter, r *http.Req
shareID := ExtractShareIdFromResourceId(itemID)
if err := api.drivesDriveItemService.UnmountShare(ctx, shareID); err != nil {
api.logger.Debug().Err(err).Msg(ErrUnmountShare.Error())
ErrUnmountShare.Render(w, r)
api.logger.Debug().Err(err).Msg(err.Error())
errorcode.RenderError(w, r, err)
return
}
@@ -484,11 +516,10 @@ func (api DrivesDriveItemApi) CreateDriveItem(w http.ResponseWriter, r *http.Req
return
}
mountedShares, err := api.drivesDriveItemService.
MountShare(ctx, &resourceId, requestDriveItem.GetName())
mountedShares, err := api.drivesDriveItemService.MountShare(ctx, &resourceId, requestDriveItem.GetName())
if err != nil {
api.logger.Debug().Err(err).Msg(ErrMountShare.Error())
ErrMountShare.Render(w, r)
api.logger.Debug().Err(err).Msg(err.Error())
errorcode.RenderError(w, r, err)
return
}

93
services/graph/pkg/service/v0/api_drives_drive_item_test.go
View File

@@ -297,14 +297,33 @@ var _ = Describe("DrivesDriveItemService", func() {
EXPECT().
ListReceivedShares(context.Background(), mock.Anything, mock.Anything, mock.Anything).
RunAndReturn(func(ctx context.Context, request *collaborationv1beta1.ListReceivedSharesRequest, option ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) {
Expect(request.Filters).To(HaveLen(2))
Expect(request.Filters).To(HaveLen(3))
Expect(request.Filters[0].Type).To(Equal(collaborationv1beta1.Filter_TYPE_RESOURCE_ID))
Expect(request.Filters[1].Term.(*collaborationv1beta1.Filter_State).State).To(Equal(collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED))
return nil, nil
Expect(request.Filters[2].Term.(*collaborationv1beta1.Filter_State).State).To(Equal(collaborationv1beta1.ShareState_SHARE_STATE_REJECTED))
return &collaborationv1beta1.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaborationv1beta1.ReceivedShare{
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_REJECTED},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED},
},
}, nil
}).
Once()
_ = drivesDriveItemService.UnmountShare(context.Background(), &collaborationv1beta1.ShareId{})
gatewayClient.
EXPECT().
UpdateReceivedShare(context.Background(), mock.Anything, mock.Anything).
RunAndReturn(func(ctx context.Context, request *collaborationv1beta1.UpdateReceivedShareRequest, option ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) {
return &collaborationv1beta1.UpdateReceivedShareResponse{
Status: status.NewOK(ctx),
}, nil
}).
Times(2)
err := drivesDriveItemService.UnmountShare(context.Background(), &collaborationv1beta1.ShareId{})
Expect(err).To(BeNil())
})
It("reports some error if one or multiple shares could not be unmounted", func() {
@@ -323,7 +342,7 @@ var _ = Describe("DrivesDriveItemService", func() {
Status: status.NewOK(context.Background()),
Shares: []*collaborationv1beta1.ReceivedShare{
{},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED},
{},
},
}, nil).
@@ -334,19 +353,13 @@ var _ = Describe("DrivesDriveItemService", func() {
EXPECT().
UpdateReceivedShare(context.Background(), mock.Anything, mock.Anything).
RunAndReturn(func(ctx context.Context, request *collaborationv1beta1.UpdateReceivedShareRequest, option ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) {
if request.GetShare().GetShare().GetId() != nil {
return &collaborationv1beta1.UpdateReceivedShareResponse{
Status: status.NewOK(ctx),
}, nil
}
return nil, someErr
}).
Times(3)
Times(1)
err := drivesDriveItemService.UnmountShare(context.Background(), &collaborationv1beta1.ShareId{})
Expect(err).To(MatchError(errorcode.New(errorcode.GeneralException, someErr.Error())))
Expect(err.(interface{ Unwrap() []error }).Unwrap()).To(HaveLen(2))
Expect(err.(interface{ Unwrap() []error }).Unwrap()).To(HaveLen(1))
})
})
@@ -362,10 +375,8 @@ var _ = Describe("DrivesDriveItemService", func() {
EXPECT().
ListReceivedShares(context.Background(), mock.Anything, mock.Anything, mock.Anything).
RunAndReturn(func(ctx context.Context, request *collaborationv1beta1.ListReceivedSharesRequest, option ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) {
Expect(request.Filters).To(HaveLen(3))
Expect(request.Filters).To(HaveLen(1))
Expect(request.Filters[0].Type).To(Equal(collaborationv1beta1.Filter_TYPE_RESOURCE_ID))
Expect(request.Filters[1].Term.(*collaborationv1beta1.Filter_State).State).To(Equal(collaborationv1beta1.ShareState_SHARE_STATE_PENDING))
Expect(request.Filters[2].Term.(*collaborationv1beta1.Filter_State).State).To(Equal(collaborationv1beta1.ShareState_SHARE_STATE_REJECTED))
return nil, nil
}).
Once()
@@ -373,6 +384,36 @@ var _ = Describe("DrivesDriveItemService", func() {
_, _ = drivesDriveItemService.MountShare(context.Background(), nil, "some")
})
It("reports no errors when shares have mounted", func() {
gatewayClient.
EXPECT().
ListReceivedShares(context.Background(), mock.Anything, mock.Anything, mock.Anything).
RunAndReturn(func(ctx context.Context, request *collaborationv1beta1.ListReceivedSharesRequest, option ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) {
return &collaborationv1beta1.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaborationv1beta1.ReceivedShare{
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_REJECTED},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_PENDING},
},
}, nil
}).
Once()
gatewayClient.
EXPECT().
UpdateReceivedShare(context.Background(), mock.Anything, mock.Anything).
RunAndReturn(func(ctx context.Context, request *collaborationv1beta1.UpdateReceivedShareRequest, option ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) {
return &collaborationv1beta1.UpdateReceivedShareResponse{
Status: status.NewOK(ctx),
}, nil
}).
Times(2)
shares, err := drivesDriveItemService.MountShare(context.Background(), nil, "some")
Expect(err).To(BeNil())
Expect(shares).To(HaveLen(2))
})
It("fails if the update instructions produce as many errors as there are shares", func() {
gatewayClient.
EXPECT().
@@ -380,9 +421,9 @@ var _ = Describe("DrivesDriveItemService", func() {
Return(&collaborationv1beta1.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaborationv1beta1.ReceivedShare{
{},
{},
{},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_PENDING},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_PENDING},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_PENDING},
},
}, nil).
Once()
@@ -408,7 +449,7 @@ var _ = Describe("DrivesDriveItemService", func() {
Status: status.NewOK(context.Background()),
Shares: []*collaborationv1beta1.ReceivedShare{
{},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}},
{Share: &collaborationv1beta1.Share{Id: &collaborationv1beta1.ShareId{}}, State: collaborationv1beta1.ShareState_SHARE_STATE_PENDING},
{},
},
}, nil).
@@ -430,7 +471,7 @@ var _ = Describe("DrivesDriveItemService", func() {
Status: status.NewOK(ctx),
}, nil
}).
Times(3)
Once()
shares, err := drivesDriveItemService.MountShare(context.Background(), nil, "some")
Expect(err).To(BeNil())
@@ -518,7 +559,7 @@ var _ = Describe("DrivesDriveItemApi", func() {
failOnNonShareJailDriveID(drivesDriveItemApi.DeleteDriveItem)
It("fails if unmounting the share fails", func() {
It("fails if unmounting the share fails with the general error", func() {
rCTX.URLParams.Add("driveID", "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668")
rCTX.URLParams.Add("itemID", "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668!1")
w := httptest.NewRecorder()
@@ -534,10 +575,10 @@ var _ = Describe("DrivesDriveItemApi", func() {
Once()
drivesDriveItemApi.DeleteDriveItem(w, r)
Expect(w.Code).To(Equal(http.StatusBadRequest))
Expect(w.Code).To(Equal(http.StatusInternalServerError))
jsonData := gjson.Get(w.Body.String(), "error")
Expect(jsonData.Get("code").String() + ": " + jsonData.Get("message").String()).To(Equal(svc.ErrUnmountShare.Error()))
Expect(jsonData.Get("code").String() + ": " + jsonData.Get("message").String()).To(Equal("generalException: some error"))
})
It("successfully unmounts the share", func() {
@@ -1021,7 +1062,7 @@ var _ = Describe("DrivesDriveItemApi", func() {
Expect(jsonData.Get("code").String() + ": " + jsonData.Get("message").String()).To(Equal(svc.ErrInvalidID.Error()))
})
It("fails if mounting the share fails", func() {
It("fails if mounting the share fails with the general error", func() {
rCTX.URLParams.Add("driveID", "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668")
rCTX.URLParams.Add("itemID", "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668!1")
@@ -1046,10 +1087,10 @@ var _ = Describe("DrivesDriveItemApi", func() {
)
drivesDriveItemApi.CreateDriveItem(w, r)
Expect(w.Code).To(Equal(http.StatusBadRequest))
Expect(w.Code).To(Equal(http.StatusInternalServerError))
jsonData := gjson.Get(w.Body.String(), "error")
Expect(jsonData.Get("code").String() + ": " + jsonData.Get("message").String()).To(Equal(svc.ErrMountShare.Error()))
Expect(jsonData.Get("code").String() + ": " + jsonData.Get("message").String()).To(Equal("generalException: some error"))
})
It("fails if drive item conversion fails", func() {

31
tests/acceptance/features/apiSharingNg/enableDisableShareSync.feature
View File

@@ -298,11 +298,10 @@ Feature: enable or disable sync of incoming shares
| textfile0.txt |
| FolderToShare |
@issue-8724
Scenario: try to enable share sync of a non-existent resource
Given user "Brian" has disabled the auto-sync share
When user "Brian" tries to enable share sync of a resource "nonexistent" using the Graph API
Then the HTTP status code should be "400"
Then the HTTP status code should be "404"
And the JSON data of the response should match
"""
{
@@ -318,7 +317,7 @@ Feature: enable or disable sync of incoming shares
],
"properties": {
"code" : {
"const": "invalidRequest"
"const": "itemNotFound"
},
"innererror" : {
"type": "object",
@@ -328,7 +327,7 @@ Feature: enable or disable sync of incoming shares
]
},
"message" : {
"const": "mounting share failed"
"const": "no shares found"
}
}
}
@@ -380,7 +379,7 @@ Feature: enable or disable sync of incoming shares
And user "Alice" has uploaded file with content "some data" to "/fileNotShared.txt"
And we save it into "FILEID"
When user "Brian" tries to enable share sync of a resource "<<FILEID>>" using the Graph API
Then the HTTP status code should be "400"
Then the HTTP status code should be "404"
And the JSON data of the response should match
"""
{
@@ -396,7 +395,7 @@ Feature: enable or disable sync of incoming shares
],
"properties": {
"code" : {
"const": "invalidRequest"
"const": "itemNotFound"
},
"innererror" : {
"type": "object",
@@ -406,7 +405,7 @@ Feature: enable or disable sync of incoming shares
]
},
"message" : {
"const": "mounting share failed"
"const": "no shares found"
}
}
}
@@ -474,7 +473,7 @@ Feature: enable or disable sync of incoming shares
| permissionsRole | Viewer |
And the user "Admin" has deleted a user "Alice"
When user "Brian" tries to disable sync of share "textfile0.txt" using the Graph API
Then the HTTP status code should be "400"
Then the HTTP status code should be "404"
And the JSON data of the response should match
"""
{
@@ -490,7 +489,7 @@ Feature: enable or disable sync of incoming shares
],
"properties": {
"code" : {
"const": "invalidRequest"
"const": "itemNotFound"
},
"innererror" : {
"type": "object",
@@ -500,7 +499,7 @@ Feature: enable or disable sync of incoming shares
]
},
"message" : {
"const": "unmounting share failed"
"const": "no shares found"
}
}
}
@@ -559,7 +558,7 @@ Feature: enable or disable sync of incoming shares
| permissionsRole | Viewer |
And group "grp1" has been deleted
When user "Brian" tries to enable share sync of a resource "<<FILEID>>" using the Graph API
Then the HTTP status code should be "400"
Then the HTTP status code should be "404"
And the JSON data of the response should match
"""
{
@@ -575,7 +574,7 @@ Feature: enable or disable sync of incoming shares
],
"properties": {
"code" : {
"const": "invalidRequest"
"const": "itemNotFound"
},
"innererror" : {
"type": "object",
@@ -585,7 +584,7 @@ Feature: enable or disable sync of incoming shares
]
},
"message" : {
"const": "mounting share failed"
"const": "no shares found"
}
}
}
@@ -609,7 +608,7 @@ Feature: enable or disable sync of incoming shares
| permissionsRole | Viewer |
And group "grp1" has been deleted
When user "Brian" tries to disable sync of share "textfile0.txt" using the Graph API
Then the HTTP status code should be "400"
Then the HTTP status code should be "404"
And the JSON data of the response should match
"""
{
@@ -625,7 +624,7 @@ Feature: enable or disable sync of incoming shares
],
"properties": {
"code" : {
"const": "invalidRequest"
"const": "itemNotFound"
},
"innererror" : {
"type": "object",
@@ -635,7 +634,7 @@ Feature: enable or disable sync of incoming shares
]
},
"message" : {
"const": "unmounting share failed"
"const": "error getting received share"
}
}
}