Use getter setter for SyncItem::instruction to simplify debugging

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;
     }

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)

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;
     }

src/libsync/discoveryphase.cpp

@@ -157,7 +157,7 @@ QPair<bool, QString> 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<bool, QString> 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;
         }

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();

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,15 +446,15 @@ 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<PropagateDirectory *>(currentRemoveDirectoryJob)) {
                     // increase the number of subjobs that would be there.
                     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,22 +502,21 @@ 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
                 // of the file we're about to delete to decide whether uploading
                 // 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,15 +539,15 @@ 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);
             } else {
                 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;

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);

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;
     }
 

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);
     }
 
     /**

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)) {