From: Christian Kamm Date: Wed, 28 Aug 2019 12:20:40 +0000 (+0200) Subject: Discovery: If a move is forbidden, restore the source X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~468^2~188 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=28797baa39b22b35c871730ccb2b479c230ae6f0;p=nextcloud-desktop.git Discovery: If a move is forbidden, restore the source Previously the source was deleted (or attempted to be deleted), even if the new location was not acceptable for upload. This could make data unavilable on the server. For #7410 --- diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index d08adbb79..de5704588 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -654,7 +654,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( // conflict we don't need to recurse into it. (local c1.owncloud, c1/ ; remote: c1) if (item->_instruction == CSYNC_INSTRUCTION_CONFLICT && !item->isDirectory()) recurse = false; - if (_queryLocal != NormalQuery && _queryServer != NormalQuery && !item->_isRestoration) + if (_queryLocal != NormalQuery && _queryServer != NormalQuery) recurse = false; auto recurseQueryLocal = _queryLocal == ParentNotChanged ? ParentNotChanged : localEntry.isDirectory || item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist; @@ -689,13 +689,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_type = ItemTypeVirtualFile; } else if (!serverModified) { // Removed locally: also remove on the server. - if (_dirItem && _dirItem->_isRestoration && _dirItem->_instruction == CSYNC_INSTRUCTION_NEW) { - // Also restore everything - item->_instruction = CSYNC_INSTRUCTION_NEW; - item->_direction = SyncFileItem::Down; - item->_isRestoration = true; - item->_errorString = tr("Not allowed to remove, restoring"); - } else if (!dbEntry._serverHasIgnoredFiles) { + if (!dbEntry._serverHasIgnoredFiles) { item->_instruction = CSYNC_INSTRUCTION_REMOVE; item->_direction = SyncFileItem::Up; } @@ -900,20 +894,48 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( return false; } - // Check local permission if we are allowed to put move the file here - // Technically we should use the one from the server, but we'll assume it is the same - if (!checkMovePermissions(base._remotePerm, originalPath, item->isDirectory())) { - qCInfo(lcDisco) << "Not a move, no permission to rename base file"; - return false; - } - return true; }; - // Finally make it a NEW or a RENAME + // If it's not a move it's just a local-NEW if (!moveCheck()) { postProcessLocalNew(); } else { + // Check local permission if we are allowed to put move the file here + // Technically we should use the permissions from the server, but we'll assume it is the same + auto movePerms = checkMovePermissions(base._remotePerm, originalPath, item->isDirectory()); + if (!movePerms.sourceOk || !movePerms.destinationOk) { + qCInfo(lcDisco) << "Move without permission to rename base file, " + << "source:" << movePerms.sourceOk + << ", target:" << movePerms.destinationOk + << ", targetNew:" << movePerms.destinationNewOk; + + // If we can create the destination, do that. + // Permission errors on the destination will be handled by checkPermissions later. + postProcessLocalNew(); + finalize(); + + // If the destination upload will work, we're fine with the source deletion. + // If the source deletion can't work, checkPermissions will error. + if (movePerms.destinationNewOk) + return; + + // Here we know the new location can't be uploaded: must prevent the source delete. + // Two cases: either the source item was already processed or not. + auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath); + if (wasDeletedOnClient.first) { + // More complicated. The REMOVE is canceled. Restore will happen next sync. + qCInfo(lcDisco) << "Undid remove instruction on source" << originalPath; + _discoveryData->_statedb->deleteFileRecord(originalPath, true); + _discoveryData->_anotherSyncNeeded = true; + } else { + // Signal to future checkPermissions() to forbid the REMOVE and set to restore instead + qCInfo(lcDisco) << "Preventing future remove on source" << originalPath; + _discoveryData->_forbiddenDeletes[originalPath + '/'] = true; + } + return; + } + auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath); auto processRename = [item, originalPath, base, this](PathTuple &path) { @@ -1084,8 +1106,12 @@ void ProcessDirectoryJob::processFileFinalize( if (item->isDirectory() && item->_instruction == CSYNC_INSTRUCTION_SYNC) item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; - if (!checkPermissions(item)) + if (checkPermissions(item)) { + if (item->_isRestoration && item->isDirectory()) + recurse = true; + } else { recurse = false; + } if (recurse) { auto job = new ProcessDirectoryJob(path, item, recurseQueryLocal, recurseQueryServer, this); if (item->_instruction == CSYNC_INSTRUCTION_REMOVE) { @@ -1185,6 +1211,20 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) break; } case CSYNC_INSTRUCTION_REMOVE: { + QString fileSlash = item->_file + '/'; + auto forbiddenIt = _discoveryData->_forbiddenDeletes.upperBound(fileSlash); + if (forbiddenIt != _discoveryData->_forbiddenDeletes.begin()) + forbiddenIt -= 1; + if (forbiddenIt != _discoveryData->_forbiddenDeletes.end() + && fileSlash.startsWith(forbiddenIt.key())) { + + qCWarning(lcDisco) << "checkForPermission: RESTORING" << item->_file; + item->_instruction = CSYNC_INSTRUCTION_NEW; + item->_direction = SyncFileItem::Down; + item->_isRestoration = true; + item->_errorString = tr("Moved to invalid target, restoring"); + return true; // restore sub items + } const auto perms = item->_remotePerm; if (perms.isNull()) { // No permissions set @@ -1207,8 +1247,9 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) } -bool ProcessDirectoryJob::checkMovePermissions(RemotePermissions srcPerm, const QString &srcPath, +auto ProcessDirectoryJob::checkMovePermissions(RemotePermissions srcPerm, const QString &srcPath, bool isDirectory) + -> MovePermissionResult { auto destPerms = !_rootPermissions.isNull() ? _rootPermissions : _dirItem ? _dirItem->_remotePerm : _rootPermissions; @@ -1218,12 +1259,15 @@ bool ProcessDirectoryJob::checkMovePermissions(RemotePermissions srcPerm, const && srcPath.lastIndexOf('/') == _currentFolder._original.size(); // Check if we are allowed to move to the destination. bool destinationOK = true; - if (isRename || destPerms.isNull()) { - // no need to check for the destination dir permission - destinationOK = true; + bool destinationNewOK = true; + if (destPerms.isNull()) { } else if (isDirectory && !destPerms.hasPermission(RemotePermissions::CanAddSubDirectories)) { - destinationOK = false; + destinationNewOK = false; } else if (!isDirectory && !destPerms.hasPermission(RemotePermissions::CanAddFile)) { + destinationNewOK = false; + } + if (!isRename && !destinationNewOK) { + // no need to check for the destination dir permission for renames destinationOK = false; } @@ -1235,16 +1279,7 @@ bool ProcessDirectoryJob::checkMovePermissions(RemotePermissions srcPerm, const // We are not allowed to move or rename this file sourceOK = false; } - if (!sourceOK || !destinationOK) { - qCInfo(lcDisco) << "Not a move because permission does not allow it." << sourceOK << destinationOK; - if (!sourceOK) { - // This is the behavior that we had in the client <= 2.5. - // but that might not be needed anymore - _discoveryData->_statedb->avoidRenamesOnNextSync(srcPath); - } - return false; - } - return true; + return MovePermissionResult{sourceOK, destinationOK, destinationNewOK}; } void ProcessDirectoryJob::subJobFinished() diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h index 20d27b73d..96295c9db 100644 --- a/src/libsync/discovery.h +++ b/src/libsync/discovery.h @@ -167,11 +167,21 @@ private: */ bool checkPermissions(const SyncFileItemPtr &item); + struct MovePermissionResult + { + // whether moving/renaming the source is ok + bool sourceOk = false; + // whether the destination accepts (always true for renames) + bool destinationOk = false; + // whether creating a new file/dir in the destination is ok + bool destinationNewOk = false; + }; + /** * Check if the move is of a specified file within this directory is allowed. * Return true if it is allowed, false otherwise */ - bool checkMovePermissions(RemotePermissions srcPerm, const QString &srcPath, bool isDirectory); + MovePermissionResult checkMovePermissions(RemotePermissions srcPerm, const QString &srcPath, bool isDirectory); void processBlacklisted(const PathTuple &, const LocalInfo &, const SyncJournalFileRecord &dbEntry); void subJobFinished(); diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index beddbecc5..8fbaafc7c 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -155,12 +155,22 @@ class DiscoveryPhase : public QObject QMap _renamedItemsRemote; QMap _renamedItemsLocal; + // set of paths that should not be removed even though they are removed locally: + // there was a move to an invalid destination and now the source should be restored + // + // This applies recursively to subdirectories. + // All entries should have a trailing slash (even files), so lookup with + // lowerBound() is reliable. + // + // The value of this map doesn't matter. + QMap _forbiddenDeletes; + /** Returns whether the db-path has been renamed locally or on the remote. * * Useful for avoiding processing of items that have already been claimed in * a rename (would otherwise be discovered as deletions). */ - bool isRenamed(const QString &p) { return _renamedItemsLocal.contains(p) || _renamedItemsRemote.contains(p); } + bool isRenamed(const QString &p) const { return _renamedItemsLocal.contains(p) || _renamedItemsRemote.contains(p); } int _currentlyActiveJobs = 0; @@ -217,6 +227,7 @@ public: // output QByteArray _dataFingerprint; + bool _anotherSyncNeeded = false; signals: void fatalError(const QString &errorString); diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 8c874a019..2cc537d5b 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -191,7 +191,7 @@ public: return false; } const char *instruction_str = csync_instruction_str(_item->_instruction); - qCInfo(lcPropagator) << "Starting" << instruction_str << "propagation of" << _item->_file << "by" << this; + qCInfo(lcPropagator) << "Starting" << instruction_str << "propagation of" << _item->destination() << "by" << this; _state = Running; QMetaObject::invokeMethod(this, "start"); // We could be in a different thread (neon jobs) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 4e75da8e0..33ad10c03 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -738,6 +738,10 @@ void SyncEngine::slotDiscoveryFinished() restoreOldFiles(_syncItems); } + if (_discoveryPhase->_anotherSyncNeeded && _anotherSyncNeeded == NoFollowUpSync) { + _anotherSyncNeeded = ImmediateFollowUp; + } + // Sort items per destination std::sort(_syncItems.begin(), _syncItems.end()); diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index 276e34435..01e385323 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -581,6 +581,8 @@ public: QString fileName = getFilePathFromUrl(request.url()); Q_ASSERT(!fileName.isEmpty()); fileInfo = remoteRootFileInfo.find(fileName); + if (!fileInfo) + qWarning() << "Could not find file" << fileName << "on the remote"; QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection); } diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index ad02ca0a9..6640d50f2 100644 --- a/test/testpermissions.cpp +++ b/test/testpermissions.cpp @@ -41,6 +41,36 @@ static void assertCsyncJournalOk(SyncJournalDb &journal) #endif } +SyncFileItemPtr findItem(const QSignalSpy &spy, const QString &path) +{ + for (const QList &args : spy) { + auto item = args[0].value(); + if (item->destination() == path) + return item; + } + return SyncFileItemPtr(new SyncFileItem); +} + +SyncFileItemPtr findDiscoveryItem(const SyncFileItemVector &spy, const QString &path) +{ + for (const auto &item : spy) { + if (item->destination() == path) + return item; + } + return SyncFileItemPtr(new SyncFileItem); +} + +bool itemInstruction(const QSignalSpy &spy, const QString &path, const csync_instructions_e instr) +{ + auto item = findItem(spy, path); + return item->_instruction == instr; +} + +bool discoveryInstruction(const SyncFileItemVector &spy, const QString &path, const csync_instructions_e instr) +{ + auto item = findDiscoveryItem(spy, path); + return item->_instruction == instr; +} class TestPermissions : public QObject { @@ -188,7 +218,16 @@ private slots: assertCsyncJournalOk(fakeFolder.syncJournal()); currentLocalState = fakeFolder.currentLocalState(); QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/cannotBeRemoved_PERM_WVN_.data")); - QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data")); + QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_")); + // the subdirectory had delete permissions, so the contents were deleted + QVERIFY(!currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_")); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // restore + fakeFolder.remoteModifier().mkdir("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_"); + fakeFolder.remoteModifier().insert("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data"); + applyPermissionsFromName(fakeFolder.remoteModifier()); + QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); @@ -203,14 +242,23 @@ private slots: currentLocalState = fakeFolder.currentLocalState(); // old name restored - QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_")); - QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data")); + QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_")); + // contents moved (had move permissions) + QVERIFY(!currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_")); + QVERIFY(!currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data")); // new still exist (and is uploaded) QVERIFY(currentLocalState.find("normalDirectory_PERM_CKDNV_/subdir_PERM_CKDNV_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data")); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + // restore for further tests + fakeFolder.remoteModifier().mkdir("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_"); + fakeFolder.remoteModifier().insert("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data"); + applyPermissionsFromName(fakeFolder.remoteModifier()); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + //###################################################################### qInfo( "rename a directory in a read only folder and move a directory to a read-only" ); @@ -220,6 +268,8 @@ private slots: QVERIFY(fakeFolder.syncOnce()); assertCsyncJournalOk(fakeFolder.syncJournal()); + QVERIFY(fakeFolder.currentLocalState().find("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data" )); + //1. rename a directory in a read only folder //Missing directory should be restored //new directory should stay but not be uploaded @@ -234,6 +284,8 @@ private slots: //1. // old name restored + QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_" )); + // including contents QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data" )); // new still exist QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/newname_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data" )); @@ -257,6 +309,10 @@ private slots: //###################################################################### qInfo( "multiple restores of a file create different conflict files" ); + fakeFolder.remoteModifier().insert("readonlyDirectory_PERM_M_/cannotBeModified_PERM_DVN_.data"); + applyPermissionsFromName(fakeFolder.remoteModifier()); + QVERIFY(fakeFolder.syncOnce()); + editReadOnly("readonlyDirectory_PERM_M_/cannotBeModified_PERM_DVN_.data"); fakeFolder.localModifier().setContents("readonlyDirectory_PERM_M_/cannotBeModified_PERM_DVN_.data", 's'); //do the sync @@ -286,6 +342,118 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + static void setAllPerm(FileInfo *fi, RemotePermissions perm) + { + fi->permissions = perm; + for (auto &subFi : fi->children) + setAllPerm(&subFi, perm); + } + + // What happens if the source can't be moved or the target can't be created? + void testForbiddenMoves() + { + FakeFolder fakeFolder{FileInfo{}}; + + auto &lm = fakeFolder.localModifier(); + auto &rm = fakeFolder.remoteModifier(); + rm.mkdir("allowed"); + rm.mkdir("norename"); + rm.mkdir("nomove"); + rm.mkdir("nocreatefile"); + rm.mkdir("nocreatedir"); + rm.mkdir("zallowed"); // order of discovery matters + + rm.mkdir("allowed/sub"); + rm.mkdir("allowed/sub2"); + rm.insert("allowed/file"); + rm.insert("allowed/sub/file"); + rm.insert("allowed/sub2/file"); + rm.mkdir("norename/sub"); + rm.insert("norename/file"); + rm.insert("norename/sub/file"); + rm.mkdir("nomove/sub"); + rm.insert("nomove/file"); + rm.insert("nomove/sub/file"); + rm.mkdir("zallowed/sub"); + rm.mkdir("zallowed/sub2"); + rm.insert("zallowed/file"); + rm.insert("zallowed/sub/file"); + rm.insert("zallowed/sub2/file"); + + setAllPerm(rm.find("norename"), RemotePermissions::fromServerString("WDVCK")); + setAllPerm(rm.find("nomove"), RemotePermissions::fromServerString("WDNCK")); + setAllPerm(rm.find("nocreatefile"), RemotePermissions::fromServerString("WDNVK")); + setAllPerm(rm.find("nocreatedir"), RemotePermissions::fromServerString("WDNVC")); + + QVERIFY(fakeFolder.syncOnce()); + + // Renaming errors + lm.rename("norename/file", "norename/file_renamed"); + lm.rename("norename/sub", "norename/sub_renamed"); + // Moving errors + lm.rename("nomove/file", "allowed/file_moved"); + lm.rename("nomove/sub", "allowed/sub_moved"); + // Createfile errors + lm.rename("allowed/file", "nocreatefile/file"); + lm.rename("zallowed/file", "nocreatefile/zfile"); + lm.rename("allowed/sub", "nocreatefile/sub"); // TODO: probably forbidden because it contains file children? + // Createdir errors + lm.rename("allowed/sub2", "nocreatedir/sub2"); + lm.rename("zallowed/sub2", "nocreatedir/zsub2"); + + // also hook into discovery!! + SyncFileItemVector discovery; + connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToPropagate, this, [&discovery](auto v) { discovery = v; }); + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + QVERIFY(!fakeFolder.syncOnce()); + + // if renaming doesn't work, just delete+create + QVERIFY(itemInstruction(completeSpy, "norename/file", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "norename/sub", CSYNC_INSTRUCTION_NONE)); + QVERIFY(discoveryInstruction(discovery, "norename/sub", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "norename/file_renamed", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "norename/sub_renamed", CSYNC_INSTRUCTION_NEW)); + // the contents can _move_ + QVERIFY(itemInstruction(completeSpy, "norename/sub_renamed/file", CSYNC_INSTRUCTION_RENAME)); + + // simiilarly forbidding moves becomes delete+create + QVERIFY(itemInstruction(completeSpy, "nomove/file", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "nomove/sub", CSYNC_INSTRUCTION_NONE)); + QVERIFY(discoveryInstruction(discovery, "nomove/sub", CSYNC_INSTRUCTION_REMOVE)); + // nomove/sub/file is removed as part of the dir + QVERIFY(itemInstruction(completeSpy, "allowed/file_moved", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "allowed/sub_moved", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "allowed/sub_moved/file", CSYNC_INSTRUCTION_NEW)); + + // when moving to an invalid target, the targets should be an error + QVERIFY(itemInstruction(completeSpy, "nocreatefile/file", CSYNC_INSTRUCTION_ERROR)); + QVERIFY(itemInstruction(completeSpy, "nocreatefile/zfile", CSYNC_INSTRUCTION_ERROR)); + QVERIFY(itemInstruction(completeSpy, "nocreatefile/sub", CSYNC_INSTRUCTION_RENAME)); // TODO: What does a real server say? + QVERIFY(itemInstruction(completeSpy, "nocreatedir/sub2", CSYNC_INSTRUCTION_ERROR)); + QVERIFY(itemInstruction(completeSpy, "nocreatedir/zsub2", CSYNC_INSTRUCTION_ERROR)); + + // and the sources of the invalid moves should be restored, not deleted + // (depending on the order of discovery a follow-up sync is needed) + QVERIFY(itemInstruction(completeSpy, "allowed/file", CSYNC_INSTRUCTION_NONE)); + QVERIFY(itemInstruction(completeSpy, "allowed/sub2", CSYNC_INSTRUCTION_NONE)); + QVERIFY(itemInstruction(completeSpy, "zallowed/file", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "zallowed/sub2", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "zallowed/sub2/file", CSYNC_INSTRUCTION_NEW)); + QCOMPARE(fakeFolder.syncEngine().isAnotherSyncNeeded(), ImmediateFollowUp); + + // A follow-up sync will restore allowed/file and allowed/sub2 and maintain the createdir/file errors + completeSpy.clear(); + QVERIFY(!fakeFolder.syncOnce()); + + QVERIFY(itemInstruction(completeSpy, "nocreatefile/file", CSYNC_INSTRUCTION_ERROR)); + QVERIFY(itemInstruction(completeSpy, "nocreatefile/zfile", CSYNC_INSTRUCTION_ERROR)); + QVERIFY(itemInstruction(completeSpy, "nocreatedir/sub2", CSYNC_INSTRUCTION_ERROR)); + QVERIFY(itemInstruction(completeSpy, "nocreatedir/zsub2", CSYNC_INSTRUCTION_ERROR)); + + QVERIFY(itemInstruction(completeSpy, "allowed/file", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "allowed/sub2", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "allowed/sub2/file", CSYNC_INSTRUCTION_NEW)); + } }; QTEST_GUILESS_MAIN(TestPermissions) diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index ae1e16efc..c3bc04991 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -11,22 +11,34 @@ using namespace OCC; -bool itemDidComplete(const QSignalSpy &spy, const QString &path) +SyncFileItemPtr findItem(const QSignalSpy &spy, const QString &path) { - for(const QList &args : spy) { + for (const QList &args : spy) { auto item = args[0].value(); if (item->destination() == path) - return item->_instruction != CSYNC_INSTRUCTION_NONE && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA; + return item; + } + return SyncFileItemPtr(new SyncFileItem); +} + +bool itemDidComplete(const QSignalSpy &spy, const QString &path) +{ + if (auto item = findItem(spy, path)) { + return item->_instruction != CSYNC_INSTRUCTION_NONE && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA; } return false; } +bool itemInstruction(const QSignalSpy &spy, const QString &path, const csync_instructions_e instr) +{ + auto item = findItem(spy, path); + return item->_instruction == instr; +} + bool itemDidCompleteSuccessfully(const QSignalSpy &spy, const QString &path) { - for(const QList &args : spy) { - auto item = args[0].value(); - if (item->destination() == path) - return item->_status == SyncFileItem::Success; + if (auto item = findItem(spy, path)) { + return item->_status == SyncFileItem::Success; } return false; }