From b508b84610d649778bdda133b41846f2cc89cc41 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 7 May 2026 23:41:42 +0200 Subject: [PATCH 01/21] fix(quota): protect local files from deletion/move when quota blocked upload. If a file failed to upload because the server quota was exceeded (HTTP 507), it was never actually stored on the server but the sync client could still delete or the local copy when the remote parent folder was later moved or deleted via the web interface. - checkNewDeleteConflict() now checks the error blacklist before issuing a local REMOVE. If the file carries an InsufficientRemoteStorage entry, it is kept locally and reported as a soft error instead. - The same _httpErrorCode = 507 is now also set in the discovery-phase quota check. - renameErrorBlacklistPaths() updates all blacklist entries under the renamed path at move propagation time, keeping the InsufficientRemoteStorage entry aligned with the file's actual location across rename cycles. - Add renameErrorBlacklistPaths call to PropagateLocalRename. Without it, quota blocked files in a server renamed folder still lost their InsufficientRemoteStorage entry, leaving them unprotected when the renamed folder was subsequently deleted. - Add tests. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/common/syncjournaldb.cpp | 20 ++++ src/common/syncjournaldb.h | 1 + src/libsync/discovery.cpp | 42 ++++++-- src/libsync/discovery.h | 2 +- src/libsync/propagateremotemove.cpp | 1 + src/libsync/propagatorjobs.cpp | 1 + test/testsyncdelete.cpp | 146 +++++++++++++++++++++++++++- 7 files changed, 204 insertions(+), 9 deletions(-) diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index ab99460a0f2a6..4c25ef54e1029 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -2171,6 +2171,26 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry(const QString return entry; } +void SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString &to) +{ + QMutexLocker locker(&_mutex); + if (!checkConnect()) { + return; + } + + // 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'. + SqlQuery query(_db); + query.prepare("UPDATE blacklist " + "SET path = ?2 || substr(path, length(?1) + 1) " + "WHERE path == ?1 OR (path > (?1 || '/') AND path < (?1 || '0'))"); + query.bindValue(1, from); + query.bindValue(2, to); + if (!query.exec()) { + sqlFail(QStringLiteral("renameErrorBlacklistPaths"), query); + } +} + bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet &keep) { QMutexLocker locker(&_mutex); diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h index 8cd868e1f0b71..36db279d68665 100644 --- a/src/common/syncjournaldb.h +++ b/src/common/syncjournaldb.h @@ -145,6 +145,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject SyncJournalErrorBlacklistRecord errorBlacklistEntry(const QString &); [[nodiscard]] bool deleteStaleErrorBlacklistEntries(const QSet &keep); + void renameErrorBlacklistPaths(const QString &from, const QString &to); /// Delete flags table entries that have no metadata correspondent void deleteStaleFlagsEntries(); diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 394a7b22fd2bb..f8b02f1012304 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1231,6 +1231,11 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( } item->_status = SyncFileItem::Status::NormalError; + // Mark as a quota error so blacklistUpdate writes an InsufficientRemoteStorage entry. + // Without this, _httpErrorCode would be 0 and blacklistUpdate would not create an + // entry, leaving the file unprotected by checkNewDeleteConflict if the remote parent + // folder is deleted before the quota situation is resolved. + item->_httpErrorCode = 507; _discoveryData->_anotherSyncNeeded = true; _discoveryData->_filesNeedingScheduledSync.insert(path._original, delayIntervalForSyncRetryForFilesExceedQuotaSeconds); } @@ -2504,18 +2509,41 @@ bool ProcessDirectoryJob::maybeRenameForWindowsCompatibility(const QString &abso return result; } -bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item) const +bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item) { - 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; + // Keep _httpErrorCode at 507 so blacklistUpdate recreates the InsufficientRemoteStorage + // entry if the current ignore-duration later expires. + item->_httpErrorCode = 507; + 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; + 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; } } diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h index f59cf75250863..f0ee7516a2ab3 100644 --- a/src/libsync/discovery.h +++ b/src/libsync/discovery.h @@ -252,7 +252,7 @@ class ProcessDirectoryJob : public QObject bool maybeRenameForWindowsCompatibility(const QString &absoluteFileName, CSYNC_EXCLUDE_TYPE excludeReason); - [[nodiscard]] bool checkNewDeleteConflict(const SyncFileItemPtr &item) const; + [[nodiscard]] bool checkNewDeleteConflict(const SyncFileItemPtr &item); qint64 _lastSyncTimestamp = 0; diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 8fd55d43c3dd8..087f8b01a9a59 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -305,6 +305,7 @@ void PropagateRemoteMove::finalize() done(SyncFileItem::FatalError, tr("Error writing metadata to the database"), ErrorCategory::GenericError); return; } + propagator()->_journal->renameErrorBlacklistPaths(_item->_file, _item->_renameTarget); } propagator()->_journal->commit("Remote Rename"); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 8346916544b55..4c0f9bba83022 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -561,6 +561,7 @@ void PropagateLocalRename::start() done(SyncFileItem::FatalError, tr("Failed to rename file"), ErrorCategory::GenericError); return; } + propagator()->_journal->renameErrorBlacklistPaths(oldFile, _item->_renameTarget); } if (pinState != PinState::Inherited && !vfs->setPinState(_item->_renameTarget, pinState)) { done(SyncFileItem::NormalError, tr("Error setting pin state"), ErrorCategory::GenericError); diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index e7f075bd89d7e..0b251c1951605 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -2,7 +2,7 @@ * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors * SPDX-FileCopyrightText: 2018 ownCloud, Inc. * SPDX-License-Identifier: CC0-1.0 - * + * * This software is in the public domain, furnished "as is", without technical * support, and with no warranty, express or implied, as to its usefulness for * any purpose. @@ -52,6 +52,150 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + // Verify that a file blocked from uploading by a remote storage quota error is + // protected from local deletion when the remote parent folder is subsequently deleted. + void testQuotaBlockedFileProtectedFromParentFolderDeletion() + { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + + // Establish an initial clean sync: a1 and a2 exist on both sides. + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Add a new local file that will fail to upload due to remote quota (HTTP 507). + fakeFolder.localModifier().insert(QStringLiteral("A/quota_blocked.txt"), 100); + fakeFolder.serverErrorPaths().append(QStringLiteral("A/quota_blocked.txt"), 507); + + // Upload fails so file is blacklisted as InsufficientRemoteStorage. + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/quota_blocked.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + + // The file was never uploaded. + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("A/quota_blocked.txt"))); + + // Remove the server error so further requests to that path don't return 507. + fakeFolder.serverErrorPaths().clear(); + + // Server side: folder A is moved/deleted. + fakeFolder.remoteModifier().remove(QStringLiteral("A")); + + ItemCompletedSpy completeSpy(fakeFolder); + fakeFolder.syncOnce(); + + // File that does not exist in the server and is blocked from upload with error must not have been deleted locally. + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/quota_blocked.txt"))); + + // Parent folder A must also survive because it still contains the protected file. + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A"))); + + // Previously synced siblings must have been deleted locally (trust the server). + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/a1"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/a2"))); + + // File that does not exist in the server and has error must still not exist on the server. + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("A/quota_blocked.txt"))); + + // The protected file must be reported as an error item with the quota specific message, + // not silently ignored or overwritten with a generic blacklist message. + { + auto item = completeSpy.findItem(QStringLiteral("A/quota_blocked.txt")); + QVERIFY(item); + const bool isErrorItem = item->_instruction == CSYNC_INSTRUCTION_ERROR + || item->_instruction == CSYNC_INSTRUCTION_IGNORE; // BlacklistedError reuses IGNORE + QVERIFY(isErrorItem); + const bool isErrorStatus = item->_status == SyncFileItem::SoftError + || item->_status == SyncFileItem::BlacklistedError; + QVERIFY(isErrorStatus); + } + } + + // A quota upload blocked file must survive even if the parent folder is first moved then deleted on the server. + void testQuotaBlockedFileProtectedAfterParentFolderMoveThenDelete() + { + // Use a custom initial state with only folder A to avoid rename collisions. + FileInfo initialState{QString{}, { + {QStringLiteral("A"), { + {QStringLiteral("a1"), 4}, + {QStringLiteral("a2"), 4}, + }}, + }}; + FakeFolder fakeFolder{initialState}; + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // New local file that fails to upload due to quota. + fakeFolder.localModifier().insert(QStringLiteral("A/quota_blocked.txt"), 100); + fakeFolder.serverErrorPaths().append(QStringLiteral("A/quota_blocked.txt"), 507); + + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/quota_blocked.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("A/quota_blocked.txt"))); + + // Switch the quota error to the new path so the file remains blocked after the rename. + fakeFolder.serverErrorPaths().clear(); + fakeFolder.serverErrorPaths().append(QStringLiteral("D/quota_blocked.txt"), 507); + + // Server side: folder A is renamed to D. The quota blocked file was never on the + // server, so the blacklist entry path is still "A/quota_blocked.txt" while the + // local file moves to "D/" as the client follows the rename. + fakeFolder.remoteModifier().rename(QStringLiteral("A"), QStringLiteral("D")); + // Sync: local A is renamed to D; any upload attempt for D/quota_blocked.txt fails with 507. + fakeFolder.syncOnce(); + + // Blacklist entry must have been updated to the new path. + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("D/quota_blocked.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("D/quota_blocked.txt"))); + + // Server side: now delete folder D. + fakeFolder.serverErrorPaths().clear(); + fakeFolder.remoteModifier().remove(QStringLiteral("D")); + ItemCompletedSpy completeSpy(fakeFolder); + fakeFolder.syncOnce(); + + // File must not have been deleted locally. + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("D/quota_blocked.txt"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("D"))); + + // Previously synced siblings must be gone. + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("D/a1"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("D/a2"))); + + // The protected file must be reported with the quota specific error, not silently ignored. + { + const auto item = completeSpy.findItem(QStringLiteral("D/quota_blocked.txt")); + QVERIFY(item); + QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_ERROR); + QVERIFY(!item->_errorString.contains(QStringLiteral("skipped due to earlier error"))); + } + } + + // Regression: new files in a server deleted folder must still be deleted locally + void testDeleteDirectoryWithNewFileNoQuotaError() + { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + fakeFolder.remoteModifier().remove(QStringLiteral("A")); + fakeFolder.localModifier().insert(QStringLiteral("A/newfile.txt"), 100); + + QVERIFY(fakeFolder.syncOnce()); + + // New local file with no quota blacklist entry must be removed when parent is deleted on server. + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/newfile.txt"))); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + } + void issue1329() { FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; From a4fc8a0dc54ffb874c44d0b74621dcb573364061 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Tue, 12 May 2026 23:08:10 +0200 Subject: [PATCH 02/21] fix(quota): keep error context for user and add tests. When a local file is blocked from uploading due to remote quota (HTTP 507) and the parent folder is subsequently deleted on the server, checkErrorBlacklisting() in would overwrite the deliberate CSYNC_INSTRUCTION_ERROR set by checkNewDeleteConflict() with a generic CSYNC_INSTRUCTION_IGNORE so the user would loose the error message with the reason for the permanence of some files. Also update tests. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/discovery.cpp | 11 +++++++++-- src/libsync/syncengine.cpp | 10 ++++++++++ test/testsyncdelete.cpp | 21 ++++++++------------- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index f8b02f1012304..92662e7c0dc71 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -2218,6 +2218,13 @@ int ProcessDirectoryJob::processSubJobs(int nbJobs) // 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()); } } emit finished(); @@ -2526,8 +2533,8 @@ bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item) << item->_file; item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_status = SyncFileItem::SoftError; - // Keep _httpErrorCode at 507 so blacklistUpdate recreates the InsufficientRemoteStorage - // entry if the current ignore-duration later expires. + // Keep _httpErrorCode at 507 so blacklistUpdate recreates the InsufficientRemoteStorage + // entry if the ignore duration later expires. item->_httpErrorCode = 507; 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.") diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 8c754d3eac718..edfb82c9b2df1 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -143,6 +143,16 @@ bool SyncEngine::checkErrorBlacklisting(SyncFileItem &item) 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) { diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 0b251c1951605..e11d4c7e02609 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -52,8 +52,7 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } - // Verify that a file blocked from uploading by a remote storage quota error is - // protected from local deletion when the remote parent folder is subsequently deleted. + // Files blocked from upload because of quota errors must survive remote parent folder deletion. void testQuotaBlockedFileProtectedFromParentFolderDeletion() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; @@ -86,7 +85,7 @@ private slots: ItemCompletedSpy completeSpy(fakeFolder); fakeFolder.syncOnce(); - // File that does not exist in the server and is blocked from upload with error must not have been deleted locally. + // File must survive local deletion when its parent folder is removed on the server. QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/quota_blocked.txt"))); // Parent folder A must also survive because it still contains the protected file. @@ -96,24 +95,20 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/a1"))); QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/a2"))); - // File that does not exist in the server and has error must still not exist on the server. + // File must remain absent on the server. QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("A/quota_blocked.txt"))); - // The protected file must be reported as an error item with the quota specific message, - // not silently ignored or overwritten with a generic blacklist message. + // Must be reported as an error with the quota message, not silently ignored. { auto item = completeSpy.findItem(QStringLiteral("A/quota_blocked.txt")); QVERIFY(item); - const bool isErrorItem = item->_instruction == CSYNC_INSTRUCTION_ERROR - || item->_instruction == CSYNC_INSTRUCTION_IGNORE; // BlacklistedError reuses IGNORE - QVERIFY(isErrorItem); - const bool isErrorStatus = item->_status == SyncFileItem::SoftError - || item->_status == SyncFileItem::BlacklistedError; - QVERIFY(isErrorStatus); + QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_ERROR); + QVERIFY(item->_status == SyncFileItem::SoftError || item->_status == SyncFileItem::NormalError); + QVERIFY(!item->_errorString.contains(QStringLiteral("skipped due to earlier error"))); } } - // A quota upload blocked file must survive even if the parent folder is first moved then deleted on the server. + // Files blocked from upload because of quota errors must survive parent folder rename then delete. void testQuotaBlockedFileProtectedAfterParentFolderMoveThenDelete() { // Use a custom initial state with only folder A to avoid rename collisions. From 6df87706fce0e0c5b77612de15110105ad4fdc39 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 14 May 2026 01:15:28 +0200 Subject: [PATCH 03/21] fix(quota): retry upload of files with quota errors on every sync instead of stopping. When an upload fails with HTTP 507 (Insufficient Storage), the blacklist entry was given a growing _ignoreDuration, causing the file to be silently ignored on subsequent syncs even after the user freed storage quota. Set _ignoreDuration = 0 for InsufficientRemoteStorage entries so that the file retries on every sync cycle. The blacklist entry is still written (checkNewDeleteConflict can still see it to protect the file from accidental local deletion), but checkErrorBlacklisting treats duration 0 entries as immediately expired and lets the upload proceed. Update testQuotaBlockedFileProtectedAfterParentFolderMoveThenDelete to keep the 507 error active during the delete parent sync so the test verifies the correct new behavior: the file is reported as a failed new upload (DetailError) rather than a silently-skipped BlacklistedError. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/owncloudpropagator.cpp | 3 +++ test/testsyncdelete.cpp | 13 +++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index b0a2b1a94c9d4..4ed72ba2c6df0 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -160,6 +160,9 @@ static SyncJournalErrorBlacklistRecord createBlacklistEntry( if (item._httpErrorCode == 507) { 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; diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index e11d4c7e02609..008fa253ccce4 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -154,8 +154,10 @@ private slots: } QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("D/quota_blocked.txt"))); - // Server side: now delete folder D. - fakeFolder.serverErrorPaths().clear(); + // Server side: now delete folder D. Keep the quota error active so we verify + // that the file retries (and fails with a quota error) rather than being silently + // skipped by the blacklist backoff timer. + fakeFolder.serverErrorPaths().append(QStringLiteral("D/quota_blocked.txt"), 507); fakeFolder.remoteModifier().remove(QStringLiteral("D")); ItemCompletedSpy completeSpy(fakeFolder); fakeFolder.syncOnce(); @@ -168,11 +170,14 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("D/a1"))); QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("D/a2"))); - // The protected file must be reported with the quota specific error, not silently ignored. + // The file must be reported as a failed upload attempt, not silently ignored. + // With _ignoreDuration = 0 for quota errors, the file retries every sync and + // produces a real quota error instead of a "skipped due to earlier error" message. { const auto item = completeSpy.findItem(QStringLiteral("D/quota_blocked.txt")); QVERIFY(item); - QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_ERROR); + QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_NEW); + QVERIFY(item->_status == SyncFileItem::DetailError || item->_status == SyncFileItem::NormalError); QVERIFY(!item->_errorString.contains(QStringLiteral("skipped due to earlier error"))); } } From f3ef8439c8461112b125c49f6698de8c5dc0ad65 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 14 May 2026 11:51:58 +0200 Subject: [PATCH 04/21] test(quota): add retry test for file with quota error protection. Verifies that setting _ignoreDuration = 0 for HTTP 507 errors causes the file to retry and upload successfully on the very next sync once quota is freed, rather than waiting out an exponential backoff. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- test/testsyncdelete.cpp | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 008fa253ccce4..eafbb5990b9a7 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -196,6 +196,43 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + // Files blocked from upload because of quota errors must retry on the next sync once quota is freed. + void testQuotaBlockedFileRetriesWhenQuotaResolved() + { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Add a new local file that will fail to upload due to quota (HTTP 507). + fakeFolder.localModifier().insert(QStringLiteral("A/quota_blocked.txt"), 100); + fakeFolder.serverErrorPaths().append(QStringLiteral("A/quota_blocked.txt"), 507); + + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/quota_blocked.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + // Must be 0 so the file retries on the very next sync rather than backing off. + QCOMPARE(entry._ignoreDuration, 0); + } + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("A/quota_blocked.txt"))); + + // Quota freed, remove the server error. + fakeFolder.serverErrorPaths().clear(); + + // Next sync: file must retry immediately (ignoreDuration = 0) and upload. + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentRemoteState().find(QStringLiteral("A/quota_blocked.txt"))); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Blacklist entry must be cleared after a successful upload. + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/quota_blocked.txt")); + QVERIFY(!entry.isValid()); + } + } + void issue1329() { FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; From a1b40cb455e02e7eaaa8bdff72a6a342016c941d Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 14 May 2026 18:14:53 +0200 Subject: [PATCH 05/21] fix(quota): emit storage full notification files are not yet uploaded. When a folder containing files that failed to upload because of quota errors was renamed on the server, the sync client correctly followed the rename but emitted no storage notification for that sync cycle. During discovery the files were not rediscovered as local items because the local folder still had the old name; the notification only reappeared on the following sync when the upload was reattempted and the server returned 507 again. Make renameErrorBlacklistPaths return true when it renames one or more InsufficientRemoteStorage entries. Callers in PropagateLocalRename and PropagateRemoteMove emit insufficientRemoteStorage immediately so the storage full message is shown in the same sync cycle as the rename. Add testStorageNotificationEmittedOnParentFolderRename to verify that the syncError signal with InsufficientRemoteStorage is emitted during the rename sync, not only on a subsequent retry. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/common/syncjournaldb.cpp | 14 ++++++- src/common/syncjournaldb.h | 2 +- src/libsync/propagateremotemove.cpp | 4 +- src/libsync/propagatorjobs.cpp | 4 +- test/testsyncdelete.cpp | 63 ++++++++++++++++++++++++----- 5 files changed, 71 insertions(+), 16 deletions(-) diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index 4c25ef54e1029..32d5483da23ef 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -2171,13 +2171,21 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry(const QString return entry; } -void SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString &to) +bool SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString &to) { QMutexLocker locker(&_mutex); if (!checkConnect()) { - return; + return false; } + SqlQuery countQuery(_db); + countQuery.prepare("SELECT COUNT(*) FROM blacklist " + "WHERE errorCategory = ?1 " + "AND (path = ?2 OR (path > (?2 || '/') AND path < (?2 || '0')))"); + countQuery.bindValue(1, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + countQuery.bindValue(2, from); + const bool hasQuotaEntries = countQuery.exec() && countQuery.next().hasData && countQuery.intValue(0) > 0; + // 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'. SqlQuery query(_db); @@ -2189,6 +2197,8 @@ void SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString if (!query.exec()) { sqlFail(QStringLiteral("renameErrorBlacklistPaths"), query); } + + return hasQuotaEntries; } bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet &keep) diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h index 36db279d68665..c2106e16a4579 100644 --- a/src/common/syncjournaldb.h +++ b/src/common/syncjournaldb.h @@ -145,7 +145,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject SyncJournalErrorBlacklistRecord errorBlacklistEntry(const QString &); [[nodiscard]] bool deleteStaleErrorBlacklistEntries(const QSet &keep); - void renameErrorBlacklistPaths(const QString &from, const QString &to); + [[nodiscard]] bool renameErrorBlacklistPaths(const QString &from, const QString &to); /// Delete flags table entries that have no metadata correspondent void deleteStaleFlagsEntries(); diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 087f8b01a9a59..fd9d60ee1a235 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -305,7 +305,9 @@ void PropagateRemoteMove::finalize() done(SyncFileItem::FatalError, tr("Error writing metadata to the database"), ErrorCategory::GenericError); return; } - propagator()->_journal->renameErrorBlacklistPaths(_item->_file, _item->_renameTarget); + if (propagator()->_journal->renameErrorBlacklistPaths(_item->_file, _item->_renameTarget)) { + emit propagator()->insufficientRemoteStorage(); + } } propagator()->_journal->commit("Remote Rename"); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 4c0f9bba83022..b9a27f7ce6ffd 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -561,7 +561,9 @@ void PropagateLocalRename::start() done(SyncFileItem::FatalError, tr("Failed to rename file"), ErrorCategory::GenericError); return; } - propagator()->_journal->renameErrorBlacklistPaths(oldFile, _item->_renameTarget); + 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); diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index eafbb5990b9a7..225474e87a917 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -139,9 +139,8 @@ private slots: fakeFolder.serverErrorPaths().clear(); fakeFolder.serverErrorPaths().append(QStringLiteral("D/quota_blocked.txt"), 507); - // Server side: folder A is renamed to D. The quota blocked file was never on the - // server, so the blacklist entry path is still "A/quota_blocked.txt" while the - // local file moves to "D/" as the client follows the rename. + // Server renames A to D. The blacklist entry stays at "A/quota_blocked.txt" until + // the next sync updates it; the local file follows to "D/". fakeFolder.remoteModifier().rename(QStringLiteral("A"), QStringLiteral("D")); // Sync: local A is renamed to D; any upload attempt for D/quota_blocked.txt fails with 507. fakeFolder.syncOnce(); @@ -154,9 +153,7 @@ private slots: } QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("D/quota_blocked.txt"))); - // Server side: now delete folder D. Keep the quota error active so we verify - // that the file retries (and fails with a quota error) rather than being silently - // skipped by the blacklist backoff timer. + // Server deletes folder D. Keep quota error active to verify the file retries. fakeFolder.serverErrorPaths().append(QStringLiteral("D/quota_blocked.txt"), 507); fakeFolder.remoteModifier().remove(QStringLiteral("D")); ItemCompletedSpy completeSpy(fakeFolder); @@ -170,14 +167,12 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("D/a1"))); QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("D/a2"))); - // The file must be reported as a failed upload attempt, not silently ignored. - // With _ignoreDuration = 0 for quota errors, the file retries every sync and - // produces a real quota error instead of a "skipped due to earlier error" message. + // Must be reported as a failed upload, not silently ignored. { const auto item = completeSpy.findItem(QStringLiteral("D/quota_blocked.txt")); QVERIFY(item); - QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_NEW); - QVERIFY(item->_status == SyncFileItem::DetailError || item->_status == SyncFileItem::NormalError); + QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_ERROR); + QVERIFY(item->_status == SyncFileItem::SoftError || item->_status == SyncFileItem::NormalError); QVERIFY(!item->_errorString.contains(QStringLiteral("skipped due to earlier error"))); } } @@ -233,6 +228,52 @@ private slots: } } + // The storage full notification must fire in the same sync as a folder rename, not only when the upload is retried. + void testStorageNotificationEmittedOnParentFolderRename() + { + FileInfo initialState{QString{}, { + {QStringLiteral("A"), { + {QStringLiteral("small.txt"), 4}, + }}, + }}; + FakeFolder fakeFolder{initialState}; + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Block big.txt from uploading permanently (quota exceeded). + fakeFolder.localModifier().insert(QStringLiteral("A/big.txt"), 1000); + fakeFolder.setServerOverride([](QNetworkAccessManager::Operation op, + const QNetworkRequest &req, + QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::PutOperation + && req.url().path().contains(QLatin1String("big.txt"))) { + return new FakeErrorReply{op, req, nullptr, 507}; + } + return nullptr; + }); + + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + + // Server renames A to B. The storage full notification must fire in this sync, + // not only after the next retry of the upload. + fakeFolder.remoteModifier().rename(QStringLiteral("A"), QStringLiteral("B")); + QSignalSpy storageSpy(&fakeFolder.syncEngine(), &SyncEngine::syncError); + fakeFolder.syncOnce(); + + const bool hadStorageNotification = std::any_of(storageSpy.begin(), storageSpy.end(), + [](const QList &args) { + return args.at(1).value() == ErrorCategory::InsufficientRemoteStorage; + }); + QVERIFY(hadStorageNotification); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/big.txt"))); + } + void issue1329() { FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; From 48259230217d91ee5641d1920b68af6956813bde Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 14 May 2026 18:15:04 +0200 Subject: [PATCH 06/21] fix(quota): persist renamed directory record when children have upload errors. When a server side folder rename is applied locally, PropagateDirectory only wrote the renamed directory's record in the sync journal on a full Success result. If a child upload failed the directory record was never written, so the next sync could not match the directory by fileId and treated a follow up server side rename as DELETE + NEW instead of following the move. Write the directory metadata record for DOWN renames even when child jobs have errors, as long as the directory rename job itself succeeded. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/owncloudpropagator.cpp | 16 +++++++ test/testsyncdelete.cpp | 67 ++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 4ed72ba2c6df0..fdd6e5550cbf6 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -1589,6 +1589,22 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) } } } + + // 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); diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 225474e87a917..97f5cbbc1d52d 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -191,6 +191,73 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + // Files blocked from upload because of quota errors must follow their parent folder through + // multiple successive server side renames. + void testQuotaBlockedFileFollowsParentFolderMove() + { + // Initial state: folder A with small.txt and empty sibling B. + FileInfo initialState{QString{}, { + {QStringLiteral("A"), { + {QStringLiteral("small.txt"), 4}, + }}, + {QStringLiteral("B"), {}}, + }}; + FakeFolder fakeFolder{initialState}; + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Block big.txt from uploading permanently (507). + fakeFolder.localModifier().insert(QStringLiteral("A/big.txt"), 1000); + fakeFolder.setServerOverride([](QNetworkAccessManager::Operation op, + const QNetworkRequest &req, + QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::PutOperation + && req.url().path().contains(QLatin1String("big.txt"))) { + return new FakeErrorReply{op, req, nullptr, 507}; + } + return nullptr; + }); + + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("A/big.txt"))); + + // Server: move A into B (server now has B/A). Client must follow the rename. + fakeFolder.remoteModifier().rename(QStringLiteral("A"), QStringLiteral("B/A")); + fakeFolder.syncOnce(); + + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A/big.txt"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A/small.txt"))); + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("B/A/big.txt"))); + + // Server: rename B/A to B/C. + fakeFolder.remoteModifier().rename(QStringLiteral("B/A"), QStringLiteral("B/C")); + fakeFolder.syncOnce(); + + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/C"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("B/A"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/C/big.txt"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/C/small.txt"))); + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("B/C/big.txt"))); + + // Server: move B/C to root (C). Client must follow without creating a ghost B/C. + fakeFolder.remoteModifier().rename(QStringLiteral("B/C"), QStringLiteral("C")); + fakeFolder.syncOnce(); + + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("C/big.txt"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("B/C"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("B/C/big.txt"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("C/small.txt"))); + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("C/big.txt"))); + } + // Files blocked from upload because of quota errors must retry on the next sync once quota is freed. void testQuotaBlockedFileRetriesWhenQuotaResolved() { From 6e388cb40927173f2af8e3f24a87ef7cabc9a940 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 14 May 2026 18:15:14 +0200 Subject: [PATCH 07/21] fix(quota): skip rename source candidates when checking for deleted parents. When discovery issues an async 404 to verify whether a path was deleted on the server (a step in rename detection), children blocked from upload because of quota errors inside a queued delete directory job could fire recursiveCheckForDeletedParents and cancel the parent's REMOVE instruction before the 404 response arrived and rename detection could claim the path. Track paths that have an async 404 in flight in _pendingRenameSourcePaths and skip them in recursiveCheckForDeletedParents so rename detection is not interrupted by quota-protected children. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/discovery.cpp | 5 +++++ src/libsync/discoveryphase.cpp | 6 ++++++ src/libsync/discoveryphase.h | 8 ++++++++ 3 files changed, 19 insertions(+) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 92662e7c0dc71..d3a9a357eb92b 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1059,9 +1059,14 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it } 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 &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 diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 602ded62931ba..ab0b136cf3e31 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -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; diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 5c076afd54aed..30c8c241cbd6f 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -283,6 +283,14 @@ class DiscoveryPhase : public QObject /// contains files/folder names that are requested to be deleted permanently QSet _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 _pendingRenameSourcePaths; + void markPermanentDeletionRequests(); public: From 4741a1418ee284983493aa4539a35d08b174095f Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 14 May 2026 22:39:22 +0200 Subject: [PATCH 08/21] fix(quota): protect files blocked from upload when folder and quota error occur in same sync. When a file blocked from upload due to a quota error has no prior blacklist entry (upload was never attempted), fall back to the parent folder's last known quota from the DB to decide whether to protect the file from local deletion when its remote parent is deleted. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/discovery.cpp | 31 +++++++++++++++++++++++++--- src/libsync/discovery.h | 2 +- test/testsyncdelete.cpp | 43 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index d3a9a357eb92b..352a3bcef0e3c 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1477,7 +1477,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( return; } - if (checkNewDeleteConflict(item)) { + if (checkNewDeleteConflict(item, localEntry.size)) { return; } @@ -2521,13 +2521,13 @@ bool ProcessDirectoryJob::maybeRenameForWindowsCompatibility(const QString &abso return result; } -bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item) +bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item, int64_t localFileSize) { if (!_discoveryData->recursiveCheckForDeletedParents(item->_file)) { return false; } - // Deleting the local copy could result in permanent data loss if the file was never in the + // 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); @@ -2550,6 +2550,31 @@ bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item) return true; } + // 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->_httpErrorCode = 507; + 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; diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h index f0ee7516a2ab3..4cdb4a0b55906 100644 --- a/src/libsync/discovery.h +++ b/src/libsync/discovery.h @@ -252,7 +252,7 @@ class ProcessDirectoryJob : public QObject bool maybeRenameForWindowsCompatibility(const QString &absoluteFileName, CSYNC_EXCLUDE_TYPE excludeReason); - [[nodiscard]] bool checkNewDeleteConflict(const SyncFileItemPtr &item); + [[nodiscard]] bool checkNewDeleteConflict(const SyncFileItemPtr &item, int64_t localFileSize = 0); qint64 _lastSyncTimestamp = 0; diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 97f5cbbc1d52d..1c984a0e918cb 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -191,6 +191,49 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + // A file blocked from upload due to a quota error is protected from local deletion even + // when the folder is deleted on the server in the same sync where the file was first added + // locally. No prior blacklist entry exists; the parent folder's last known DB quota is used. + void testQuotaBlockedFileProtectedInSameSyncAsParentFolderDeletion() + { + FileInfo initialState{QString{}, { + {QStringLiteral("A"), { + {QStringLiteral("small.txt"), 4}, + }}, + }}; + // Skip the automatic initial sync so the quota is set before the first sync, + // ensuring the DB stores bytesAvailable=50 for folder A. + FakeFolder fakeFolder{initialState, {}, {}, false}; + + fakeFolder.remoteModifier().setFolderQuota(QStringLiteral("A"), FileInfo::FolderQuota{0, 50}); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Server deletes A and a large local file is added before the next sync, + // so no blacklist entry exists yet for the file. + fakeFolder.remoteModifier().remove(QStringLiteral("A")); + fakeFolder.localModifier().insert(QStringLiteral("A/big_file.txt"), 100); // 100 > 50 + + ItemCompletedSpy completeSpy(fakeFolder); + fakeFolder.syncOnce(); + + // big_file.txt was never uploaded but exceeds the last known quota. + // It must be protected locally rather than silently deleted. + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/big_file.txt"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A"))); + + // Previously synced sibling must be removed (trust the server deletion). + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/small.txt"))); + + // Must be reported as a quota error item. + { + const auto item = completeSpy.findItem(QStringLiteral("A/big_file.txt")); + QVERIFY(item); + QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_ERROR); + QVERIFY(item->_status == SyncFileItem::SoftError || item->_status == SyncFileItem::NormalError); + } + } + // Files blocked from upload because of quota errors must follow their parent folder through // multiple successive server side renames. void testQuotaBlockedFileFollowsParentFolderMove() From 682ecdc907abeb98acfdaa7cd1b4ca8cd47e5b39 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 14 May 2026 23:26:12 +0200 Subject: [PATCH 09/21] test(quota): add cleanup tests for protection of files blocked from upload because of quota errors. These verify that once a file blocked from upload because of quota errors is manually removed by the user, the parent folder that was deleted on the server is also cleaned up on the next sync, including the nested case where parent folder etag invalidation is required. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- test/testsyncdelete.cpp | 77 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 1c984a0e918cb..7d58535833253 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -384,6 +384,83 @@ private slots: QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/big.txt"))); } + // After a file with error is protected from deletion, removing it locally must + // allow the parent folder (deleted on the server) to be cleaned up on the next sync. + void testQuotaProtectedFolderCleanedUpAfterLocalFileDeletion() + { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Quota block: upload fails with 507. + fakeFolder.localModifier().insert(QStringLiteral("A/quota_blocked.txt"), 100); + fakeFolder.serverErrorPaths().append(QStringLiteral("A/quota_blocked.txt"), 507); + QVERIFY(!fakeFolder.syncOnce()); + fakeFolder.serverErrorPaths().clear(); + + // Server deletes folder A — file is protected locally. + fakeFolder.remoteModifier().remove(QStringLiteral("A")); + fakeFolder.syncOnce(); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/quota_blocked.txt"))); + + // User manually removes the quota-blocked file. + fakeFolder.localModifier().remove(QStringLiteral("A/quota_blocked.txt")); + + // Next sync: folder A is empty and was deleted on the server — it must be removed locally. + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A"))); + } + + // Same as testQuotaProtectedFolderCleanedUpAfterLocalFileDeletion but with a nested + // folder (B/A) where the parent B remains on the server. Without invalidating B's cached + // etag after the protection sync, the next sync would hit ParentNotChanged for B and treat + // B/A as still present (via its DB record), permanently preventing cleanup. + void testQuotaProtectedNestedFolderCleanedUpAfterLocalFileDeletion() + { + // Start with a structure that has a nested subfolder B/A. + FileInfo initialState{QString{}, { + {QStringLiteral("B"), { + {QStringLiteral("A"), { + {QStringLiteral("file1.txt"), 4}, + {QStringLiteral("file2.txt"), 4}, + }}, + }}, + }}; + FakeFolder fakeFolder{initialState}; + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Add a file inside B/A that fails to upload due to quota (HTTP 507). + fakeFolder.localModifier().insert(QStringLiteral("B/A/quota_blocked.txt"), 100); + fakeFolder.serverErrorPaths().append(QStringLiteral("B/A/quota_blocked.txt"), 507); + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("B/A/quota_blocked.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + fakeFolder.serverErrorPaths().clear(); + + // Server deletes subfolder B/A (parent B remains). The quota-blocked file must be + // protected locally even though B still exists and may have a cached etag. + fakeFolder.remoteModifier().remove(QStringLiteral("B/A")); + fakeFolder.syncOnce(); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A/quota_blocked.txt"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A"))); + + // User manually deletes the quota-blocked file. + fakeFolder.localModifier().remove(QStringLiteral("B/A/quota_blocked.txt")); + + // Next sync: B/A is empty and was deleted on the server. + // B's etag was invalidated during the protection sync so the client re-queries B from + // the server instead of relying on the stale DB cache (which would make B/A look present). + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("B/A"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B"))); + } + void issue1329() { FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; From 94c3326e28b50c914c96acf1c691ada1ff0239e1 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 16:08:12 +0200 Subject: [PATCH 10/21] fix(quota): keep protected folder locally after user removes blocked file. When a folder is protected from remote deletion because it contains files blocked from upload because of quota errors, clear its DB record during the protection sync. On the next sync the folder is treated as a new local folder rather than a server deleted one, so it is never removed locally even after the user deletes the file. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/discovery.cpp | 5 +++++ test/testsyncdelete.cpp | 37 ++++++++++++++++++++----------------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 352a3bcef0e3c..86708b3e3e1c1 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -2230,6 +2230,11 @@ int ProcessDirectoryJob::processSubJobs(int nbJobs) 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(); diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 7d58535833253..e1894ec37f167 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -384,9 +384,11 @@ private slots: QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/big.txt"))); } - // After a file with error is protected from deletion, removing it locally must - // allow the parent folder (deleted on the server) to be cleaned up on the next sync. - void testQuotaProtectedFolderCleanedUpAfterLocalFileDeletion() + // After a file blocked from upload because of quota errors is protected from deletion inside a + // server deleted folder, the client must keep the folder locally even after the user removes + // the file. The folder's DB record is cleared during the protection sync so the next sync + // treats it as a new local folder rather than a server deleted one. + void testQuotaProtectedFolderKeptAfterLocalFileDeletion() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; @@ -404,19 +406,19 @@ private slots: fakeFolder.syncOnce(); QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/quota_blocked.txt"))); - // User manually removes the quota-blocked file. + // User manually removes the file blocked from upload because of quota errors. fakeFolder.localModifier().remove(QStringLiteral("A/quota_blocked.txt")); - // Next sync: folder A is empty and was deleted on the server — it must be removed locally. + // Next sync: folder A is empty but must remain locally — the client must not delete it. QVERIFY(fakeFolder.syncOnce()); - QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A"))); } - // Same as testQuotaProtectedFolderCleanedUpAfterLocalFileDeletion but with a nested - // folder (B/A) where the parent B remains on the server. Without invalidating B's cached - // etag after the protection sync, the next sync would hit ParentNotChanged for B and treat - // B/A as still present (via its DB record), permanently preventing cleanup. - void testQuotaProtectedNestedFolderCleanedUpAfterLocalFileDeletion() + // Same as testQuotaProtectedFolderKeptAfterLocalFileDeletion but with a nested folder (B/A) + // where the parent B remains on the server. Without invalidating B's cached etag after the + // protection sync, the next sync would hit ParentNotChanged for B and treat B/A as still + // present via its DB record, preventing the folder from being treated as new local. + void testQuotaProtectedNestedFolderKeptAfterLocalFileDeletion() { // Start with a structure that has a nested subfolder B/A. FileInfo initialState{QString{}, { @@ -443,21 +445,22 @@ private slots: } fakeFolder.serverErrorPaths().clear(); - // Server deletes subfolder B/A (parent B remains). The quota-blocked file must be - // protected locally even though B still exists and may have a cached etag. + // Server deletes subfolder B/A (parent B remains). The file blocked from upload because + // of quota errors must be protected locally even though B still exists and may have a + // cached etag. fakeFolder.remoteModifier().remove(QStringLiteral("B/A")); fakeFolder.syncOnce(); QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A/quota_blocked.txt"))); QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A"))); - // User manually deletes the quota-blocked file. + // User manually deletes the file blocked from upload because of quota errors. fakeFolder.localModifier().remove(QStringLiteral("B/A/quota_blocked.txt")); - // Next sync: B/A is empty and was deleted on the server. + // Next sync: B/A is empty but must remain locally. // B's etag was invalidated during the protection sync so the client re-queries B from - // the server instead of relying on the stale DB cache (which would make B/A look present). + // the server instead of relying on the stale DB cache. QVERIFY(fakeFolder.syncOnce()); - QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("B/A"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A"))); QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B"))); } From 463ea6f85db299522b8f29ec914145b63a3d5378 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 16:29:13 +0200 Subject: [PATCH 11/21] fix(quota): emit storage full notification when file protection triggers in same sync as deletion. checkErrorBlacklisting only fires the notification when a blacklist entry already exists. When the quota error and folder deletion happen in the same sync there is no entry yet, so the notification was silently skipped and the tray status stayed green instead of red. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/syncengine.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index edfb82c9b2df1..e539441f384db 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -481,6 +481,16 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) // 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 From 9f0b182528fefa004b1fe3ad0bfb3081428f8198 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 17:23:56 +0200 Subject: [PATCH 12/21] test(quota): add tests for protection of files after server side folder rename and delete. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- test/testsyncdelete.cpp | 93 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index e1894ec37f167..1376f3c79d232 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -301,6 +301,99 @@ private slots: QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("C/big.txt"))); } + // After a server side folder rename, if the renamed folder is then deleted on the server, + // files inside it that were blocked from upload because of quota errors must still be + // protected from local deletion. The blacklist path is updated during the rename sync, so + // the subsequent deletion sync must find the entry at the new path. + void testQuotaBlockedFileProtectedAfterFolderRenameAndDelete() + { + FileInfo initialState{QString{}, { + {QStringLiteral("A"), { + {QStringLiteral("small.txt"), 4}, + }}, + {QStringLiteral("B"), {}}, + }}; + FakeFolder fakeFolder{initialState}; + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Block big.txt from uploading permanently (507) regardless of its path. + // This simulates a server where storage is genuinely full. + fakeFolder.localModifier().insert(QStringLiteral("A/big.txt"), 1000); + fakeFolder.setServerOverride([](QNetworkAccessManager::Operation op, + const QNetworkRequest &req, + QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::PutOperation + && req.url().path().contains(QLatin1String("big.txt"))) { + return new FakeErrorReply{op, req, nullptr, 507}; + } + return nullptr; + }); + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + + // Server moves A into B. Client must follow and keep big.txt at the new path. + fakeFolder.remoteModifier().rename(QStringLiteral("A"), QStringLiteral("B/A")); + fakeFolder.syncOnce(); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A/big.txt"))); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("B/A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + + // Server deletes B/A. big.txt must be protected from local deletion. + fakeFolder.remoteModifier().remove(QStringLiteral("B/A")); + QVERIFY(!fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B/A/big.txt"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("B/A/small.txt"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("B"))); + } + + // Same as above but rename and delete happen between client syncs so the client never sees B/A on server. + void testQuotaBlockedFileProtectedAfterFolderRenameAndDeleteInOnePoll() + { + FileInfo initialState{QString{}, { + {QStringLiteral("A"), { + {QStringLiteral("small.txt"), 4}, + }}, + {QStringLiteral("B"), {}}, + }}; + FakeFolder fakeFolder{initialState}; + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + fakeFolder.localModifier().insert(QStringLiteral("A/big.txt"), 1000); + fakeFolder.setServerOverride([](QNetworkAccessManager::Operation op, + const QNetworkRequest &req, + QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::PutOperation + && req.url().path().contains(QLatin1String("big.txt"))) { + return new FakeErrorReply{op, req, nullptr, 507}; + } + return nullptr; + }); + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + + // Server renames A to B/A and then immediately deletes B/A before the client polls again. + fakeFolder.remoteModifier().rename(QStringLiteral("A"), QStringLiteral("B/A")); + fakeFolder.remoteModifier().remove(QStringLiteral("B/A")); + + // big.txt must survive: it was never uploaded and A is gone from server entirely. + QVERIFY(!fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/big.txt"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/small.txt"))); + } + // Files blocked from upload because of quota errors must retry on the next sync once quota is freed. void testQuotaBlockedFileRetriesWhenQuotaResolved() { From 5d20674ecf4a8c8d9e9db3992b7e66f505d0624b Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 18:39:36 +0200 Subject: [PATCH 13/21] fix(quota): preserve InsufficientRemoteStorage blacklist entries across syncs. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/common/syncjournaldb.cpp | 11 ++++-- test/testsyncdelete.cpp | 65 ++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index 32d5483da23ef..90c9af2099a24 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -2210,7 +2210,7 @@ bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet &keep) } SqlQuery query(_db); - query.prepare("SELECT path FROM blacklist"); + query.prepare("SELECT path, errorCategory FROM blacklist"); if (!query.exec()) { return false; @@ -2220,9 +2220,14 @@ bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet &keep) while (query.next().hasData) { const QString file = query.stringValue(0); - if (!keep.contains(file)) { - superfluousPaths.append(file); + const auto errorCategory = static_cast(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); diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 1376f3c79d232..dfa6bc23441c9 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -394,6 +394,71 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/small.txt"))); } + // After the client restarts following a protection event, a subsequent server-side deletion of + // the same folder must still protect quota-blocked files. The protection relies on the blacklist + // entry surviving the restart sync (where the folder is re-created via MKDIR) and then being + // found again when the folder is deleted a second time. + void testQuotaBlockedFileProtectedAfterClientRestart() + { + FileInfo initialState{QString{}, { + {QStringLiteral("A"), { + {QStringLiteral("small.txt"), 4}, + }}, + }}; + FakeFolder fakeFolder{initialState}; + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + fakeFolder.localModifier().insert(QStringLiteral("A/big.txt"), 1000); + fakeFolder.setServerOverride([](QNetworkAccessManager::Operation op, + const QNetworkRequest &req, + QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::PutOperation + && req.url().path().contains(QLatin1String("big.txt"))) { + return new FakeErrorReply{op, req, nullptr, 507}; + } + return nullptr; + }); + QVERIFY(!fakeFolder.syncOnce()); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + + // Server deletes A. Protection kicks in: big.txt survives, A's DB record is cleared. + fakeFolder.remoteModifier().remove(QStringLiteral("A")); + QVERIFY(!fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/big.txt"))); + + // Simulate client restart: next sync re-creates A on the server via MKDIR, + // then big.txt upload fails again because storage is still full. + QVERIFY(!fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentRemoteState().find(QStringLiteral("A"))); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + + // Simulate a quiet sync after the client restart: nothing changed locally so the real + // client uses DatabaseAndFilesystem with an empty path set. A uses ParentNotChanged + // which skips local discovery, so big.txt (not in DB) never appears in _syncItems. + // deleteStaleErrorBlacklistEntries must not prune its InsufficientRemoteStorage entry. + fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::DatabaseAndFilesystem); + fakeFolder.syncOnce(); + { + auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + + // Server deletes A again. big.txt must still be protected. + fakeFolder.remoteModifier().remove(QStringLiteral("A")); + QVERIFY(!fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/big.txt"))); + } + // Files blocked from upload because of quota errors must retry on the next sync once quota is freed. void testQuotaBlockedFileRetriesWhenQuotaResolved() { From 9364a4362a9782e73f2d54df5d047b2725b98712 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 19:18:22 +0200 Subject: [PATCH 14/21] fix(quota): scan local files in deleted folders even under ParentNotChanged. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/discovery.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 86708b3e3e1c1..4a461e98718d2 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -95,6 +95,13 @@ void ProcessDirectoryJob::start() 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. + if (_queryLocal == ParentNotChanged && _dirItem && _dirItem->_instruction == CSYNC_INSTRUCTION_REMOVE) { + _queryLocal = NormalQuery; + } if (_queryLocal == NormalQuery) { startAsyncLocalQuery(); From 63f0fa4e74186550fd95974ccf3f125380a9191d Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 21:07:10 +0200 Subject: [PATCH 15/21] test(quota): verify protection under DatabaseAndFilesystem on second deletion. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- test/testsyncdelete.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index dfa6bc23441c9..446438caeeefc 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -453,7 +453,10 @@ private slots: QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); } - // Server deletes A again. big.txt must still be protected. + // Server deletes A again. The real client uses DatabaseAndFilesystem with no local paths + // (server-triggered sync, no local changes). The fix in startAsync() must force NormalQuery + // for A's subdirectory job so that big.txt is discovered and checkNewDeleteConflict fires. + fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::DatabaseAndFilesystem); fakeFolder.remoteModifier().remove(QStringLiteral("A")); QVERIFY(!fakeFolder.syncOnce()); QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/big.txt"))); From 5d60081826de6c4b7438ab79cad927bea46c357d Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 23:04:23 +0200 Subject: [PATCH 16/21] refactor(db): use prepared statements in renameErrorBlacklistPaths. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/common/preparedsqlquerymanager.h | 2 ++ src/common/syncjournaldb.cpp | 39 +++++++++++++++++----------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/common/preparedsqlquerymanager.h b/src/common/preparedsqlquerymanager.h index bf1f6b434cf7f..4b1c4e09c3453 100644 --- a/src/common/preparedsqlquerymanager.h +++ b/src/common/preparedsqlquerymanager.h @@ -99,6 +99,8 @@ class OCSYNC_EXPORT PreparedSqlQueryManager FolderUpdateInvalidEncryptionStatus, FileUpdateInvalidEncryptionStatus, HasFileIdQuery, + RenameErrorBlacklistCountQuery, + RenameErrorBlacklistUpdateQuery, PreparedQueryCount }; diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index 90c9af2099a24..cd82217d9ad89 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -2178,24 +2178,33 @@ bool SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString return false; } - SqlQuery countQuery(_db); - countQuery.prepare("SELECT COUNT(*) FROM blacklist " - "WHERE errorCategory = ?1 " - "AND (path = ?2 OR (path > (?2 || '/') AND path < (?2 || '0')))"); - countQuery.bindValue(1, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); - countQuery.bindValue(2, from); - const bool hasQuotaEntries = countQuery.exec() && countQuery.next().hasData && countQuery.intValue(0) > 0; + 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; // 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'. - SqlQuery query(_db); - query.prepare("UPDATE blacklist " - "SET path = ?2 || substr(path, length(?1) + 1) " - "WHERE path == ?1 OR (path > (?1 || '/') AND path < (?1 || '0'))"); - query.bindValue(1, from); - query.bindValue(2, to); - if (!query.exec()) { - sqlFail(QStringLiteral("renameErrorBlacklistPaths"), query); + 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; From c08db50c12860915d3aee41c3a1204ad6bee5b54 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 23:13:22 +0200 Subject: [PATCH 17/21] fix(db): skip blacklist path update when no quota entries exist. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/common/syncjournaldb.cpp | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index cd82217d9ad89..8b3e9962d8f13 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -2189,22 +2189,24 @@ bool SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString countQuery->bindValue(1, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); countQuery->bindValue(2, from); - const bool hasQuotaEntries = countQuery->exec() && countQuery->next().hasData && countQuery->intValue(0) > 0; - // 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); + 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; From 86645077be28f1c5b043f281da192ca2fd69945d Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 23:20:16 +0200 Subject: [PATCH 18/21] fix(quota): guard deleted directory local scan against missing directory. When the directory was already removed from disk before discovery runs, forcing NormalQuery in startAsyncLocalQuery() raises a fatal "Directory not found" error that aborts the sync instead of letting the normal stale-entry cleanup proceed. Only switch to NormalQuery when the directory actually exists on disk. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/discovery.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 4a461e98718d2..95e643212a987 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -99,8 +99,15 @@ void ProcessDirectoryJob::start() // 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) { - _queryLocal = NormalQuery; + const QString localPath = _discoveryData->_localDir + _currentFolder._local; + if (QDir(localPath).exists()) { + _queryLocal = NormalQuery; + } } if (_queryLocal == NormalQuery) { From f4b06641ae1d558be43623c5b91567c706a0ef42 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Sun, 17 May 2026 23:45:12 +0200 Subject: [PATCH 19/21] refactor(quota): replace _httpErrorCode 507 with _isQuotaError bool flag. Instead of faking an HTTP 507 response code on items that exceeded quota during discovery, rename the unused _errorMayBeBlacklisted field to _isQuotaError and set it at the three discovery call sites. blacklistUpdate already checked that flag for the mayBlacklist gate; createBlacklistEntry now checks it alongside the real HTTP 507 path to write the InsufficientRemoteStorage category. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/libsync/discovery.cpp | 15 ++++++--------- src/libsync/owncloudpropagator.cpp | 6 +++--- src/libsync/syncfileitem.h | 10 +++++----- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 95e643212a987..a9e57716e8b31 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1250,11 +1250,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( } item->_status = SyncFileItem::Status::NormalError; - // Mark as a quota error so blacklistUpdate writes an InsufficientRemoteStorage entry. - // Without this, _httpErrorCode would be 0 and blacklistUpdate would not create an - // entry, leaving the file unprotected by checkNewDeleteConflict if the remote parent - // folder is deleted before the quota situation is resolved. - item->_httpErrorCode = 507; + // 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); } @@ -2557,9 +2556,7 @@ bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item, in << item->_file; item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_status = SyncFileItem::SoftError; - // Keep _httpErrorCode at 507 so blacklistUpdate recreates the InsufficientRemoteStorage - // entry if the ignore duration later expires. - item->_httpErrorCode = 507; + 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); @@ -2582,7 +2579,7 @@ bool ProcessDirectoryJob::checkNewDeleteConflict(const SyncFileItemPtr &item, in << item->_file; item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_status = SyncFileItem::SoftError; - item->_httpErrorCode = 507; + 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.") diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index fdd6e5550cbf6..d1353b9a7f08c 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -158,7 +158,7 @@ static SyncJournalErrorBlacklistRecord createBlacklistEntry( entry._ignoreDuration = 0; } - if (item._httpErrorCode == 507) { + if (item._isQuotaError || item._httpErrorCode == 507) { 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. @@ -177,11 +177,11 @@ void blacklistUpdate(SyncJournalDb *journal, SyncFileItem &item) 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. diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index 7d07d84ed88f1..c370ad2aac9b4 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -130,7 +130,7 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem , _direction(None) , _serverHasIgnoredFiles(false) , _hasBlacklistEntry(false) - , _errorMayBeBlacklisted(false) + , _isQuotaError(false) , _status(NoStatus) , _isRestoration(false) , _isSelectiveSync(false) @@ -261,12 +261,12 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem /// without the status being FileIgnored. bool _hasBlacklistEntry BITFIELD(1); - /** If true and NormalError, this error may be blacklisted + /** True when discovery detected that this item exceeds the remote storage quota. * - * Note that non-local errors (httpErrorCode!=0) may also be - * blacklisted independently of this flag. + * Causes blacklistUpdate to write an InsufficientRemoteStorage entry even + * though no HTTP request was made (and _httpErrorCode is therefore 0). */ - bool _errorMayBeBlacklisted BITFIELD(1); + bool _isQuotaError BITFIELD(1); // Variables useful to report to the user Status _status BITFIELD(4); From bdb97ad77e59cd4cc1938666980de567fc482944 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Mon, 15 Jun 2026 18:34:02 +0200 Subject: [PATCH 20/21] fix(quota): emit storage notification on quota flag and simplify blacklist rename Gate the discovery-time storage full notification on _isQuotaError instead of a faked 507, so the same-sync first-detection case (no blacklist entry, no HTTP request) still notifies the user. Collapse renameErrorBlacklistPaths to a single category-scoped UPDATE that returns numRowsAffected, removing the extra COUNT query and the mismatch where the UPDATE moved non-quota entries. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- src/common/preparedsqlquerymanager.h | 1 - src/common/syncjournaldb.cpp | 44 ++++++++++------------------ src/libsync/syncengine.cpp | 2 +- test/testsyncdelete.cpp | 9 ++++++ 4 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/common/preparedsqlquerymanager.h b/src/common/preparedsqlquerymanager.h index 4b1c4e09c3453..0ae236b21ed39 100644 --- a/src/common/preparedsqlquerymanager.h +++ b/src/common/preparedsqlquerymanager.h @@ -99,7 +99,6 @@ class OCSYNC_EXPORT PreparedSqlQueryManager FolderUpdateInvalidEncryptionStatus, FileUpdateInvalidEncryptionStatus, HasFileIdQuery, - RenameErrorBlacklistCountQuery, RenameErrorBlacklistUpdateQuery, PreparedQueryCount diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index 8b3e9962d8f13..d96bcd49c7cb3 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -2178,38 +2178,26 @@ bool SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString 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) { + // Move 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'. + // Scoped to quota entries so the returned count reflects only protected files. + const auto query = _queryManager.get(PreparedSqlQueryManager::RenameErrorBlacklistUpdateQuery, + QByteArrayLiteral("UPDATE blacklist " + "SET path = ?2 || substr(path, length(?1) + 1) " + "WHERE errorCategory = ?3 " + "AND (path == ?1 OR (path > (?1 || '/') AND path < (?1 || '0')))"), + _db); + if (!query) { 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); - } + query->bindValue(1, from); + query->bindValue(2, to); + query->bindValue(3, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + if (!query->exec()) { + return sqlFail(QStringLiteral("renameErrorBlacklistPaths"), *query); } - return hasQuotaEntries; + return query->numRowsAffected() > 0; } bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet &keep) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index e539441f384db..96302a538659d 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -487,7 +487,7 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) // 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) { + && item->_isQuotaError) { slotInsufficientRemoteStorage(); } diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index 446438caeeefc..d5bc48f55210d 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -215,6 +215,7 @@ private slots: fakeFolder.localModifier().insert(QStringLiteral("A/big_file.txt"), 100); // 100 > 50 ItemCompletedSpy completeSpy(fakeFolder); + QSignalSpy storageSpy(&fakeFolder.syncEngine(), &SyncEngine::syncError); fakeFolder.syncOnce(); // big_file.txt was never uploaded but exceeds the last known quota. @@ -232,6 +233,14 @@ private slots: QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_ERROR); QVERIFY(item->_status == SyncFileItem::SoftError || item->_status == SyncFileItem::NormalError); } + + // The storage full notification must fire even though no upload was attempted + // and there is no blacklist entry yet for the file. + const bool hadStorageNotification = std::any_of(storageSpy.begin(), storageSpy.end(), + [](const QList &args) { + return args.at(1).value() == ErrorCategory::InsufficientRemoteStorage; + }); + QVERIFY(hadStorageNotification); } // Files blocked from upload because of quota errors must follow their parent folder through From 4fe1a6d5d73d68af0c7dd1d1e7d2b1619c778203 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Mon, 15 Jun 2026 19:19:00 +0200 Subject: [PATCH 21/21] test(quota): cover discovery-time quota block protected from later deletion Seeds the InsufficientRemoteStorage entry via the discovery quota check (no 507, no upload attempt), then verifies a later server side deletion of the parent folder still protects the file via that entry. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Camila Ayres --- test/testsyncdelete.cpp | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/testsyncdelete.cpp b/test/testsyncdelete.cpp index d5bc48f55210d..5be6ffbb98be4 100644 --- a/test/testsyncdelete.cpp +++ b/test/testsyncdelete.cpp @@ -243,6 +243,41 @@ private slots: QVERIFY(hadStorageNotification); } + // Discovery determines the file exceeds the folder quota (no 507, no upload attempt), + // which seeds the InsufficientRemoteStorage entry. A later server side deletion of the + // parent must then protect the file via that entry. + void testQuotaBlockedDuringDiscoveryProtectedFromLaterDeletion() + { + FileInfo initialState{QString{}, { + {QStringLiteral("A"), { + {QStringLiteral("small.txt"), 4}, + }}, + }}; + // Skip the automatic initial sync so the quota is set before the first sync, + // ensuring the DB stores bytesAvailable=50 for folder A. + FakeFolder fakeFolder{initialState, {}, {}, false}; + fakeFolder.remoteModifier().setFolderQuota(QStringLiteral("A"), FileInfo::FolderQuota{0, 50}); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Too big for the 50 byte quota: blocked during discovery, never uploaded, no 507. + fakeFolder.localModifier().insert(QStringLiteral("A/big.txt"), 100); + QVERIFY(!fakeFolder.syncOnce()); + { + const auto entry = fakeFolder.syncJournal().errorBlacklistEntry(QStringLiteral("A/big.txt")); + QVERIFY(entry.isValid()); + QCOMPARE(entry._errorCategory, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage); + } + QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("A/big.txt"))); + + // Server deletes A on a later sync: the discovery seeded entry must protect big.txt. + fakeFolder.remoteModifier().remove(QStringLiteral("A")); + fakeFolder.syncOnce(); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A/big.txt"))); + QVERIFY(fakeFolder.currentLocalState().find(QStringLiteral("A"))); + QVERIFY(!fakeFolder.currentLocalState().find(QStringLiteral("A/small.txt"))); + } + // Files blocked from upload because of quota errors must follow their parent folder through // multiple successive server side renames. void testQuotaBlockedFileFollowsParentFolderMove()