From 276524786861883c3d2dac31e263c4f5f1ae80c6 Mon Sep 17 00:00:00 2001 From: alex-z Date: Mon, 11 Dec 2023 12:57:58 +0100 Subject: [PATCH] Fix review comments from round II. Signed-off-by: alex-z --- src/libsync/clientstatusreporting.cpp | 20 ++++------ src/libsync/clientstatusreportingcommon.cpp | 40 ++++++++++--------- src/libsync/clientstatusreportingcommon.h | 2 +- src/libsync/clientstatusreportingdatabase.cpp | 4 +- src/libsync/clientstatusreportingnetwork.cpp | 36 ++++++++--------- src/libsync/owncloudpropagator_p.h | 2 - test/testclientstatusreporting.cpp | 6 +-- 7 files changed, 53 insertions(+), 57 deletions(-) diff --git a/src/libsync/clientstatusreporting.cpp b/src/libsync/clientstatusreporting.cpp index 86a7025961959..682ff206e1c31 100644 --- 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(ClientStatusReportingStatus::Count); ++i) { const auto statusString = clientStatusstatusStringFromNumber(static_cast(i)); _statusStrings[i] = statusString; } - if (_statusStrings.size() < ClientStatusReportingStatus::Count) { + if (_statusStrings.size() < static_cast(ClientStatusReportingStatus::Count)) { return; } @@ -46,11 +46,7 @@ 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 { @@ -58,15 +54,15 @@ void ClientStatusReporting::reportClientStatus(const ClientStatusReportingStatus 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(status) >= 0 && static_cast(status) < static_cast(ClientStatusReportingStatus::Count)); + if (static_cast(status) < 0 || static_cast(status) >= static_cast(ClientStatusReportingStatus::Count)) { + qCDebug(lcClientStatusReporting) << "Trying to report invalid status:" << static_cast(status); return; } ClientStatusReportingRecord record; - record._name = _statusStrings[status]; - record._status = status; + record._name = _statusStrings[static_cast(status)]; + record._status = static_cast(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 index a57c11bf16063..fabbf2b0fbbaf 100644 --- a/src/libsync/clientstatusreportingcommon.cpp +++ b/src/libsync/clientstatusreportingcommon.cpp @@ -13,47 +13,49 @@ */ #include "clientstatusreportingcommon.h" -#include +#include 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(status) >= 0 && static_cast(status) < static_cast(ClientStatusReportingStatus::Count)); + if (static_cast(status) < 0 || static_cast(status) >= static_cast(ClientStatusReportingStatus::Count)) { + qCDebug(lcClientStatusReportingCommon) << "Invalid status:" << static_cast(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 index db18cd1580fa0..4e08ca1efa6fd 100644 --- a/src/libsync/clientstatusreportingcommon.h +++ b/src/libsync/clientstatusreportingcommon.h @@ -17,7 +17,7 @@ #include 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 index b1cde5bb291b8..b272e284ddabd 100644 --- 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(ClientStatusReportingStatus::Count); ++i) { statusNamesContatenated += clientStatusstatusStringFromNumber(static_cast(i)); } - statusNamesContatenated += QByteArray::number(ClientStatusReportingStatus::Count); + statusNamesContatenated += QByteArray::number(static_cast(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 index 9413879d20188..bee15f37bb564 100644 --- 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(status) >= 0 && static_cast(status) < static_cast(ClientStatusReportingStatus::Count)); + if (static_cast(status) < 0 || static_cast(status) >= static_cast(ClientStatusReportingStatus::Count)) { + qCDebug(lcClientStatusReportingNetwork) << "Invalid status:" << static_cast(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 index cc905c2d4c3a8..beb150648db87 100644 --- 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 index e0f625ae00876..b018f20336666 100644 --- 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();