[Pkg-owncloud-commits] [owncloud-client] 51/218: PropagateLocalRemove: remove entries from the DB even if there was an error.

[Sandro Knauß]

This is an automated email from the git hooks/post-receive script.

hefee-guest pushed a commit to branch master
in repository owncloud-client.

commit e54253d8457f5e0eb071a7a704699149aec040e1
Author: Olivier Goffart <ogoffart at woboq.com>
Date:   Wed Sep 2 15:19:34 2015 +0200

    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
---
 src/libsync/propagatorjobs.cpp | 56 +++++++++++++++++++++++++++++++-----------
 src/libsync/propagatorjobs.h   |  3 +++
 2 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
index 4cb7872..7c4df9c 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 537bba9..5dd21e6 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;
 };
 
 /**

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud-client.git