fix delete in the owncloud storage driver (#1833)

changelog/unreleased/fix-delete-owncloud-storage-driver.md

@@ -0,0 +1,6 @@
+Bugfix: Properly handle name collisions for deletes in the owncloud driver 
+
+In the owncloud storage driver when we delete a file we append the deletion time to the file name.
+If two fast consecutive deletes happened, the deletion time would be the same and if the two files had the same name we ended up with only one file in the trashbin.
+
+https://github.com/cs3org/reva/pull/1833

pkg/storage/fs/owncloud/owncloud.go

@@ -1526,15 +1526,19 @@ func (fs *ocfs) trash(ctx context.Context, ip string, rp string, origin string)
 	// move to trash location
 	dtime := time.Now().Unix()
 	tgt := filepath.Join(rp, fmt.Sprintf("%s.d%d", filepath.Base(ip), dtime))
+
+	// The condition reads: "if the file exists"
+	// I know this check is hard to read because of the double negation
+	// but this way we avoid to duplicate the code following the if block.
+	// If two deletes happen fast consecutively they will have the same `dtime`,
+	// therefore we have to increase the 'dtime' to avoid collisions.
+	if _, err := os.Stat(tgt); !errors.Is(err, os.ErrNotExist) {
+		// timestamp collision, try again with higher value:
+		dtime++
+		tgt = filepath.Join(rp, fmt.Sprintf("%s.d%d", filepath.Base(ip), dtime))
+	}
 	if err := os.Rename(ip, tgt); err != nil {
-		if os.IsExist(err) {
-			// timestamp collision, try again with higher value:
-			dtime++
-			tgt := filepath.Join(rp, fmt.Sprintf("%s.d%d", filepath.Base(ip), dtime))
-			if err := os.Rename(ip, tgt); err != nil {
-				return errors.Wrap(err, "ocfs: could not move item to trash")
-			}
-		}
+		return errors.Wrap(err, "ocfs: could not move item to trash")
 	}
 
 	return fs.propagate(ctx, filepath.Dir(ip))

tests/acceptance/expected-failures-on-OWNCLOUD-storage.md

@@ -27,12 +27,6 @@ The following scenarios fail on OWNCLOUD storage but not on OCIS storage:
 -   [apiTrashbin/trashbinFilesFolders.feature:278](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L278)
 -   [apiTrashbin/trashbinFilesFolders.feature:279](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L279)
 
-#### [cannot restore to a different file-name](https://github.com/owncloud/ocis/issues/1122)
--   [apiTrashbin/trashbinRestore.feature:108](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinRestore.feature#L108)
--   [apiTrashbin/trashbinRestore.feature:109](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinRestore.feature#L109)
--   [apiTrashbin/trashbinRestore.feature:110](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinRestore.feature#L110)
--   [apiTrashbin/trashbinRestore.feature:111](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinRestore.feature#L111)
-
 #### [href in trashbin PROPFIND response is wrong](https://github.com/owncloud/ocis/issues/1120)
 #### [cannot restore to a different file-name](https://github.com/owncloud/ocis/issues/1122)
 -   [apiTrashbin/trashbinRestore.feature:309](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinRestore.feature#L309)