From 65573bdc24824c6ebb57f36e5cf5e7f3c0def4cd Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 6 Feb 2024 18:12:19 +0100 Subject: [PATCH] fix an error when move using destination id --- changelog/unreleased/fix-move.md | 6 + internal/http/services/owncloud/ocdav/move.go | 6 +- .../owncloud/ocdav/ocdav_blackbox_test.go | 121 ++++++++++++++++++ 3 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/fix-move.md diff --git a/changelog/unreleased/fix-move.md b/changelog/unreleased/fix-move.md new file mode 100644 index 0000000000..f408abc5df --- /dev/null +++ b/changelog/unreleased/fix-move.md @@ -0,0 +1,6 @@ +Bugfix: Fix an error when move + +We fixed a bug that caused Internal Server Error when move using destination id + +https://github.com/cs3org/reva/pull/4503 +https://github.com/owncloud/ocis/issues/6739 diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index fbd51ee72f..4706d20e9d 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -266,7 +266,11 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req } // TODO what if intermediate is a file? } - + // resolve the destination path + if dst.Path == "." { + dst.Path = utils.MakeRelativePath(dstStatRes.GetInfo().GetName()) + dst.ResourceId = dstStatRes.GetInfo().GetParentId() + } mReq := &provider.MoveRequest{Source: src, Destination: dst} mRes, err := client.Move(ctx, mReq) if err != nil { diff --git a/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go b/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go index 47dfb9fa4b..7f339629eb 100644 --- a/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go +++ b/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go @@ -932,6 +932,127 @@ var _ = Describe("ocdav", func() { BeforeEach(func() { basePath = "/dav/spaces" + + userspace.Id = &cs3storageprovider.StorageSpaceId{OpaqueId: storagespace.FormatResourceID(cs3storageprovider.ResourceId{StorageId: "provider-1", SpaceId: "userspace", OpaqueId: "userspace"})} + userspace.Root = &cs3storageprovider.ResourceId{StorageId: "provider-1", SpaceId: "userspace", OpaqueId: "userspace"} + // path based requests at the /webdav endpoint first look up the storage space + client.On("ListStorageSpaces", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.ListStorageSpacesRequest) bool { + p := string(req.Opaque.Map["path"].Value) + return p == "/" || strings.HasPrefix(p, "/users") + })).Return(&cs3storageprovider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*cs3storageprovider.StorageSpace{userspace}, + }, nil) + }) + + Describe("MOVE", func() { + // The variables that used in a JustBeforeEach must be defined in the BeforeEach + var reqPath, dstPath, dstFileName string + + JustBeforeEach(func() { + // setup the request + rr = httptest.NewRecorder() + req, err = http.NewRequest("MOVE", basePath+reqPath, strings.NewReader("")) + Expect(err).ToNot(HaveOccurred()) + req = req.WithContext(ctx) + req.Header.Set(net.HeaderDestination, basePath+dstPath) + req.Header.Set("Overwrite", "T") + + client.On("GetPath", mock.Anything, mock.Anything).Return(&cs3storageprovider.GetPathResponse{ + Status: status.NewOK(ctx), + Path: "/file", + }, nil).Once() + client.On("GetPath", mock.Anything, mock.Anything).Return(&cs3storageprovider.GetPathResponse{ + Status: status.NewOK(ctx), + Path: "/dstFileName", + }, nil).Once() + + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool { + return req.Ref.Path == mReq.Source.Path + })).Return(&cs3storageprovider.StatResponse{ + Status: status.NewOK(ctx), + Info: &cs3storageprovider.ResourceInfo{Id: mReq.Source.ResourceId}, + }, nil).Once() + + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool { + return req.Ref.Path == mReq.Destination.Path + })).Return(&cs3storageprovider.StatResponse{ + Status: status.NewOK(ctx), + Info: &cs3storageprovider.ResourceInfo{ + Id: mReq.Source.ResourceId, + ParentId: &cs3storageprovider.ResourceId{StorageId: "provider-1", OpaqueId: "dstId", SpaceId: "userspace"}, + Name: dstFileName, + }, + }, 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) + + }) + + When("use the id as a destination. the gateway returns OK when moving file", func() { + BeforeEach(func() { + reqPath = "/provider-1$userspace/file" + dstPath = "/provider-1$userspace!dstId" + dstFileName = "dstFileName" + + mReq = &cs3storageprovider.MoveRequest{ + Source: mockReference("userspace", "./file"), + Destination: mockReference("dstId", "."), + } + }) + It("the source and the destination exist", func() { + + expReq := &cs3storageprovider.MoveRequest{ + Source: &cs3storageprovider.Reference{ResourceId: &cs3storageprovider.ResourceId{ + StorageId: "provider-1", SpaceId: "userspace"}, Path: "./file"}, + Destination: &cs3storageprovider.Reference{ResourceId: &cs3storageprovider.ResourceId{ + StorageId: "provider-1", OpaqueId: "dstId", SpaceId: "userspace"}, Path: "./dstFileName"}, + } + + client.On("Move", mock.Anything, expReq).Return(&cs3storageprovider.MoveResponse{ + Status: status.NewOK(ctx), + }, nil) + + mockPathStat("./dstFileName", status.NewOK(ctx), &cs3storageprovider.ResourceInfo{Id: &cs3storageprovider.ResourceId{}}) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusNoContent)) + }) + }) + When("use the id as a source and destination. the gateway returns OK when moving file", func() { + BeforeEach(func() { + reqPath = "/provider-1$userspace!srcId" + dstPath = "/provider-1$userspace!dstId" + dstFileName = "" + + mReq = &cs3storageprovider.MoveRequest{ + Source: mockReference("srcId", "."), + Destination: mockReference("dstId", "."), + } + }) + It("the source and the destination exist", func() { + + expReq := &cs3storageprovider.MoveRequest{ + Source: &cs3storageprovider.Reference{ResourceId: &cs3storageprovider.ResourceId{ + StorageId: "provider-1", OpaqueId: "srcId", SpaceId: "userspace"}, Path: "."}, + Destination: &cs3storageprovider.Reference{ResourceId: &cs3storageprovider.ResourceId{ + StorageId: "provider-1", OpaqueId: "dstId", SpaceId: "userspace"}, Path: "."}, + } + + client.On("Move", mock.Anything, expReq).Return(&cs3storageprovider.MoveResponse{ + Status: status.NewOK(ctx), + }, nil) + + mockPathStat(".", status.NewOK(ctx), &cs3storageprovider.ResourceInfo{Id: &cs3storageprovider.ResourceId{}}) + + handler.Handler().ServeHTTP(rr, req) + Expect(rr).To(HaveHTTPStatus(http.StatusNoContent)) + }) + }) }) })