diff --git a/src/common/preparedsqlquerymanager.h b/src/common/preparedsqlquerymanager.h index beea7d74b1041..2f4bd9b40fc39 100644 --- a/src/common/preparedsqlquerymanager.h +++ b/src/common/preparedsqlquerymanager.h @@ -107,6 +107,7 @@ class OCSYNC_EXPORT PreparedSqlQueryManager GetE2EeLockedFolderQuery, GetE2EeLockedFoldersQuery, DeleteE2EeLockedFolderQuery, + MoveFilesInPathQuery, PreparedQueryCount }; diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index e8c1f55619194..59e3e3f197584 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -394,6 +394,18 @@ bool SyncJournalDb::checkConnect() end - text, 0)); }, nullptr, nullptr); + sqlite3_create_function(_db.sqliteDb(), "path_hash", 1, SQLITE_UTF8 | SQLITE_DETERMINISTIC, nullptr, + [] (sqlite3_context *ctx,int, sqlite3_value **argv) { + const auto text = reinterpret_cast(sqlite3_value_text(argv[0])); + sqlite3_result_int64(ctx, c_jhash64(reinterpret_cast(text), strlen(text), 0)); + }, nullptr, nullptr); + + sqlite3_create_function(_db.sqliteDb(), "path_length", 1, SQLITE_UTF8 | SQLITE_DETERMINISTIC, nullptr, + [] (sqlite3_context *ctx,int, sqlite3_value **argv) { + const auto text = reinterpret_cast(sqlite3_value_text(argv[0])); + sqlite3_result_int64(ctx, strlen(text)); + }, nullptr, nullptr); + /* Because insert is so slow, we do everything in a transaction, and only need one call to commit */ startTransaction(); @@ -1030,6 +1042,28 @@ Result SyncJournalDb::setFileRecord(const SyncJournalFileRecord & return {}; } +bool SyncJournalDb::updateParentForAllChildren(const QByteArray &oldParentPath, const QByteArray &newParentPath) +{ + qCInfo(lcDb) << "Moving files from path" << oldParentPath << "to path" << newParentPath; + + if (!checkConnect()) { + qCWarning(lcDb) << "Failed to connect database."; + return false; + } + + const auto query = _queryManager.get(PreparedSqlQueryManager::MoveFilesInPathQuery, + QByteArrayLiteral("UPDATE metadata" + " SET path = REPLACE(path, ?1, ?2), phash = path_hash(REPLACE(path, ?1, ?2)), pathlen = path_length(REPLACE(path, ?1, ?2))" + " WHERE " IS_PREFIX_PATH_OF("?1", "path")), + _db); + if (!query) { + return false; + } + query->bindValue(1, oldParentPath); + query->bindValue(2, newParentPath); + return query->exec(); +} + void SyncJournalDb::keyValueStoreSet(const QString &key, QVariant value) { QMutexLocker locker(&_mutex); diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h index 01cc1e39012be..10f25b1474a23 100644 --- a/src/common/syncjournaldb.h +++ b/src/common/syncjournaldb.h @@ -70,6 +70,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject [[nodiscard]] bool getFilesBelowPath(const QByteArray &path, const std::function &rowCallback); [[nodiscard]] bool listFilesInPath(const QByteArray &path, const std::function &rowCallback); [[nodiscard]] Result setFileRecord(const SyncJournalFileRecord &record); + [[nodiscard]] bool updateParentForAllChildren(const QByteArray &oldParentPath, const QByteArray &newParentPath); void keyValueStoreSet(const QString &key, QVariant value); [[nodiscard]] qint64 keyValueStoreGetInt(const QString &key, qint64 defaultValue); diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 488a83a3f0ff5..1592d7920ad52 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -479,6 +479,33 @@ void OwncloudPropagator::adjustDeletedFoldersWithNewChildren(SyncFileItemVector } } +void OwncloudPropagator::cleanupLocallyMovedFoldersFromNestedItems(SyncFileItemVector &items) +{ + QString previousFolderPath; + const auto eraseBeginIt = std::remove_if(std::begin(items), std::end(items), [&previousFolderPath](const SyncFileItemPtr &item) { + // we assume items is always sorted such that parent folder go before children + if (item->_instruction != CSYNC_INSTRUCTION_RENAME || item->_direction != SyncFileItem::Up) { + previousFolderPath.clear(); + return false; + } + + if (item->isDirectory() && (previousFolderPath.isEmpty() || !item->_originalFile.startsWith(previousFolderPath))) { + // if it is a directory and there was no previous directory set, do it now, same for a directory which is not a child of previous directory, assume + // starting a new hierarchy + previousFolderPath = item->_originalFile; + return false; + } + + if (previousFolderPath.isEmpty()) { + return false; + } + + // remove only children of previousFolderPath, not previousFolderPath itself such that parent always remains in the vector + return item->_originalFile.startsWith(previousFolderPath); + }); + items.erase(eraseBeginIt, std::end(items)); +} + qint64 OwncloudPropagator::smallFileSize() { const qint64 smallFileSize = 100 * 1024; //default to 1 MB. Not dynamic right now. @@ -518,15 +545,12 @@ void OwncloudPropagator::start(SyncFileItemVector &&items) items.end()); } - QStringList files; - - for (const auto &item : items) { - files.push_back(item->_file); - } - // process each item that is new and is a directory and make sure every parent in its tree has the instruction NEW instead of REMOVE adjustDeletedFoldersWithNewChildren(items); + // when a folder is moved on the local device we only need to perform one MOVE on the server and then just update database, so we only keep unique moves (topmost moved folder items) + cleanupLocallyMovedFoldersFromNestedItems(items); + resetDelayedUploadTasks(); _rootJob.reset(new PropagateRootDirectory(this)); QStack> directories; diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 46f0ea4e5ec2e..a3f02cd01d9c9 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -670,6 +670,7 @@ private slots: void resetDelayedUploadTasks(); static void adjustDeletedFoldersWithNewChildren(SyncFileItemVector &items); + static void cleanupLocallyMovedFoldersFromNestedItems(SyncFileItemVector &items); AccountPtr _account; QScopedPointer _rootJob; diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 09cb179d385a9..dad9bb5b9043a 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -249,6 +249,7 @@ void PropagateRemoteMove::finalize() return; } auto &vfs = propagator()->syncOptions()._vfs; + // TODO: vfs->pinState(_item->_originalFile); does not make sense as item is already gone from original location, do we need this? auto pinState = vfs->pinState(_item->_originalFile); const auto targetFile = propagator()->fullLocalPath(_item->_renameTarget); @@ -260,6 +261,7 @@ void PropagateRemoteMove::finalize() done(SyncFileItem::NormalError, tr("Could not delete file record %1 from local DB").arg(_item->_originalFile), ErrorCategory::GenericError); return; } + // TODO: vfs->setPinState(_item->_originalFile, PinState::Inherited) will always fail as item is already gone from original location, do we need this? if (!vfs->setPinState(_item->_originalFile, PinState::Inherited)) { qCWarning(lcPropagateRemoteMove) << "Could not set pin state of" << _item->_originalFile << "to inherited"; } @@ -305,6 +307,66 @@ void PropagateRemoteMove::finalize() } } + if (_item->isDirectory()) { + if (!propagator()->_journal->updateParentForAllChildren(_item->_originalFile.toUtf8(), _item->_renameTarget.toUtf8())) { + done(SyncFileItem::FatalError, tr("Failed to move folder: %1").arg(_item->_file), ErrorCategory::GenericError); + return; + } + + if (!propagator()->_journal->getFilesBelowPath(_item->_renameTarget.toUtf8(), [&](const SyncJournalFileRecord &rec) { + // not sure if this is needed, inode seems to never change for move/rename + auto newItem = SyncFileItem::fromSyncJournalFileRecord(rec); + newItem->_originalFile = QString(newItem->_file).replace(_item->_renameTarget, _item->_originalFile); + newItem->_renameTarget = newItem->_file; + newItem->_instruction = CSYNC_INSTRUCTION_RENAME; + newItem->_direction = SyncFileItem::Up; + const auto fsPath = propagator()->fullLocalPath(newItem->_renameTarget); + quint64 inode = rec._inode; + if (!FileSystem::getInode(fsPath, &inode)) { + qCWarning(lcPropagateRemoteMove) << "Could not get inode for moved file" << fsPath; + return; + } + if (inode != rec._inode) { + auto newRec = rec; + newRec._inode = inode; + if (!propagator()->_journal->setFileRecord(newRec)) { + qCWarning(lcPropagateRemoteMove) << "Could not update inode for moved file" << newRec.path(); + return; + } + } + auto &vfs = propagator()->syncOptions()._vfs; + // TODO: vfs->pinState(_item->_originalFile); does not make sense as item is already gone from original location, do we need this? + auto pinState = vfs->pinState(newItem->_originalFile); + const auto targetFile = propagator()->fullLocalPath(newItem->_renameTarget); + + if (QFileInfo::exists(targetFile)) { + // Delete old db data. + if (!propagator()->_journal->deleteFileRecord(newItem->_originalFile)) { + qCWarning(lcPropagateRemoteMove) << "could not delete file from local DB" << newItem->_originalFile; + } + // TODO: vfs->setPinState(_item->_originalFile, PinState::Inherited) will always fail as item is already gone from original location, do we + // need this? + if (!vfs->setPinState(newItem->_originalFile, PinState::Inherited)) { + qCWarning(lcPropagateRemoteMove) << "Could not set pin state of" << newItem->_originalFile << "to inherited"; + } + } + const auto result = propagator()->updateMetadata(*newItem); + if (!result) { + done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error()), ErrorCategory::GenericError); + return; + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + done(SyncFileItem::SoftError, tr("The file %1 is currently in use").arg(newItem->_file), ErrorCategory::GenericError); + return; + } + if (pinState && *pinState != PinState::Inherited && !vfs->setPinState(newItem->_renameTarget, *pinState)) { + done(SyncFileItem::NormalError, tr("Error setting pin state"), ErrorCategory::GenericError); + return; + } + })) { + qCWarning(lcPropagateRemoteMove) << "Could not update inode for moved files in" << _item->_renameTarget; + } + } + propagator()->_journal->commit("Remote Rename"); done(SyncFileItem::Success, {}, ErrorCategory::NoError); }