From: Olivier Goffart <ogoffart@woboq.com>
Date: Wed, 2 Sep 2015 13:19:34 +0000 (+0200)
Subject: PropagateLocalRemove:  remove entries from the DB even if there was an error.
X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~1408^2~132
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=e54253d8457f5e0eb071a7a704699149aec040e1;p=nextcloud-desktop.git

PropagateLocalRemove:  remove entries from the DB even if there was an error.

Previously, in case of an error while deleting a directory, we would not
remove the entries from the local db, despite most of the files would
be deleted.

Which means that if the files re-appear on the server with the same etag,
we would think the file were deleted from the client and propagate the change.

In 1.8.0 we had this bug that we would not see some directory in the server in
some cases. This would lead us to delete the file on the client.  Normaly the
files are deleted from the local database and next sync would re-download the files.
But in the cases where there was an error deleting one of the file (for example
if it was locked) we would then propagate the delete to the server.

Fix this by always deleting from the database the files that we deleted.

Issue #3206
---

diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
index 4cb7872b1..7c4df9cb8 100644
--- a/src/libsync/propagatorjobs.cpp
+++ b/src/libsync/propagatorjobs.cpp
@@ -39,38 +39,65 @@
 
 namespace OCC {
 
-// Code copied from Qt5's QDir::removeRecursively
-// (and modified to report the error)
-static bool removeRecursively(const QString &path, QString &error)
+/**
+ * Code inspired from Qt5's QDir::removeRecursively
+ * The code will update the database in case of error.
+ * If everything goes well (no error, returns true), the caller is responsible of removing the entries
+ * in the database.  But in case of error, we need to remove the entries from the database of the files
+ * that were deleted.
+ *
+ * \a path is relative to _propagator->_localDir + _item->_file and should start with a slash
+ */
+bool PropagateLocalRemove::removeRecursively(const QString& path)
 {
     bool success = true;
-    QDirIterator di(path, QDir::AllEntries | QDir::Hidden | QDir::System | QDir::NoDotAndDotDot);
+    QString absolute = _propagator->_localDir + _item->_file + path;
+    QDirIterator di(absolute, QDir::AllEntries | QDir::Hidden | QDir::System | QDir::NoDotAndDotDot);
+
+    QVector<QPair<QString, bool>> deleted;
+
     while (di.hasNext()) {
         di.next();
         const QFileInfo& fi = di.fileInfo();
         bool ok;
         // The use of isSymLink here is okay:
         // we never want to go into this branch for .lnk files
-        if (fi.isDir() && !fi.isSymLink()) {
-            ok = removeRecursively(di.filePath(), error); // recursive
+        bool isDir = fi.isDir() && !fi.isSymLink();
+        if (isDir) {
+            ok = removeRecursively(path + QLatin1Char('/') + di.fileName()); // recursive
         } else {
             QFile f(di.filePath());
             ok = f.remove();
             if (!ok) {
-                error += PropagateLocalRemove::tr("Error removing '%1': %2;").
+                _error += PropagateLocalRemove::tr("Error removing '%1': %2;").
                     arg(QDir::toNativeSeparators(f.fileName()), f.errorString()) + " ";
                 qDebug() << "Error removing " << f.fileName() << ':' << f.errorString();
             }
         }
-        if (!ok)
+        if (success && !ok) {
+            // We need to delete the entries from the database now from the deleted vector
+            foreach(const auto &it, deleted) {
+                _propagator->_journal->deleteFileRecord(_item->_originalFile + path + QLatin1Char('/') + it.first,
+                                                        it.second);
+            }
             success = false;
+            deleted.clear();
+        }
+        if (success) {
+            deleted.append(qMakePair(di.fileName(), isDir));
+        }
+        if (!success && ok) {
+            // This succeeded, so we need to delete it from the database now because the caller won't
+            _propagator->_journal->deleteFileRecord(_item->_originalFile + path + QLatin1Char('/') + di.fileName(),
+                                                    isDir);
+        }
     }
     if (success) {
-        success = QDir().rmdir(path);
+        success = QDir().rmdir(absolute);
         if (!success) {
-            error += PropagateLocalRemove::tr("Could not remove directory '%1';")
-                .arg(QDir::toNativeSeparators(path)) + " ";
-            qDebug() << "Error removing directory" << path;
+            _error += PropagateLocalRemove::tr("Could not remove directory '%1';")
+                .arg(QDir::toNativeSeparators(absolute)) + " ";
+            qDebug() << "Error removing directory" << absolute;
         }
     }
     return success;
@@ -89,9 +116,8 @@ void PropagateLocalRemove::start()
     }
 
     if (_item->_isDirectory) {
-        QString error;
-        if (QDir(filename).exists() && !removeRecursively(filename, error)) {
-            done(SyncFileItem::NormalError, error);
+        if (QDir(filename).exists() && !removeRecursively(QString())) {
+            done(SyncFileItem::NormalError, _error);
             return;
         }
     } else {
diff --git a/src/libsync/propagatorjobs.h b/src/libsync/propagatorjobs.h
index 537bba940..5dd21e682 100644
--- a/src/libsync/propagatorjobs.h
+++ b/src/libsync/propagatorjobs.h
@@ -43,6 +43,9 @@ class PropagateLocalRemove : public PropagateItemJob {
 public:
     PropagateLocalRemove (OwncloudPropagator* propagator,const SyncFileItemPtr& item)  : PropagateItemJob(propagator, item) {}
     void start() Q_DECL_OVERRIDE;
+private:
+    bool removeRecursively(const QString &path);
+    QString _error;
 };
 
 /**