Client Status Reporting. Only report statuses listed on the server.

Signed-off-by: alex-z <blackslayer4@gmail.com>
nextcloud · Feb 22, 2024 · 62909a1 · 62909a1
1 parent 094947b
commit 62909a1
Show file tree

Hide file tree

Showing 8 changed files with 30 additions and 79 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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();
         }