Handle file locks with deletes

Fixes: #9293
owncloud · Dec 16, 2021 · 14ecfee · 14ecfee
1 parent 610ef5b
commit 14ecfee
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 21 deletions.
diff --git a/changelog/unreleased/9293 b/changelog/unreleased/9293
@@ -0,0 +1,9 @@
+Bugfix: Handle file locks for delete jobs
+
+We no longer report an error when the client tries to delete a locked file
+but wait for the lock to be removed.
+
+This only works when a file is deleted not on folders.
+
+https://github.com/owncloud/client/issues/9293
+https://github.com/owncloud/client/pull/9295
diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp
@@ -90,7 +90,7 @@ void PropagateUploadFileNG::doStartUpload()
     const auto lockMode = propagator()->syncOptions().requiredLockMode();
     if (FileSystem::isFileLocked(fileName, lockMode)) {
         emit propagator()->seenLockedFile(fileName, lockMode);
-        abortWithError(SyncFileItem::SoftError, tr("%1 the file is currently in use").arg(fileName));
+        abortWithError(SyncFileItem::SoftError, tr("%1 the file is currently in use").arg(QDir::toNativeSeparators(fileName)));
         return;
     }
 

diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
@@ -47,10 +47,10 @@ Q_LOGGING_CATEGORY(lcPropagateLocalRename, "sync.propagator.localrename", QtInfo
  *
  * \a path is relative to propagator()->_localDir + _item->_file and should start with a slash
  */
-bool PropagateLocalRemove::removeRecursively(const QString &path)
+bool PropagateLocalRemove::removeRecursively(const QString &absolute, QString *error)
 {
-    QString absolute = propagator()->fullLocalPath(_item->_file + path);
     QStringList errors;
+    // path, isDir
     QList<QPair<QString, bool>> deleted;
     bool success = FileSystem::removeRecursively(
         absolute,
@@ -75,7 +75,7 @@ bool PropagateLocalRemove::removeRecursively(const QString &path)
             propagator()->_journal->deleteFileRecord(it.first.mid(propagator()->localPath().size()), it.second);
         }
 
-        _error = errors.join(QStringLiteral(", "));
+        *error = errors.join(QStringLiteral(", "));
     }
     return success;
 }
@@ -95,26 +95,29 @@ void PropagateLocalRemove::start()
         return;
     }
 
-    QString removeError;
-    if (_moveToTrash) {
-        if ((QDir(filename).exists() || FileSystem::fileExists(filename))
-            && !FileSystem::moveToTrash(filename, &removeError)) {
-            done(SyncFileItem::NormalError, removeError);
+    if (FileSystem::fileExists(filename)) {
+        bool ok;
+        QString removeError;
+        const auto lockMode = propagator()->syncOptions().requiredLockMode();
+        if (FileSystem::isFileLocked(filename, lockMode)) {
+            emit propagator()->seenLockedFile(filename, lockMode);
+            done(SyncFileItem::SoftError, tr("%1 the file is currently in use").arg(QDir::toNativeSeparators(filename)));
             return;
         }
-    } else {
-        if (_item->isDirectory()) {
-            if (QDir(filename).exists() && !removeRecursively(QString())) {
-                done(SyncFileItem::NormalError, _error);
-                return;
-            }
+
+        if (_moveToTrash) {
+            ok = FileSystem::moveToTrash(filename, &removeError);
         } else {
-            if (FileSystem::fileExists(filename)
-                && !FileSystem::remove(filename, &removeError)) {
-                done(SyncFileItem::NormalError, removeError);
-                return;
+            if (_item->isDirectory()) {
+                ok = removeRecursively(filename, &removeError);
+            } else {
+                ok = FileSystem::remove(filename, &removeError);
             }
         }
+        if (!ok) {
+            done(SyncFileItem::NormalError, removeError);
+            return;
+        }
     }
     propagator()->reportProgress(*_item, 0);
     propagator()->_journal->deleteFileRecord(_item->_originalFile, _item->isDirectory());

diff --git a/src/libsync/propagatorjobs.h b/src/libsync/propagatorjobs.h
@@ -42,8 +42,7 @@ class PropagateLocalRemove : public PropagateItemJob
     void start() override;
 
 private:
-    bool removeRecursively(const QString &path);
-    QString _error;
+    bool removeRecursively(const QString &absolute, QString *error);
     bool _moveToTrash;
 };