From 65261091fc40bac55723b21f8885943b81188bc8 Mon Sep 17 00:00:00 2001 From: alex-z Date: Thu, 7 Dec 2023 11:52:58 +0100 Subject: [PATCH] Review comments. Part IV. Signed-off-by: alex-z --- src/libsync/clientstatusreporting.cpp | 1 - src/libsync/clientstatusreporting.h | 2 -- src/libsync/clientstatusreportingcommon.cpp | 1 - src/libsync/clientstatusreportingdatabase.cpp | 3 ++- src/libsync/clientstatusreportingnetwork.cpp | 8 +++++++- src/libsync/clientstatusreportingnetwork.h | 3 ++- src/libsync/networkjobs.h | 7 +++++++ src/libsync/owncloudpropagator.cpp | 5 +++-- 8 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/libsync/clientstatusreporting.cpp b/src/libsync/clientstatusreporting.cpp index 0566995737282..10033d76f1f26 100644 --- a/src/libsync/clientstatusreporting.cpp +++ b/src/libsync/clientstatusreporting.cpp @@ -23,7 +23,6 @@ namespace OCC Q_LOGGING_CATEGORY(lcClientStatusReporting, "nextcloud.sync.clientstatusreporting", QtInfoMsg) ClientStatusReporting::ClientStatusReporting(Account *account) - : _account(account) { for (int i = 0; i < ClientStatusReportingStatus::Count; ++i) { const auto statusString = clientStatusstatusStringFromNumber(static_cast(i)); diff --git a/src/libsync/clientstatusreporting.h b/src/libsync/clientstatusreporting.h index 669c4dc4fa1d4..e0005e06c2461 100644 --- a/src/libsync/clientstatusreporting.h +++ b/src/libsync/clientstatusreporting.h @@ -40,8 +40,6 @@ class OWNCLOUDSYNC_EXPORT ClientStatusReporting // reporting must happen via Account void reportClientStatus(const ClientStatusReportingStatus status) const; - Account *_account = nullptr; - bool _isInitialized = false; QHash _statusStrings; diff --git a/src/libsync/clientstatusreportingcommon.cpp b/src/libsync/clientstatusreportingcommon.cpp index 62e48aa2d6669..a57c11bf16063 100644 --- a/src/libsync/clientstatusreportingcommon.cpp +++ b/src/libsync/clientstatusreportingcommon.cpp @@ -11,7 +11,6 @@ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License * for more details. */ -#pragma once #include "clientstatusreportingcommon.h" #include diff --git a/src/libsync/clientstatusreportingdatabase.cpp b/src/libsync/clientstatusreportingdatabase.cpp index 6b8a2da8adcee..b1cde5bb291b8 100644 --- a/src/libsync/clientstatusreportingdatabase.cpp +++ b/src/libsync/clientstatusreportingdatabase.cpp @@ -173,7 +173,8 @@ QVector ClientStatusReportingDatabase::getTableColumns(const QString QVector columns; QSqlQuery query; const auto prepareResult = query.prepare(QStringLiteral("PRAGMA table_info('%1');").arg(table)); - if (!query.exec()) { + if (!prepareResult || !query.exec()) { + qCDebug(lcClientStatusReportingDatabase) << "Could get table columns" << query.lastError().text(); return columns; } while (query.next()) { diff --git a/src/libsync/clientstatusreportingnetwork.cpp b/src/libsync/clientstatusreportingnetwork.cpp index 0ec7d2e978a4b..0b6caf0fd9fc2 100644 --- a/src/libsync/clientstatusreportingnetwork.cpp +++ b/src/libsync/clientstatusreportingnetwork.cpp @@ -78,11 +78,17 @@ void ClientStatusReportingNetwork::sendReportToServer() return; } + if (!_account) { + return; + } + const auto clientStatusReportingJob = new JsonApiJob(_account->sharedFromThis(), QStringLiteral("ocs/v2.php/apps/security_guard/diagnostics")); clientStatusReportingJob->setBody(QJsonDocument::fromVariant(report)); clientStatusReportingJob->setVerb(SimpleApiJob::Verb::Put); connect(clientStatusReportingJob, &JsonApiJob::jsonReceived, [this](const QJsonDocument &json, int statusCode) { - if (statusCode == 0 || statusCode == 200 || statusCode == 201 || statusCode == 204) { + const auto isSuccess = statusCode == HttpErrorCodeNone || statusCode == HttpErrorCodeSuccess || statusCode == HttpErrorCodeSuccessCreated + || statusCode == HttpErrorCodeSuccessNoContent; + if (isSuccess) { const auto metaFromJson = json.object().value(QStringLiteral("ocs")).toObject().value(QStringLiteral("meta")).toObject(); const auto codeFromJson = metaFromJson.value(QStringLiteral("statuscode")).toInt(); if (codeFromJson == 0 || codeFromJson == 200 || codeFromJson == 201 || codeFromJson == 204) { diff --git a/src/libsync/clientstatusreportingnetwork.h b/src/libsync/clientstatusreportingnetwork.h index e41bc5859ff0f..cc71ee0c0ef22 100644 --- a/src/libsync/clientstatusreportingnetwork.h +++ b/src/libsync/clientstatusreportingnetwork.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -57,7 +58,7 @@ private slots: static QString dbPathForTesting; private: - Account *_account = nullptr; + QPointer _account = nullptr; QSharedPointer _database; diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index 4fc397a7a4129..b299bdb0aa387 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -30,6 +30,13 @@ class QDomDocument; namespace OCC { +constexpr auto HttpErrorCodeNone = 0; +constexpr auto HttpErrorCodeSuccess = 200; +constexpr auto HttpErrorCodeSuccessCreated = 201; +constexpr auto HttpErrorCodeSuccessNoContent = 204; +constexpr auto HttpErrorCodeBadRequest = 400; +constexpr auto HttpErrorCodeUnsupportedMediaType = 415; + struct HttpError { int code; // HTTP error code diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index e2628d48a1b99..118bf86d55630 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -356,9 +356,10 @@ void PropagateItemJob::reportClientStatuses() propagator()->account()->reportClientStatus(ClientStatusReportingStatus::UploadError_ConflictInvalidCharacters); } else if (_item->_status == SyncFileItem::Status::FileNameInvalid) { propagator()->account()->reportClientStatus(ClientStatusReportingStatus::DownloadError_ConflictInvalidCharacters); - } else if (_item->_httpErrorCode != 0 && _item->_httpErrorCode != 200 && _item->_httpErrorCode != 201 && _item->_httpErrorCode != 204) { + } else if (_item->_httpErrorCode != HttpErrorCodeNone && _item->_httpErrorCode != HttpErrorCodeSuccess && _item->_httpErrorCode != HttpErrorCodeSuccessCreated + && _item->_httpErrorCode != HttpErrorCodeSuccessNoContent) { if (_item->_direction == SyncFileItem::Up) { - const auto isCodeBadReqOrUnsupportedMediaType = (_item->_httpErrorCode == 400 || _item->_httpErrorCode == 415); + 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"))