On folder move execute only one UPDATE query for all nested items.

Signed-off-by: alex-z <[email protected]>

src/common/preparedsqlquerymanager.h

@@ -107,6 +107,7 @@ class OCSYNC_EXPORT PreparedSqlQueryManager
         GetE2EeLockedFolderQuery,
         GetE2EeLockedFoldersQuery,
         DeleteE2EeLockedFolderQuery,
+        MoveFilesInPathQuery,
 
         PreparedQueryCount
     };

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<const char*>(sqlite3_value_text(argv[0]));
+                                    sqlite3_result_int64(ctx, c_jhash64(reinterpret_cast<const uint8_t*>(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<const char*>(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<void, QString> 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);

src/common/syncjournaldb.h

@@ -70,6 +70,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
     [[nodiscard]] bool getFilesBelowPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
     [[nodiscard]] bool listFilesInPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
     [[nodiscard]] Result<void, QString> 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);

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<QPair<QString /* directory name */, PropagateDirectory * /* job */>> directories;

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<PropagateRootDirectory> _rootJob;

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