diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index d6b1bd29f..a101feb12 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -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; } diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index d2c865b7c..9ad3c66a6 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -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) diff --git a/test/testsyncfilestatustracker.cpp b/test/testsyncfilestatustracker.cpp index 34e66f401..e87bb6575 100644 --- a/test/testsyncfilestatustracker.cpp +++ b/test/testsyncfilestatustracker.cpp @@ -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(); diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp index f73c84838..f1488c818 100644 --- a/test/testsyncmove.cpp +++ b/test/testsyncmove.cpp @@ -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));