From 558480862263e4d00c1190615333a044bb6ff335 Mon Sep 17 00:00:00 2001 From: Jyrki Gadinger Date: Fri, 24 Oct 2025 16:46:24 +0200 Subject: [PATCH] fix(vfs/cfapi): disable on-demand folder fetching This temporarily reverts the on-demand parts of #8359 and adds some fallbacks when encountering virtual directories from the sync db. Signed-off-by: Jyrki Gadinger --- src/libsync/discovery.cpp | 27 ++++++++++++++-------- src/libsync/discoveryphase.cpp | 6 ++--- src/libsync/vfs/cfapi/cfapiwrapper.cpp | 21 +++++++++++++---- src/libsync/vfs/cfapi/vfs_cfapi.cpp | 4 ++-- test/testsynccfapi.cpp | 32 +++++++++++++------------- 5 files changed, 55 insertions(+), 35 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 08677e49b6..44f7f7e55c 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -132,6 +132,10 @@ void ProcessDirectoryJob::process() auto &dbEntry = entries[name].dbEntry; dbEntry = rec; setupDbPinStateActions(dbEntry); + if (_discoveryData->_syncOptions._vfs->mode() == Vfs::WindowsCfApi && dbEntry._type == ItemTypeVirtualDirectory) { + // invalidate etag for virtual directories to avoid upsyncing deletions + dbEntry._etag = QByteArrayLiteral("_invalid_"); + } })) { dbError(); return; @@ -671,13 +675,13 @@ void ProcessDirectoryJob::postProcessServerNew(const SyncFileItemPtr &item, const auto opts = _discoveryData->_syncOptions; if (item->isDirectory()) { - // Turn new remote folders into virtual folders if the option is enabled. - if (!localEntry.isValid() && - opts._vfs->mode() == Vfs::WindowsCfApi && - _pinState != PinState::AlwaysLocal && - !FileSystem::isExcludeFile(item->_file)) { - item->_type = ItemTypeVirtualDirectory; - } + // // Turn new remote folders into virtual folders if the option is enabled. + // if (!localEntry.isValid() && + // opts._vfs->mode() == Vfs::WindowsCfApi && + // _pinState != PinState::AlwaysLocal && + // !FileSystem::isExcludeFile(item->_file)) { + // item->_type = ItemTypeVirtualDirectory; + // } _pendingAsyncJobs++; _discoveryData->checkSelectiveSyncNewFolder(path._server, @@ -1456,7 +1460,8 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_checksumHeader.clear(); item->_size = localEntry.size; item->_modtime = localEntry.modtime; - item->_type = localEntry.isDirectory && !localEntry.isVirtualFile ? ItemTypeDirectory : localEntry.isDirectory ? ItemTypeVirtualDirectory : localEntry.isVirtualFile ? ItemTypeVirtualFile : ItemTypeFile; + // item->_type = localEntry.isDirectory && !localEntry.isVirtualFile ? ItemTypeDirectory : localEntry.isDirectory ? ItemTypeVirtualDirectory : localEntry.isVirtualFile ? ItemTypeVirtualFile : ItemTypeFile; + item->_type = localEntry.isDirectory ? ItemTypeDirectory : localEntry.isVirtualFile ? ItemTypeVirtualFile : ItemTypeFile; _childModified = true; if (!localEntry.caseClashConflictingName.isEmpty()) { @@ -1903,8 +1908,10 @@ void ProcessDirectoryJob::processFileFinalize( } if (item->_type == ItemTypeVirtualDirectory) { - qCDebug(lcDisco()) << "do not recurse inside a virtual folder" << item->_file; - recurse = false; + // qCDebug(lcDisco()) << "do not recurse inside a virtual folder" << item->_file; + // recurse = false; + qCInfo(lcDisco()).nospace() << "changing virtual directory to normal directory file=" << item->_file; + item->_type = ItemTypeDirectory; } if (!(item->isDirectory() || diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index da10dd0e28..c560166aff 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -110,9 +110,9 @@ void DiscoveryPhase::checkSelectiveSyncNewFolder(const QString &path, return callback(false); } - if (_syncOptions._vfs->mode() == Vfs::WindowsCfApi) { - return callback(true); - } + // if (_syncOptions._vfs->mode() == Vfs::WindowsCfApi) { + // return callback(true); + // } checkFolderSizeLimit(path, [this, path, callback](const bool bigFolder) { if (bigFolder) { diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp index 2e87850682..9b3964bf9b 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp +++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp @@ -418,12 +418,17 @@ OCC::Result updatePlaceholderStat OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.ChangeTime); qCInfo(lcCfApiWrapper) << "updatePlaceholderState" << path << modtime; - const auto updateFlags = item.isDirectory() ? CF_UPDATE_FLAG_MARK_IN_SYNC | CF_UPDATE_FLAG_ENABLE_ON_DEMAND_POPULATION : CF_UPDATE_FLAG_MARK_IN_SYNC; + const auto updateFlags = CF_UPDATE_FLAG_MARK_IN_SYNC; // item.isDirectory() ? CF_UPDATE_FLAG_MARK_IN_SYNC | CF_UPDATE_FLAG_ENABLE_ON_DEMAND_POPULATION : CF_UPDATE_FLAG_MARK_IN_SYNC; const auto result = CfUpdatePlaceholder(OCC::CfApiWrapper::handleForPath(path).get(), updateType == CfApiUpdateMetadataType::AllMetadata ? &metadata : nullptr, fileId.data(), static_cast(fileId.size()), nullptr, 0, updateFlags, nullptr, nullptr); + if (result != S_OK) { + const auto errorMessage = createErrorMessageForPlaceholderUpdateAndCreate(path, "Couldn't update placeholder info"); + qCWarning(lcCfApiWrapper) << errorMessage << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage()) << replacesPath; + } + // Pin state tends to be lost on updates, so restore it every time if (!setPinState(path, previousPinState, OCC::CfApiWrapper::NoRecurse)) { return { "Couldn't restore pin state" }; @@ -549,13 +554,18 @@ void CALLBACK cfApiFetchPlaceHolders(const CF_CALLBACK_INFO *callbackInfo, const auto rootPath = QFileInfo{vfs->params().filesystemPath}.canonicalFilePath(); if (!pathString.startsWith(rootPath)) { - qCCritical(lcCfApiWrapper) << "wrong path" << pathString << rootPath, + qCCritical(lcCfApiWrapper) << "wrong path" << pathString << rootPath; sendTransferError(); return; } const auto remoteSyncRootPath = vfs->params().remotePath; // with leading slash const auto serverPath = QString{remoteSyncRootPath + pathString.mid(rootPath.length() + 1)}.mid(1); + // to allow navigating to a folder if it's still a virtual directory: + qCDebug(lcCfApiWrapper) << "sending empty placeholders for" << path << serverPath << requestId; + sendTransferInfo({}, serverPath); + +#if 0 qCDebug(lcCfApiWrapper) << "fetch placeholder:" << path << serverPath << requestId; QEventLoop localEventLoop; @@ -609,6 +619,7 @@ void CALLBACK cfApiFetchPlaceHolders(const CF_CALLBACK_INFO *callbackInfo, const if (!newPlaceholdersResult) { sendTransferError(); } +#endif qCInfo(lcCfApiWrapper) << "call for finalizeNewPlaceholders succeeded"; } @@ -865,7 +876,7 @@ OCC::Result OCC::CfApiWrapper::registerSyncRoot(const QString &pa policies.StructSize = sizeof(CF_SYNC_POLICIES); policies.Hydration.Primary = CF_HYDRATION_POLICY_FULL; policies.Hydration.Modifier = CF_HYDRATION_POLICY_MODIFIER_NONE; - policies.Population.Primary = CF_POPULATION_POLICY_PARTIAL; + policies.Population.Primary = CF_POPULATION_POLICY_ALWAYS_FULL; // CF_POPULATION_POLICY_PARTIAL; policies.Population.Modifier = CF_POPULATION_POLICY_MODIFIER_NONE; policies.InSync = CF_INSYNC_POLICY_PRESERVE_INSYNC_FOR_SYNC_ENGINE; policies.HardLink = CF_HARDLINK_POLICY_NONE; @@ -1198,7 +1209,9 @@ OCC::Result OCC::CfApiWrapper::co const QByteArray &fileId = item._fileId; const auto fileIdentity = QString::fromUtf8(fileId).toStdWString(); const auto fileIdentitySize = (fileIdentity.length() + 1) * sizeof(wchar_t); - const auto createPlaceholderFlags = CF_CONVERT_FLAG_MARK_IN_SYNC | (item.isDirectory() ? (item._type == ItemType::ItemTypeVirtualDirectory ? CF_CONVERT_FLAG_ENABLE_ON_DEMAND_POPULATION : CF_CONVERT_FLAG_ALWAYS_FULL) : CF_CONVERT_FLAG_MARK_IN_SYNC); + // const auto createPlaceholderFlags = CF_CONVERT_FLAG_MARK_IN_SYNC | (item.isDirectory() ? (item._type == ItemType::ItemTypeVirtualDirectory ? CF_CONVERT_FLAG_ENABLE_ON_DEMAND_POPULATION : CF_CONVERT_FLAG_ALWAYS_FULL) : CF_CONVERT_FLAG_MARK_IN_SYNC); + const auto directoryPlaceholderFlags = item.isDirectory() ? CF_CONVERT_FLAG_ALWAYS_FULL : CF_CONVERT_FLAG_NONE; + const auto createPlaceholderFlags = CF_CONVERT_FLAG_MARK_IN_SYNC | directoryPlaceholderFlags; const auto result = CfConvertToPlaceholder(handleForPath(path).get(), fileIdentity.data(), sizeToDWORD(fileIdentitySize), createPlaceholderFlags, nullptr, nullptr); Q_ASSERT(result == S_OK); diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 38d4f5a478..9a18d1ef7f 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp +++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp @@ -291,8 +291,8 @@ bool VfsCfApi::statTypeVirtualFile(csync_file_stat_t *stat, void *statData) if (isDirectory) { if (hasCloudTag) { ffd->dwFileAttributes &= ~FILE_ATTRIBUTE_REPARSE_POINT; - stat->type = CSyncEnums::ItemTypeVirtualDirectory; - return true; + // stat->type = CSyncEnums::ItemTypeVirtualDirectory; + // return true; } return false; } else if (isSparseFile && isPinned) { diff --git a/test/testsynccfapi.cpp b/test/testsynccfapi.cpp index eb4b8570d9..797752ac52 100644 --- a/test/testsynccfapi.cpp +++ b/test/testsynccfapi.cpp @@ -223,7 +223,7 @@ private slots: auto someDate = QDateTime(QDate(1984, 07, 30), QTime(1,3,2)); fakeFolder.remoteModifier().setModTime("A/a1", someDate); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); CFVERIFY_VIRTUAL(fakeFolder, "A/a1"); QCOMPARE(QFileInfo(fakeFolder.localPath() + "A/a1").size(), 64); QCOMPARE(QFileInfo(fakeFolder.localPath() + "A/a1").lastModified(), someDate); @@ -352,7 +352,7 @@ private slots: fakeFolder.remoteModifier().mkdir("B"); fakeFolder.remoteModifier().insert("B/b1", 21); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); CFVERIFY_VIRTUAL(fakeFolder, "A/a1"); CFVERIFY_VIRTUAL(fakeFolder, "A/a2"); CFVERIFY_VIRTUAL(fakeFolder, "B/b1"); @@ -455,7 +455,7 @@ private slots: fakeFolder.remoteModifier().insert("A/b4"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); CFVERIFY_VIRTUAL(fakeFolder, "A/a1"); CFVERIFY_VIRTUAL(fakeFolder, "A/a2"); CFVERIFY_VIRTUAL(fakeFolder, "A/a3"); @@ -548,7 +548,7 @@ private slots: fakeFolder.remoteModifier().mkdir("A"); fakeFolder.remoteModifier().insert("A/a1"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); CFVERIFY_VIRTUAL(fakeFolder, "A/a1"); cleanup(); @@ -578,7 +578,7 @@ private slots: fakeFolder.remoteModifier().mkdir("A"); fakeFolder.remoteModifier().insert("A/a1"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); CFVERIFY_VIRTUAL(fakeFolder, "A/a1"); ::setPinState(fakeFolder.localPath(), PinState::AlwaysLocal, cfapi::NoRecurse); @@ -613,7 +613,7 @@ private slots: fakeFolder.remoteModifier().insert("B/Sub/b2"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); CFVERIFY_VIRTUAL(fakeFolder, "A/a1"); CFVERIFY_VIRTUAL(fakeFolder, "A/a2"); CFVERIFY_VIRTUAL(fakeFolder, "A/Sub/a3"); @@ -902,7 +902,7 @@ private slots: QVERIFY(fakeFolder.syncOnce()); CFVERIFY_VIRTUAL(fakeFolder, "f1"); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); CFVERIFY_VIRTUAL(fakeFolder, "A/a1"); // CFVERIFY_VIRTUAL(fakeFolder, "A/a3"); CFVERIFY_VIRTUAL(fakeFolder, "A/B/b1"); @@ -951,7 +951,7 @@ private slots: fakeFolder.remoteModifier().mkdir("online"); fakeFolder.remoteModifier().mkdir("unspec"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); ::setPinState(fakeFolder.localPath() + "local", PinState::AlwaysLocal, cfapi::Recurse); @@ -1035,7 +1035,7 @@ private slots: fakeFolder.remoteModifier().mkdir("online/sub"); fakeFolder.remoteModifier().mkdir("unspec"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); ::setPinState(fakeFolder.localPath() + "local", PinState::AlwaysLocal, cfapi::Recurse); @@ -1096,7 +1096,7 @@ private slots: fakeFolder.remoteModifier().mkdir("online"); fakeFolder.remoteModifier().mkdir("unspec"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); ::setPinState(fakeFolder.localPath() + "local", PinState::AlwaysLocal, cfapi::NoRecurse); @@ -1202,7 +1202,7 @@ private slots: fakeFolder.remoteModifier().mkdir("local"); fakeFolder.remoteModifier().mkdir("online"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); ::setPinState(fakeFolder.localPath() + "local", PinState::AlwaysLocal, cfapi::NoRecurse); @@ -1256,7 +1256,7 @@ private slots: fakeFolder.remoteModifier().mkdir("online"); fakeFolder.remoteModifier().mkdir("online/sub"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); ::setPinState(fakeFolder.localPath() + "online", PinState::OnlineOnly, cfapi::Recurse); @@ -1335,7 +1335,7 @@ private slots: fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::DatabaseAndFilesystem); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } @@ -1355,7 +1355,7 @@ private slots: QCOMPARE(completeSpy.findItem(QStringLiteral("A/a1"))->_locked, OCC::SyncFileItem::LockStatus::UnlockedItem); OCC::SyncJournalFileRecord fileRecordBefore; QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordBefore)); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); QVERIFY(fileRecordBefore.isValid()); QVERIFY(!fileRecordBefore._lockstate._locked); @@ -1453,7 +1453,7 @@ private slots: fakeFolder.remoteModifier().remove("a/TESTFILE"); fakeFolder.remoteModifier().mkdir("a/TESTFILE"); QVERIFY(fakeFolder.syncOnce()); - QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); + // QEXPECT_FAIL("", "folders on-demand breaks existing tests", Abort); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); @@ -1556,7 +1556,7 @@ private slots: void testSyncFolderNewDeleteConflictExpectDeletion() { - QSKIP("folders on-demand breaks existing tests"); + // QSKIP("folders on-demand breaks existing tests"); FakeFolder fakeFolder{FileInfo{}}; setupVfs(fakeFolder);