Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bugfix/virtual files more reliable #7327

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+11 −73
Open

Bugfix/virtual files more reliable #7327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb43cc0
revert changes to the stale virtual files clean up function
mgallien Oct 16, 2024
bd74f61
prevent out of bounds access in automated tests
mgallien Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 10 additions & 72 deletions src/libsync/discovery.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/discovery.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/discovery.cpp 

File src/libsync/discovery.cpp does not conform to Custom style guidelines. (lines 1313)
* Copyright (C) by Olivier Goffart <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -1303,80 +1303,18 @@
}

auto postProcessLocalNew = [item, localEntry, path, this]() {
// TODO: We may want to execute the same logic for non-VFS mode, as, moving/renaming the same folder by 2 or more clients at the same time is not possible in Web UI.
// Keeping it like this (for VFS files and folders only) just to fix a user issue.

if (!(_discoveryData && _discoveryData->_syncOptions._vfs && _discoveryData->_syncOptions._vfs->mode() != Vfs::Off)) {
// for VFS files and folders only
return;
}

if (!localEntry.isVirtualFile && !localEntry.isDirectory) {
return;
}

if (localEntry.isDirectory && _discoveryData->_syncOptions._vfs->mode() != Vfs::WindowsCfApi) {
// for VFS folders on Windows only
return;
}

Q_ASSERT(item->_instruction == CSYNC_INSTRUCTION_NEW);
if (item->_instruction != CSYNC_INSTRUCTION_NEW) {
qCWarning(lcDisco) << "Trying to wipe a virtual item" << path._local << " with item->_instruction" << item->_instruction;
return;
}

// must be a dehydrated placeholder
const bool isFilePlaceHolder = !localEntry.isDirectory && _discoveryData->_syncOptions._vfs->isDehydratedPlaceholder(_discoveryData->_localDir + path._local);

// either correct availability, or a result with error if the folder is new or otherwise has no availability set yet
const auto folderPlaceHolderAvailability = localEntry.isDirectory ? _discoveryData->_syncOptions._vfs->availability(path._local, Vfs::AvailabilityRecursivity::RecursiveAvailability) : Vfs::AvailabilityResult(Vfs::AvailabilityError::NoSuchItem);

const auto folderPinState = localEntry.isDirectory ? _discoveryData->_syncOptions._vfs->pinState(path._local) : Optional<PinState>(PinState::Unspecified);

if (!isFilePlaceHolder && !folderPlaceHolderAvailability.isValid() && !folderPinState.isValid()) {
// not a file placeholder and not a synced folder placeholder (new local folder)
return;
}

const auto isFolderPinStateOnlineOnly = (folderPinState.isValid() && *folderPinState == PinState::OnlineOnly);

const auto isfolderPlaceHolderAvailabilityOnlineOnly = (folderPlaceHolderAvailability.isValid() && *folderPlaceHolderAvailability == VfsItemAvailability::OnlineOnly);

// a folder is considered online-only if: no files are hydrated, or, if it's an empty folder
const auto isOnlineOnlyFolder = isfolderPlaceHolderAvailabilityOnlineOnly || (!folderPlaceHolderAvailability && isFolderPinStateOnlineOnly);

if (!isFilePlaceHolder && !isOnlineOnlyFolder) {
if (localEntry.isDirectory && folderPlaceHolderAvailability.isValid() && !isOnlineOnlyFolder) {
// a VFS folder but is not online-only (has some files hydrated)
qCInfo(lcDisco) << "Virtual directory without db entry for" << path._local << "but it contains hydrated file(s), so let's keep it and reupload.";
return;
}
qCWarning(lcDisco) << "Virtual file without db entry for" << path._local
<< "but looks odd, keeping";
item->_instruction = CSYNC_INSTRUCTION_IGNORE;

return;
}

if (isOnlineOnlyFolder) {
// if we're wiping a folder, we will only get this function called once and will wipe a folder along with it's files and also display one error in GUI
qCInfo(lcDisco) << "Wiping virtual folder without db entry for" << path._local;
if (isfolderPlaceHolderAvailabilityOnlineOnly && folderPlaceHolderAvailability.isValid()) {
qCInfo(lcDisco) << "*folderPlaceHolderAvailability:" << *folderPlaceHolderAvailability;
}
if (isFolderPinStateOnlineOnly && folderPinState.isValid()) {
qCInfo(lcDisco) << "*folderPinState:" << *folderPinState;
if (localEntry.isVirtualFile) {
const bool isPlaceHolder = _discoveryData->_syncOptions._vfs->isDehydratedPlaceholder(_discoveryData->_localDir + path._local);
if (isPlaceHolder) {
qCWarning(lcDisco) << "Wiping virtual file without db entry for" << path._local;
item->_instruction = CSYNC_INSTRUCTION_REMOVE;
item->_direction = SyncFileItem::Down;
} else {
qCWarning(lcDisco) << "Virtual file without db entry for" << path._local
<< "but looks odd, keeping";
item->_instruction = CSYNC_INSTRUCTION_IGNORE;
}
emit _discoveryData->addErrorToGui(SyncFileItem::SoftError, tr("Conflict when uploading a folder. It's going to get cleared!"), path._local, ErrorCategory::GenericError);
} else {
qCInfo(lcDisco) << "Wiping virtual file without db entry for" << path._local;
emit _discoveryData->addErrorToGui(SyncFileItem::SoftError, tr("Conflict when uploading a file. It's going to get removed!"), path._local, ErrorCategory::GenericError);
}
item->_instruction = CSYNC_INSTRUCTION_REMOVE;
item->_direction = SyncFileItem::Down;
// this flag needs to be unset, otherwise a folder would get marked as new in the processSubJobs
_childModified = false;
};

// Check if it is a move
Expand Down
2 changes: 1 addition & 1 deletion test/syncenginetestutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ FileInfo *FakePutReply::perform(FileInfo &remoteRootFileInfo, const QNetworkRequ
fileInfo->contentChar = putPayload.at(0);
} else {
// Assume that the file is filled with the same character
fileInfo = remoteRootFileInfo.create(fileName, putPayload.size(), putPayload.at(0));
fileInfo = remoteRootFileInfo.create(fileName, putPayload.size(), putPayload.isEmpty() ? ' ' : putPayload.at(0));
}
fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(request.rawHeader("X-OC-Mtime").toLongLong());
remoteRootFileInfo.find(fileName, /*invalidateEtags=*/true);
Expand Down
Loading