From 290cc316021ea35a72e94c80ff52612dd86b4ca2 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 11 Apr 2023 12:21:41 +0200 Subject: [PATCH 1/4] Added the test coverage for the file moving. Fixed the typo. --- internal/http/services/owncloud/ocdav/move.go | 2 +- .../owncloud/ocdav/ocdav_blackbox_test.go | 257 +++++++++++++++++- 2 files changed, 257 insertions(+), 2 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 172490fc9a..f85b7db617 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -198,7 +198,7 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req return } if dstStatRes.Status.Code != rpc.Code_CODE_OK && dstStatRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - errors.HandleErrorStatus(&log, w, srcStatRes.Status) + errors.HandleErrorStatus(&log, w, dstStatRes.Status) return } diff --git a/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go b/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go index b6d3298591..39004610cf 100644 --- a/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go +++ b/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go @@ -71,7 +71,7 @@ var _ = Describe("ocdav", func() { })).Return(&cs3storageprovider.StatResponse{ Status: s, Info: info, - }, nil) + }, nil).Once() } mockStat = func(ref *cs3storageprovider.Reference, s *cs3rpc.Status, info *cs3storageprovider.ResourceInfo) { client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool { @@ -124,6 +124,7 @@ var _ = Describe("ocdav", func() { Size: m["size"].(uint64), } } + mReq *cs3storageprovider.MoveRequest ) BeforeEach(func() { @@ -574,7 +575,261 @@ var _ = Describe("ocdav", func() { }) }) + + Describe("MOVE", func() { + + BeforeEach(func() { + // setup the request + rr = httptest.NewRecorder() + req, err = http.NewRequest("MOVE", basePath+"/file", strings.NewReader("")) + Expect(err).ToNot(HaveOccurred()) + req = req.WithContext(ctx) + req.Header.Set("Destination", basePath+"/newfile") + req.Header.Set("Overwrite", "T") + + mReq = &cs3storageprovider.MoveRequest{ + Source: &cs3storageprovider.Reference{ + ResourceId: userspace.Root, + Path: "./file", + }, + Destination: &cs3storageprovider.Reference{ + ResourceId: userspace.Root, + Path: "./newfile", + }, + } + }) + + When("the gateway returns OK when moving file", func() { + It("the source exists, the destination doesn't exists", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + mockPathStat(".", status.NewOK(ctx), nil) + + client.On("Move", mock.Anything, mReq).Return(&cs3storageprovider.MoveResponse{ + Status: status.NewOK(ctx), + }, nil) + + mockPathStat(mReq.Destination.Path, status.NewOK(ctx), &cs3storageprovider.ResourceInfo{Id: &cs3storageprovider.ResourceId{}}) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusCreated)) + }) + + It("the source and the destination exist", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewOK(ctx), nil) + + client.On("Delete", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.DeleteRequest) bool { + return utils.ResourceEqual(req.Ref, mReq.Destination) + })).Return(&cs3storageprovider.DeleteResponse{ + Status: status.NewOK(ctx), + }, nil) + + client.On("Move", mock.Anything, mReq).Return(&cs3storageprovider.MoveResponse{ + Status: status.NewOK(ctx), + }, nil) + + mockPathStat(mReq.Destination.Path, status.NewOK(ctx), &cs3storageprovider.ResourceInfo{Id: &cs3storageprovider.ResourceId{}}) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusNoContent)) + }) + }) + + When("the gateway returns error when moving file", func() { + It("the source Stat error", func() { + + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool { + return utils.ResourceEqual(req.Ref, mReq.Source) + })).Return(nil, fmt.Errorf("unexpected io error")) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusInternalServerError)) + }) + + It("moves a file. the source not found", func() { + + mockPathStat(mReq.Source.Path, status.NewNotFound(ctx, ""), nil) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusNotFound)) + }) + + It("the destination Stat error", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool { + return utils.ResourceEqual(req.Ref, mReq.Destination) + })).Return(nil, fmt.Errorf("unexpected io error")) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusInternalServerError)) + }) + + It("error when the 'Overwrite' header is 'F'", func() { + + req.Header.Set("Overwrite", "F") + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewOK(ctx), nil) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusPreconditionFailed)) + }) + + It("error when deleting an existing tree", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewOK(ctx), nil) + + client.On("Delete", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.DeleteRequest) bool { + return utils.ResourceEqual(req.Ref, mReq.Destination) + })).Return(nil, fmt.Errorf("unexpected io error")) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusInternalServerError)) + }) + + It("error when destination Stat returns unexpected code", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewInternal(ctx, ""), nil) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusInternalServerError)) + }) + + It("error when Delete returns unexpected code", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewOK(ctx), nil) + + client.On("Delete", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.DeleteRequest) bool { + return utils.ResourceEqual(req.Ref, mReq.Destination) + })).Return(&cs3storageprovider.DeleteResponse{ + Status: status.NewInvalid(ctx, ""), + }, nil) + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusBadRequest)) + }) + + It("the destination Stat error", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool { + return utils.ResourceEqual(req.Ref, &cs3storageprovider.Reference{ + ResourceId: userspace.Root, + Path: ".", + }) + })).Return(nil, fmt.Errorf("unexpected io error")).Once() + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusInternalServerError)) + }) + + It("error when destination Stat is not found", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + mockPathStat(".", status.NewNotFound(ctx, ""), nil) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusConflict)) + }) + + It("an unexpected error when destination Stat", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + mockPathStat(".", status.NewInvalid(ctx, ""), nil) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusBadRequest)) + }) + + It("error when removing", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + mockPathStat(".", status.NewOK(ctx), nil) + client.On("Move", mock.Anything, mReq).Return(nil, fmt.Errorf("unexpected io error")) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusInternalServerError)) + }) + + It("status 'Aborted' when removing", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + mockPathStat(".", status.NewOK(ctx), nil) + + client.On("Move", mock.Anything, mReq).Return(&cs3storageprovider.MoveResponse{ + Status: status.NewAborted(ctx, fmt.Errorf("aborted"), ""), + }, nil) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusPreconditionFailed)) + }) + + It("status 'Unimplemented' when removing", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + mockPathStat(".", status.NewOK(ctx), nil) + + client.On("Move", mock.Anything, mReq).Return(&cs3storageprovider.MoveResponse{ + Status: status.NewUnimplemented(ctx, fmt.Errorf("unimplemeted"), ""), + }, nil) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusBadGateway)) + }) + + It("the destination Stat error after moving", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + mockPathStat(".", status.NewOK(ctx), nil) + + client.On("Move", mock.Anything, mReq).Return(&cs3storageprovider.MoveResponse{ + Status: status.NewOK(ctx), + }, nil) + + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool { + return utils.ResourceEqual(req.Ref, &cs3storageprovider.Reference{ + ResourceId: userspace.Root, + Path: mReq.Destination.Path, + }) + })).Return(nil, fmt.Errorf("unexpected io error")) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusInternalServerError)) + }) + + It("the destination Stat returned not OK status after moving", func() { + + mockPathStat(mReq.Source.Path, status.NewOK(ctx), nil) + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + mockPathStat(".", status.NewOK(ctx), nil) + + client.On("Move", mock.Anything, mReq).Return(&cs3storageprovider.MoveResponse{ + Status: status.NewOK(ctx), + }, nil) + + mockPathStat(mReq.Destination.Path, status.NewNotFound(ctx, ""), nil) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusNotFound)) + }) + }) + }) }) + Context("at the /dav/avatars endpoint", func() { BeforeEach(func() { From f35c5d3414837118fa52cdb925aa12407fdd913c Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 11 Apr 2023 13:05:12 +0200 Subject: [PATCH 2/4] pr numder added --- changelog/unreleased/test-coverage.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/unreleased/test-coverage.md diff --git a/changelog/unreleased/test-coverage.md b/changelog/unreleased/test-coverage.md new file mode 100644 index 0000000000..9b2581cf2d --- /dev/null +++ b/changelog/unreleased/test-coverage.md @@ -0,0 +1,3 @@ +Enhancement: Increase unit test coverage in the ocdav service + +https://github.com/cs3org/reva/pull/3785 From 263d37680bc884b7a20ca4f37097436bc0a7ce79 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 11 Apr 2023 14:42:46 +0200 Subject: [PATCH 3/4] the APITESTS_BRANCH has been changed to master --- .drone.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.env b/.drone.env index f52f3ba146..455350ffe3 100644 --- a/.drone.env +++ b/.drone.env @@ -1,4 +1,4 @@ # The test runner source for API tests APITESTS_COMMITID=2cd625777122e292c92b30bc0ec0c6f4fc29a178 -APITESTS_BRANCH=dont-share-versions +APITESTS_BRANCH=master APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git From cd3d45dc96867d7daac0e737bc592d624893d773 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 11 Apr 2023 15:08:08 +0200 Subject: [PATCH 4/4] the APITESTS_COMMITID updated --- .drone.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.env b/.drone.env index 455350ffe3..2705b86a96 100644 --- a/.drone.env +++ b/.drone.env @@ -1,4 +1,4 @@ # The test runner source for API tests -APITESTS_COMMITID=2cd625777122e292c92b30bc0ec0c6f4fc29a178 +APITESTS_COMMITID=26c9f889e79611c05be078723ea253dbef486538 APITESTS_BRANCH=master APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git