From e974771796ec6ef9ddae2e1cdd512ccb08ca825d Mon Sep 17 00:00:00 2001 From: Jocelyn Turcotte Date: Mon, 15 Aug 2016 14:17:51 +0200 Subject: [PATCH] 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 | 1 - csync/src/csync.h | 33 ++++++------- csync/src/csync_private.h | 2 - csync/src/csync_reconcile.c | 59 +++++++++++----------- csync/src/csync_update.c | 35 ++++++------- csync/src/csync_util.c | 1 + src/gui/folder.cpp | 8 ++- src/gui/syncrunfilelog.cpp | 3 ++ src/libsync/owncloudpropagator.cpp | 34 ++++++------- src/libsync/progressdispatcher.cpp | 19 +++---- src/libsync/syncengine.cpp | 79 ++++++++++++++---------------- src/libsync/syncfileitem.h | 3 +- src/libsync/syncjournaldb.cpp | 41 ++++++++++++++++ src/libsync/syncjournaldb.h | 3 ++ 14 files changed, 174 insertions(+), 147 deletions(-) diff --git a/csync/src/csync.c b/csync/src/csync.c index e6c9ad016..281a00ee9 100644 --- a/csync/src/csync.c +++ b/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; diff --git a/csync/src/csync.h b/csync/src/csync.h index 422e746ab..678985d32 100644 --- a/csync/src/csync.h +++ b/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_EVAL = 0x00000001, /* There was changed compared to the DB (UPDATE) */ - CSYNC_INSTRUCTION_REMOVE = 0x00000002, /* The file need to be removed (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_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_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_STAT_ERROR = 0x00000080, - CSYNC_INSTRUCTION_ERROR = 0x00000100, - CSYNC_INSTRUCTION_TYPE_CHANGE = 0x0000200, /* Like NEW, but deletes the old entity first (RECONCILE) - Used when the type of something changes from directory to file - or back. */ + CSYNC_INSTRUCTION_NONE = 0x00000000, /* Nothing to do (UPDATE|RECONCILE) */ + 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_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_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_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_STAT_ERROR = 0x00000080, + CSYNC_INSTRUCTION_ERROR = 0x00000100, + 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 + 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; diff --git a/csync/src/csync_private.h b/csync/src/csync_private.h index 779587033..1ceede56d 100644 --- a/csync/src/csync_private.h +++ b/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 */ diff --git a/csync/src/csync_reconcile.c b/csync/src/csync_reconcile.c index ab25b1c87..3c906fedd 100644 --- a/csync/src/csync_reconcile.c +++ b/csync/src/csync_reconcile.c @@ -20,6 +20,7 @@ #include "config_csync.h" +#include #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) { - /* The files are considered equal. */ - cur->instruction = CSYNC_INSTRUCTION_NONE; + if (ctx->current == REMOTE_REPLICA) { + // If the files are considered equal, only update the DB with the etag from remote + 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; - other->instruction = CSYNC_INSTRUCTION_CONFLICT; + cur->instruction = CSYNC_INSTRUCTION_NONE; + 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); diff --git a/csync/src/csync_update.c b/csync/src/csync_update.c index 85ebecb37..895a8c113 100644 --- a/csync/src/csync_update.c +++ b/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->should_update_metadata = true; + st->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; 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) { - ctx->current_fs->should_update_metadata = true; - } + if (ctx->current == REMOTE_REPLICA) { + ctx->current_fs->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + } 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); diff --git a/csync/src/csync_util.c b/csync/src/csync_util.c index b1746b17b..1bc09c35c 100644 --- a/csync/src/csync_util.c +++ b/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 } }; diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 87f75d55a..f94ba71b4 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -410,11 +410,9 @@ void Folder::bubbleUpSyncResult() firstItemDeleted = item; break; case CSYNC_INSTRUCTION_SYNC: - if (!item->_isDirectory) { - updatedItems++; - if (!firstItemUpdated) - firstItemUpdated = item; - } + updatedItems++; + if (!firstItemUpdated) + firstItemUpdated = item; break; case CSYNC_INSTRUCTION_ERROR: qDebug() << "Got Instruction ERROR. " << _syncResult.errorString(); diff --git a/src/gui/syncrunfilelog.cpp b/src/gui/syncrunfilelog.cpp index bc312cc3d..04d1763ad 100644 --- a/src/gui/syncrunfilelog.cpp +++ b/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; diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 7de6b0ad2..9d21d5d24 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/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) { - // Should we set the mtime? - return 0; - } - { - if (item->_direction != SyncFileItem::Up) { - auto job = new PropagateDownloadFileQNAM(this, item); - job->setDeleteExistingFolder(deleteExisting); - return job; - } else { - auto job = new PropagateUploadFileQNAM(this, item); - job->setDeleteExisting(deleteExisting); - return job; - } + if (item->_direction != SyncFileItem::Up) { + auto job = new PropagateDownloadFileQNAM(this, item); + job->setDeleteExistingFolder(deleteExisting); + 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(_firstJob.data())) { // special case from MKDIR, get the fileId from the job there if (_item->_fileId.isEmpty() && !mkdir->_item->_fileId.isEmpty()) { diff --git a/src/libsync/progressdispatcher.cpp b/src/libsync/progressdispatcher.cpp index 3e9540a16..30c493c31 100644 --- a/src/libsync/progressdispatcher.cpp +++ b/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 - // jobs associated with them. - if (item._isDirectory - && (instruction == CSYNC_INSTRUCTION_NONE - || 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 + // 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) { return false; } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index b0aa783eb..68385e5e8 100644 --- a/src/libsync/syncengine.cpp +++ b/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 - // the file system in the DB, this is to avoid spurious upload on the next sync - item->_modtime = file->other.modtime; - // same for the size - item->_size = file->other.size; + // 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 + item->_modtime = file->other.modtime; + // same for the size + item->_size = file->other.size; - // If the 'W' remote permission changed, update the local filesystem - SyncJournalFileRecord prev = _journal->getFileRecord(item->_file); - if (prev.isValid() && prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) { - const bool isReadOnly = !item->_remotePerm.contains('W'); - FileSystem::setFileReadOnlyWeak(filePath, isReadOnly); + // If the 'W' remote permission changed, update the local filesystem + SyncJournalFileRecord prev = _journal->getFileRecord(item->_file); + if (prev.isValid() && prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) { + const bool isReadOnly = !item->_remotePerm.contains('W'); + 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)); - 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); + // Technically we're done with this 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; diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index ed825a722..f07bb283e 100644 --- a/src/libsync/syncfileitem.h +++ b/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; diff --git a/src/libsync/syncjournaldb.cpp b/src/libsync/syncjournaldb.cpp index afea5ec77..d624fac1c 100644 --- a/src/libsync/syncjournaldb.cpp +++ b/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); diff --git a/src/libsync/syncjournaldb.h b/src/libsync/syncjournaldb.h index ef03cdb0c..78f73d55f 100644 --- a/src/libsync/syncjournaldb.h +++ b/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 _getFileRecordQuery; QScopedPointer _setFileRecordQuery; QScopedPointer _setFileRecordChecksumQuery; + QScopedPointer _setFileRecordLocalMetadataQuery; QScopedPointer _getDownloadInfoQuery; QScopedPointer _setDownloadInfoQuery; QScopedPointer _deleteDownloadInfoQuery;