From c1899935423d6bb8caadcb7077466e1687cef388 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 22 Aug 2023 15:09:42 +0200 Subject: [PATCH] Use getter setter for SyncItem::instruction to simplify debugging --- src/gui/folder.cpp | 3 +- src/gui/owncloudgui.cpp | 2 +- src/gui/syncrunfilelog.cpp | 9 +- src/libsync/discovery.cpp | 159 +++++++++++++------------- src/libsync/discoveryphase.cpp | 4 +- src/libsync/localdiscoverytracker.cpp | 2 +- src/libsync/owncloudpropagator.cpp | 52 ++++----- src/libsync/owncloudpropagator.h | 4 +- src/libsync/progressdispatcher.cpp | 11 +- src/libsync/progressdispatcher.h | 8 +- src/libsync/propagatedownload.cpp | 10 +- src/libsync/propagateupload.cpp | 6 +- src/libsync/propagatorjobs.cpp | 6 +- src/libsync/syncengine.cpp | 34 +++--- src/libsync/syncfileitem.cpp | 17 ++- src/libsync/syncfileitem.h | 9 +- src/libsync/syncfilestatustracker.cpp | 26 ++--- src/libsync/syncresult.cpp | 12 +- test/testblacklist.cpp | 10 +- test/testchunkingng.cpp | 4 +- test/testpermissions.cpp | 4 +- test/testremotediscovery.cpp | 14 +-- test/testsyncconflict.cpp | 4 +- test/testsyncengine.cpp | 10 +- test/testsyncfileitem.cpp | 2 +- test/testsyncmove.cpp | 4 +- test/testsyncvirtualfiles.cpp | 2 +- 27 files changed, 206 insertions(+), 222 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 4c8e7839250..2064c667c04 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -1086,8 +1086,7 @@ void Folder::slotSyncFinished(bool success) // a item is completed: count the errors and forward to the ProgressDispatcher void Folder::slotItemCompleted(const SyncFileItemPtr &item) { - if (item->_status == SyncFileItem::Success - && (item->_instruction & (CSYNC_INSTRUCTION_NONE | CSYNC_INSTRUCTION_UPDATE_METADATA))) { + if (item->_status == SyncFileItem::Success && (item->instruction() & (CSYNC_INSTRUCTION_NONE | CSYNC_INSTRUCTION_UPDATE_METADATA))) { // We only care about the updates that deserve to be shown in the UI return; } diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp index 9517fdb00b3..27f82b232f4 100644 --- a/src/gui/owncloudgui.cpp +++ b/src/gui/owncloudgui.cpp @@ -756,7 +756,7 @@ void ownCloudGui::slotRebuildRecentMenus() /// 'Recent Activity' menu static bool shouldShowInRecentsMenu(const SyncFileItem &item) { - return !Progress::isIgnoredKind(item._status) && item._instruction != CSYNC_INSTRUCTION_NONE; + return !Progress::isIgnoredKind(item._status) && item.instruction() != CSYNC_INSTRUCTION_NONE; } void ownCloudGui::slotUpdateProgress(Folder *folder, const ProgressInfo &progress) diff --git a/src/gui/syncrunfilelog.cpp b/src/gui/syncrunfilelog.cpp index b3732641923..599bdc1b2bd 100644 --- a/src/gui/syncrunfilelog.cpp +++ b/src/gui/syncrunfilelog.cpp @@ -79,17 +79,16 @@ void SyncRunFileLog::start(const QString &folderPath) void SyncRunFileLog::logItem(const SyncFileItem &item) { // don't log the directory items that are in the list - if (item._direction == SyncFileItem::None - || item._instruction == CSYNC_INSTRUCTION_IGNORE) { + if (item._direction == SyncFileItem::None || item.instruction() == CSYNC_INSTRUCTION_IGNORE) { return; } const QChar L = QLatin1Char('|'); QString tmp; { QDebug(&tmp).noquote() << dateTimeStr(Utility::parseRFC1123Date(QString::fromUtf8(item._responseTimeStamp))) << L - << ((item._instruction != CSYNC_INSTRUCTION_RENAME) ? item.destination() - : item._file + QStringLiteral(" -> ") + item._renameTarget) - << L << item._instruction << L << item._direction << L << L << item._modtime << L << item._etag << L << item._size << L + << ((item.instruction() != CSYNC_INSTRUCTION_RENAME) ? item.destination() + : item._file + QStringLiteral(" -> ") + item._renameTarget) + << L << item.instruction() << L << item._direction << L << L << item._modtime << L << item._etag << L << item._size << L << item._fileId << L << item._status << L << item._errorString << L << item._httpErrorCode << L << item._previousSize << L << item._previousModtime << L << item._requestId << L << Qt::endl; } diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 45f97ad41a6..4d70c739863 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -229,7 +229,7 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const QString &loc auto item = SyncFileItemPtr::create(); item->_file = path; item->_originalFile = path; - item->_instruction = CSYNC_INSTRUCTION_IGNORE; + item->setInstruction(CSYNC_INSTRUCTION_IGNORE); if (isSymlink) { /* Symbolic links are ignored. */ @@ -358,7 +358,7 @@ void ProcessDirectoryJob::processFile(const PathTuple &path, || localEntry.type == ItemTypeVirtualFileDownload) && (localEntry.isValid() || _queryLocal == ParentNotChanged)) { item->_direction = SyncFileItem::Down; - item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->setInstruction(CSYNC_INSTRUCTION_SYNC); item->_type = ItemTypeVirtualFileDownload; } @@ -405,7 +405,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( if (serverEntry.fileId.isEmpty()) missingData.append(tr("file id")); if (!missingData.isEmpty()) { - item->_instruction = CSYNC_INSTRUCTION_ERROR; + item->setInstruction(CSYNC_INSTRUCTION_ERROR); _childIgnored = true; item->_errorString = tr("server reported no %1").arg(missingData.join(QLatin1String(", "))); emit _discoveryData->itemDiscovered(item); @@ -418,7 +418,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( if (serverEntry.isDirectory != dbEntry.isDirectory()) { // If the type of the entity changed, it's like NEW, but // needs to delete the other entity first. - item->_instruction = CSYNC_INSTRUCTION_TYPE_CHANGE; + item->setInstruction(CSYNC_INSTRUCTION_TYPE_CHANGE); item->_direction = SyncFileItem::Down; item->_modtime = serverEntry.modtime; item->_size = serverEntry.size; @@ -427,7 +427,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( // The above check for the localEntry existing is important. Otherwise it breaks // the case where a file is moved and simultaneously tagged for download in the db. item->_direction = SyncFileItem::Down; - item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->setInstruction(CSYNC_INSTRUCTION_SYNC); item->_type = ItemTypeVirtualFileDownload; item->_size = serverEntry.size; } else if (dbEntry._etag != serverEntry.etag.toUtf8()) { @@ -436,15 +436,15 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( item->_size = serverEntry.size; if (serverEntry.isDirectory) { OC_ENFORCE(dbEntry.isDirectory()); - item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + item->setInstruction(CSYNC_INSTRUCTION_UPDATE_METADATA); } else if (!localEntry.isValid() && _queryLocal != ParentNotChanged) { // Deleted locally, changed on server - item->_instruction = CSYNC_INSTRUCTION_NEW; + item->setInstruction(CSYNC_INSTRUCTION_NEW); } else { - item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->setInstruction(CSYNC_INSTRUCTION_SYNC); } } else if (dbEntry._remotePerm != serverEntry.remotePerm || dbEntry._fileId != serverEntry.fileId) { - item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + item->setInstruction(CSYNC_INSTRUCTION_UPDATE_METADATA); item->_direction = SyncFileItem::Down; } else { processFileAnalyzeLocalInfo(item, path, localEntry, serverEntry, dbEntry, ParentNotChanged); @@ -458,7 +458,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( // Unknown in db: new file on the server Q_ASSERT(!dbEntry.isValid()); - item->_instruction = CSYNC_INSTRUCTION_NEW; + item->setInstruction(CSYNC_INSTRUCTION_NEW); item->_direction = SyncFileItem::Down; item->_modtime = serverEntry.modtime; item->_size = serverEntry.size; @@ -579,7 +579,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( _discoveryData->_renamedItemsRemote.insert(originalPath, path._target); item->_modtime = base._modtime; item->_inode = base._inode; - item->_instruction = CSYNC_INSTRUCTION_RENAME; + item->setInstruction(CSYNC_INSTRUCTION_RENAME); item->_direction = SyncFileItem::Down; item->_renameTarget = path._target; item->_file = adjustedOriginalPath; @@ -612,7 +612,8 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( _discoveryData->findAndCancelDeletedJob(originalPath); postProcessRename(path); - processFileFinalize(item, path, item->isDirectory(), item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist, _queryServer); + processFileFinalize( + item, path, item->isDirectory(), item->instruction() == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist, _queryServer); return; } abort(); @@ -630,7 +631,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( return; // We went async } - if (item->_instruction == CSYNC_INSTRUCTION_NEW) { + if (item->instruction() == CSYNC_INSTRUCTION_NEW) { postProcessServerNew(); return; } @@ -649,13 +650,13 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( const bool serverModified = [&] { bool modifiedOnServer = - (item->_instruction & (CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_SYNC | CSYNC_INSTRUCTION_RENAME | CSYNC_INSTRUCTION_TYPE_CHANGE)); + (item->instruction() & (CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_SYNC | CSYNC_INSTRUCTION_RENAME | CSYNC_INSTRUCTION_TYPE_CHANGE)); // Decay server modifications to UPDATE_METADATA if the local virtual exists const bool hasLocalVirtual = localEntry.isVirtualFile || (_queryLocal == ParentNotChanged && dbEntry.isVirtualFile()); - const bool virtualFileDownload = item->_type == ItemTypeVirtualFileDownload || item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE; + const bool virtualFileDownload = item->_type == ItemTypeVirtualFileDownload || item->instruction() == CSYNC_INSTRUCTION_TYPE_CHANGE; if (modifiedOnServer && !virtualFileDownload && hasLocalVirtual) { - item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + item->setInstruction(CSYNC_INSTRUCTION_UPDATE_METADATA); modifiedOnServer = false; item->_type = ItemTypeVirtualFile; } @@ -672,12 +673,14 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( bool recurse = item->isDirectory() || localEntry.isDirectory || serverEntry.isDirectory; // Even if we have a local directory: If the remote is a file that's propagated as a // conflict we don't need to recurse into it. (local c1.owncloud, c1/ ; remote: c1) - if (item->_instruction == CSYNC_INSTRUCTION_CONFLICT && !item->isDirectory()) + if (item->instruction() == CSYNC_INSTRUCTION_CONFLICT && !item->isDirectory()) recurse = false; if (_queryLocal != NormalQuery && _queryServer != NormalQuery) recurse = false; - auto recurseQueryLocal = _queryLocal == ParentNotChanged ? ParentNotChanged : localEntry.isDirectory || item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist; + auto recurseQueryLocal = _queryLocal == ParentNotChanged ? ParentNotChanged + : localEntry.isDirectory || item->instruction() == CSYNC_INSTRUCTION_RENAME ? NormalQuery + : ParentDontExist; processFileFinalize(item, path, recurse, recurseQueryLocal, recurseQueryServer); }; @@ -686,12 +689,12 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( // Not modified locally (ParentNotChanged) if (noServerEntry) { // not on the server: Removed on the server, delete locally - item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->setInstruction(CSYNC_INSTRUCTION_REMOVE); item->_direction = SyncFileItem::Down; } else if (dbEntry._type == ItemTypeVirtualFileDehydration) { // dehydration requested item->_direction = SyncFileItem::Down; - item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->setInstruction(CSYNC_INSTRUCTION_SYNC); item->_type = ItemTypeVirtualFileDehydration; } } else if (noServerEntry) { @@ -704,13 +707,13 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( // This is a precaution since the suffix files don't look like the real ones // and we don't want users to accidentally delete server data because they // might not expect that deleting the placeholder will have a remote effect. - item->_instruction = CSYNC_INSTRUCTION_NEW; + item->setInstruction(CSYNC_INSTRUCTION_NEW); item->_direction = SyncFileItem::Down; item->_type = ItemTypeVirtualFile; } else if (!serverModified) { // Removed locally: also remove on the server. if (!dbEntry._serverHasIgnoredFiles) { - item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->setInstruction(CSYNC_INSTRUCTION_REMOVE); item->_direction = SyncFileItem::Up; } } @@ -722,12 +725,11 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( Q_ASSERT(localEntry.isValid()); item->_inode = localEntry.inode; - if (dbEntry.isValid()) { - bool typeChange = localEntry.isDirectory != dbEntry.isDirectory(); + const bool typeChange = localEntry.isDirectory != dbEntry.isDirectory(); if (!typeChange && localEntry.isVirtualFile) { if (noServerEntry) { - item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->setInstruction(CSYNC_INSTRUCTION_REMOVE); item->_direction = SyncFileItem::Down; } else if (!dbEntry.isVirtualFile() && isVfsWithSuffix()) { // If we find what looks to be a spurious "abc.owncloud" the base file "abc" @@ -736,28 +738,28 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( if (dbEntry._modtime == localEntry.modtime && dbEntry._fileSize == localEntry.size) { qCInfo(lcDisco) << "Base file was renamed to virtual file:" << item->_file; item->_direction = SyncFileItem::Down; - item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->setInstruction(CSYNC_INSTRUCTION_SYNC); item->_type = ItemTypeVirtualFileDehydration; addVirtualFileSuffix(item->_file); item->_renameTarget = item->_file; } else { qCInfo(lcDisco) << "Virtual file with non-virtual db entry, ignoring:" << item->_file; - item->_instruction = CSYNC_INSTRUCTION_IGNORE; + item->setInstruction(CSYNC_INSTRUCTION_IGNORE); } } } else if (!typeChange && ((dbEntry._modtime == localEntry.modtime && dbEntry._fileSize == localEntry.size) || localEntry.isDirectory)) { // Local file unchanged. if (noServerEntry) { - item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->setInstruction(CSYNC_INSTRUCTION_REMOVE); item->_direction = SyncFileItem::Down; } else if (dbEntry._type == ItemTypeVirtualFileDehydration || localEntry.type == ItemTypeVirtualFileDehydration) { item->_direction = SyncFileItem::Down; - item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->setInstruction(CSYNC_INSTRUCTION_SYNC); item->_type = ItemTypeVirtualFileDehydration; } else if (!serverModified && (dbEntry._inode != localEntry.inode || _discoveryData->_syncOptions._vfs->needsMetadataUpdate(*item))) { - item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + item->setInstruction(CSYNC_INSTRUCTION_UPDATE_METADATA); item->_direction = SyncFileItem::Down; } } else if (!typeChange && isVfsWithSuffix() @@ -769,10 +771,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( // This check leaks some details of VfsSuffix, particularly the size of placeholders. item->_direction = SyncFileItem::Down; if (noServerEntry) { - item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->setInstruction(CSYNC_INSTRUCTION_REMOVE); item->_type = ItemTypeFile; } else { - item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->setInstruction(CSYNC_INSTRUCTION_SYNC); item->_type = ItemTypeVirtualFileDownload; item->_previousSize = 1; } @@ -785,7 +787,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( // a regular SYNC upwards when there's no server change. processFileConflict(item, path, localEntry, serverEntry, dbEntry); } else if (typeChange) { - item->_instruction = CSYNC_INSTRUCTION_TYPE_CHANGE; + item->setInstruction(CSYNC_INSTRUCTION_TYPE_CHANGE); item->_direction = SyncFileItem::Up; item->_checksumHeader.clear(); item->_size = localEntry.size; @@ -794,10 +796,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( _childModified = true; } else { // Local file was changed - item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->setInstruction(CSYNC_INSTRUCTION_SYNC); if (noServerEntry) { // Special case! deleted on server, modified on client, the instruction is then NEW - item->_instruction = CSYNC_INSTRUCTION_NEW; + item->setInstruction(CSYNC_INSTRUCTION_NEW); } item->_direction = SyncFileItem::Up; item->_checksumHeader.clear(); @@ -812,7 +814,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( if (computeLocalChecksum(dbEntry._checksumHeader, _discoveryData->_localDir + path._local, item) && item->_checksumHeader == dbEntry._checksumHeader) { qCInfo(lcDisco) << "NOTE: Checksums are identical, file did not actually change: " << path._local; - item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + item->setInstruction(CSYNC_INSTRUCTION_UPDATE_METADATA); } } } @@ -826,7 +828,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( if (localEntry.isVirtualFile && !noServerEntry) { // Somehow there is a missing DB entry while the virtual file already exists. // The instruction should already be set correctly. - OC_ASSERT(item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA); + OC_ASSERT(item->instruction() == CSYNC_INSTRUCTION_UPDATE_METADATA); OC_ASSERT(item->_type == ItemTypeVirtualFile); finalize(path, recurseQueryServer); return; @@ -837,7 +839,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( } // New local file or rename - item->_instruction = CSYNC_INSTRUCTION_NEW; + item->setInstruction(CSYNC_INSTRUCTION_NEW); item->_direction = SyncFileItem::Up; item->_checksumHeader.clear(); item->_size = localEntry.size; @@ -850,12 +852,12 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( const bool isPlaceHolder = _discoveryData->_syncOptions._vfs->isDehydratedPlaceholder(_discoveryData->_localDir + path._local); if (isPlaceHolder) { qCWarning(lcDisco) << "Wiping virtual file without db entry for" << path._local; - item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->setInstruction(CSYNC_INSTRUCTION_REMOVE); item->_direction = SyncFileItem::Down; } else { qCWarning(lcDisco) << "Virtual file without db entry for" << path._local << "but looks odd, keeping"; - item->_instruction = CSYNC_INSTRUCTION_IGNORE; + item->setInstruction(CSYNC_INSTRUCTION_IGNORE); } } }; @@ -965,6 +967,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( auto processRename = [item, originalPath, base, this](PathTuple path) { auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath, SyncFileItem::Down); _discoveryData->_renamedItemsLocal.insert(originalPath, path._target); + // TODO: move to SyncFileItem so its easier to refactor if item changes in any way... item->_renameTarget = path._target; path._server = adjustedOriginalPath; item->_file = path._server; @@ -972,7 +975,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_originalFile = path._original; item->_modtime = base._modtime; item->_inode = base._inode; - item->_instruction = CSYNC_INSTRUCTION_RENAME; + item->setInstruction(CSYNC_INSTRUCTION_RENAME); item->_direction = SyncFileItem::Up; item->_fileId = base._fileId; item->_remotePerm = base._remotePerm; @@ -982,10 +985,11 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( // Discard any download/dehydrate tags on the base file. // They could be preserved and honored in a follow-up sync, // but it complicates handling a lot and will happen rarely. - if (item->_type == ItemTypeVirtualFileDownload) + if (item->_type == ItemTypeVirtualFileDownload) { item->_type = ItemTypeVirtualFile; - if (item->_type == ItemTypeVirtualFileDehydration) + } else if (item->_type == ItemTypeVirtualFileDehydration) { item->_type = ItemTypeFile; + } qCInfo(lcDisco) << "Rename detected (up) " << item->_file << " -> " << item->_renameTarget; return path; @@ -1026,7 +1030,7 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, const if (serverEntry.isDirectory && localEntry.isDirectory) { // Folders of the same path are always considered equals - item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + item->setInstruction(CSYNC_INSTRUCTION_UPDATE_METADATA); return; } @@ -1058,7 +1062,7 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, const // In particular this kind of NEW/NEW situation with identical // sizes and mtimes pops up when the local database is lost for // whatever reason. - item->_instruction = isConflict ? CSYNC_INSTRUCTION_CONFLICT : CSYNC_INSTRUCTION_UPDATE_METADATA; + item->setInstruction(isConflict ? CSYNC_INSTRUCTION_CONFLICT : CSYNC_INSTRUCTION_UPDATE_METADATA); item->_direction = isConflict ? SyncFileItem::None : SyncFileItem::Down; return; } @@ -1069,12 +1073,10 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, const auto up = _discoveryData->_statedb->getUploadInfo(path._original); if (up._valid && up._contentChecksum == serverEntry.checksumHeader) { // Solve the conflict into an upload, or update meta data - item->_instruction = up._modtime == localEntry.modtime && up._size == localEntry.size - ? CSYNC_INSTRUCTION_UPDATE_METADATA - : CSYNC_INSTRUCTION_SYNC; + item->setInstruction(up._modtime == localEntry.modtime && up._size == localEntry.size ? CSYNC_INSTRUCTION_UPDATE_METADATA : CSYNC_INSTRUCTION_SYNC); item->_direction = SyncFileItem::Up; - if (item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA) { + if (item->instruction() == CSYNC_INSTRUCTION_UPDATE_METADATA) { // Update the etag and other server metadata in the journal already Q_ASSERT(item->_file == path._original); Q_ASSERT(item->_size == serverEntry.size); @@ -1089,7 +1091,7 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, const } // Rely on content hash comparisons to optimize away non-conflicts inside the job - item->_instruction = CSYNC_INSTRUCTION_CONFLICT; + item->setInstruction(CSYNC_INSTRUCTION_CONFLICT); item->_direction = SyncFileItem::None; } @@ -1101,12 +1103,11 @@ void ProcessDirectoryJob::processFileFinalize( if (isVfsWithSuffix()) { if (item->_type == ItemTypeVirtualFile) { addVirtualFileSuffix(path._target); - if (item->_instruction == CSYNC_INSTRUCTION_RENAME) + if (item->instruction() == CSYNC_INSTRUCTION_RENAME) addVirtualFileSuffix(item->_renameTarget); else addVirtualFileSuffix(item->_file); - } else if (item->_type == ItemTypeVirtualFileDehydration - && item->_instruction == CSYNC_INSTRUCTION_SYNC) { + } else if (item->_type == ItemTypeVirtualFileDehydration && item->instruction() == CSYNC_INSTRUCTION_SYNC) { if (item->_renameTarget.isEmpty()) { item->_renameTarget = item->_file; addVirtualFileSuffix(item->_renameTarget); @@ -1114,20 +1115,20 @@ void ProcessDirectoryJob::processFileFinalize( } } - if (path._original != path._target && (item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA || item->_instruction == CSYNC_INSTRUCTION_NONE)) { - OC_ASSERT(_dirItem && _dirItem->_instruction == CSYNC_INSTRUCTION_RENAME); + if (path._original != path._target && (item->instruction() & (CSYNC_INSTRUCTION_UPDATE_METADATA | CSYNC_INSTRUCTION_NONE))) { + OC_ASSERT(_dirItem && _dirItem->instruction() == CSYNC_INSTRUCTION_RENAME); // This is because otherwise subitems are not updated! (ideally renaming a directory could // update the database for all items! See PropagateDirectory::slotSubJobsFinished) - item->_instruction = CSYNC_INSTRUCTION_RENAME; + item->setInstruction(CSYNC_INSTRUCTION_RENAME); item->_renameTarget = path._target; item->_direction = _dirItem->_direction; } - qCInfo(lcDisco) << "Discovered" << item->_file << item->_instruction << item->_direction << item->_type; + qCInfo(lcDisco) << "Discovered" << item->_file << item->instruction() << item->_direction << item->_type; - if (item->isDirectory() && item->_instruction == CSYNC_INSTRUCTION_SYNC) - item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; - bool removed = item->_instruction == CSYNC_INSTRUCTION_REMOVE; + if (item->isDirectory() && item->instruction() == CSYNC_INSTRUCTION_SYNC) + item->setInstruction(CSYNC_INSTRUCTION_UPDATE_METADATA); + bool removed = item->instruction() == CSYNC_INSTRUCTION_REMOVE; if (checkPermissions(item)) { if (item->_isRestoration && item->isDirectory()) recurse = true; @@ -1146,7 +1147,7 @@ void ProcessDirectoryJob::processFileFinalize( } else { if (removed // For the purpose of rename deletion, restored deleted placeholder is as if it was deleted - || (item->_type == ItemTypeVirtualFile && item->_instruction == CSYNC_INSTRUCTION_NEW)) { + || (item->_type == ItemTypeVirtualFile && item->instruction() == CSYNC_INSTRUCTION_NEW)) { _discoveryData->_deletedItem[path._original] = item; } emit _discoveryData->itemDiscovered(item); @@ -1165,18 +1166,18 @@ void ProcessDirectoryJob::processBlacklisted(const PathTuple &path, const OCC::L item->_inode = localEntry.inode; item->_isSelectiveSync = true; if (dbEntry.isValid() && ((dbEntry._modtime == localEntry.modtime && dbEntry._fileSize == localEntry.size) || (localEntry.isDirectory && dbEntry.isDirectory()))) { - item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->setInstruction(CSYNC_INSTRUCTION_REMOVE); item->_direction = SyncFileItem::Down; } else { - item->_instruction = CSYNC_INSTRUCTION_IGNORE; + item->setInstruction(CSYNC_INSTRUCTION_IGNORE); item->_status = SyncFileItem::FileIgnored; item->_errorString = tr("SelectiveSync: Ignored because its path is deselected"); _childIgnored = true; } - qCInfo(lcDisco) << "Discovered (blacklisted) " << item->_file << item->_instruction << item->_direction << item->isDirectory(); + qCInfo(lcDisco) << "Discovered (blacklisted) " << item->_file << item->instruction() << item->_direction << item->isDirectory(); - if (item->isDirectory() && item->_instruction != CSYNC_INSTRUCTION_IGNORE) { + if (item->isDirectory() && item->instruction() != CSYNC_INSTRUCTION_IGNORE) { auto job = new ProcessDirectoryJob(path, item, NormalQuery, InBlackList, this); connect(job, &ProcessDirectoryJob::finished, this, &ProcessDirectoryJob::subJobFinished); _queuedJobs.push_back(job); @@ -1192,7 +1193,7 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) return true; } - switch (item->_instruction) { + switch (item->instruction()) { case CSYNC_INSTRUCTION_TYPE_CHANGE: case CSYNC_INSTRUCTION_NEW: { const auto perms = !_rootPermissions.isNull() ? _rootPermissions @@ -1202,12 +1203,12 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) return true; } else if (item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddSubDirectories)) { qCWarning(lcDisco) << "checkForPermission: ERROR" << item->_file; - item->_instruction = CSYNC_INSTRUCTION_ERROR; + item->setInstruction(CSYNC_INSTRUCTION_ERROR); item->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder"); return false; } else if (!item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddFile)) { qCWarning(lcDisco) << "checkForPermission: ERROR" << item->_file; - item->_instruction = CSYNC_INSTRUCTION_ERROR; + item->setInstruction(CSYNC_INSTRUCTION_ERROR); item->_errorString = tr("Not allowed because you don't have permission to add files in that folder"); return false; } @@ -1220,7 +1221,7 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) return true; } if (!perms.hasPermission(RemotePermissions::CanWrite)) { - item->_instruction = CSYNC_INSTRUCTION_CONFLICT; + item->setInstruction(CSYNC_INSTRUCTION_CONFLICT); item->_errorString = tr("Not allowed to upload this file because it is read-only on the server, restoring"); item->_direction = SyncFileItem::Down; item->_isRestoration = true; @@ -1241,7 +1242,7 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) } if (forbiddenIt != _discoveryData->_forbiddenDeletes.cend() && fileSlash.startsWith(*forbiddenIt)) { - item->_instruction = CSYNC_INSTRUCTION_NEW; + item->setInstruction(CSYNC_INSTRUCTION_NEW); item->_direction = SyncFileItem::Down; item->_isRestoration = true; item->_errorString = tr("Moved to invalid target, restoring"); @@ -1254,7 +1255,7 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) return true; } if (!perms.hasPermission(RemotePermissions::CanDelete)) { - item->_instruction = CSYNC_INSTRUCTION_NEW; + item->setInstruction(CSYNC_INSTRUCTION_NEW); item->_direction = SyncFileItem::Down; item->_isRestoration = true; item->_errorString = tr("Not allowed to remove, restoring"); @@ -1327,22 +1328,22 @@ int ProcessDirectoryJob::processSubJobs(int nbJobs) if (_queuedJobs.empty() && _runningJobs.empty() && _pendingAsyncJobs == 0) { _pendingAsyncJobs = -1; // We're finished, we don't want to emit finished again if (_dirItem) { - if (_childModified && _dirItem->_instruction == CSYNC_INSTRUCTION_REMOVE) { + if (_childModified && _dirItem->instruction() == CSYNC_INSTRUCTION_REMOVE) { // re-create directory that has modified contents - _dirItem->_instruction = CSYNC_INSTRUCTION_NEW; + _dirItem->setInstruction(CSYNC_INSTRUCTION_NEW); _dirItem->_direction = _dirItem->_direction == SyncFileItem::Up ? SyncFileItem::Down : SyncFileItem::Up; } - if (_childModified && _dirItem->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE && !_dirItem->isDirectory()) { + if (_childModified && _dirItem->instruction() == CSYNC_INSTRUCTION_TYPE_CHANGE && !_dirItem->isDirectory()) { // Replacing a directory by a file is a conflict, if the directory had modified children - _dirItem->_instruction = CSYNC_INSTRUCTION_CONFLICT; + _dirItem->setInstruction(CSYNC_INSTRUCTION_CONFLICT); if (_dirItem->_direction == SyncFileItem::Up) { _dirItem->_type = ItemTypeDirectory; _dirItem->_direction = SyncFileItem::Down; } } - if (_childIgnored && _dirItem->_instruction == CSYNC_INSTRUCTION_REMOVE) { + if (_childIgnored && _dirItem->instruction() == CSYNC_INSTRUCTION_REMOVE) { // Do not remove a directory that has ignored files - _dirItem->_instruction = CSYNC_INSTRUCTION_NONE; + _dirItem->setInstruction(CSYNC_INSTRUCTION_NONE); } } emit finished(); @@ -1428,7 +1429,7 @@ DiscoverySingleDirectoryJob *ProcessDirectoryJob::startAsyncServerQuery() // is returned too. Thus we can't distinguish the two and will treat any // 503 as request to ignore the folder. See #3113 #2884. // Similarly, the server might also return 404 or 50x in case of bugs. #7199 #7586 - _dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE; + _dirItem->setInstruction(CSYNC_INSTRUCTION_IGNORE); _dirItem->_errorString = results.error().message; emit this->finished(); return; @@ -1473,7 +1474,7 @@ void ProcessDirectoryJob::startAsyncLocalQuery() _pendingAsyncJobs--; if (_dirItem) { - _dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE; + _dirItem->setInstruction(CSYNC_INSTRUCTION_IGNORE); _dirItem->_errorString = msg; emit this->finished(); } else { diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index d262dca35dc..bfe9ef4593d 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -157,7 +157,7 @@ QPair DiscoveryPhase::findAndCancelDeletedJob(const QString &orig auto it = _deletedItem.constFind(originalPath); if (it != _deletedItem.cend()) { const auto &item = *it; - const SyncInstructions instruction = item->_instruction; + const SyncInstructions instruction = item->instruction(); if (instruction == CSYNC_INSTRUCTION_IGNORE && item->_type == ItemTypeVirtualFile) { // re-creation of virtual files count as a delete // restoration after a prohibited move @@ -187,7 +187,7 @@ QPair DiscoveryPhase::findAndCancelDeletedJob(const QString &orig qCWarning(lcDiscovery) << "item->_remotePerm" << item->_remotePerm; OC_ENFORCE(false); } - item->_instruction = CSYNC_INSTRUCTION_NONE; + item->setInstruction(CSYNC_INSTRUCTION_NONE); result = true; oldEtag = item->_etag; } diff --git a/src/libsync/localdiscoverytracker.cpp b/src/libsync/localdiscoverytracker.cpp index 466c0f8fd6e..387d1599b09 100644 --- a/src/libsync/localdiscoverytracker.cpp +++ b/src/libsync/localdiscoverytracker.cpp @@ -68,7 +68,7 @@ void LocalDiscoveryTracker::slotItemCompleted(const SyncFileItemPtr &item) switch (item->_status) { case SyncFileItem::NoStatus: // we can't use the flags operator with CSYNC_INSTRUCTION_NONE - if (item->_instruction != CSYNC_INSTRUCTION_NONE && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA) { + if (item->instruction() & ~(CSYNC_INSTRUCTION_NONE | CSYNC_INSTRUCTION_UPDATE_METADATA)) { break; } Q_FALLTHROUGH(); diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 1b5fe0c4fa5..c3ae44b7fe9 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -322,13 +322,14 @@ void PropagateItemJob::done(SyncFileItem::Status statusArg, const QString &error PropagateItemJob *OwncloudPropagator::createJob(const SyncFileItemPtr &item) { qCDebug(lcPropagator) << "Propagating:" << item; - const bool deleteExisting = item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE; - switch (item->_instruction) { + const bool deleteExisting = item->instruction() == CSYNC_INSTRUCTION_TYPE_CHANGE; + switch (item->instruction()) { case CSYNC_INSTRUCTION_REMOVE: - if (item->_direction == SyncFileItem::Down) + if (item->_direction == SyncFileItem::Down) { return new PropagateLocalRemove(this, item); - else + } else { return new PropagateRemoteDelete(this, item); + } case CSYNC_INSTRUCTION_NEW: case CSYNC_INSTRUCTION_TYPE_CHANGE: case CSYNC_INSTRUCTION_CONFLICT: @@ -445,7 +446,7 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) // First check if this is an item in a directory which is going to be removed. if (currentRemoveDirectoryJob && FileSystem::isChildPathOf(item->_file, currentRemoveDirectoryJob->path())) { // Check the sync instruction for the item: - if (item->_instruction == CSYNC_INSTRUCTION_REMOVE) { + if (item->instruction() == CSYNC_INSTRUCTION_REMOVE) { // already taken care of. (by the removal of the parent directory) if (auto delDirJob = qobject_cast(currentRemoveDirectoryJob)) { @@ -453,7 +454,7 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) delDirJob->increaseAffectedCount(); } continue; - } else if (item->isDirectory() && (item->_instruction & (CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_TYPE_CHANGE))) { + } else if (item->isDirectory() && (item->instruction() & (CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_TYPE_CHANGE))) { // Create a new directory within a deleted directory? That can happen if the directory // etag was not fetched properly on the previous sync because the sync was aborted // while uploading this directory (which is now removed). We can ignore it. @@ -462,11 +463,11 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) delDirJob->increaseAffectedCount(); } continue; - } else if (item->_instruction & (CSYNC_INSTRUCTION_IGNORE | CSYNC_INSTRUCTION_NONE)) { + } else if (item->instruction() & (CSYNC_INSTRUCTION_IGNORE | CSYNC_INSTRUCTION_NONE)) { continue; - } else if (item->_instruction != CSYNC_INSTRUCTION_RENAME) { + } else if (item->instruction() != CSYNC_INSTRUCTION_RENAME) { // all is good, the rename will be executed before the directory deletion - qCWarning(lcPropagator) << "WARNING: Job within a removed directory? This should not happen!" << item->_file << item->_instruction; + qCWarning(lcPropagator) << "WARNING: Job within a removed directory? This should not happen!" << item->_file << item->instruction(); Q_ASSERT(false); // we shouldn't land here, but assert for debug purposes } } @@ -477,9 +478,8 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) if (!maybeConflictDirectory.isEmpty()) { if (FileSystem::isChildPathOf(item->destination(), maybeConflictDirectory)) { // We're processing an item in a CONFLICT directory. - qCInfo(lcPropagator) << "Skipping job inside CONFLICT directory" - << item->_file << item->_instruction; - item->_instruction = CSYNC_INSTRUCTION_NONE; + qCInfo(lcPropagator) << "Skipping job inside CONFLICT directory" << item->_file << item->instruction(); + item->setInstruction(CSYNC_INSTRUCTION_NONE); continue; } else { // This is not an item inside the conflict directory, which means we're done @@ -502,8 +502,7 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) if (item->isDirectory()) { PropagateDirectory *dir = new PropagateDirectory(this, item); - if (item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE - && item->_direction == SyncFileItem::Up) { + if (item->instruction() == CSYNC_INSTRUCTION_TYPE_CHANGE && item->_direction == SyncFileItem::Up) { // Skip all potential uploads to the new folder. // Processing them now leads to problems with permissions: // checkForPermissions() has already run and used the permissions @@ -511,13 +510,13 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) // to the new dir is ok... for (const auto &item2 : qAsConst(items)) { if (item2 != item && FileSystem::isChildPathOf(item2->destination(), item->destination())) { - item2->_instruction = CSYNC_INSTRUCTION_NONE; + item2->setInstruction(CSYNC_INSTRUCTION_NONE); _anotherSyncNeeded = true; } } } - if (item->_instruction == CSYNC_INSTRUCTION_REMOVE) { + if (item->instruction() == CSYNC_INSTRUCTION_REMOVE) { // We do the removal of directories at the end, because there might be moves from // these directories that will happen later. currentRemoveDirectoryJob = dir; @@ -527,9 +526,9 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) // since it would be done before the actual remove (issue #1845) // NOTE: Currently this means that we don't update those etag at all in this sync, // but it should not be a problem, they will be updated in the next sync. - for (auto &directory : directories) { - if (directory.second->item()->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA) { - directory.second->item()->_instruction = CSYNC_INSTRUCTION_NONE; + for (auto &dir : directories) { + if (dir.second->item()->instruction() == CSYNC_INSTRUCTION_UPDATE_METADATA) { + dir.second->item()->setInstruction(CSYNC_INSTRUCTION_NONE); _anotherSyncNeeded = true; } } @@ -540,7 +539,7 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) directories.push(qMakePair(item->destination(), dir)); } else { // The item is not a directory, but a file. - if (item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE) { + if (item->instruction() == CSYNC_INSTRUCTION_TYPE_CHANGE) { // will delete directories, so defer execution currentRemoveDirectoryJob = createJob(item); _rootJob->addDeleteJob(currentRemoveDirectoryJob); @@ -548,7 +547,7 @@ void OwncloudPropagator::start(SyncFileItemSet &&items) directories.top().second->appendTask(item); } - if (item->_instruction == CSYNC_INSTRUCTION_CONFLICT) { + if (item->instruction() == CSYNC_INSTRUCTION_CONFLICT) { // This might be a file or a directory on the local side. If it's a // directory we want to skip processing items inside it. maybeConflictDirectory = item->_file; @@ -799,7 +798,7 @@ bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item, conflictItem->_file = conflictFileName; conflictItem->_type = ItemTypeFile; conflictItem->_direction = SyncFileItem::Up; - conflictItem->_instruction = CSYNC_INSTRUCTION_NEW; + conflictItem->setInstruction(CSYNC_INSTRUCTION_NEW); conflictItem->_modtime = conflictModTime; conflictItem->_size = item->_previousSize; emit newItem(conflictItem); @@ -932,7 +931,7 @@ bool PropagatorCompositeJob::scheduleSelfOrChild() _tasksToDo.erase(_tasksToDo.begin()); PropagatorJob *job = propagator()->createJob(nextTask); if (!job) { - qCWarning(lcDirectory) << "Useless task found for file" << nextTask->destination() << "instruction" << nextTask->_instruction; + qCWarning(lcDirectory) << "Useless task found for file" << nextTask->destination() << "instruction" << nextTask->instruction(); continue; } appendJob(job); @@ -1103,13 +1102,12 @@ void PropagateDirectory::slotSubJobsFinished(const SyncFileItem::Status status) if (status == SyncFileItem::Success) { // If a directory is renamed, recursively delete any stale items // that may still exist below the old path. - if (_item->_instruction == CSYNC_INSTRUCTION_RENAME - && _item->_originalFile != _item->_renameTarget) { + if (_item->instruction() == CSYNC_INSTRUCTION_RENAME && _item->_originalFile != _item->_renameTarget) { // TODO: check result but it breaks TestDatabaseError propagator()->_journal->deleteFileRecord(_item->_originalFile, true); } - if (_item->_instruction == CSYNC_INSTRUCTION_NEW && _item->_direction == SyncFileItem::Down) { + if (_item->instruction() == CSYNC_INSTRUCTION_NEW && _item->_direction == SyncFileItem::Down) { // special case for local MKDIR, set local directory mtime // (it's not synced later at all, but can be nice to have it set initially) OC_ASSERT(FileSystem::setModTime(propagator()->fullLocalPath(_item->destination()), _item->_modtime)); @@ -1118,7 +1116,7 @@ void PropagateDirectory::slotSubJobsFinished(const SyncFileItem::Status status) // the directory has been propagated. Otherwise the directory // could appear locally without being added to the database. // Additionally we need to convert those folders to placeholders with cfapi vfs. - if (_item->_instruction & (CSYNC_INSTRUCTION_RENAME | CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_UPDATE_METADATA)) { + if (_item->instruction() & (CSYNC_INSTRUCTION_RENAME | CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_UPDATE_METADATA)) { // metatdata changes are relevant _item->_relevantDirectoyInstruction = true; _item->_status = SyncFileItem::Success; diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 613a7696b95..d5b2c0b6542 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -365,11 +365,11 @@ class PropagateIgnoreJob : public PropagateItemJob { SyncFileItem::Status status = _item->_status; if (status == SyncFileItem::NoStatus) { - if (_item->_instruction == CSYNC_INSTRUCTION_ERROR) { + if (_item->instruction() == CSYNC_INSTRUCTION_ERROR) { status = SyncFileItem::NormalError; } else { status = SyncFileItem::FileIgnored; - OC_ASSERT(_item->_instruction == CSYNC_INSTRUCTION_IGNORE); + OC_ASSERT(_item->instruction() == CSYNC_INSTRUCTION_IGNORE); } } done(status, _item->_errorString); diff --git a/src/libsync/progressdispatcher.cpp b/src/libsync/progressdispatcher.cpp index ab7e863e390..cd641df702a 100644 --- a/src/libsync/progressdispatcher.cpp +++ b/src/libsync/progressdispatcher.cpp @@ -24,7 +24,7 @@ ProgressDispatcher *ProgressDispatcher::_instance = nullptr; QString Progress::asResultString(const SyncFileItem &item) { - switch (item._instruction) { + switch (item.instruction()) { case CSYNC_INSTRUCTION_SYNC: case CSYNC_INSTRUCTION_NEW: case CSYNC_INSTRUCTION_TYPE_CHANGE: @@ -59,7 +59,7 @@ QString Progress::asResultString(const SyncFileItem &item) QString Progress::asActionString(const SyncFileItem &item) { - switch (item._instruction) { + switch (item.instruction()) { case CSYNC_INSTRUCTION_CONFLICT: case CSYNC_INSTRUCTION_SYNC: case CSYNC_INSTRUCTION_NEW: @@ -157,13 +157,10 @@ bool ProgressInfo::isUpdatingEstimates() const static bool shouldCountProgress(const SyncFileItem &item) { - const auto instruction = item._instruction; + const auto instruction = item.instruction(); // Skip any ignored, error or non-propagated files and directories. - if (instruction == CSYNC_INSTRUCTION_NONE - || instruction == CSYNC_INSTRUCTION_UPDATE_METADATA - || instruction == CSYNC_INSTRUCTION_IGNORE - || instruction == CSYNC_INSTRUCTION_ERROR) { + if (instruction & (CSYNC_INSTRUCTION_NONE | CSYNC_INSTRUCTION_UPDATE_METADATA | CSYNC_INSTRUCTION_IGNORE | CSYNC_INSTRUCTION_ERROR)) { return false; } diff --git a/src/libsync/progressdispatcher.h b/src/libsync/progressdispatcher.h index 9bdcd4a9c2c..96de8afd1da 100644 --- a/src/libsync/progressdispatcher.h +++ b/src/libsync/progressdispatcher.h @@ -118,12 +118,8 @@ class OWNCLOUDSYNC_EXPORT ProgressInfo : public QObject static inline bool isSizeDependent(const SyncFileItem &item) { return !item.isDirectory() - && (item._instruction == CSYNC_INSTRUCTION_CONFLICT - || item._instruction == CSYNC_INSTRUCTION_SYNC - || item._instruction == CSYNC_INSTRUCTION_NEW - || item._instruction == CSYNC_INSTRUCTION_TYPE_CHANGE) - && !(item._type == ItemTypeVirtualFile - || item._type == ItemTypeVirtualFileDehydration); + && (item.instruction() & (CSYNC_INSTRUCTION_CONFLICT | CSYNC_INSTRUCTION_SYNC | CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_TYPE_CHANGE)) + && !(item._type == ItemTypeVirtualFile || item._type == ItemTypeVirtualFileDehydration); } /** diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 848eccc9d64..59573356f01 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -483,11 +483,8 @@ void PropagateDownloadFile::start() return true; }; - if (_item->_instruction == CSYNC_INSTRUCTION_CONFLICT - && _item->_size == _item->_previousSize - && !_item->_checksumHeader.isEmpty() - && (csync_is_collision_safe_hash(_item->_checksumHeader) - || _item->_modtime == _item->_previousModtime)) { + if (_item->instruction() == CSYNC_INSTRUCTION_CONFLICT && _item->_size == _item->_previousSize && !_item->_checksumHeader.isEmpty() + && (csync_is_collision_safe_hash(_item->_checksumHeader) || _item->_modtime == _item->_previousModtime)) { qCDebug(lcPropagateDownload) << _item->_file << "may not need download, computing checksum"; auto computeChecksum = new ComputeChecksum(this); const auto checksumHeader = ChecksumHeader::parseChecksumHeader(_item->_checksumHeader); @@ -927,8 +924,7 @@ void PropagateDownloadFile::downloadFinished() } } - bool isConflict = _item->_instruction == CSYNC_INSTRUCTION_CONFLICT - && (QFileInfo(fn).isDir() || !FileSystem::fileEquals(fn, _tmpFile.fileName())); + bool isConflict = _item->instruction() == CSYNC_INSTRUCTION_CONFLICT && (QFileInfo(fn).isDir() || !FileSystem::fileEquals(fn, _tmpFile.fileName())); if (isConflict) { QString error; if (!propagator()->createConflict(_item, _associatedComposite, &error)) { diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 1386b74ea30..e3878bb7b6a 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -539,8 +539,7 @@ QMap PropagateUploadFileCommon::headers() } if (!_item->_etag.isEmpty() && _item->_etag != QLatin1String("empty_etag") - && _item->_instruction != CSYNC_INSTRUCTION_NEW // On new files never send a If-Match - && _item->_instruction != CSYNC_INSTRUCTION_TYPE_CHANGE + && (_item->instruction() & ~(CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_TYPE_CHANGE)) // On new files never send a If-Match && !_deleteExisting) { // We add quotes because the owncloud server always adds quotes around the etag, and // csync_owncloud.c's owncloud_file_id always strips the quotes. @@ -597,8 +596,7 @@ void PropagateUploadFileCommon::finalize() // Files that were new on the remote shouldn't have online-only pin state // even if their parent folder is online-only. - if (_item->_instruction == CSYNC_INSTRUCTION_NEW - || _item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE) { + if (_item->instruction() & (CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_TYPE_CHANGE)) { auto &vfs = propagator()->syncOptions()._vfs; const auto pin = vfs->pinState(_item->_file); if (pin && *pin == PinState::OnlineOnly) { diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 29916c4b1d0..72f0a3603db 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -163,7 +163,7 @@ void PropagateLocalMkdir::start() .arg(newDirStr, removeError)); return; } - } else if (_item->_instruction == CSYNC_INSTRUCTION_CONFLICT) { + } else if (_item->instruction() == CSYNC_INSTRUCTION_CONFLICT) { QString error; if (!propagator()->createConflict(_item, _associatedComposite, &error)) { done(SyncFileItem::SoftError, error); @@ -200,9 +200,7 @@ void PropagateLocalMkdir::start() } propagator()->_journal->commit(QStringLiteral("localMkdir")); - auto resultStatus = _item->_instruction == CSYNC_INSTRUCTION_CONFLICT - ? SyncFileItem::Conflict - : SyncFileItem::Success; + auto resultStatus = _item->instruction() == CSYNC_INSTRUCTION_CONFLICT ? SyncFileItem::Conflict : SyncFileItem::Success; done(resultStatus); } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index ae10bebef02..814ba21e095 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -148,7 +148,7 @@ bool SyncEngine::checkErrorBlacklisting(SyncFileItem &item) // for reporting and for making sure we don't update the blacklist // entry yet. // Classification is this _instruction and _status - item._instruction = CSYNC_INSTRUCTION_IGNORE; + item.setInstruction(CSYNC_INSTRUCTION_IGNORE); if (entry._errorCategory == SyncJournalErrorBlacklistRecord::Category::LocalSoftError) { item._status = SyncFileItem::SoftError; item._errorString = entry._errorString; @@ -178,9 +178,7 @@ void SyncEngine::deleteStaleDownloadInfos(const SyncFileItemSet &syncItems) // Find all downloadinfo paths that we want to preserve. QSet download_file_paths; for (const auto &it : syncItems) { - if (it->_direction == SyncFileItem::Down - && it->_type == ItemTypeFile - && isFileTransferInstruction(it->_instruction)) { + if (it->_direction == SyncFileItem::Down && it->_type == ItemTypeFile && isFileTransferInstruction(it->instruction())) { download_file_paths.insert(it->_file); } } @@ -200,9 +198,7 @@ void SyncEngine::deleteStaleUploadInfos(const SyncFileItemSet &syncItems) // Find all blacklisted paths that we want to preserve. QSet upload_file_paths; for (const auto &it : syncItems) { - if (it->_direction == SyncFileItem::Up - && it->_type == ItemTypeFile - && isFileTransferInstruction(it->_instruction)) { + if (it->_direction == SyncFileItem::Up && it->_type == ItemTypeFile && isFileTransferInstruction(it->instruction())) { upload_file_paths.insert(it->_file); } } @@ -277,25 +273,24 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) { if (Utility::isConflictFile(item->_file)) _seenConflictFiles.insert(item->_file); - if (item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA && !item->isDirectory()) { + if (item->instruction() == CSYNC_INSTRUCTION_UPDATE_METADATA && !item->isDirectory()) { _hasNoneFiles = true; - } else if (item->_instruction == CSYNC_INSTRUCTION_NONE) { + } else if (item->instruction() == CSYNC_INSTRUCTION_NONE) { _hasNoneFiles = true; if (_account->capabilities().uploadConflictFiles() && Utility::isConflictFile(item->_file)) { // For uploaded conflict files, files with no action performed on them should // be displayed: but we mustn't overwrite the instruction if something happens // to the file! item->_errorString = tr("Unresolved conflict."); - item->_instruction = CSYNC_INSTRUCTION_IGNORE; + item->setInstruction(CSYNC_INSTRUCTION_IGNORE); item->_status = SyncFileItem::Conflict; } return; - } else if (item->_instruction == CSYNC_INSTRUCTION_REMOVE && !item->_isSelectiveSync) { + } else if (item->instruction() == CSYNC_INSTRUCTION_REMOVE && !item->_isSelectiveSync) { _hasRemoveFile = true; - } else if (item->_instruction == CSYNC_INSTRUCTION_RENAME) { + } else if (item->instruction() == CSYNC_INSTRUCTION_RENAME) { _hasNoneFiles = true; // If a file (or every file) has been renamed, it means not al files where deleted - } else if (item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE - || item->_instruction == CSYNC_INSTRUCTION_SYNC) { + } else if (item->instruction() & (CSYNC_INSTRUCTION_TYPE_CHANGE & CSYNC_INSTRUCTION_SYNC)) { if (item->_direction == SyncFileItem::Up) { // An upload of an existing file means that the file was left unchanged on the server // This counts as a NONE for detecting if all the files on the server were changed @@ -312,7 +307,8 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) const auto it = _syncItems.find(item); if (it != _syncItems.cend()) { const auto &item2 = it->get(); - qCWarning(lcEngine) << "We already have an item for " << item2->_file << ":" << item2->_instruction << item2->_direction << "|" << item->_instruction << item->_direction; + qCWarning(lcEngine) << "We already have an item for " << item2->_file << ":" << item2->instruction() << item2->_direction << "|" + << item->instruction() << item->_direction; return false; } return true; @@ -645,7 +641,7 @@ void SyncEngine::slotDiscoveryFinished() if (_promptRemoveAllFiles) { int side = 0; // > 0 means more deleted on the server. < 0 means more deleted on the client for (const auto &it : qAsConst(_syncItems)) { - if (it->_instruction == CSYNC_INSTRUCTION_REMOVE) { + if (it->instruction() == CSYNC_INSTRUCTION_REMOVE) { side += it->_direction == SyncFileItem::Down ? 1 : -1; } } @@ -765,14 +761,14 @@ void SyncEngine::restoreOldFiles(SyncFileItemSet &syncItems) if ((*it)->_direction != SyncFileItem::Down) continue; - switch ((*it)->_instruction) { + switch ((*it)->instruction()) { case CSYNC_INSTRUCTION_SYNC: qCWarning(lcEngine) << "restoreOldFiles: RESTORING" << (*it)->_file; - (*it)->_instruction = CSYNC_INSTRUCTION_CONFLICT; + (*it)->setInstruction(CSYNC_INSTRUCTION_CONFLICT); break; case CSYNC_INSTRUCTION_REMOVE: qCWarning(lcEngine) << "restoreOldFiles: RESTORING" << (*it)->_file; - (*it)->_instruction = CSYNC_INSTRUCTION_NEW; + (*it)->setInstruction(CSYNC_INSTRUCTION_NEW); (*it)->_direction = SyncFileItem::Up; break; case CSYNC_INSTRUCTION_RENAME: diff --git a/src/libsync/syncfileitem.cpp b/src/libsync/syncfileitem.cpp index b7fa9475e38..89349eedd81 100644 --- a/src/libsync/syncfileitem.cpp +++ b/src/libsync/syncfileitem.cpp @@ -21,6 +21,8 @@ #include +#include + namespace OCC { Q_LOGGING_CATEGORY(lcFileItem, "sync.fileitem", QtInfoMsg) @@ -75,6 +77,19 @@ SyncFileItemPtr SyncFileItem::fromSyncJournalFileRecord(const SyncJournalFileRec return item; } +SyncInstruction SyncFileItem::instruction() const +{ + return _instruction; +} + +void SyncFileItem::setInstruction(SyncInstruction instruction) +{ + // only one instruction is allowed + // with c++23 this can be made a static_assert + Q_ASSERT(std::bitset(instruction).count() == 1); + _instruction = instruction; +} + template <> QString Utility::enumToDisplayName(SyncFileItem::Status s) { @@ -123,7 +138,7 @@ QDebug operator<<(QDebug debug, const OCC::SyncFileItem *item) if (!item->_renameTarget.isEmpty()) { debug << ", destination=" << item->destination(); } - debug << ", type=" << item->_type << ", instruction=" << item->_instruction << ", status=" << item->_status << ")"; + debug << ", type=" << item->_type << ", instruction=" << item->instruction() << ", status=" << item->_status << ")"; } return debug; } diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index e71cc9f75a2..e9c3ec85ebc 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -151,7 +151,6 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem , _isSelectiveSync(false) , _httpErrorCode(0) , _affectedItems(1) - , _instruction(CSYNC_INSTRUCTION_NONE) , _modtime(0) , _size(0) , _inode(0) @@ -290,7 +289,6 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem // usually this value is 1, but for removes on dirs, it might be much higher. // Variables used by the propagator - SyncInstructions _instruction; time_t _modtime; QString _etag; qint64 _size; @@ -324,6 +322,13 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem out._size = _size; return out; } + + + SyncInstruction instruction() const; + void setInstruction(SyncInstruction instruction); + +private: + SyncInstruction _instruction = CSYNC_INSTRUCTION_NONE; }; diff --git a/src/libsync/syncfilestatustracker.cpp b/src/libsync/syncfilestatustracker.cpp index 32f773fcc0c..fcff2c1298b 100644 --- a/src/libsync/syncfilestatustracker.cpp +++ b/src/libsync/syncfilestatustracker.cpp @@ -71,20 +71,14 @@ SyncFileStatus::SyncFileStatusTag SyncFileStatusTracker::lookupProblem(const QSt static inline bool hasErrorStatus(const SyncFileItem &item) { const auto status = item._status; - return item._instruction == CSYNC_INSTRUCTION_ERROR - || status == SyncFileItem::NormalError - || status == SyncFileItem::FatalError - || status == SyncFileItem::DetailError - || status == SyncFileItem::BlacklistedError - || item._hasBlacklistEntry; + return item.instruction() == CSYNC_INSTRUCTION_ERROR || status == SyncFileItem::NormalError || status == SyncFileItem::FatalError + || status == SyncFileItem::DetailError || status == SyncFileItem::BlacklistedError || item._hasBlacklistEntry; } static inline bool hasExcludedStatus(const SyncFileItem &item) { const auto status = item._status; - return item._instruction == CSYNC_INSTRUCTION_IGNORE - || status == SyncFileItem::FileIgnored - || status == SyncFileItem::Conflict + return item.instruction() == CSYNC_INSTRUCTION_IGNORE || status == SyncFileItem::FileIgnored || status == SyncFileItem::Conflict || status == SyncFileItem::Restoration; } @@ -209,7 +203,7 @@ void SyncFileStatusTracker::slotAboutToPropagate(const SyncFileItemSet &items) std::swap(_syncProblems, oldProblems); for (const auto &item : qAsConst(items)) { - qCDebug(lcStatusTracker) << "Investigating" << item->destination() << item->_status << item->_instruction; + qCDebug(lcStatusTracker) << "Investigating" << item->destination() << item->_status << item->instruction(); _dirtyPaths.remove(item->destination()); _dirtyPaths.remove(item->_originalFile); @@ -221,10 +215,7 @@ void SyncFileStatusTracker::slotAboutToPropagate(const SyncFileItemSet &items) } SharedFlag sharedFlag = item->_remotePerm.hasPermission(RemotePermissions::IsShared) ? Shared : NotShared; - if (item->_instruction != CSYNC_INSTRUCTION_NONE - && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA - && item->_instruction != CSYNC_INSTRUCTION_IGNORE - && item->_instruction != CSYNC_INSTRUCTION_ERROR) { + if (item->instruction() & ~(CSYNC_INSTRUCTION_NONE | CSYNC_INSTRUCTION_UPDATE_METADATA | CSYNC_INSTRUCTION_IGNORE | CSYNC_INSTRUCTION_ERROR)) { // Mark this path as syncing for instructions that will result in propagation. incSyncCountAndEmitStatusChanged(item->destination(), sharedFlag); } else { @@ -255,7 +246,7 @@ void SyncFileStatusTracker::slotAboutToPropagate(const SyncFileItemSet &items) void SyncFileStatusTracker::slotItemCompleted(const SyncFileItemPtr &item) { - qCDebug(lcStatusTracker) << "Item completed" << item->destination() << item->_status << item->_instruction; + qCDebug(lcStatusTracker) << "Item completed" << item->destination() << item->_status << item->instruction(); if (hasErrorStatus(*item)) { _syncProblems[item->destination()] = SyncFileStatus::StatusError; @@ -267,10 +258,7 @@ void SyncFileStatusTracker::slotItemCompleted(const SyncFileItemPtr &item) } SharedFlag sharedFlag = item->_remotePerm.hasPermission(RemotePermissions::IsShared) ? Shared : NotShared; - if (item->_instruction != CSYNC_INSTRUCTION_NONE - && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA - && item->_instruction != CSYNC_INSTRUCTION_IGNORE - && item->_instruction != CSYNC_INSTRUCTION_ERROR) { + if (item->instruction() & ~(CSYNC_INSTRUCTION_NONE | CSYNC_INSTRUCTION_UPDATE_METADATA | CSYNC_INSTRUCTION_IGNORE | CSYNC_INSTRUCTION_ERROR)) { // decSyncCount calls *must* be symetric with incSyncCount calls in slotAboutToPropagate decSyncCountAndEmitStatusChanged(item->destination(), sharedFlag); } else { diff --git a/src/libsync/syncresult.cpp b/src/libsync/syncresult.cpp index a610308c9b9..f33be635a75 100644 --- a/src/libsync/syncresult.cpp +++ b/src/libsync/syncresult.cpp @@ -104,10 +104,8 @@ void SyncResult::processCompletedItem(const SyncFileItemPtr &item) _foundFilesNotSynced = true; } - if (item->isDirectory() && (item->_instruction == CSYNC_INSTRUCTION_NEW - || item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE - || item->_instruction == CSYNC_INSTRUCTION_REMOVE - || item->_instruction == CSYNC_INSTRUCTION_RENAME)) { + if (item->isDirectory() + && (item->instruction() & (CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_TYPE_CHANGE | CSYNC_INSTRUCTION_REMOVE | CSYNC_INSTRUCTION_RENAME))) { _folderStructureWasChanged = true; } @@ -120,7 +118,7 @@ void SyncResult::processCompletedItem(const SyncFileItemPtr &item) _firstItemError = item; } } else if (item->_status == SyncFileItem::Conflict) { - if (item->_instruction == CSYNC_INSTRUCTION_CONFLICT) { + if (item->instruction() == CSYNC_INSTRUCTION_CONFLICT) { _numNewConflictItems++; if (!_firstNewConflictItem) { _firstNewConflictItem = item; @@ -130,7 +128,7 @@ void SyncResult::processCompletedItem(const SyncFileItemPtr &item) } } else { if (!item->hasErrorStatus() && item->_status != SyncFileItem::FileIgnored && item->_direction == SyncFileItem::Down) { - switch (item->_instruction) { + switch (item->instruction()) { case CSYNC_INSTRUCTION_NEW: case CSYNC_INSTRUCTION_TYPE_CHANGE: _numNewItems++; @@ -157,7 +155,7 @@ void SyncResult::processCompletedItem(const SyncFileItemPtr &item) // nothing. break; } - } else if (item->_instruction == CSYNC_INSTRUCTION_IGNORE && item->_hasBlacklistEntry) { + } else if (item->instruction() == CSYNC_INSTRUCTION_IGNORE && item->_hasBlacklistEntry) { if (item->_hasBlacklistEntry) { _numBlacklistErrors++; } diff --git a/test/testblacklist.cpp b/test/testblacklist.cpp index 15d5c70253c..975272cc670 100644 --- a/test/testblacklist.cpp +++ b/test/testblacklist.cpp @@ -96,7 +96,7 @@ private slots: auto it = completeSpy.findItem(testFileName); QVERIFY(it); QCOMPARE(it->_status, SyncFileItem::NormalError); // initial error visible - QCOMPARE(it->_instruction, CSYNC_INSTRUCTION_NEW); + QCOMPARE(it->instruction(), CSYNC_INSTRUCTION_NEW); auto entry = fakeFolder.syncJournal().errorBlacklistEntry(testFileName); QVERIFY(entry.isValid()); @@ -118,7 +118,7 @@ private slots: auto it = completeSpy.findItem(testFileName); QVERIFY(it); QCOMPARE(it->_status, SyncFileItem::BlacklistedError); - QCOMPARE(it->_instruction, CSYNC_INSTRUCTION_IGNORE); // no retry happened! + QCOMPARE(it->instruction(), CSYNC_INSTRUCTION_IGNORE); // no retry happened! auto entry = fakeFolder.syncJournal().errorBlacklistEntry(testFileName); QVERIFY(entry.isValid()); @@ -145,7 +145,7 @@ private slots: auto it = completeSpy.findItem(testFileName); QVERIFY(it); QCOMPARE(it->_status, SyncFileItem::BlacklistedError); // blacklisted as it's just a retry - QCOMPARE(it->_instruction, CSYNC_INSTRUCTION_NEW); // retry! + QCOMPARE(it->instruction(), CSYNC_INSTRUCTION_NEW); // retry! auto entry = fakeFolder.syncJournal().errorBlacklistEntry(testFileName); QVERIFY(entry.isValid()); @@ -171,7 +171,7 @@ private slots: auto it = completeSpy.findItem(testFileName); QVERIFY(it); QCOMPARE(it->_status, SyncFileItem::BlacklistedError); - QCOMPARE(it->_instruction, CSYNC_INSTRUCTION_NEW); // retry! + QCOMPARE(it->instruction(), CSYNC_INSTRUCTION_NEW); // retry! auto entry = fakeFolder.syncJournal().errorBlacklistEntry(testFileName); QVERIFY(entry.isValid()); @@ -199,7 +199,7 @@ private slots: auto it = completeSpy.findItem(testFileName); QVERIFY(it); QCOMPARE(it->_status, SyncFileItem::Success); - QCOMPARE(it->_instruction, CSYNC_INSTRUCTION_NEW); + QCOMPARE(it->instruction(), CSYNC_INSTRUCTION_NEW); auto entry = fakeFolder.syncJournal().errorBlacklistEntry(testFileName); QVERIFY(!entry.isValid()); diff --git a/test/testchunkingng.cpp b/test/testchunkingng.cpp index aa4fe8dc69c..82bdf7e3a62 100644 --- a/test/testchunkingng.cpp +++ b/test/testchunkingng.cpp @@ -268,10 +268,10 @@ private slots: QCOMPARE(items.size(), 2); auto it = items.cbegin(); QCOMPARE(it->get()->_file, QLatin1String("A")); - QCOMPARE(it->get()->_instruction, CSYNC_INSTRUCTION_UPDATE_METADATA); + QCOMPARE(it->get()->instruction(), CSYNC_INSTRUCTION_UPDATE_METADATA); it++; QCOMPARE(it->get()->_file, QLatin1String("A/a0")); - QCOMPARE(it->get()->_instruction, CSYNC_INSTRUCTION_UPDATE_METADATA); + QCOMPARE(it->get()->instruction(), CSYNC_INSTRUCTION_UPDATE_METADATA); QCOMPARE(it->get()->_etag, QString::fromUtf8(fakeFolder.remoteModifier().find(QStringLiteral("A/a0"))->etag)); }; auto checkEtagUpdated = [&](const SyncFileItemPtr &item) { diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index edb16c47ff9..5acb04bae80 100644 --- a/test/testpermissions.cpp +++ b/test/testpermissions.cpp @@ -55,13 +55,13 @@ bool itemInstruction(const ItemCompletedSpy &spy, const QString &path, const Syn { auto item = spy.findItem(path); Q_ASSERT(!item.isNull()); - return item->_instruction == instr; + return item->instruction() == instr; } bool discoveryInstruction(const SyncFileItemSet &spy, const QString &path, const SyncInstructions instr) { auto item = findDiscoveryItem(spy, path); - return item->_instruction == instr; + return item->instruction() == instr; } class TestPermissions : public QObject diff --git a/test/testremotediscovery.cpp b/test/testremotediscovery.cpp index 893bf6dbe8c..b07dd77722c 100644 --- a/test/testremotediscovery.cpp +++ b/test/testremotediscovery.cpp @@ -140,13 +140,13 @@ private slots: QCOMPARE(errorSpy.size(), 1); QCOMPARE(errorSpy[0][0].toString(), QString(fatalErrorPrefix + expectedErrorString)); } else { - QCOMPARE(completeSpy.findItem(QStringLiteral("B"))->_instruction, CSYNC_INSTRUCTION_IGNORE); + QCOMPARE(completeSpy.findItem(QStringLiteral("B"))->instruction(), CSYNC_INSTRUCTION_IGNORE); QVERIFY(completeSpy.findItem(QStringLiteral("B"))->_errorString.contains(expectedErrorString)); // The other folder should have been sync'ed as the sync just ignored the faulty dir QCOMPARE(fakeFolder.currentRemoteState().children[QStringLiteral("A")], fakeFolder.currentLocalState().children[QStringLiteral("A")]); QCOMPARE(fakeFolder.currentRemoteState().children[QStringLiteral("C")], fakeFolder.currentLocalState().children[QStringLiteral("C")]); - QCOMPARE(completeSpy.findItem(QStringLiteral("A/z1"))->_instruction, CSYNC_INSTRUCTION_NEW); + QCOMPARE(completeSpy.findItem(QStringLiteral("A/z1"))->instruction(), CSYNC_INSTRUCTION_NEW); } // @@ -183,11 +183,11 @@ private slots: ItemCompletedSpy completeSpy(fakeFolder); QVERIFY(!fakeFolder.applyLocalModificationsAndSync()); - QCOMPARE(completeSpy.findItem(QStringLiteral("good"))->_instruction, CSYNC_INSTRUCTION_NEW); - QCOMPARE(completeSpy.findItem(QStringLiteral("noetag"))->_instruction, CSYNC_INSTRUCTION_ERROR); - QCOMPARE(completeSpy.findItem(QStringLiteral("nofileid"))->_instruction, CSYNC_INSTRUCTION_ERROR); - QCOMPARE(completeSpy.findItem(QStringLiteral("nopermissions"))->_instruction, CSYNC_INSTRUCTION_NEW); - QCOMPARE(completeSpy.findItem(QStringLiteral("nopermissions/A"))->_instruction, CSYNC_INSTRUCTION_ERROR); + QCOMPARE(completeSpy.findItem(QStringLiteral("good"))->instruction(), CSYNC_INSTRUCTION_NEW); + QCOMPARE(completeSpy.findItem(QStringLiteral("noetag"))->instruction(), CSYNC_INSTRUCTION_ERROR); + QCOMPARE(completeSpy.findItem(QStringLiteral("nofileid"))->instruction(), CSYNC_INSTRUCTION_ERROR); + QCOMPARE(completeSpy.findItem(QStringLiteral("nopermissions"))->instruction(), CSYNC_INSTRUCTION_NEW); + QCOMPARE(completeSpy.findItem(QStringLiteral("nopermissions/A"))->instruction(), CSYNC_INSTRUCTION_ERROR); QVERIFY(completeSpy.findItem(QStringLiteral("noetag"))->_errorString.contains(QStringLiteral("etag"))); QVERIFY(completeSpy.findItem(QStringLiteral("nofileid"))->_errorString.contains(QStringLiteral("file id"))); QVERIFY(completeSpy.findItem(QStringLiteral("nopermissions/A"))->_errorString.contains(QStringLiteral("permissions"))); diff --git a/test/testsyncconflict.cpp b/test/testsyncconflict.cpp index 881a6064ff2..ef2488f166b 100644 --- a/test/testsyncconflict.cpp +++ b/test/testsyncconflict.cpp @@ -26,14 +26,14 @@ bool itemSuccessful(const ItemCompletedSpy &spy, const QString &path, const Sync { auto item = spy.findItem(path); Q_ASSERT(item); - return item->_status == SyncFileItem::Success && item->_instruction == instr; + return item->_status == SyncFileItem::Success && item->instruction() == instr; } bool itemConflict(const ItemCompletedSpy &spy, const QString &path) { auto item = spy.findItem(path); Q_ASSERT(item); - return item->_status == SyncFileItem::Conflict && item->_instruction == CSYNC_INSTRUCTION_CONFLICT; + return item->_status == SyncFileItem::Conflict && item->instruction() == CSYNC_INSTRUCTION_CONFLICT; } QStringList findConflicts(const FileInfo &dir) diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index e847a5053d6..bb4931354a5 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -19,7 +19,7 @@ using namespace OCC; bool itemDidComplete(const ItemCompletedSpy &spy, const QString &path) { if (auto item = spy.findItem(path)) { - return item->_instruction != CSYNC_INSTRUCTION_NONE && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA; + return item->instruction() & ~(CSYNC_INSTRUCTION_NONE | CSYNC_INSTRUCTION_UPDATE_METADATA); } return false; } @@ -27,7 +27,7 @@ bool itemDidComplete(const ItemCompletedSpy &spy, const QString &path) bool itemInstruction(const ItemCompletedSpy &spy, const QString &path, const SyncInstructions instr) { auto item = spy.findItem(path); - return item->_instruction == instr; + return item->instruction() == instr; } bool itemDidCompleteSuccessfully(const ItemCompletedSpy &spy, const QString &path) @@ -503,7 +503,7 @@ private slots: // a1: should have local size and modtime QVERIFY(a1); - QCOMPARE(a1->_instruction, CSYNC_INSTRUCTION_SYNC); + QCOMPARE(a1->instruction(), CSYNC_INSTRUCTION_SYNC); QCOMPARE(a1->_direction, SyncFileItem::Up); QCOMPARE(a1->_size, qint64(5)); @@ -513,7 +513,7 @@ private slots: // b2: should have remote size and modtime QVERIFY(b1); - QCOMPARE(b1->_instruction, CSYNC_INSTRUCTION_SYNC); + QCOMPARE(b1->instruction(), CSYNC_INSTRUCTION_SYNC); QCOMPARE(b1->_direction, SyncFileItem::Down); QCOMPARE(b1->_size, qint64(17)); QCOMPARE(Utility::qDateTimeFromTime_t(b1->_modtime), changedMtime); @@ -522,7 +522,7 @@ private slots: // c1: conflicts are downloads, so remote size and modtime QVERIFY(c1); - QCOMPARE(c1->_instruction, CSYNC_INSTRUCTION_CONFLICT); + QCOMPARE(c1->instruction(), CSYNC_INSTRUCTION_CONFLICT); QCOMPARE(c1->_direction, SyncFileItem::None); QCOMPARE(c1->_size, qint64(25)); QCOMPARE(Utility::qDateTimeFromTime_t(c1->_modtime), changedMtime2); diff --git a/test/testsyncfileitem.cpp b/test/testsyncfileitem.cpp index ac7b40e7a19..397436d9990 100644 --- a/test/testsyncfileitem.cpp +++ b/test/testsyncfileitem.cpp @@ -39,7 +39,7 @@ private slots: SyncFileItem movedItem1; movedItem1._file = QStringLiteral("folder/source/file.f"); movedItem1._renameTarget = QStringLiteral("folder/destination/file.f"); - movedItem1._instruction = CSYNC_INSTRUCTION_RENAME; + movedItem1.setInstruction(CSYNC_INSTRUCTION_RENAME); QTest::newRow("move1") << createItem(QStringLiteral("folder/destination")) << movedItem1 << createItem(QStringLiteral("folder/destination-2")); QTest::newRow("move2") << createItem(QStringLiteral("folder/destination/1")) << movedItem1 << createItem(QStringLiteral("folder/source")); diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp index bb05e0b4b27..15654f980a9 100644 --- a/test/testsyncmove.cpp +++ b/test/testsyncmove.cpp @@ -17,13 +17,13 @@ using namespace OCC; bool itemSuccessful(const ItemCompletedSpy &spy, const QString &path, const SyncInstructions instr) { auto item = spy.findItem(path); - return item->_status == SyncFileItem::Success && item->_instruction == instr; + return item->_status == SyncFileItem::Success && item->instruction() == instr; } bool itemConflict(const ItemCompletedSpy &spy, const QString &path) { auto item = spy.findItem(path); - return item->_status == SyncFileItem::Conflict && item->_instruction == CSYNC_INSTRUCTION_CONFLICT; + return item->_status == SyncFileItem::Conflict && item->instruction() == CSYNC_INSTRUCTION_CONFLICT; } bool itemSuccessfulMove(const ItemCompletedSpy &spy, const QString &path) diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp index 81ae8969ffb..eb237a63df5 100644 --- a/test/testsyncvirtualfiles.cpp +++ b/test/testsyncvirtualfiles.cpp @@ -17,7 +17,7 @@ auto itemInstruction(const ItemCompletedSpy &spy, const QString &path) { auto item = spy.findItem(path); Q_ASSERT(!item.isNull()); - return item->_instruction; + return item->instruction(); } SyncJournalFileRecord dbRecord(FakeFolder &folder, const QString &path)