csync: Use an explicit instruction for should_update_metadata

The current way of tracking the need to update the metadata without
propagation using a separate flag makes it difficult to track
priorities between the local and remote tree. The logic is also
difficult to logically cover since the possibilities matrix isn't
100% covered, leaving the flag only used in a few situations
(mostly involving folders, but not only).

The reason we need to change this is to be able to track the sync
state of files for overlay icons. The instruction alone can't be
used since CSYNC_INSTRUCTION_SYNC is used for folders even though
they won't be propagated. Removing this logic is however not possible
without using something else than CSYNC_INSTRUCTION_NONE since too
many codepath interpret (rightfully) this as meaning "nothing to do".

This patch adds a new CSYNC_INSTRUCTION_UPDATE_METADATA instruction
to let the update and reconcile steps tell the SyncEngine to update
the metadata of a file without any propagation. Other flags are left
to be interpretted by the implementation as implicitly needing
metadata update or not, as this was already the case for most file
propagation jobs. For example, CSYNC_INSTRUCTION_NEW for directories
now also implicitly update the metadata.

Since it's not impossible for folders to emit CSYNC_INSTRUCTION_SYNC
or CSYNC_INSTRUCTION_CONFLICT, the corresponding code paths in the
sync engine have been removed.

Since the reconcile step can now know if the local tree needs metadata
update while the remote side might want propagation, the
localMetadataUpdate logic in SyncEngine::treewalkFile now simply use
a CSYNC_INSTRUCTION_UPDATE_METADATA for the local side, which is now
implemented as a different database query.

csync/src/csync.c

@@ -387,7 +387,6 @@ static int _csync_treewalk_visitor(void *obj, void *data) {
       trav.inode        = cur->inode;
 
       trav.error_status = cur->error_status;
-      trav.should_update_metadata = cur->should_update_metadata;
       trav.has_ignored_files = cur->has_ignored_files;
       trav.checksum = cur->checksum;
       trav.checksumTypeId = cur->checksumTypeId;

csync/src/csync.h

@@ -125,20 +125,22 @@ typedef enum csync_status_codes_e CSYNC_STATUS;
   * the csync state of a file.
   */
 enum csync_instructions_e {
-  CSYNC_INSTRUCTION_NONE       = 0x00000000,  /* Nothing to do (UPDATE|RECONCILE) */
+  CSYNC_INSTRUCTION_NONE            = 0x00000000,  /* Nothing to do (UPDATE|RECONCILE) */
-  CSYNC_INSTRUCTION_EVAL       = 0x00000001,  /* There was changed compared to the DB (UPDATE) */
+  CSYNC_INSTRUCTION_EVAL            = 0x00000001,  /* There was changed compared to the DB (UPDATE) */
-  CSYNC_INSTRUCTION_REMOVE     = 0x00000002,  /* The file need to be removed (RECONCILE) */
+  CSYNC_INSTRUCTION_REMOVE          = 0x00000002,  /* The file need to be removed (RECONCILE) */
-  CSYNC_INSTRUCTION_RENAME     = 0x00000004,  /* The file need to be renamed (RECONCILE) */
+  CSYNC_INSTRUCTION_RENAME          = 0x00000004,  /* The file need to be renamed (RECONCILE) */
-  CSYNC_INSTRUCTION_EVAL_RENAME= 0x00000800,  /* The file is new, it is the destination of a rename (UPDATE) */
+  CSYNC_INSTRUCTION_EVAL_RENAME     = 0x00000800,  /* The file is new, it is the destination of a rename (UPDATE) */
-  CSYNC_INSTRUCTION_NEW        = 0x00000008,  /* The file is new compared to the db (UPDATE) */
+  CSYNC_INSTRUCTION_NEW             = 0x00000008,  /* The file is new compared to the db (UPDATE) */
-  CSYNC_INSTRUCTION_CONFLICT   = 0x00000010,  /* The file need to be downloaded because it is a conflict (RECONCILE) */
+  CSYNC_INSTRUCTION_CONFLICT        = 0x00000010,  /* The file need to be downloaded because it is a conflict (RECONCILE) */
-  CSYNC_INSTRUCTION_IGNORE     = 0x00000020,  /* The file is ignored (UPDATE|RECONCILE) */
+  CSYNC_INSTRUCTION_IGNORE          = 0x00000020,  /* The file is ignored (UPDATE|RECONCILE) */
-  CSYNC_INSTRUCTION_SYNC       = 0x00000040,  /* The file need to be pushed to the other remote (RECONCILE) */
+  CSYNC_INSTRUCTION_SYNC            = 0x00000040,  /* The file need to be pushed to the other remote (RECONCILE) */
-  CSYNC_INSTRUCTION_STAT_ERROR = 0x00000080,
+  CSYNC_INSTRUCTION_STAT_ERROR      = 0x00000080,
-  CSYNC_INSTRUCTION_ERROR      = 0x00000100,
+  CSYNC_INSTRUCTION_ERROR           = 0x00000100,
-  CSYNC_INSTRUCTION_TYPE_CHANGE = 0x0000200,  /* Like NEW, but deletes the old entity first (RECONCILE)
+  CSYNC_INSTRUCTION_TYPE_CHANGE     = 0x00000200,  /* Like NEW, but deletes the old entity first (RECONCILE)
-                                                 Used when the type of something changes from directory to file
+                                                      Used when the type of something changes from directory to file
-                                                 or back. */
+                                                      or back. */
+  CSYNC_INSTRUCTION_UPDATE_METADATA = 0x00000400,  /* If the etag has been updated and need to be writen to the db,
+                                                      but without any propagation (UPDATE|RECONCILE) */
 };
 
 enum csync_ftw_type_e {
@@ -254,9 +256,6 @@ struct csync_tree_walk_file_s {
     enum csync_ftw_type_e     type;
     enum csync_instructions_e instruction;
 
-    /* For directories: If the etag has been updated and need to be writen on the db */
-    int         should_update_metadata;
-
     /* For directories: Does it have children that were ignored (hidden or ignore pattern) */
     int         has_ignored_files;
 

csync/src/csync_private.h

@@ -186,8 +186,6 @@ struct csync_file_stat_s {
   mode_t mode;      /* u32 */
   unsigned int type                   : 4;
   unsigned int child_modified         : 1;
-  unsigned int should_update_metadata : 1; /*specify that the etag, or the remote perm or fileid has
-                                changed and need to be updated on the db even for INSTRUCTION_NONE */
   unsigned int has_ignored_files      : 1; /* specify that a directory, or child directory contains ignored files */
 
   char *destpath;   /* for renames */

csync/src/csync_reconcile.c

@@ -20,6 +20,7 @@
 
 #include "config_csync.h"
 
+#include <assert.h>
 #include "csync_private.h"
 #include "csync_reconcile.h"
 #include "csync_util.h"
@@ -130,6 +131,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
             break;
             /* file has been removed on the opposite replica */
         case CSYNC_INSTRUCTION_NONE:
+        case CSYNC_INSTRUCTION_UPDATE_METADATA:
             if (cur->has_ignored_files) {
                 /* Do not remove a directory that has ignored files */
                 break;
@@ -181,13 +183,8 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
 
                 if(!other) {
                     cur->instruction = CSYNC_INSTRUCTION_NEW;
-                    if (cur->type == CSYNC_FTW_TYPE_DIR) {
-                        // For new directories we always want to update the etag once
-                        // the directory has been propagated. Otherwise the directory
-                        // could appear locally without being added to the database.
-                        cur->should_update_metadata = true;
-                    }
                 } else if (other->instruction == CSYNC_INSTRUCTION_NONE
+                           || other->instruction == CSYNC_INSTRUCTION_UPDATE_METADATA
                            || cur->type == CSYNC_FTW_TYPE_DIR) {
                     other->instruction = CSYNC_INSTRUCTION_RENAME;
                     other->destpath = c_strdup( cur->path );
@@ -195,7 +192,6 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
                         csync_vio_set_file_id( other->file_id, cur->file_id );
                     }
                     other->inode = cur->inode;
-                    other->should_update_metadata = true;
                     cur->instruction = CSYNC_INSTRUCTION_NONE;
                 } else if (other->instruction == CSYNC_INSTRUCTION_REMOVE) {
                     other->instruction = CSYNC_INSTRUCTION_RENAME;
@@ -205,12 +201,12 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
                         csync_vio_set_file_id( other->file_id, cur->file_id );
                     }
                     other->inode = cur->inode;
-                    other->should_update_metadata = true;
                     cur->instruction = CSYNC_INSTRUCTION_NONE;
                 } else if (other->instruction == CSYNC_INSTRUCTION_NEW) {
                     CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "OOOO=> NEW detected in other tree!");
                     cur->instruction = CSYNC_INSTRUCTION_CONFLICT;
                 } else {
+                    assert(other->type != CSYNC_FTW_TYPE_DIR);
                     cur->instruction = CSYNC_INSTRUCTION_NONE;
                     other->instruction = CSYNC_INSTRUCTION_SYNC;
                 }
@@ -222,13 +218,19 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
             break;
         }
     } else {
-        bool is_equal_files = false;
+        bool is_conflict = true;
         /*
      * file found on the other replica
      */
         other = (csync_file_stat_t *) node->data;
 
         switch (cur->instruction) {
+        case CSYNC_INSTRUCTION_UPDATE_METADATA:
+            if (other->instruction == CSYNC_INSTRUCTION_UPDATE_METADATA && ctx->current == LOCAL_REPLICA) {
+                // Remote wins, the SyncEngine will pick relevant local metadata since the remote tree is walked last.
+                cur->instruction = CSYNC_INSTRUCTION_NONE;
+            }
+            break;
         case CSYNC_INSTRUCTION_EVAL_RENAME:
             /* If the file already exist on the other side, we have a conflict.
                Abort the rename and consider it is a new file. */
@@ -253,42 +255,39 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
             case CSYNC_INSTRUCTION_EVAL:
                 if (other->type == CSYNC_FTW_TYPE_DIR &&
                         cur->type == CSYNC_FTW_TYPE_DIR) {
-                    is_equal_files = (other->modtime == cur->modtime);
+                    // Folders of the same path are always considered equals
+                    is_conflict = false;
                 } else {
-                    is_equal_files = ((other->size == cur->size) && (other->modtime == cur->modtime));
+                    is_conflict = ((other->size != cur->size) || (other->modtime != cur->modtime));
                     // FIXME: do a binary comparision of the file here because of the following
                     // edge case:
                     // The files could still have different content, even though the mtime
                     // and size are the same.
                 }
-                if (is_equal_files) {
+                if (ctx->current == REMOTE_REPLICA) {
-                    /* The files are considered equal. */
+                    // If the files are considered equal, only update the DB with the etag from remote
-                    cur->instruction = CSYNC_INSTRUCTION_NONE;
+                    cur->instruction = is_conflict ? CSYNC_INSTRUCTION_CONFLICT : CSYNC_INSTRUCTION_UPDATE_METADATA;
                     other->instruction = CSYNC_INSTRUCTION_NONE;
-
-                    /* update DB with new etag from remote */
-                    if (ctx->current == LOCAL_REPLICA) {
-                        other->should_update_metadata = true;
-                    } else {
-                        cur->should_update_metadata = true;
-                    }
-                } else if(ctx->current == REMOTE_REPLICA) {
-                        cur->instruction = CSYNC_INSTRUCTION_CONFLICT;
-                        other->instruction = CSYNC_INSTRUCTION_NONE;
                 } else {
-                        cur->instruction = CSYNC_INSTRUCTION_NONE;
+                    cur->instruction = CSYNC_INSTRUCTION_NONE;
-                        other->instruction = CSYNC_INSTRUCTION_CONFLICT;
+                    other->instruction = is_conflict ? CSYNC_INSTRUCTION_CONFLICT : CSYNC_INSTRUCTION_UPDATE_METADATA;
                 }
 
                 break;
                 /* file on the other replica has not been modified */
             case CSYNC_INSTRUCTION_NONE:
+            case CSYNC_INSTRUCTION_UPDATE_METADATA:
                 if (cur->type != other->type) {
                     // If the type of the entity changed, it's like NEW, but
                     // needs to delete the other entity first.
                     cur->instruction = CSYNC_INSTRUCTION_TYPE_CHANGE;
+                    other->instruction = CSYNC_INSTRUCTION_NONE;
+                } else if (cur->type == CSYNC_FTW_TYPE_DIR) {
+                    cur->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
+                    other->instruction = CSYNC_INSTRUCTION_NONE;
                 } else {
                     cur->instruction = CSYNC_INSTRUCTION_SYNC;
+                    other->instruction = CSYNC_INSTRUCTION_NONE;
                 }
                 break;
             case CSYNC_INSTRUCTION_IGNORE:
@@ -310,7 +309,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
         if(cur->type == CSYNC_FTW_TYPE_DIR)
         {
             CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE,
-                      "%-20s %s dir:  %s",
+                      "%-30s %s dir:  %s",
                       csync_instruction_str(cur->instruction),
                       repo,
                       cur->path);
@@ -318,7 +317,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
         else
         {
             CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE,
-                      "%-20s %s file: %s",
+                      "%-30s %s file: %s",
                       csync_instruction_str(cur->instruction),
                       repo,
                       cur->path);
@@ -329,7 +328,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
         if(cur->type == CSYNC_FTW_TYPE_DIR)
         {
             CSYNC_LOG(CSYNC_LOG_PRIORITY_DEBUG,
-                      "%-20s %s dir:  %s",
+                      "%-30s %s dir:  %s",
                       csync_instruction_str(cur->instruction),
                       repo,
                       cur->path);
@@ -337,7 +336,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
         else
         {
             CSYNC_LOG(CSYNC_LOG_PRIORITY_DEBUG,
-                      "%-20s %s file: %s",
+                      "%-30s %s file: %s",
                       csync_instruction_str(cur->instruction),
                       repo,
                       cur->path);

csync/src/csync_update.c

@@ -314,8 +314,7 @@ static int _csync_detect_update(CSYNC *ctx, const char *file,
                 }
                 if (checksumIdentical) {
                     CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "NOTE: Checksums are identical, file did not actually change: %s", path);
-                    st->instruction = CSYNC_INSTRUCTION_NONE;
+                    st->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
-                    st->should_update_metadata = true;
                     goto out;
                 }
             }
@@ -341,18 +340,19 @@ static int _csync_detect_update(CSYNC *ctx, const char *file,
             CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "Reading from database: %s", path);
             ctx->remote.read_from_db = true;
         }
-        if (metadata_differ) {
-            /* file id or permissions has changed. Which means we need to update them in the DB. */
-            CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "Need to update metadata for: %s", path);
-            st->should_update_metadata = true;
-        }
         /* If it was remembered in the db that the remote dir has ignored files, store
          * that so that the reconciler can make advantage of.
          */
         if( ctx->current == REMOTE_REPLICA ) {
             st->has_ignored_files = tmp->has_ignored_files;
         }
-        st->instruction = CSYNC_INSTRUCTION_NONE;
+        if (metadata_differ) {
+            /* file id or permissions has changed. Which means we need to update them in the DB. */
+            CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "Need to update metadata for: %s", path);
+            st->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
+        } else {
+            st->instruction = CSYNC_INSTRUCTION_NONE;
+        }
     } else {
         enum csync_vio_file_type_e tmp_vio_type = CSYNC_VIO_FILE_TYPE_UNKNOWN;
 
@@ -488,7 +488,9 @@ out:
           }
       }
   }
-  if (st->instruction != CSYNC_INSTRUCTION_NONE && st->instruction != CSYNC_INSTRUCTION_IGNORE
+  if (st->instruction != CSYNC_INSTRUCTION_NONE
+      && st->instruction != CSYNC_INSTRUCTION_IGNORE
+      && st->instruction != CSYNC_INSTRUCTION_UPDATE_METADATA
       && type != CSYNC_FTW_TYPE_DIR) {
     st->child_modified = 1;
   }
@@ -877,10 +879,11 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
 
       if (ctx->current_fs && !ctx->current_fs->child_modified
           && ctx->current_fs->instruction == CSYNC_INSTRUCTION_EVAL) {
-        ctx->current_fs->instruction = CSYNC_INSTRUCTION_NONE;
+          if (ctx->current == REMOTE_REPLICA) {
-        if (ctx->current == REMOTE_REPLICA) {
+              ctx->current_fs->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
-          ctx->current_fs->should_update_metadata = true;
+          } else {
-        }
+              ctx->current_fs->instruction = CSYNC_INSTRUCTION_NONE;
+          }
       }
 
       if (ctx->current_fs && previous_fs && ctx->current_fs->has_ignored_files) {
@@ -894,12 +897,6 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
         previous_fs->child_modified = ctx->current_fs->child_modified;
     }
 
-    if (flag == CSYNC_FTW_FLAG_DIR && ctx->current_fs
-        && (ctx->current_fs->instruction == CSYNC_INSTRUCTION_EVAL ||
-            ctx->current_fs->instruction == CSYNC_INSTRUCTION_NEW)) {
-        ctx->current_fs->should_update_metadata = true;
-    }
-
     ctx->current_fs = previous_fs;
     ctx->remote.read_from_db = read_from_db;
     SAFE_FREE(filename);

csync/src/csync_util.c

@@ -56,6 +56,7 @@ static const _instr_code_struct _instr[] =
   { "INSTRUCTION_STAT_ERR", CSYNC_INSTRUCTION_STAT_ERROR },
   { "INSTRUCTION_ERROR", CSYNC_INSTRUCTION_ERROR },
   { "INSTRUCTION_TYPE_CHANGE", CSYNC_INSTRUCTION_TYPE_CHANGE },
+  { "INSTRUCTION_UPDATE_METADATA", CSYNC_INSTRUCTION_UPDATE_METADATA },
   { NULL, CSYNC_INSTRUCTION_ERROR }
 };
 

src/gui/folder.cpp

@@ -410,11 +410,9 @@ void Folder::bubbleUpSyncResult()
                         firstItemDeleted = item;
                     break;
                 case CSYNC_INSTRUCTION_SYNC:
-                    if (!item->_isDirectory) {
+                    updatedItems++;
-                        updatedItems++;
+                    if (!firstItemUpdated)
-                        if (!firstItemUpdated)
+                        firstItemUpdated = item;
-                            firstItemUpdated = item;
-                    }
                     break;
                 case CSYNC_INSTRUCTION_ERROR:
                     qDebug() << "Got Instruction ERROR. " << _syncResult.errorString();

src/gui/syncrunfilelog.cpp

@@ -81,6 +81,9 @@ QString SyncRunFileLog::instructionToStr( csync_instructions_e inst )
     case CSYNC_INSTRUCTION_TYPE_CHANGE:
         re = "INST_TYPE_CHANGE";
         break;
+    case CSYNC_INSTRUCTION_UPDATE_METADATA:
+        re = "INST_METADATA";
+        break;
     }
 
     return re;

src/libsync/owncloudpropagator.cpp

@@ -206,7 +206,7 @@ bool PropagateItemJob::checkForProblemsWithShared(int httpStatusCode, const QStr
             // Directories are harder to recover.
             // But just re-create the directory, next sync will be able to recover the files
             SyncFileItemPtr mkdirItem(new SyncFileItem(*_item));
-            mkdirItem->_instruction = CSYNC_INSTRUCTION_SYNC;
+            mkdirItem->_instruction = CSYNC_INSTRUCTION_NEW;
             mkdirItem->_direction = SyncFileItem::Down;
             newJob = new PropagateLocalMkdir(_propagator, mkdirItem);
             // Also remove the inodes and fileid from the db so no further renames are tried for
@@ -265,20 +265,14 @@ PropagateItemJob* OwncloudPropagator::createJob(const SyncFileItemPtr &item) {
             }   //fall through
         case CSYNC_INSTRUCTION_SYNC:
         case CSYNC_INSTRUCTION_CONFLICT:
-            if (item->_isDirectory) {
+            if (item->_direction != SyncFileItem::Up) {
-                // Should we set the mtime?
+                auto job = new PropagateDownloadFileQNAM(this, item);
-                return 0;
+                job->setDeleteExistingFolder(deleteExisting);
-            }
+                return job;
-            {
+            } else {
-                if (item->_direction != SyncFileItem::Up) {
+                auto job = new PropagateUploadFileQNAM(this, item);
-                    auto job = new PropagateDownloadFileQNAM(this, item);
+                job->setDeleteExisting(deleteExisting);
-                    job->setDeleteExistingFolder(deleteExisting);
+                return job;
-                    return job;
-                } else {
-                    auto job = new PropagateUploadFileQNAM(this, item);
-                    job->setDeleteExisting(deleteExisting);
-                    return job;
-                }
             }
         case CSYNC_INSTRUCTION_RENAME:
             if (item->_direction == SyncFileItem::Up) {
@@ -377,7 +371,8 @@ void OwncloudPropagator::start(const SyncFileItemVector& items)
                 // 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 (int i = 0; i < directories.size(); ++i) {
-                    directories[i].second->_item->_should_update_metadata = false;
+                    if (directories[i].second->_item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA)
+                        directories[i].second->_item->_instruction = CSYNC_INSTRUCTION_NONE;
                 }
             } else {
                 PropagateDirectory* currentDirJob = directories.top().second;
@@ -677,7 +672,12 @@ void PropagateDirectory::finalize()
             _item->_file = _item->_renameTarget;
         }
 
-        if (_item->_should_update_metadata && _item->_instruction != CSYNC_INSTRUCTION_REMOVE) {
+        // For new directories we always want to update the etag once
+        // the directory has been propagated. Otherwise the directory
+        // could appear locally without being added to the database.
+        if (_item->_instruction == CSYNC_INSTRUCTION_RENAME
+            || _item->_instruction == CSYNC_INSTRUCTION_NEW
+            || _item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA) {
             if (PropagateRemoteMkdir* mkdir = qobject_cast<PropagateRemoteMkdir*>(_firstJob.data())) {
                 // special case from MKDIR, get the fileId from the job there
                 if (_item->_fileId.isEmpty() && !mkdir->_item->_fileId.isEmpty()) {

src/libsync/progressdispatcher.cpp

@@ -46,6 +46,8 @@ QString Progress::asResultString( const SyncFileItem& item)
         return QCoreApplication::translate( "progress", "Filesystem access error");
     case CSYNC_INSTRUCTION_ERROR:
         return QCoreApplication::translate( "progress", "Error");
+    case CSYNC_INSTRUCTION_UPDATE_METADATA:
+        return QCoreApplication::translate( "progress", "Updated local metadata");
     case CSYNC_INSTRUCTION_NONE:
     case CSYNC_INSTRUCTION_EVAL:
         return QCoreApplication::translate( "progress", "Unknown");
@@ -76,6 +78,8 @@ QString Progress::asActionString( const SyncFileItem &item )
         return QCoreApplication::translate( "progress", "error");
     case CSYNC_INSTRUCTION_ERROR:
         return QCoreApplication::translate( "progress", "error");
+    case CSYNC_INSTRUCTION_UPDATE_METADATA:
+        return QCoreApplication::translate( "progress", "updating local metadata");
     case CSYNC_INSTRUCTION_NONE:
     case CSYNC_INSTRUCTION_EVAL:
         break;
@@ -159,17 +163,10 @@ static bool shouldCountProgress(const SyncFileItem &item)
 {
     const auto instruction = item._instruction;
 
-    // Don't worry about directories that won't have propagation
+    // Skip any ignored, error or non-propagated files and directories.
-    // jobs associated with them.
+    if (instruction == CSYNC_INSTRUCTION_NONE
-    if (item._isDirectory
+            || instruction == CSYNC_INSTRUCTION_UPDATE_METADATA
-            && (instruction == CSYNC_INSTRUCTION_NONE
+            || instruction == CSYNC_INSTRUCTION_IGNORE
-                || instruction == CSYNC_INSTRUCTION_SYNC
-                || instruction == CSYNC_INSTRUCTION_CONFLICT)) {
-        return false;
-    }
-
-    // Skip any ignored or error files, we do nothing with them.
-    if (instruction == CSYNC_INSTRUCTION_IGNORE
             || instruction == CSYNC_INSTRUCTION_ERROR) {
         return false;
     }

src/libsync/syncengine.cpp

@@ -395,8 +395,6 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
         item->_remotePerm = QByteArray(file->remotePerm);
     }
 
-    item->_should_update_metadata = item->_should_update_metadata || file->should_update_metadata;
-
     /* The flag "serverHasIgnoredFiles" is true if item in question is a directory
      * that has children which are ignored in sync, either because the files are
      * matched by an ignore pattern, or because they are hidden.
@@ -514,10 +512,23 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
     int re = 0;
     switch(file->instruction) {
     case CSYNC_INSTRUCTION_NONE: {
-        if (remote && item->_should_update_metadata && !isDirectory && item->_instruction == CSYNC_INSTRUCTION_NONE) {
+        // Any files that are instruction NONE?
+        if (!isDirectory && file->other.instruction == CSYNC_INSTRUCTION_NONE) {
+            _hasNoneFiles = true;
+        }
+        // No syncing or update to be done.
+        return re;
+    }
+    case CSYNC_INSTRUCTION_UPDATE_METADATA:
+        dir = SyncFileItem::None;
+        // For directories, metadata-only updates will be done after all their files are propagated.
+        if (!isDirectory) {
+            item->_isDirectory = isDirectory;
+            emit syncItemDiscovered(*item);
+
             // Update the database now already:  New remote fileid or Etag or RemotePerm
             // Or for files that were detected as "resolved conflict".
-            // Or a local inode/mtime change (see localMetadataUpdate below)
+            // Or a local inode/mtime change
 
             // In case of "resolved conflict": there should have been a conflict because they
             // both were new, or both had their local mtime or remote etag modified, but the
@@ -529,47 +540,33 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
             // quick to do and we don't want to create a potentially large number of
             // mini-jobs later on, we just update metadata right now.
 
-            QString filePath = _localPath + item->_file;
+            if (remote) {
+                QString filePath = _localPath + item->_file;
 
-            // Even if the mtime is different on the server, we always want to keep the mtime from
+                // Even if the mtime is different on the server, we always want to keep the mtime from
-            // the file system in the DB, this is to avoid spurious upload on the next sync
+                // the file system in the DB, this is to avoid spurious upload on the next sync
-            item->_modtime = file->other.modtime;
+                item->_modtime = file->other.modtime;
-            // same for the size
+                // same for the size
-            item->_size = file->other.size;
+                item->_size = file->other.size;
 
-            // If the 'W' remote permission changed, update the local filesystem
+                // If the 'W' remote permission changed, update the local filesystem
-            SyncJournalFileRecord prev = _journal->getFileRecord(item->_file);
+                SyncJournalFileRecord prev = _journal->getFileRecord(item->_file);
-            if (prev.isValid() && prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) {
+                if (prev.isValid() && prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) {
-                const bool isReadOnly = !item->_remotePerm.contains('W');
+                    const bool isReadOnly = !item->_remotePerm.contains('W');
-                FileSystem::setFileReadOnlyWeak(filePath, isReadOnly);
+                    FileSystem::setFileReadOnlyWeak(filePath, isReadOnly);
+                }
+
+                _journal->setFileRecordMetadata(SyncJournalFileRecord(*item, filePath));
+            } else {
+                // The local tree is walked first and doesn't have all the info from the server.
+                // Update only outdated data from the disk.
+                _journal->updateLocalMetadata(item->_file, item->_modtime, item->_size, item->_inode);
             }
 
-            _journal->setFileRecordMetadata(SyncJournalFileRecord(*item, filePath));
+            // Technically we're done with this item.
-            item->_should_update_metadata = false;
-
-            // Technically we're done with this item. See localMetadataUpdate hack below.
-            _syncItemMap.remove(key);
-        }
-        // Any files that are instruction NONE?
-        if (!isDirectory && file->other.instruction == CSYNC_INSTRUCTION_NONE) {
-            _hasNoneFiles = true;
-        }
-        // We want to still update etags of directories, other NONE
-        // items can be ignored.
-        bool directoryEtagUpdate = isDirectory && file->should_update_metadata;
-        bool localMetadataUpdate = !remote && file->should_update_metadata;
-        if (!directoryEtagUpdate) {
-            item->_isDirectory = isDirectory;
-            if (localMetadataUpdate) {
-                // Hack, we want a local metadata update to happen, but only if the
-                // remote tree doesn't ask us to do some kind of propagation.
-                _syncItemMap.insert(key, item);
-            }
-            emit syncItemDiscovered(*item);
             return re;
         }
         break;
-    }
     case CSYNC_INSTRUCTION_RENAME:
         dir = !remote ? SyncFileItem::Down : SyncFileItem::Up;
         item->_renameTarget = renameTarget;
@@ -1180,9 +1177,8 @@ void SyncEngine::checkForPermission()
                 if (perms.isNull()) {
                     // No permissions set
                     break;
-                } if (!(*it)->_isDirectory && !perms.contains("W")) {
+                } if (!perms.contains("W")) {
                     qDebug() << "checkForPermission: RESTORING" << (*it)->_file;
-                    (*it)->_should_update_metadata = true;
                     (*it)->_instruction = CSYNC_INSTRUCTION_CONFLICT;
                     (*it)->_direction = SyncFileItem::Down;
                     (*it)->_isRestoration = true;
@@ -1204,7 +1200,6 @@ void SyncEngine::checkForPermission()
                 }
                 if (!perms.contains("D")) {
                     qDebug() << "checkForPermission: RESTORING" << (*it)->_file;
-                    (*it)->_should_update_metadata = true;
                     (*it)->_instruction = CSYNC_INSTRUCTION_NEW;
                     (*it)->_direction = SyncFileItem::Down;
                     (*it)->_isRestoration = true;
@@ -1223,7 +1218,6 @@ void SyncEngine::checkForPermission()
                             }
 
                             qDebug() << "checkForPermission: RESTORING" << (*it)->_file;
-                            (*it)->_should_update_metadata = true;
 
                             (*it)->_instruction = CSYNC_INSTRUCTION_NEW;
                             (*it)->_direction = SyncFileItem::Down;
@@ -1372,7 +1366,6 @@ void SyncEngine::restoreOldFiles()
             break;
         case CSYNC_INSTRUCTION_REMOVE:
             qDebug() << "restoreOldFiles: RESTORING" << (*it)->_file;
-            (*it)->_should_update_metadata = true;
             (*it)->_instruction = CSYNC_INSTRUCTION_NEW;
             (*it)->_direction = SyncFileItem::Up;
             break;

src/libsync/syncfileitem.h

@@ -66,7 +66,7 @@ public:
     SyncFileItem() : _type(UnknownType),  _direction(None), _isDirectory(false),
          _serverHasIgnoredFiles(false), _hasBlacklistEntry(false),
          _errorMayBeBlacklisted(false), _status(NoStatus),
-        _isRestoration(false), _should_update_metadata(false),
+        _isRestoration(false),
         _httpErrorCode(0), _requestDuration(0), _affectedItems(1),
         _instruction(CSYNC_INSTRUCTION_NONE), _modtime(0), _size(0), _inode(0)
     {
@@ -156,7 +156,6 @@ public:
     // Variables useful to report to the user
     Status               _status BITFIELD(4);
     bool                 _isRestoration BITFIELD(1); // The original operation was forbidden, and this is a restoration
-    bool                 _should_update_metadata BITFIELD(1);
     quint16              _httpErrorCode;
     QString              _errorString; // Contains a string only in case of error
     QByteArray           _responseTimeStamp;

src/libsync/syncjournaldb.cpp

@@ -385,6 +385,12 @@ bool SyncJournalDb::checkConnect()
             " SET contentChecksum = ?2, contentChecksumTypeId = ?3"
             " WHERE phash == ?1;");
 
+    _setFileRecordLocalMetadataQuery.reset(new SqlQuery(_db));
+    _setFileRecordLocalMetadataQuery->prepare(
+            "UPDATE metadata"
+            " SET inode=?2, modtime=?3, filesize=?4"
+            " WHERE phash == ?1;");
+ 
     _getDownloadInfoQuery.reset(new SqlQuery(_db) );
     _getDownloadInfoQuery->prepare( "SELECT tmpfile, etag, errorcount FROM "
                                     "downloadinfo WHERE path=?1" );
@@ -473,6 +479,7 @@ void SyncJournalDb::close()
     _getFileRecordQuery.reset(0);
     _setFileRecordQuery.reset(0);
     _setFileRecordChecksumQuery.reset(0);
+    _setFileRecordLocalMetadataQuery.reset(0);
     _getDownloadInfoQuery.reset(0);
     _setDownloadInfoQuery.reset(0);
     _deleteDownloadInfoQuery.reset(0);
@@ -964,6 +971,40 @@ bool SyncJournalDb::updateFileRecordChecksum(const QString& filename,
     return true;
 }
 
+bool SyncJournalDb::updateLocalMetadata(const QString& filename,
+                                        qint64 modtime, quint64 size, quint64 inode)
+
+{
+    QMutexLocker locker(&_mutex);
+
+    qlonglong phash = getPHash(filename);
+    if( !checkConnect() ) {
+        qDebug() << "Failed to connect database.";
+        return false;
+    }
+
+    auto & query = _setFileRecordLocalMetadataQuery;
+
+    query->reset_and_clear_bindings();
+    query->bindValue(1, QString::number(phash));
+    query->bindValue(2, inode);
+    query->bindValue(3, modtime);
+    query->bindValue(4, size);
+
+    if( !query->exec() ) {
+        qWarning() << "Error SQL statement updateLocalMetadata: "
+                   << query->lastQuery() <<  " :"
+                   << query->error();
+        return false;
+    }
+
+    qDebug() << query->lastQuery() << phash << inode
+             << modtime << size;
+
+    query->reset_and_clear_bindings();
+    return true;
+}
+
 bool SyncJournalDb::setFileRecordMetadata(const SyncJournalFileRecord& record)
 {
     SyncJournalFileRecord existing = getFileRecord(record._path);

src/libsync/syncjournaldb.h

@@ -52,6 +52,8 @@ public:
     bool updateFileRecordChecksum(const QString& filename,
                                   const QByteArray& contentChecksum,
                                   const QByteArray& contentChecksumType);
+    bool updateLocalMetadata(const QString& filename,
+                             qint64 modtime, quint64 size, quint64 inode);
     bool exists();
     void walCheckpoint();
 
@@ -188,6 +190,7 @@ private:
     QScopedPointer<SqlQuery> _getFileRecordQuery;
     QScopedPointer<SqlQuery> _setFileRecordQuery;
     QScopedPointer<SqlQuery> _setFileRecordChecksumQuery;
+    QScopedPointer<SqlQuery> _setFileRecordLocalMetadataQuery;
     QScopedPointer<SqlQuery> _getDownloadInfoQuery;
     QScopedPointer<SqlQuery> _setDownloadInfoQuery;
     QScopedPointer<SqlQuery> _deleteDownloadInfoQuery;