Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
7a687ba
fix(quota): protect local files from deletion/move when quota blocked…
camilasan May 7, 2026
f9a8e30
fix(quota): keep error context for user and add tests.
camilasan May 12, 2026
b16c06e
fix(quota): retry upload of files with quota errors on every sync ins…
camilasan May 13, 2026
deb8ca5
test(quota): add retry test for file with quota error protection.
camilasan May 14, 2026
6539361
fix(quota): emit storage full notification files are not yet uploaded.
camilasan May 14, 2026
0148944
fix(quota): persist renamed directory record when children have uploa…
camilasan May 14, 2026
d3f408b
fix(quota): skip rename source candidates when checking for deleted p…
camilasan May 14, 2026
fc91778
fix(quota): protect files blocked from upload when folder and quota e…
camilasan May 14, 2026
cc9e51e
test(quota): add cleanup tests for protection of files blocked from u…
camilasan May 14, 2026
bce31fe
fix(quota): keep protected folder locally after user removes blocked …
camilasan May 17, 2026
3d36f51
fix(quota): emit storage full notification when file protection trigg…
camilasan May 17, 2026
378de44
test(quota): add tests for protection of files after server side fold…
camilasan May 17, 2026
0395c97
fix(quota): preserve InsufficientRemoteStorage blacklist entries acro…
camilasan May 17, 2026
1050a22
fix(quota): scan local files in deleted folders even under ParentNotC…
camilasan May 17, 2026
1559ae3
test(quota): verify protection under DatabaseAndFilesystem on second …
camilasan May 17, 2026
20f1e04
refactor(db): use prepared statements in renameErrorBlacklistPaths.
camilasan May 17, 2026
f63c796
fix(db): skip blacklist path update when no quota entries exist.
camilasan May 17, 2026
fa7f0b5
fix(quota): guard deleted directory local scan against missing direct…
camilasan May 17, 2026
73ca037
refactor(quota): replace _httpErrorCode 507 with _isQuotaError bool f…
camilasan May 17, 2026
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
2 changes: 2 additions & 0 deletions src/common/preparedsqlquerymanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

#pragma once

#include "ocsynclib.h"

Check failure on line 9 in src/common/preparedsqlquerymanager.h

View workflow job for this annotation

GitHub Actions / build

src/common/preparedsqlquerymanager.h:9:10 [clang-diagnostic-error]

'ocsynclib.h' file not found
#include "ownsql.h"
#include "common/asserts.h"

Expand Down Expand Up @@ -99,6 +99,8 @@
FolderUpdateInvalidEncryptionStatus,
FileUpdateInvalidEncryptionStatus,
HasFileIdQuery,
RenameErrorBlacklistCountQuery,
RenameErrorBlacklistUpdateQuery,

PreparedQueryCount
};
Expand Down
52 changes: 49 additions & 3 deletions src/common/syncjournaldb.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/*
* SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2014 ownCloud GmbH
* SPDX-License-Identifier: LGPL-2.1-or-later
*/

#include <QCryptographicHash>

Check failure on line 7 in src/common/syncjournaldb.cpp

View workflow job for this annotation

GitHub Actions / build

src/common/syncjournaldb.cpp:7:10 [clang-diagnostic-error]

'QCryptographicHash' file not found
#include <QFile>
#include <QJsonArray>
#include <QJsonDocument>
Expand Down Expand Up @@ -2171,6 +2171,47 @@
return entry;
}

bool SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString &to)
{
QMutexLocker locker(&_mutex);
if (!checkConnect()) {
return false;
}

const auto countQuery = _queryManager.get(PreparedSqlQueryManager::RenameErrorBlacklistCountQuery,
QByteArrayLiteral("SELECT COUNT(*) FROM blacklist "
"WHERE errorCategory = ?1 "
"AND (path = ?2 OR (path > (?2 || '/') AND path < (?2 || '0')))"),
_db);
if (!countQuery) {
return false;
}

countQuery->bindValue(1, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage);
countQuery->bindValue(2, from);

const bool hasQuotaEntries = countQuery->exec() && countQuery->next().hasData && countQuery->intValue(0) > 0;
if (hasQuotaEntries) {
// Update the exact folder entry and all entries whose path starts with "from/".
// Uses the same range trick as IS_PREFIX_PATH_OR_EQUAL: '/' + 1 == '0'.
const auto query = _queryManager.get(PreparedSqlQueryManager::RenameErrorBlacklistUpdateQuery,
QByteArrayLiteral("UPDATE blacklist "
"SET path = ?2 || substr(path, length(?1) + 1) "
"WHERE path == ?1 OR (path > (?1 || '/') AND path < (?1 || '0'))"),
_db);
if (!query) {
return false;
}
query->bindValue(1, from);
query->bindValue(2, to);
if (!query->exec()) {
sqlFail(QStringLiteral("renameErrorBlacklistPaths"), *query);
}
}

return hasQuotaEntries;
}

bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet<QString> &keep)
{
QMutexLocker locker(&_mutex);
Expand All @@ -2180,7 +2221,7 @@
}

SqlQuery query(_db);
query.prepare("SELECT path FROM blacklist");
query.prepare("SELECT path, errorCategory FROM blacklist");

if (!query.exec()) {
return false;
Expand All @@ -2190,9 +2231,14 @@

while (query.next().hasData) {
const QString file = query.stringValue(0);
if (!keep.contains(file)) {
superfluousPaths.append(file);
const auto errorCategory = static_cast<SyncJournalErrorBlacklistRecord::Category>(query.intValue(1));
// Never prune quota entries: the file was never uploaded and may not appear in
// _syncItems during a ParentNotChanged sync. The entry is removed by blacklistUpdate
// when the upload eventually succeeds, or when the user manually removes the file.
if (keep.contains(file) || errorCategory == SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage) {
continue;
}
superfluousPaths.append(file);
}

SqlQuery delQuery(_db);
Expand Down
1 change: 1 addition & 0 deletions src/common/syncjournaldb.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#ifndef SYNCJOURNALDB_H
#define SYNCJOURNALDB_H

#include <QObject>

Check failure on line 10 in src/common/syncjournaldb.h

View workflow job for this annotation

GitHub Actions / build

src/common/syncjournaldb.h:10:10 [clang-diagnostic-error]

'QObject' file not found
#include <QDateTime>
#include <QHash>
#include <QMutex>
Expand Down Expand Up @@ -145,6 +145,7 @@

SyncJournalErrorBlacklistRecord errorBlacklistEntry(const QString &);
[[nodiscard]] bool deleteStaleErrorBlacklistEntries(const QSet<QString> &keep);
[[nodiscard]] bool renameErrorBlacklistPaths(const QString &from, const QString &to);

/// Delete flags table entries that have no metadata correspondent
void deleteStaleFlagsEntries();
Expand Down
97 changes: 89 additions & 8 deletions src/libsync/discovery.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/*
* SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2018 ownCloud GmbH
Expand Down Expand Up @@ -95,6 +95,20 @@
qCDebug(lcDisco) << "adjusted discovery policy" << _currentFolder._server << _queryServer << _currentFolder._local << _queryLocal;
}
}
// Deleted directories must always scan local files so that checkNewDeleteConflict
// can find and protect quota-blocked files (not in DB) inside them. Under
// ParentNotChanged those files are invisible and would be deleted by the propagator
// along with the folder even though they were never uploaded.
// Only force NormalQuery when the directory actually exists on disk. If it is
// already gone (e.g. deleted by a previous interrupted sync), forcing NormalQuery
// would trigger a fatal "Directory not found" error from startAsyncLocalQuery()
// and abort the sync instead of letting the normal stale-entry cleanup proceed.
if (_queryLocal == ParentNotChanged && _dirItem && _dirItem->_instruction == CSYNC_INSTRUCTION_REMOVE) {
const QString localPath = _discoveryData->_localDir + _currentFolder._local;
if (QDir(localPath).exists()) {
_queryLocal = NormalQuery;
}
}

if (_queryLocal == NormalQuery) {
startAsyncLocalQuery();
Expand Down Expand Up @@ -1059,9 +1073,14 @@
} else {
// we need to make a request to the server to know that the original file is deleted on the server
_pendingAsyncJobs++;
// Mark this path as a pending rename check so that children blocked from upload
// because of quota errors inside a queued deleted-directory job do not cancel the
// parent's REMOVE instruction before rename detection can claim it.
_discoveryData->_pendingRenameSourcePaths.insert(originalPath);
const auto job = new RequestEtagJob(_discoveryData->_account, _discoveryData->_remoteFolder + originalPath, this);
connect(job, &RequestEtagJob::finishedWithResult, this, [=, this](const HttpResult<QByteArray> &etag) mutable {
_pendingAsyncJobs--;
_discoveryData->_pendingRenameSourcePaths.remove(originalPath);
QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs);
if (etag || etag.error().code != 404 ||
// Somehow another item claimed this original path, consider as if it existed
Expand Down Expand Up @@ -1231,6 +1250,10 @@
}

item->_status = SyncFileItem::Status::NormalError;
// Flag as a quota error so blacklistUpdate writes an InsufficientRemoteStorage entry,
// which checkNewDeleteConflict uses to protect the file if its parent folder is later
// deleted on the server before the quota situation is resolved.
item->_isQuotaError = true;
_discoveryData->_anotherSyncNeeded = true;
_discoveryData->_filesNeedingScheduledSync.insert(path._original, delayIntervalForSyncRetryForFilesExceedQuotaSeconds);
}
Expand Down Expand Up @@ -1467,7 +1490,7 @@
return;
}

if (checkNewDeleteConflict(item)) {
if (checkNewDeleteConflict(item, localEntry.size)) {
return;
}

Expand Down Expand Up @@ -2213,6 +2236,18 @@
// Do not remove a directory that has ignored files
qCInfo(lcDisco) << "Child ignored for a folder to remove" << _dirItem->_file << "direction" << _dirItem->_direction;
_dirItem->_instruction = CSYNC_INSTRUCTION_NONE;
// Invalidate the parent directory's etag so the next sync queries it from
// the server instead of using a ParentNotChanged cache hit. Without this, a
// parent whose etag has not changed since the server side deletion uses its DB
// record as a proxy for server state and keeps issuing NONE for this folder.
const auto slashPos = _dirItem->_file.lastIndexOf(QLatin1Char('/'));
const auto parentPath = slashPos >= 0 ? _dirItem->_file.left(slashPos) : QString();
_discoveryData->_statedb->schedulePathForRemoteDiscovery(parentPath.toUtf8());
// Clear the folder's own DB record so the next sync treats it as a new local
// folder rather than a server deleted one. Without this, once all protected files
// inside it are removed by the user, the empty folder would be deleted locally
// on the next sync because the DB record still points to a server side deletion.
_discoveryData->_statedb->deleteFileRecord(_dirItem->_file, false);
}
}
emit finished();
Expand Down Expand Up @@ -2504,18 +2539,64 @@
return result;
}

bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item) const
bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item, int64_t localFileSize)
{
if (_discoveryData->recursiveCheckForDeletedParents(item->_file)) {
qCWarning(lcDisco) << "Removing local file inside a remotely deleted folder" << item->_file;
item->_instruction = CSYNC_INSTRUCTION_REMOVE;
item->_direction = SyncFileItem::Down;
item->_wantsSpecificActions = SyncFileItem::SynchronizationOptions::MoveToClientTrashBin;
if (!_discoveryData->recursiveCheckForDeletedParents(item->_file)) {
return false;
}

// Deleting the local copy could result in permanent data loss if the file was never in the
// server and blocked from being uploaded by a quota error.
// Protect it instead and let the user resolve the storage situation first.
if (const auto blacklistEntry = _discoveryData->_statedb->errorBlacklistEntry(item->_file);
blacklistEntry.isValid()
&& blacklistEntry._errorCategory == SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage) {
qCWarning(lcDisco) << "Not removing local file inside a remotely deleted folder: "
"file was never uploaded due to storage quota —"
<< item->_file;
item->_instruction = CSYNC_INSTRUCTION_ERROR;
item->_status = SyncFileItem::SoftError;
item->_isQuotaError = true;
item->_errorString = tr("%1 could not be removed: it has unsynced changes due to full server storage. "
"Please manage your files and try syncing again.")
.arg(item->_file);
// Prevent the parent directory from being deleted while a file with an error exists.
_childIgnored = true;
emit _discoveryData->itemDiscovered(item);
return true;
}

return false;
// No prior blacklist entry. For files blocked from upload due to a quota error in the same
// sync as the folder deletion, check the parent folder's last known quota from the DB.
if (!item->isDirectory() && localFileSize > 0 && _dirItem) {
SyncJournalFileRecord dirItemDbRecord;
if (_discoveryData->_statedb->getFileRecord(_dirItem->_file, &dirItemDbRecord)
&& dirItemDbRecord.isValid()) {
const auto bytesAvailable = dirItemDbRecord._folderQuota.bytesAvailable;
if (bytesAvailable >= 0 && localFileSize > bytesAvailable) {
qCWarning(lcDisco) << "Not removing local file inside a remotely deleted folder: "
"file would exceed last known parent folder quota —"
<< item->_file;
item->_instruction = CSYNC_INSTRUCTION_ERROR;
item->_status = SyncFileItem::SoftError;
item->_isQuotaError = true;
item->_errorString = tr("\"%1\" was not deleted because its latest changes were not synced "
"and your server quota was exceeded. "
"Please manage your storage and try syncing again.")
.arg(item->_file);
_childIgnored = true;
emit _discoveryData->itemDiscovered(item);
return true;
}
}
}

qCWarning(lcDisco) << "Removing local file inside a remotely deleted folder" << item->_file;
item->_instruction = CSYNC_INSTRUCTION_REMOVE;
item->_direction = SyncFileItem::Down;
item->_wantsSpecificActions = SyncFileItem::SynchronizationOptions::MoveToClientTrashBin;
emit _discoveryData->itemDiscovered(item);
return true;
}

}
2 changes: 1 addition & 1 deletion src/libsync/discovery.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

#pragma once

#include <QObject>

Check failure on line 9 in src/libsync/discovery.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/discovery.h:9:10 [clang-diagnostic-error]

'QObject' file not found
#include <cstdint>
#include "csync_exclude.h"
#include "discoveryphase.h"
Expand Down Expand Up @@ -252,7 +252,7 @@
bool maybeRenameForWindowsCompatibility(const QString &absoluteFileName,
CSYNC_EXCLUDE_TYPE excludeReason);

[[nodiscard]] bool checkNewDeleteConflict(const SyncFileItemPtr &item) const;
[[nodiscard]] bool checkNewDeleteConflict(const SyncFileItemPtr &item, int64_t localFileSize = 0);

qint64 _lastSyncTimestamp = 0;

Expand Down
6 changes: 6 additions & 0 deletions src/libsync/discoveryphase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,12 @@ bool DiscoveryPhase::recursiveCheckForDeletedParents(const QString &itemPath) co
continue;
}

// Async 404 in flight: rename source candidate, not a confirmed deletion.
if (_pendingRenameSourcePaths.contains(currentParentFolder)) {
qCDebug(lcDiscovery()) << "deleted parent is a pending rename candidate, skipping" << currentParentFolder;
continue;
}

qCDebug(lcDiscovery()) << "deleted parent found";
result = true;
break;
Expand Down
8 changes: 8 additions & 0 deletions src/libsync/discoveryphase.h
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,14 @@ class DiscoveryPhase : public QObject
/// contains files/folder names that are requested to be deleted permanently
QSet<QString> _permanentDeletionRequests;

/** Paths with an async 404 in flight to confirm server deletion.
*
* While a path is here, children blocked from upload because of quota errors must not
* prevent the parent folder from being removed. Rename detection takes priority and
* will claim the path once the 404 confirms deletion.
*/
QSet<QString> _pendingRenameSourcePaths;

void markPermanentDeletionRequests();

public:
Expand Down
25 changes: 22 additions & 3 deletions src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/*
* SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2013 ownCloud GmbH
Expand Down Expand Up @@ -158,8 +158,11 @@
entry._ignoreDuration = 0;
}

if (item._httpErrorCode == 507) {
if (item._isQuotaError || item._httpErrorCode == 507) {

Check warning on line 161 in src/libsync/owncloudpropagator.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/owncloudpropagator.cpp:161:54 [cppcoreguidelines-avoid-magic-numbers]

507 is a magic number; consider replacing it with a named constant
entry._errorCategory = SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage;
// Quota can change at any time (user frees space, admin adjusts limits).
// Always retry on the next sync rather than backing off exponentially.
entry._ignoreDuration = 0;
}

return entry;
Expand All @@ -174,11 +177,11 @@
SyncJournalErrorBlacklistRecord oldEntry = journal->errorBlacklistEntry(item._file);

bool mayBlacklist =
item._errorMayBeBlacklisted // explicitly flagged for blacklisting
item._isQuotaError // quota errors always get an InsufficientRemoteStorage entry
|| ((item._status == SyncFileItem::NormalError
|| item._status == SyncFileItem::SoftError
|| item._status == SyncFileItem::DetailError)
&& item._httpErrorCode != 0 // or non-local error
&& item._httpErrorCode != 0 // non-local error
);

// No new entry? Possibly remove the old one, then done.
Expand Down Expand Up @@ -1586,6 +1589,22 @@
}
}
}

// For a DOWN rename where the directory itself succeeded (PropagateLocalRename
// ran OK) but one or more children had errors, still persist the renamed
// directory's path and fileId in the database. Without this record a
// subsequent server side rename of the same directory cannot be matched via
// fileId lookup, causing the client to treat it as a DELETE + NEW instead of
// following the move.
if (!_item->isEmpty()
&& status != SyncFileItem::Success
&& status != SyncFileItem::FatalError
&& _item->_status == SyncFileItem::Success
&& _item->_instruction == CSYNC_INSTRUCTION_RENAME
&& _item->_direction == SyncFileItem::Down) {
propagator()->updateMetadata(*_item);
}

_state = Finished;
qCDebug(lcDirectory()) << "PropagateDirectory::slotSubJobsFinished" << "emit finished" << status;
emit finished(status);
Expand Down
3 changes: 3 additions & 0 deletions src/libsync/propagateremotemove.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,9 @@ void PropagateRemoteMove::finalize()
done(SyncFileItem::FatalError, tr("Error writing metadata to the database"), ErrorCategory::GenericError);
return;
}
if (propagator()->_journal->renameErrorBlacklistPaths(_item->_file, _item->_renameTarget)) {
emit propagator()->insufficientRemoteStorage();
}
}

propagator()->_journal->commit("Remote Rename");
Expand Down
3 changes: 3 additions & 0 deletions src/libsync/propagatorjobs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,9 @@ void PropagateLocalRename::start()
done(SyncFileItem::FatalError, tr("Failed to rename file"), ErrorCategory::GenericError);
return;
}
if (propagator()->_journal->renameErrorBlacklistPaths(oldFile, _item->_renameTarget)) {
emit propagator()->insufficientRemoteStorage();
}
}
if (pinState != PinState::Inherited && !vfs->setPinState(_item->_renameTarget, pinState)) {
done(SyncFileItem::NormalError, tr("Error setting pin state"), ErrorCategory::GenericError);
Expand Down
20 changes: 20 additions & 0 deletions src/libsync/syncengine.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/*
* SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2014 ownCloud GmbH
Expand Down Expand Up @@ -143,6 +143,16 @@

item._hasBlacklistEntry = true;

// Discovery already produced a deliberate error for items protected in checkNewDeleteConflict.
// Keep its instruction and message intact so the specific reason is displayed.
// Still show the quota notification if applicable.
if (item._instruction == CSYNC_INSTRUCTION_ERROR) {
if (entry._errorCategory == SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage) {
slotInsufficientRemoteStorage();
}
return true;
}

// If duration has expired, it's not blacklisted anymore
time_t now = Utility::qDateTimeToTime_t(QDateTime::currentDateTimeUtc());
if (now >= entry._lastTryTime + entry._ignoreDuration) {
Expand Down Expand Up @@ -471,6 +481,16 @@
// check for blacklisting of this item.
// if the item is on blacklist, the instruction was set to ERROR
checkErrorBlacklisting(*item);

// When discovery protects a file from deletion because of a quota error, emit the storage
// full notification so the sync status shows as error. checkErrorBlacklisting only does this
// when a blacklist entry already exists; in the same sync as the folder deletion there is no
// entry yet, so we check here unconditionally.
if (item->_instruction == CSYNC_INSTRUCTION_ERROR
&& item->_httpErrorCode == 507) {
slotInsufficientRemoteStorage();
}

_needsUpdate = true;

// Insert sorted
Expand Down
Loading
Loading