Rollback local move when server move has failed.
authoralex-z <blackslayer4@gmail.com>
Fri, 18 Feb 2022 08:50:00 +0000 (10:50 +0200)
committeralex-z <blackslayer4@gmail.com>
Wed, 23 Feb 2022 14:53:26 +0000 (16:53 +0200)
Signed-off-by: alex-z <blackslayer4@gmail.com>
src/libsync/propagateremotemove.cpp
test/testsyncengine.cpp
test/testsyncfilestatustracker.cpp
test/testsyncmove.cpp

index d6b1bd29f1479021127a433e28f79c747c6fba43..a101feb12769bcc317b0f27ef26e233950e4c160 100644 (file)
@@ -208,6 +208,17 @@ void PropagateRemoteMove::slotMoveJobFinished()
     if (err != QNetworkReply::NoError) {
         SyncFileItem::Status status = classifyError(err, _item->_httpErrorCode,
             &propagator()->_anotherSyncNeeded);
+        const auto filePath = propagator()->fullLocalPath(_item->_renameTarget);
+        const auto filePathOriginal = propagator()->fullLocalPath(_item->_originalFile);
+        QFile file(filePath);
+        if (!file.rename(filePathOriginal)) {
+            qCWarning(lcPropagateRemoteMove) << "Could not MOVE file" << filePathOriginal << " to" << filePath
+                                             << " with error:" << _job->errorString() << " and failed to restore it !";
+        } else {
+            qCWarning(lcPropagateRemoteMove)
+                << "Could not MOVE file" << filePathOriginal << " to" << filePath
+                << " with error:" << _job->errorString() << " and successfully restored it.";
+        }
         done(status, _job->errorString());
         return;
     }
index d2c865b7c72c3dd48d748481d765f08c92d52d50..9ad3c66a63b87aa76eb69121f48222cd9424d9e5 100644 (file)
@@ -982,6 +982,106 @@ private slots:
         QCOMPARE(nPOST, 0);
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
     }
+
+    void testRemoteMoveFailedInsufficientStorageLocalMoveRolledBack()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+
+        // create a big shared folder with some files
+        fakeFolder.remoteModifier().mkdir("big_shared_folder");
+        fakeFolder.remoteModifier().mkdir("big_shared_folder/shared_files");
+        fakeFolder.remoteModifier().insert("big_shared_folder/shared_files/big_shared_file_A.data", 1000);
+        fakeFolder.remoteModifier().insert("big_shared_folder/shared_files/big_shared_file_B.data", 1000);
+
+        // make sure big shared folder is synced
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(fakeFolder.currentLocalState().find("big_shared_folder/shared_files/big_shared_file_A.data"));
+        QVERIFY(fakeFolder.currentLocalState().find("big_shared_folder/shared_files/big_shared_file_B.data"));
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        // try to move from a big shared folder to your own folder
+        fakeFolder.localModifier().mkdir("own_folder");
+        fakeFolder.localModifier().rename(
+            "big_shared_folder/shared_files/big_shared_file_A.data", "own_folder/big_shared_file_A.data");
+        fakeFolder.localModifier().rename(
+            "big_shared_folder/shared_files/big_shared_file_B.data", "own_folder/big_shared_file_B.data");
+
+        // emulate server MOVE 507 error
+        QObject parent;
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request,
+                                         QIODevice *outgoingData) -> QNetworkReply * {
+            Q_UNUSED(outgoingData)
+
+            if (op == QNetworkAccessManager::CustomOperation
+                && request.attribute(QNetworkRequest::CustomVerbAttribute).toString() == QStringLiteral("MOVE")) {
+                return new FakeErrorReply(op, request, &parent, 507);
+            }
+            return nullptr;
+        });
+
+        // make sure the first sync failes and files get restored to original folder
+        QVERIFY(!fakeFolder.syncOnce());
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        QVERIFY(fakeFolder.currentLocalState().find("big_shared_folder/shared_files/big_shared_file_A.data"));
+        QVERIFY(fakeFolder.currentLocalState().find("big_shared_folder/shared_files/big_shared_file_B.data"));
+        QVERIFY(!fakeFolder.currentLocalState().find("own_folder/big_shared_file_A.data"));
+        QVERIFY(!fakeFolder.currentLocalState().find("own_folder/big_shared_file_B.data"));
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
+    void testRemoteMoveFailedForbiddenLocalMoveRolledBack()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+
+        // create a big shared folder with some files
+        fakeFolder.remoteModifier().mkdir("big_shared_folder");
+        fakeFolder.remoteModifier().mkdir("big_shared_folder/shared_files");
+        fakeFolder.remoteModifier().insert("big_shared_folder/shared_files/big_shared_file_A.data", 1000);
+        fakeFolder.remoteModifier().insert("big_shared_folder/shared_files/big_shared_file_B.data", 1000);
+
+        // make sure big shared folder is synced
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(fakeFolder.currentLocalState().find("big_shared_folder/shared_files/big_shared_file_A.data"));
+        QVERIFY(fakeFolder.currentLocalState().find("big_shared_folder/shared_files/big_shared_file_B.data"));
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        // try to move from a big shared folder to your own folder
+        fakeFolder.localModifier().mkdir("own_folder");
+        fakeFolder.localModifier().rename(
+            "big_shared_folder/shared_files/big_shared_file_A.data", "own_folder/big_shared_file_A.data");
+        fakeFolder.localModifier().rename(
+            "big_shared_folder/shared_files/big_shared_file_B.data", "own_folder/big_shared_file_B.data");
+
+        // emulate server MOVE 507 error
+        QObject parent;
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request,
+                                         QIODevice *outgoingData) -> QNetworkReply * {
+            Q_UNUSED(outgoingData)
+
+            auto attributeCustomVerb = request.attribute(QNetworkRequest::CustomVerbAttribute).toString();
+
+            if (op == QNetworkAccessManager::CustomOperation
+                && request.attribute(QNetworkRequest::CustomVerbAttribute).toString() == QStringLiteral("MOVE")) {
+                return new FakeErrorReply(op, request, &parent, 403);
+            }
+            return nullptr;
+        });
+
+        // make sure the first sync failes and files get restored to original folder
+        QVERIFY(!fakeFolder.syncOnce());
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        QVERIFY(fakeFolder.currentLocalState().find("big_shared_folder/shared_files/big_shared_file_A.data"));
+        QVERIFY(fakeFolder.currentLocalState().find("big_shared_folder/shared_files/big_shared_file_B.data"));
+        QVERIFY(!fakeFolder.currentLocalState().find("own_folder/big_shared_file_A.data"));
+        QVERIFY(!fakeFolder.currentLocalState().find("own_folder/big_shared_file_B.data"));
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncEngine)
index 34e66f401b86a71f6ba1f84dc19bc65259cd61b2..e87bb6575ef5dde1b0566b6224afc9398b203543 100644 (file)
@@ -22,6 +22,7 @@ public:
 
     SyncFileStatus statusOf(const QString &relativePath) const {
         QFileInfo file(_syncEngine.localPath(), relativePath);
+        auto locPath = _syncEngine.localPath();
         // Start from the end to get the latest status
         for (int i = size() - 1; i >= 0; --i) {
             if (QFileInfo(at(i)[0].toString()) == file)
@@ -467,6 +468,7 @@ private slots:
     }
 
     void renameError() {
+        // when rename has failed - the old file name must be restored
         FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
         fakeFolder.serverErrorPaths().append("A/a1");
         fakeFolder.localModifier().rename("A/a1", "A/a1m");
@@ -488,22 +490,22 @@ private slots:
         fakeFolder.execUntilFinished();
         verifyThatPushMatchesPull(fakeFolder, statusSpy);
         QCOMPARE(statusSpy.statusOf("A/a1m"), SyncFileStatus(SyncFileStatus::StatusError));
-        QCOMPARE(statusSpy.statusOf("A/a1"), statusSpy.statusOf("A/a1notexist"));
+        QCOMPARE(statusSpy.statusOf("A/a1"), SyncFileStatus(SyncFileStatus::StatusUpToDate));
         QCOMPARE(statusSpy.statusOf("A"), SyncFileStatus(SyncFileStatus::StatusWarning));
         QCOMPARE(statusSpy.statusOf(""), SyncFileStatus(SyncFileStatus::StatusWarning));
         QCOMPARE(statusSpy.statusOf("B"), SyncFileStatus(SyncFileStatus::StatusUpToDate));
         QCOMPARE(statusSpy.statusOf("B/b1m"), SyncFileStatus(SyncFileStatus::StatusUpToDate));
         statusSpy.clear();
 
-        QVERIFY(!fakeFolder.syncOnce());
+        QVERIFY(fakeFolder.syncOnce());
         verifyThatPushMatchesPull(fakeFolder, statusSpy);
         statusSpy.clear();
-        QVERIFY(!fakeFolder.syncOnce());
+        QVERIFY(fakeFolder.syncOnce());
         verifyThatPushMatchesPull(fakeFolder, statusSpy);
-        QCOMPARE(statusSpy.statusOf("A/a1m"), SyncFileStatus(SyncFileStatus::StatusError));
+        QCOMPARE(statusSpy.statusOf("A/a1m"), SyncFileStatus(SyncFileStatus::StatusNone));
         QCOMPARE(statusSpy.statusOf("A/a1"), statusSpy.statusOf("A/a1notexist"));
-        QCOMPARE(statusSpy.statusOf("A"), SyncFileStatus(SyncFileStatus::StatusWarning));
-        QCOMPARE(statusSpy.statusOf(""), SyncFileStatus(SyncFileStatus::StatusWarning));
+        QCOMPARE(statusSpy.statusOf("A"), SyncFileStatus(SyncFileStatus::StatusNone));
+        QCOMPARE(statusSpy.statusOf(""), SyncFileStatus(SyncFileStatus::StatusUpToDate));
         QCOMPARE(statusSpy.statusOf("B"), SyncFileStatus(SyncFileStatus::StatusNone));
         QCOMPARE(statusSpy.statusOf("B/b1m"), SyncFileStatus(SyncFileStatus::StatusNone));
         statusSpy.clear();
index f73c84838f6a356077023d40b9989fd3b8c39a05..f1488c8189b758b6b87ff9e99d31a37228249527 100644 (file)
@@ -958,12 +958,12 @@ private slots:
             fakeFolder.syncOnce();
         }
 
-        QVERIFY(!fakeFolder.currentLocalState().find(src));
-        QVERIFY(fakeFolder.currentLocalState().find(getName(dest)));
+        QVERIFY(fakeFolder.currentLocalState().find(getName(src)));
+        QVERIFY(!fakeFolder.currentLocalState().find(getName(dest)));
         if (vfsMode == Vfs::WithSuffix)
         {
             // the placeholder was not restored as it is still in error state
-            QVERIFY(!fakeFolder.currentLocalState().find(dest));
+            QVERIFY(!fakeFolder.currentLocalState().find(getName(dest)));
         }
         QVERIFY(fakeFolder.currentRemoteState().find(src));
         QVERIFY(!fakeFolder.currentRemoteState().find(dest));