From 0fc001d1710fa7d23043a5a59d9ca442cbd3186a Mon Sep 17 00:00:00 2001 From: David Christofas Date: Mon, 10 May 2021 16:39:05 +0200 Subject: [PATCH] fix move in the owncloud storage driver (#1696) Update the filepath in redis when moving a file. Didn't implement it in delete since delete is still a bit more broken. But since we don't actively use the owncloud storage driver except for in CI it's not too important. Fixes https://github.com/cs3org/reva/issues/1693 --- .../unreleased/fix-move-owncloud-driver.md | 7 +++++++ pkg/storage/fs/owncloud/owncloud.go | 21 +++++++++++++++++++ .../expected-failures-on-OWNCLOUD-storage.md | 2 -- 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/fix-move-owncloud-driver.md diff --git a/changelog/unreleased/fix-move-owncloud-driver.md b/changelog/unreleased/fix-move-owncloud-driver.md new file mode 100644 index 0000000000..90a1ba6aac --- /dev/null +++ b/changelog/unreleased/fix-move-owncloud-driver.md @@ -0,0 +1,7 @@ +Bugfix: Fix move in owncloud storage driver + +When moving a file or folder (includes renaming) the filepath in the cache didn't get updated which caused subsequent requests to `getpath` to fail. + +https://github.com/cs3org/reva/issues/1693 +https://github.com/cs3org/reva/issues/1696 + diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index f3f16fa5dc..41fdf42d5d 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -1589,6 +1589,27 @@ func (fs *ocfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (e if err = os.Rename(oldIP, newIP); err != nil { return errors.Wrap(err, "ocfs: error moving "+oldIP+" to "+newIP) } + + log := appctx.GetLogger(ctx) + conn := fs.pool.Get() + defer conn.Close() + // Ideally if we encounter an error here we should rollback the Move/Rename. + // But since the owncloud storage driver is not being actively used by anyone other + // than the acceptance tests we should be fine by ignoring the errors. + _ = filepath.Walk(newIP, func(path string, info os.FileInfo, err error) error { + if err != nil { + // TODO(c0rby): rollback the move in case of an error + log.Error().Str("path", path).Err(err).Msg("error caching id") + return nil + } + id := readOrCreateID(context.Background(), path, nil) + _, err = conn.Do("SET", id, path) + if err != nil { + // TODO(c0rby): rollback the move in case of an error + log.Error().Str("path", path).Err(err).Msg("error caching id") + } + return nil + }) if err := fs.propagate(ctx, newIP); err != nil { return err } diff --git a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md index 8af46de70a..18b9e9bf74 100644 --- a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md +++ b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md @@ -1624,8 +1624,6 @@ The following scenarios fail on OWNCLOUD storage but not on OCIS storage: ### [Moving resource loses associated shares](https://github.com/owncloud/ocis/issues/1251) -- [apiSharePublicLink2/multilinkSharing.feature:187](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharePublicLink2/multilinkSharing.feature#L187) - #### [No way to set default folder for received shares](https://github.com/owncloud/ocis/issues/1327) - [apiShareCreateSpecialToShares2/createShareDefaultFolderForReceivedShares.feature:22](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares2/createShareDefaultFolderForReceivedShares.feature#L22) - [apiShareCreateSpecialToShares2/createShareDefaultFolderForReceivedShares.feature:23](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares2/createShareDefaultFolderForReceivedShares.feature#L23)