Fix review comments from round II.

Signed-off-by: alex-z <[email protected]>
nextcloud · Dec 11, 2023 · 2765247 · 2765247
1 parent 79e124c
commit 2765247
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 57 deletions.
diff --git a/src/libsync/clientstatusreporting.cpp b/src/libsync/clientstatusreporting.cpp
@@ -24,12 +24,12 @@ Q_LOGGING_CATEGORY(lcClientStatusReporting, "nextcloud.sync.clientstatusreportin
 
 ClientStatusReporting::ClientStatusReporting(Account *account)
 {
-    for (int i = 0; i < ClientStatusReportingStatus::Count; ++i) {
+    for (int i = 0; i < static_cast<int>(ClientStatusReportingStatus::Count); ++i) {
         const auto statusString = clientStatusstatusStringFromNumber(static_cast<ClientStatusReportingStatus>(i));
         _statusStrings[i] = statusString;
     }
 
-    if (_statusStrings.size() < ClientStatusReportingStatus::Count) {
+    if (_statusStrings.size() < static_cast<int>(ClientStatusReportingStatus::Count)) {
         return;
     }
 
@@ -46,27 +46,23 @@ ClientStatusReporting::ClientStatusReporting(Account *account)
     _isInitialized = true;
 }
 
-ClientStatusReporting::~ClientStatusReporting()
-{
-    // the sole purpose of this desrtuctor is to make unique_ptr work with forward declaration, but let's clearn the initialized flag too
-    _isInitialized = false;
-}
+ClientStatusReporting::~ClientStatusReporting() = default;
 
 void ClientStatusReporting::reportClientStatus(const ClientStatusReportingStatus status) const
 {
     if (!_isInitialized) {
         return;
     }
 
-    Q_ASSERT(status >= 0 && status < Count);
-    if (status < 0 || status >= ClientStatusReportingStatus::Count) {
-        qCDebug(lcClientStatusReporting) << "Trying to report invalid status:" << status;
+    Q_ASSERT(static_cast<int>(status) >= 0 && static_cast<int>(status) < static_cast<int>(ClientStatusReportingStatus::Count));
+    if (static_cast<int>(status) < 0 || static_cast<int>(status) >= static_cast<int>(ClientStatusReportingStatus::Count)) {
+        qCDebug(lcClientStatusReporting) << "Trying to report invalid status:" << static_cast<int>(status);
         return;
     }
 
     ClientStatusReportingRecord record;
-    record._name = _statusStrings[status];
-    record._status = status;
+    record._name = _statusStrings[static_cast<int>(status)];
+    record._status = static_cast<int>(status);
     record._lastOccurence = QDateTime::currentDateTimeUtc().toMSecsSinceEpoch();
     const auto result = _database->setClientStatusReportingRecord(record);
     if (!result.isValid()) {

diff --git a/src/libsync/clientstatusreportingcommon.cpp b/src/libsync/clientstatusreportingcommon.cpp
@@ -13,47 +13,49 @@
  */
 
 #include "clientstatusreportingcommon.h"
-#include <QDebug>
+#include <QLoggingCategory>
 
 namespace OCC {
+Q_LOGGING_CATEGORY(lcClientStatusReportingCommon, "nextcloud.sync.clientstatusreportingcommon", QtInfoMsg)
+
 QByteArray clientStatusstatusStringFromNumber(const ClientStatusReportingStatus status)
 {
-    Q_ASSERT(status >= 0 && status < Count);
-    if (status < 0 || status >= ClientStatusReportingStatus::Count) {
-        qDebug() << "Invalid status:" << status;
+    Q_ASSERT(static_cast<int>(status) >= 0 && static_cast<int>(status) < static_cast<int>(ClientStatusReportingStatus::Count));
+    if (static_cast<int>(status) < 0 || static_cast<int>(status) >= static_cast<int>(ClientStatusReportingStatus::Count)) {
+        qCDebug(lcClientStatusReportingCommon) << "Invalid status:" << static_cast<int>(status);
         return {};
     }
 
     switch (status) {
-    case DownloadError_Cannot_Create_File:
+    case ClientStatusReportingStatus::DownloadError_Cannot_Create_File:
         return QByteArrayLiteral("DownloadResult.CANNOT_CREATE_FILE");
-    case DownloadError_Conflict:
+    case ClientStatusReportingStatus::DownloadError_Conflict:
         return QByteArrayLiteral("DownloadResult.CONFLICT");
-    case DownloadError_ConflictCaseClash:
+    case ClientStatusReportingStatus::DownloadError_ConflictCaseClash:
         return QByteArrayLiteral("DownloadResult.CONFLICT_CASECLASH");
-    case DownloadError_ConflictInvalidCharacters:
+    case ClientStatusReportingStatus::DownloadError_ConflictInvalidCharacters:
         return QByteArrayLiteral("DownloadResult.CONFLICT_INVALID_CHARACTERS");
-    case DownloadError_No_Free_Space:
+    case ClientStatusReportingStatus::DownloadError_No_Free_Space:
         return QByteArrayLiteral("DownloadResult.NO_FREE_SPACE");
-    case DownloadError_ServerError:
+    case ClientStatusReportingStatus::DownloadError_ServerError:
         return QByteArrayLiteral("DownloadResult.SERVER_ERROR");
-    case DownloadError_Virtual_File_Hydration_Failure:
+    case ClientStatusReportingStatus::DownloadError_Virtual_File_Hydration_Failure:
         return QByteArrayLiteral("DownloadResult.VIRTUAL_FILE_HYDRATION_FAILURE");
-    case E2EeError_GeneralError:
+    case ClientStatusReportingStatus::E2EeError_GeneralError:
         return QByteArrayLiteral("E2EeError.General");
-    case UploadError_Conflict:
+    case ClientStatusReportingStatus::UploadError_Conflict:
         return QByteArrayLiteral("UploadResult.CONFLICT_CASECLASH");
-    case UploadError_ConflictInvalidCharacters:
+    case ClientStatusReportingStatus::UploadError_ConflictInvalidCharacters:
         return QByteArrayLiteral("UploadResult.CONFLICT_INVALID_CHARACTERS");
-    case UploadError_No_Free_Space:
+    case ClientStatusReportingStatus::UploadError_No_Free_Space:
         return QByteArrayLiteral("UploadResult.NO_FREE_SPACE");
-    case UploadError_No_Write_Permissions:
+    case ClientStatusReportingStatus::UploadError_No_Write_Permissions:
         return QByteArrayLiteral("UploadResult.NO_WRITE_PERMISSIONS");
-    case UploadError_ServerError:
+    case ClientStatusReportingStatus::UploadError_ServerError:
         return QByteArrayLiteral("UploadResult.SERVER_ERROR");
-    case UploadError_Virus_Detected:
+    case ClientStatusReportingStatus::UploadError_Virus_Detected:
         return QByteArrayLiteral("UploadResult.VIRUS_DETECTED");
-    case Count:
+    case ClientStatusReportingStatus::Count:
         return {};
     };
     return {};

diff --git a/src/libsync/clientstatusreportingcommon.h b/src/libsync/clientstatusreportingcommon.h
@@ -17,7 +17,7 @@
 #include <QtCore/qbytearray.h>
 
 namespace OCC {
-enum ClientStatusReportingStatus {
+enum class ClientStatusReportingStatus {
     DownloadError_Cannot_Create_File = 0,
     DownloadError_Conflict,
     DownloadError_ConflictCaseClash,

diff --git a/src/libsync/clientstatusreportingdatabase.cpp b/src/libsync/clientstatusreportingdatabase.cpp
@@ -147,10 +147,10 @@ QString ClientStatusReportingDatabase::makeDbPath(const Account *account) const
 bool ClientStatusReportingDatabase::updateStatusNamesHash() const
 {
     QByteArray statusNamesContatenated;
-    for (int i = 0; i < ClientStatusReportingStatus::Count; ++i) {
+    for (int i = 0; i < static_cast<int>(ClientStatusReportingStatus::Count); ++i) {
         statusNamesContatenated += clientStatusstatusStringFromNumber(static_cast<ClientStatusReportingStatus>(i));
     }
-    statusNamesContatenated += QByteArray::number(ClientStatusReportingStatus::Count);
+    statusNamesContatenated += QByteArray::number(static_cast<int>(ClientStatusReportingStatus::Count));
     const auto statusNamesHashCurrent = QCryptographicHash::hash(statusNamesContatenated, QCryptographicHash::Md5).toHex();
     const auto statusNamesHashFromDb = getStatusNamesHash();
 

diff --git a/src/libsync/clientstatusreportingnetwork.cpp b/src/libsync/clientstatusreportingnetwork.cpp
@@ -162,32 +162,32 @@ QVariantMap ClientStatusReportingNetwork::prepareReport() const
 
 QByteArray ClientStatusReportingNetwork::classifyStatus(const ClientStatusReportingStatus status)
 {
-    Q_ASSERT(status >= 0 && status < Count);
-    if (status < 0 || status >= ClientStatusReportingStatus::Count) {
-        qCDebug(lcClientStatusReportingNetwork) << "Invalid status:" << status;
+    Q_ASSERT(static_cast<int>(status) >= 0 && static_cast<int>(status) < static_cast<int>(ClientStatusReportingStatus::Count));
+    if (static_cast<int>(status) < 0 || static_cast<int>(status) >= static_cast<int>(ClientStatusReportingStatus::Count)) {
+        qCDebug(lcClientStatusReportingNetwork) << "Invalid status:" << static_cast<int>(status);
         return {};
     }
 
     switch (status) {
-    case DownloadError_Conflict:
-    case DownloadError_ConflictCaseClash:
-    case DownloadError_ConflictInvalidCharacters:
-    case UploadError_Conflict:
-    case UploadError_ConflictInvalidCharacters:
+    case ClientStatusReportingStatus::DownloadError_Conflict:
+    case ClientStatusReportingStatus::DownloadError_ConflictCaseClash:
+    case ClientStatusReportingStatus::DownloadError_ConflictInvalidCharacters:
+    case ClientStatusReportingStatus::UploadError_Conflict:
+    case ClientStatusReportingStatus::UploadError_ConflictInvalidCharacters:
         return statusReportCategorySyncConflicts;
-    case DownloadError_Cannot_Create_File:
-    case DownloadError_No_Free_Space:
-    case DownloadError_ServerError:
-    case DownloadError_Virtual_File_Hydration_Failure:
-    case UploadError_No_Free_Space:
-    case UploadError_No_Write_Permissions:
-    case UploadError_ServerError:
+    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 UploadError_Virus_Detected:
+    case ClientStatusReportingStatus::UploadError_Virus_Detected:
         return statusReportCategoryVirus;
-    case E2EeError_GeneralError:
+    case ClientStatusReportingStatus::E2EeError_GeneralError:
         return statusReportCategoryE2eErrors;
-    case Count:
+    case ClientStatusReportingStatus::Count:
         return {};
     };
     return {};

diff --git a/src/libsync/owncloudpropagator_p.h b/src/libsync/owncloudpropagator_p.h
@@ -47,8 +47,6 @@ inline bool fileIsStillChanging(const OCC::SyncFileItem &item)
 }
 
 namespace OCC {
-
-
 inline QByteArray getEtagFromReply(QNetworkReply *reply)
 {
     QByteArray ocEtag = parseEtag(reply->rawHeader("OC-ETag"));

diff --git a/test/testclientstatusreporting.cpp b/test/testclientstatusreporting.cpp
@@ -86,12 +86,12 @@ private slots:
             account->reportClientStatus(OCC::ClientStatusReportingStatus::UploadError_ServerError);
             account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_ServerError);
             account->reportClientStatus(OCC::ClientStatusReportingStatus::DownloadError_Virtual_File_Hydration_Failure);
-            // 3 occurances of UploadError_No_Write_Permissions
+            // 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);
 
-            // 3 occurances of UploadError_Virus_Detected
+            // 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);
@@ -123,7 +123,7 @@ private slots:
             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 UploadError_No_Write_Permissions
+            // among those, 3 occurances of case ClientStatusReportingStatus::UploadError_No_Write_Permissions
             QCOMPARE(problemsNoWritePermissions.value("count"), 3);
 
             bodyReceivedAndParsed.clear();
-Original file line number
+Diff line change
@@ Expand Up @@
     }
     namespace OCC {
     inline QByteArray getEtagFromReply(QNetworkReply *reply)
     {
         QByteArray ocEtag = parseEtag(reply->rawHeader("OC-ETag"));
@@ Expand Down @@