Skip to content

Commit 36aff5a

Browse files
committed
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 <hello@camilasan.com>
1 parent 888c1f5 commit 36aff5a

4 files changed

Lines changed: 26 additions & 30 deletions

File tree

src/common/preparedsqlquerymanager.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ class OCSYNC_EXPORT PreparedSqlQueryManager
9999
FolderUpdateInvalidEncryptionStatus,
100100
FileUpdateInvalidEncryptionStatus,
101101
HasFileIdQuery,
102-
RenameErrorBlacklistCountQuery,
103102
RenameErrorBlacklistUpdateQuery,
104103

105104
PreparedQueryCount

src/common/syncjournaldb.cpp

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2178,38 +2178,26 @@ bool SyncJournalDb::renameErrorBlacklistPaths(const QString &from, const QString
21782178
return false;
21792179
}
21802180

2181-
const auto countQuery = _queryManager.get(PreparedSqlQueryManager::RenameErrorBlacklistCountQuery,
2182-
QByteArrayLiteral("SELECT COUNT(*) FROM blacklist "
2183-
"WHERE errorCategory = ?1 "
2184-
"AND (path = ?2 OR (path > (?2 || '/') AND path < (?2 || '0')))"),
2185-
_db);
2186-
if (!countQuery) {
2181+
// Move the exact folder entry and all entries whose path starts with "from/".
2182+
// Uses the same range trick as IS_PREFIX_PATH_OR_EQUAL: '/' + 1 == '0'.
2183+
// Scoped to quota entries so the returned count reflects only protected files.
2184+
const auto query = _queryManager.get(PreparedSqlQueryManager::RenameErrorBlacklistUpdateQuery,
2185+
QByteArrayLiteral("UPDATE blacklist "
2186+
"SET path = ?2 || substr(path, length(?1) + 1) "
2187+
"WHERE errorCategory = ?3 "
2188+
"AND (path == ?1 OR (path > (?1 || '/') AND path < (?1 || '0')))"),
2189+
_db);
2190+
if (!query) {
21872191
return false;
21882192
}
2189-
2190-
countQuery->bindValue(1, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage);
2191-
countQuery->bindValue(2, from);
2192-
2193-
const bool hasQuotaEntries = countQuery->exec() && countQuery->next().hasData && countQuery->intValue(0) > 0;
2194-
if (hasQuotaEntries) {
2195-
// Update the exact folder entry and all entries whose path starts with "from/".
2196-
// Uses the same range trick as IS_PREFIX_PATH_OR_EQUAL: '/' + 1 == '0'.
2197-
const auto query = _queryManager.get(PreparedSqlQueryManager::RenameErrorBlacklistUpdateQuery,
2198-
QByteArrayLiteral("UPDATE blacklist "
2199-
"SET path = ?2 || substr(path, length(?1) + 1) "
2200-
"WHERE path == ?1 OR (path > (?1 || '/') AND path < (?1 || '0'))"),
2201-
_db);
2202-
if (!query) {
2203-
return false;
2204-
}
2205-
query->bindValue(1, from);
2206-
query->bindValue(2, to);
2207-
if (!query->exec()) {
2208-
sqlFail(QStringLiteral("renameErrorBlacklistPaths"), *query);
2209-
}
2193+
query->bindValue(1, from);
2194+
query->bindValue(2, to);
2195+
query->bindValue(3, SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage);
2196+
if (!query->exec()) {
2197+
return sqlFail(QStringLiteral("renameErrorBlacklistPaths"), *query);
22102198
}
22112199

2212-
return hasQuotaEntries;
2200+
return query->numRowsAffected() > 0;
22132201
}
22142202

22152203
bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet<QString> &keep)

src/libsync/syncengine.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item)
487487
// when a blacklist entry already exists; in the same sync as the folder deletion there is no
488488
// entry yet, so we check here unconditionally.
489489
if (item->_instruction == CSYNC_INSTRUCTION_ERROR
490-
&& item->_httpErrorCode == 507) {
490+
&& item->_isQuotaError) {
491491
slotInsufficientRemoteStorage();
492492
}
493493

test/testsyncdelete.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ private slots:
215215
fakeFolder.localModifier().insert(QStringLiteral("A/big_file.txt"), 100); // 100 > 50
216216

217217
ItemCompletedSpy completeSpy(fakeFolder);
218+
QSignalSpy storageSpy(&fakeFolder.syncEngine(), &SyncEngine::syncError);
218219
fakeFolder.syncOnce();
219220

220221
// big_file.txt was never uploaded but exceeds the last known quota.
@@ -232,6 +233,14 @@ private slots:
232233
QCOMPARE(item->_instruction, CSYNC_INSTRUCTION_ERROR);
233234
QVERIFY(item->_status == SyncFileItem::SoftError || item->_status == SyncFileItem::NormalError);
234235
}
236+
237+
// The storage full notification must fire even though no upload was attempted
238+
// and there is no blacklist entry yet for the file.
239+
const bool hadStorageNotification = std::any_of(storageSpy.begin(), storageSpy.end(),
240+
[](const QList<QVariant> &args) {
241+
return args.at(1).value<ErrorCategory>() == ErrorCategory::InsufficientRemoteStorage;
242+
});
243+
QVERIFY(hadStorageNotification);
235244
}
236245

237246
// Files blocked from upload because of quota errors must follow their parent folder through

0 commit comments

Comments
 (0)