fix(vfs/cfapi): avoid creating invalid db entries when using a different sync root

Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
This commit is contained in:
Jyrki Gadinger 2025-10-24 12:22:21 +02:00
parent d73bd2d7e5
commit 85d9dd5c29
4 changed files with 91 additions and 31 deletions

View File

@ -9,6 +9,7 @@
#include "accountfwd.h"
#include "capabilities.h"
#include "clientsideencryptionjobs.h"
#include "common/utility.h"
#include "configfile.h"
#include "cookiejar.h"
#include "creds/abstractcredentials.h"
@ -1174,61 +1175,64 @@ void Account::setAskUserForMnemonic(const bool ask)
emit askUserForMnemonicChanged();
}
void Account::listRemoteFolder(QPromise<OCC::PlaceholderCreateInfo> *promise, const QString &path, SyncJournalDb *journalForFolder)
void Account::listRemoteFolder(QPromise<OCC::PlaceholderCreateInfo> *promise, const QString &remoteSyncRootPath, const QString &subPath, SyncJournalDb *journalForFolder)
{
qCInfo(lcAccount()) << "ls col job requested for" << path;
qCInfo(lcAccount()) << "ls col job requested for" << subPath;
if (!credentials()->ready()) {
qCWarning(lcAccount()) << "credentials are not ready" << path;
qCWarning(lcAccount()) << "credentials are not ready" << subPath;
promise->finish();
return;
}
auto listFolderJob = new OCC::LsColJob{sharedFromThis(), path};
auto listFolderJob = new OCC::LsColJob{sharedFromThis(), subPath};
const auto props = LsColJob::defaultProperties(LsColJob::FolderType::ChildFolder, sharedFromThis());
listFolderJob->setProperties(props);
QObject::connect(listFolderJob, &OCC::LsColJob::networkError, this, [promise, path] (QNetworkReply *reply) {
QObject::connect(listFolderJob, &OCC::LsColJob::networkError, this, [promise, subPath] (QNetworkReply *reply) {
if (reply) {
qCWarning(lcAccount()) << "ls col job" << path << "error" << reply->errorString();
qCWarning(lcAccount()) << "ls col job" << subPath << "error" << reply->errorString();
}
qCWarning(lcAccount()) << "ls col job" << path << "error without a reply";
qCWarning(lcAccount()) << "ls col job" << subPath << "error without a reply";
promise->finish();
});
QObject::connect(listFolderJob, &OCC::LsColJob::finishedWithError, this, [promise, path] (QNetworkReply *reply) {
QObject::connect(listFolderJob, &OCC::LsColJob::finishedWithError, this, [promise, subPath] (QNetworkReply *reply) {
if (reply) {
qCWarning(lcAccount()) << "ls col job" << path << "error" << reply->errorString();
qCWarning(lcAccount()) << "ls col job" << subPath << "error" << reply->errorString();
}
qCWarning(lcAccount()) << "ls col job" << path << "error without a reply";
qCWarning(lcAccount()) << "ls col job" << subPath << "error without a reply";
promise->finish();
});
QObject::connect(listFolderJob, &OCC::LsColJob::finishedWithoutError, this, [promise, path] () {
qCInfo(lcAccount()) << "ls col job" << path << "finished";
QObject::connect(listFolderJob, &OCC::LsColJob::finishedWithoutError, this, [promise, subPath] () {
qCInfo(lcAccount()) << "ls col job" << subPath << "finished";
promise->finish();
});
QObject::connect(listFolderJob, &OCC::LsColJob::directoryListingIterated, this, [promise, path, journalForFolder, this](const QString &name, const QMap<QString, QString> &properties) {
// `name` is e.g. "/remote.php/dav/files/admin" or "/remote.php/dav/files/admin/SomeFolder"; whereas `path` is e.g. "" or "SomeFolder/"
// in case these two are equal we are currently iterating the entry for the current directory
const auto serverPath = name.mid(this->davPath().size());
const auto isRootCollection = serverPath.isEmpty() && path.isEmpty();
const auto isCurrentCollection = isRootCollection || serverPath == Utility::noTrailingSlashPath(path);
if (isCurrentCollection) {
listFolderJob->setProperty("ignoredFirst", false);
const auto baseRemotePath = Utility::trailingSlashPath(Utility::noLeadingSlashPath(remoteSyncRootPath));
auto syncRootPath = subPath;
if (subPath.startsWith(baseRemotePath)) {
syncRootPath = syncRootPath.mid(baseRemotePath.size());
}
QObject::connect(listFolderJob, &OCC::LsColJob::directoryListingIterated, this, [promise, remoteSyncRootPath, syncRootPath, journalForFolder, this](const QString &completeDavPath, const QMap<QString, QString> &properties) {
if (!sender()->property("ignoredFirst").toBool()) {
qCDebug(lcAccount()) << "skip first item";
sender()->setProperty("ignoredFirst", true);
return;
}
qCInfo(lcAccount()) << "ls col job" << path << "new file" << name << properties.count();
qCInfo(lcAccount()) << "ls col job" << syncRootPath << "new file" << completeDavPath << properties.count();
const auto slash = name.lastIndexOf('/');
const auto itemFileName = name.mid(slash + 1);
const auto absoluteItemPathName = (path.isEmpty() ? itemFileName : path + "/" + itemFileName);
const auto slash = completeDavPath.lastIndexOf('/');
const auto itemFileName = completeDavPath.mid(slash + 1);
const auto absoluteItemPathName = syncRootPath.isEmpty() ? itemFileName : Utility::noTrailingSlashPath(syncRootPath) + '/' + itemFileName;
auto currentItemDbRecord = SyncJournalFileRecord{};
if (journalForFolder->getFileRecord(absoluteItemPathName, &currentItemDbRecord) && currentItemDbRecord.isValid()) {

View File

@ -425,7 +425,8 @@ public slots:
void setAskUserForMnemonic(const bool ask);
void listRemoteFolder(QPromise<OCC::PlaceholderCreateInfo> *promise,
const QString &path,
const QString &remoteSyncRootPath,
const QString &subPath,
SyncJournalDb *journalForFolder);
signals:

View File

@ -553,7 +553,8 @@ void CALLBACK cfApiFetchPlaceHolders(const CF_CALLBACK_INFO *callbackInfo, const
sendTransferError();
return;
}
const auto serverPath = QString{vfs->params().remotePath + pathString.mid(rootPath.length() + 1)}.mid(1);
const auto remoteSyncRootPath = vfs->params().remotePath; // with leading slash
const auto serverPath = QString{remoteSyncRootPath + pathString.mid(rootPath.length() + 1)}.mid(1);
qCDebug(lcCfApiWrapper) << "fetch placeholder:" << path << serverPath << requestId;
@ -584,7 +585,7 @@ void CALLBACK cfApiFetchPlaceHolders(const CF_CALLBACK_INFO *callbackInfo, const
qCInfo(lcCfApiWrapper()) << "ls prop started";
});
QMetaObject::invokeMethod(vfs->params().account.data(), &OCC::Account::listRemoteFolder, &lsPropPromise, serverPath, vfs->params().journal);
QMetaObject::invokeMethod(vfs->params().account.data(), &OCC::Account::listRemoteFolder, &lsPropPromise, remoteSyncRootPath, serverPath, vfs->params().journal);
qCInfo(lcCfApiWrapper()) << "ls prop requested" << path << serverPath;

View File

@ -12,13 +12,10 @@
#include <QTemporaryDir>
#include <QtTest>
#include "common/utility.h"
#include "folderman.h"
#include "account.h"
#include "accountstate.h"
#include "configfile.h"
#include "testhelper.h"
#include "logger.h"
#include "syncenginetestutils.h"
using namespace OCC;
@ -100,7 +97,64 @@ private slots:
setLimitSettings(LimitSetting::LegacyGlobalLimit);
verifyLimitSettings(LimitSetting::NoLimit);
}
void testAccount_listRemoteFolder_data()
{
QTest::addColumn<QString>("remotePath");
QTest::addColumn<QString>("subPath");
QTest::addColumn<QStringList>("expectedPaths");
QTest::newRow("root = /; requesting ''") << "" << "" << QStringList{ "A", "B", "C", "S" };
QTest::newRow("root = /; requesting A/") << "" << "A/" << QStringList{ "A/a1", "A/a2", "A/sub1" };
QTest::newRow("root = /; requesting A/sub1/") << "" << "A/sub1/" << QStringList{ "A/sub1/sub2" };
QTest::newRow("root = /; requesting A/sub1/sub2") << "" << "A/sub1/sub2/" << QStringList{};
QTest::newRow("root = /A; requesting A/") << "/A" << "A/" << QStringList{ "a1", "a2", "sub1" };
QTest::newRow("root = /A; requesting A/sub1") << "/A" << "A/sub1/" << QStringList{ "sub1/sub2" };
QTest::newRow("root = /A; requesting A/sub1/sub2") << "/A" << "A/sub1/sub2/" << QStringList{};
}
void testAccount_listRemoteFolder()
{
QFETCH(QString, remotePath);
QFETCH(QString, subPath);
QFETCH(QStringList, expectedPaths);
FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
fakeFolder.remoteModifier().mkdir("A/sub1");
fakeFolder.remoteModifier().mkdir("A/sub1/sub2");
const auto account = fakeFolder.account();
QEventLoop localEventLoop;
auto lsPropPromise = QPromise<OCC::PlaceholderCreateInfo>{};
auto lsPropFuture = lsPropPromise.future();
auto lsPropFutureWatcher = QFutureWatcher<OCC::PlaceholderCreateInfo>{};
lsPropFutureWatcher.setFuture(lsPropFuture);
QList<QString> receivedPaths;
QObject::connect(&lsPropFutureWatcher, &QFutureWatcher<QStringList>::finished,
&localEventLoop, [&localEventLoop] () {
qInfo() << "ls prop finished";
localEventLoop.quit();
});
QObject::connect(&lsPropFutureWatcher, &QFutureWatcher<QStringList>::resultReadyAt,
&localEventLoop, [&receivedPaths, &lsPropFutureWatcher] (int resultIndex) {
qInfo() << "ls prop result at index" << resultIndex;
const auto &newResultEntries = lsPropFutureWatcher.resultAt(resultIndex);
receivedPaths.append(newResultEntries.fullPath);
});
fakeFolder.syncJournal().clearFileTable(); // pretend we only need to fetch placeholders
account->listRemoteFolder(&lsPropPromise, remotePath, subPath, &fakeFolder.syncJournal());
localEventLoop.exec();
receivedPaths.sort();
expectedPaths.sort();
QCOMPARE_EQ(receivedPaths, expectedPaths);
}
};
QTEST_APPLESS_MAIN(TestAccount)
QTEST_GUILESS_MAIN(TestAccount)
#include "testaccount.moc"