From bf27cd984387c561dac2671cc6509194651bdc81 Mon Sep 17 00:00:00 2001 From: alex-z Date: Mon, 19 Feb 2024 20:37:34 +0100 Subject: [PATCH] Client Status Reporting. Only report statuses listed on the server. Signed-off-by: alex-z --- src/libsync/clientstatusreportingcommon.cpp | 24 +++----------- src/libsync/clientstatusreportingcommon.h | 9 +---- src/libsync/clientstatusreportingnetwork.cpp | 9 +---- src/libsync/discovery.cpp | 2 -- src/libsync/owncloudpropagator.cpp | 25 ++++---------- src/libsync/propagatedownload.cpp | 2 -- src/libsync/syncengine.cpp | 3 -- test/testclientstatusreporting.cpp | 35 +++++++++----------- 8 files changed, 30 insertions(+), 79 deletions(-) diff --git a/src/libsync/clientstatusreportingcommon.cpp b/src/libsync/clientstatusreportingcommon.cpp index fabbf2b0fbbaf..16f0b0a1a8f8e 100644 --- a/src/libsync/clientstatusreportingcommon.cpp +++ b/src/libsync/clientstatusreportingcommon.cpp @@ -27,32 +27,18 @@ QByteArray clientStatusstatusStringFromNumber(const ClientStatusReportingStatus } switch (status) { - case ClientStatusReportingStatus::DownloadError_Cannot_Create_File: - return QByteArrayLiteral("DownloadResult.CANNOT_CREATE_FILE"); - case ClientStatusReportingStatus::DownloadError_Conflict: - return QByteArrayLiteral("DownloadResult.CONFLICT"); case ClientStatusReportingStatus::DownloadError_ConflictCaseClash: - return QByteArrayLiteral("DownloadResult.CONFLICT_CASECLASH"); + return QByteArrayLiteral("DownloadError.CONFLICT_CASECLASH"); case ClientStatusReportingStatus::DownloadError_ConflictInvalidCharacters: - return QByteArrayLiteral("DownloadResult.CONFLICT_INVALID_CHARACTERS"); - case ClientStatusReportingStatus::DownloadError_No_Free_Space: - return QByteArrayLiteral("DownloadResult.NO_FREE_SPACE"); + return QByteArrayLiteral("DownloadError.CONFLICT_INVALID_CHARACTERS"); case ClientStatusReportingStatus::DownloadError_ServerError: - return QByteArrayLiteral("DownloadResult.SERVER_ERROR"); + return QByteArrayLiteral("DownloadError.SERVER_ERROR"); case ClientStatusReportingStatus::DownloadError_Virtual_File_Hydration_Failure: - return QByteArrayLiteral("DownloadResult.VIRTUAL_FILE_HYDRATION_FAILURE"); + return QByteArrayLiteral("DownloadError.VIRTUAL_FILE_HYDRATION_FAILURE"); case ClientStatusReportingStatus::E2EeError_GeneralError: return QByteArrayLiteral("E2EeError.General"); - case ClientStatusReportingStatus::UploadError_Conflict: - return QByteArrayLiteral("UploadResult.CONFLICT_CASECLASH"); - case ClientStatusReportingStatus::UploadError_ConflictInvalidCharacters: - return QByteArrayLiteral("UploadResult.CONFLICT_INVALID_CHARACTERS"); - case ClientStatusReportingStatus::UploadError_No_Free_Space: - return QByteArrayLiteral("UploadResult.NO_FREE_SPACE"); - case ClientStatusReportingStatus::UploadError_No_Write_Permissions: - return QByteArrayLiteral("UploadResult.NO_WRITE_PERMISSIONS"); case ClientStatusReportingStatus::UploadError_ServerError: - return QByteArrayLiteral("UploadResult.SERVER_ERROR"); + return QByteArrayLiteral("UploadError.SERVER_ERROR"); case ClientStatusReportingStatus::UploadError_Virus_Detected: return QByteArrayLiteral("UploadResult.VIRUS_DETECTED"); case ClientStatusReportingStatus::Count: diff --git a/src/libsync/clientstatusreportingcommon.h b/src/libsync/clientstatusreportingcommon.h index 4e08ca1efa6fd..aa422ff257c90 100644 --- a/src/libsync/clientstatusreportingcommon.h +++ b/src/libsync/clientstatusreportingcommon.h @@ -18,18 +18,11 @@ namespace OCC { enum class ClientStatusReportingStatus { - DownloadError_Cannot_Create_File = 0, - DownloadError_Conflict, - DownloadError_ConflictCaseClash, + DownloadError_ConflictCaseClash = 0, DownloadError_ConflictInvalidCharacters, - DownloadError_No_Free_Space, DownloadError_ServerError, DownloadError_Virtual_File_Hydration_Failure, E2EeError_GeneralError, - UploadError_Conflict, - UploadError_ConflictInvalidCharacters, - UploadError_No_Free_Space, - UploadError_No_Write_Permissions, UploadError_ServerError, UploadError_Virus_Detected, Count, diff --git a/src/libsync/clientstatusreportingnetwork.cpp b/src/libsync/clientstatusreportingnetwork.cpp index ed0f669f8411c..08e6416fadb5d 100644 --- a/src/libsync/clientstatusreportingnetwork.cpp +++ b/src/libsync/clientstatusreportingnetwork.cpp @@ -136,7 +136,7 @@ QVariantMap ClientStatusReportingNetwork::prepareReport() const qCDebug(lcClientStatusReportingNetwork) << "Could not classify status:"; continue; } - + if (categoryKey == statusReportCategoryE2eErrors) { const auto initialCount = e2eeErrors[QStringLiteral("count")].toInt(); e2eeErrors[QStringLiteral("count")] = initialCount + record._numOccurences; @@ -169,18 +169,11 @@ QByteArray ClientStatusReportingNetwork::classifyStatus(const ClientStatusReport } switch (status) { - case ClientStatusReportingStatus::DownloadError_Conflict: case ClientStatusReportingStatus::DownloadError_ConflictCaseClash: case ClientStatusReportingStatus::DownloadError_ConflictInvalidCharacters: - case ClientStatusReportingStatus::UploadError_Conflict: - case ClientStatusReportingStatus::UploadError_ConflictInvalidCharacters: return statusReportCategorySyncConflicts; - case ClientStatusReportingStatus::DownloadError_Cannot_Create_File: - case ClientStatusReportingStatus::DownloadError_No_Free_Space: case ClientStatusReportingStatus::DownloadError_ServerError: case ClientStatusReportingStatus::DownloadError_Virtual_File_Hydration_Failure: - case ClientStatusReportingStatus::UploadError_No_Free_Space: - case ClientStatusReportingStatus::UploadError_No_Write_Permissions: case ClientStatusReportingStatus::UploadError_ServerError: return statusReportCategoryProblems; case ClientStatusReportingStatus::UploadError_Virus_Detected: diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index dc608aa7abf3a..992f0c71c1147 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1761,13 +1761,11 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) // No permissions set return true; } else if (item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddSubDirectories)) { - _discoveryData->_account->reportClientStatus(ClientStatusReportingStatus::UploadError_No_Write_Permissions); qCWarning(lcDisco) << "checkForPermission: ERROR" << item->_file; item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder"); return false; } else if (!item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddFile)) { - _discoveryData->_account->reportClientStatus(ClientStatusReportingStatus::UploadError_No_Write_Permissions); qCWarning(lcDisco) << "checkForPermission: ERROR" << item->_file; item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_errorString = tr("Not allowed because you don't have permission to add files in that folder"); diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 6664be246616b..010771a183e53 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -326,29 +326,19 @@ bool PropagateItemJob::hasEncryptedAncestor() const void PropagateItemJob::reportClientStatuses() { - if (_item->_status == SyncFileItem::Status::Conflict) { - if (_item->_direction == SyncFileItem::Direction::Up) { - propagator()->account()->reportClientStatus(ClientStatusReportingStatus::UploadError_Conflict); - } else { - propagator()->account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_Conflict); - } - } else if (_item->_status == SyncFileItem::Status::FileNameClash) { - if (_item->_direction == SyncFileItem::Direction::Up) { - propagator()->account()->reportClientStatus(ClientStatusReportingStatus::UploadError_ConflictInvalidCharacters); - } else { + if (_item->_status == SyncFileItem::Status::FileNameClash) { + if (_item->_direction != SyncFileItem::Direction::Up) { propagator()->account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_ConflictInvalidCharacters); } - } else if (_item->_status == SyncFileItem::Status::FileNameInvalidOnServer) { - propagator()->account()->reportClientStatus(ClientStatusReportingStatus::UploadError_ConflictInvalidCharacters); } else if (_item->_status == SyncFileItem::Status::FileNameInvalid) { propagator()->account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_ConflictInvalidCharacters); - } else if (_item->_httpErrorCode != HttpErrorCodeNone && _item->_httpErrorCode != HttpErrorCodeSuccess && _item->_httpErrorCode != HttpErrorCodeSuccessCreated - && _item->_httpErrorCode != HttpErrorCodeSuccessNoContent) { + } else if (_item->_httpErrorCode != HttpErrorCodeNone && _item->_httpErrorCode != HttpErrorCodeSuccess + && _item->_httpErrorCode != HttpErrorCodeSuccessCreated && _item->_httpErrorCode != HttpErrorCodeSuccessNoContent) { if (_item->_direction == SyncFileItem::Up) { - const auto isCodeBadReqOrUnsupportedMediaType = (_item->_httpErrorCode == HttpErrorCodeBadRequest || _item->_httpErrorCode == HttpErrorCodeUnsupportedMediaType); + const auto isCodeBadReqOrUnsupportedMediaType = + (_item->_httpErrorCode == HttpErrorCodeBadRequest || _item->_httpErrorCode == HttpErrorCodeUnsupportedMediaType); const auto isExceptionInfoPresent = !_item->_errorExceptionName.isEmpty() && !_item->_errorExceptionMessage.isEmpty(); - if (isCodeBadReqOrUnsupportedMediaType && isExceptionInfoPresent - && _item->_errorExceptionName.contains(QStringLiteral("UnsupportedMediaType")) + if (isCodeBadReqOrUnsupportedMediaType && isExceptionInfoPresent && _item->_errorExceptionName.contains(QStringLiteral("UnsupportedMediaType")) && _item->_errorExceptionMessage.contains(QStringLiteral("virus"), Qt::CaseInsensitive)) { propagator()->account()->reportClientStatus(ClientStatusReportingStatus::UploadError_Virus_Detected); } else { @@ -982,7 +972,6 @@ bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item, } _journal->setConflictRecord(conflictRecord); - account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_Conflict); // Create a new upload job if the new conflict file should be uploaded if (account()->capabilities().uploadConflictFiles()) { diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index caa6922dbdb24..11b9935a4e6ca 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -675,7 +675,6 @@ void PropagateDownloadFile::startDownload() if (_tmpFile.exists()) FileSystem::setFileReadOnly(_tmpFile.fileName(), false); if (!_tmpFile.open(QIODevice::Append | QIODevice::Unbuffered)) { - propagator()->account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_Cannot_Create_File); qCWarning(lcPropagateDownload) << "could not open temporary file" << _tmpFile.fileName(); done(SyncFileItem::NormalError, _tmpFile.errorString(), ErrorCategory::GenericError); return; @@ -1260,7 +1259,6 @@ void PropagateDownloadFile::downloadFinished() emit propagator()->touchedFile(filename); // The fileChanged() check is done above to generate better error messages. if (!FileSystem::uncheckedRenameReplace(_tmpFile.fileName(), filename, &error)) { - propagator()->account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_Cannot_Create_File); qCWarning(lcPropagateDownload) << QString("Rename failed: %1 => %2").arg(_tmpFile.fileName()).arg(filename); // If the file is locked, we want to retry this sync when it // becomes available again, otherwise try again directly diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 72c043fbbafc3..4c884acdc0235 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -314,7 +314,6 @@ void SyncEngine::conflictRecordMaintenance() } _journal->setConflictRecord(record); - account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_Conflict); } } } @@ -1268,7 +1267,6 @@ void SyncEngine::slotSummaryError(const QString &message) void SyncEngine::slotInsufficientLocalStorage() { - account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_No_Free_Space); slotSummaryError( tr("Disk space is low: Downloads that would reduce free space " "below %1 were skipped.") @@ -1277,7 +1275,6 @@ void SyncEngine::slotInsufficientLocalStorage() void SyncEngine::slotInsufficientRemoteStorage() { - account()->reportClientStatus(ClientStatusReportingStatus::UploadError_No_Free_Space); auto msg = tr("There is insufficient space available on the server for some uploads."); if (_uniqueErrors.contains(msg)) return; diff --git a/test/testclientstatusreporting.cpp b/test/testclientstatusreporting.cpp index d2ca07244b3c2..0d20028a2050f 100644 --- a/test/testclientstatusreporting.cpp +++ b/test/testclientstatusreporting.cpp @@ -75,35 +75,32 @@ private slots: void testReportAndSendStatuses() { for (int i = 0; i < 2; ++i) { - // 5 conflicts - account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_Conflict); - account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_ConflictInvalidCharacters); - account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_Conflict); + // 2 conflicts account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_ConflictInvalidCharacters); account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_ConflictCaseClash); - // 4 problems + // 3 problems account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_ServerError); account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_ServerError); account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_Virtual_File_Hydration_Failure); // 3 occurances of case ClientStatusReportingStatus::UploadError_No_Write_Permissions - account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_No_Write_Permissions); - account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_No_Write_Permissions); - account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_No_Write_Permissions); + account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_Virtual_File_Hydration_Failure); + account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_Virtual_File_Hydration_Failure); + + // 2 occurances of E2EeError_GeneralError + account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); + account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); // 3 occurances of case ClientStatusReportingStatus::UploadError_Virus_Detected account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_Virus_Detected); account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_Virus_Detected); account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_Virus_Detected); - // 2 occurances of E2EeError_GeneralError - account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); - account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); QTest::qWait(OCC::ClientStatusReportingNetwork::clientStatusReportingTrySendTimerInterval + OCC::ClientStatusReportingNetwork::repordSendIntervalMs); QVERIFY(!bodyReceivedAndParsed.isEmpty()); - // we must have 2 e2ee errors + // we must have 3 virus_detected category statuses const auto virusDetectedErrorsReceived = bodyReceivedAndParsed.value("virus_detected").toMap(); QVERIFY(!virusDetectedErrorsReceived.isEmpty()); QCOMPARE(virusDetectedErrorsReceived.value("count"), 3); @@ -113,18 +110,18 @@ private slots: QVERIFY(!e2eeErrorsReceived.isEmpty()); QCOMPARE(e2eeErrorsReceived.value("count"), 2); - // we must have 5 conflicts + // we must have 2 conflicts const auto conflictsReceived = bodyReceivedAndParsed.value("sync_conflicts").toMap(); QVERIFY(!conflictsReceived.isEmpty()); - QCOMPARE(conflictsReceived.value("count"), 5); + QCOMPARE(conflictsReceived.value("count"), 2); - // we must have 4 problems + // we must have 3 problems const auto problemsReceived = bodyReceivedAndParsed.value("problems").toMap(); QVERIFY(!problemsReceived.isEmpty()); - QCOMPARE(problemsReceived.size(), 4); - const auto problemsNoWritePermissions = problemsReceived.value(OCC::clientStatusstatusStringFromNumber(OCC::ClientStatusReportingStatus::UploadError_No_Write_Permissions)).toMap(); - // among those, 3 occurances of case ClientStatusReportingStatus::UploadError_No_Write_Permissions - QCOMPARE(problemsNoWritePermissions.value("count"), 3); + QCOMPARE(problemsReceived.size(), 3); + const auto specificProblemMultipleOccurances = problemsReceived.value(OCC::clientStatusstatusStringFromNumber(OCC::ClientStatusReportingStatus::DownloadError_Virtual_File_Hydration_Failure)).toMap(); + // among those, 3 occurances of specific problem + QCOMPARE(specificProblemMultipleOccurances.value("count"), 3); bodyReceivedAndParsed.clear(); }