From 07e0504a1c3243d1966d66b81c3bc1d3494f1a79 Mon Sep 17 00:00:00 2001 From: alex-z Date: Wed, 6 Dec 2023 15:33:15 +0100 Subject: [PATCH] Review comments. Part I. Signed-off-by: alex-z --- src/gui/tray/NCBusyIndicator.qml | 2 +- src/libsync/CMakeLists.txt | 1 - src/libsync/account.cpp | 10 +++--- src/libsync/account.h | 4 +-- src/libsync/clientstatusreporting.cpp | 38 +++++++++------------ src/libsync/clientstatusreporting.h | 24 ++++++++----- src/libsync/clientstatusreportingrecord.cpp | 24 ------------- src/libsync/clientstatusreportingrecord.h | 5 ++- src/libsync/owncloudpropagator_p.h | 2 +- 9 files changed, 44 insertions(+), 66 deletions(-) delete mode 100644 src/libsync/clientstatusreportingrecord.cpp diff --git a/src/gui/tray/NCBusyIndicator.qml b/src/gui/tray/NCBusyIndicator.qml index ddead28d4b07d..973d3dee1ed62 100644 --- a/src/gui/tray/NCBusyIndicator.qml +++ b/src/gui/tray/NCBusyIndicator.qml @@ -42,7 +42,7 @@ BusyIndicator { RotationAnimator { target: contentImage - running: false + running: root.running onRunningChanged: contentImage.rotation = 0 from: 0 to: 360 diff --git a/src/libsync/CMakeLists.txt b/src/libsync/CMakeLists.txt index bd7931a8670d3..0e0f85b65dda2 100644 --- a/src/libsync/CMakeLists.txt +++ b/src/libsync/CMakeLists.txt @@ -27,7 +27,6 @@ set(libsync_SRCS clientstatusreporting.h clientstatusreporting.cpp clientstatusreportingrecord.h - clientstatusreportingrecord.cpp cookiejar.h cookiejar.cpp discovery.h diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index 1d27a74ffaca2..7ceb196bdf0b2 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -286,19 +286,17 @@ void Account::setPushNotificationsReconnectInterval(int interval) void Account::trySetupClientStatusReporting() { - if (_capabilities.isClientStatusReportingEnabled()) { - if (!_clientStatusReporting) { - _clientStatusReporting.reset(new ClientStatusReporting(this)); - } + if (!_capabilities.isClientStatusReportingEnabled()) { + _clientStatusReporting.reset(); return; } if (!_clientStatusReporting) { - _clientStatusReporting.reset(); + _clientStatusReporting = std::make_unique(this); } } -void Account::reportClientStatus(const ClientStatusReporting::Status status) +void Account::reportClientStatus(const ClientStatusReporting::Status status) const { if (_clientStatusReporting) { _clientStatusReporting->reportClientStatus(status); diff --git a/src/libsync/account.h b/src/libsync/account.h index 1c51bb7a0c6b5..005115adc0ff1 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -308,7 +308,7 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject void trySetupClientStatusReporting(); - void reportClientStatus(const ClientStatusReporting::Status status); + void reportClientStatus(const ClientStatusReporting::Status status) const; [[nodiscard]] std::shared_ptr userStatusConnector() const; @@ -444,7 +444,7 @@ private slots: PushNotifications *_pushNotifications = nullptr; - QScopedPointer _clientStatusReporting; + std::unique_ptr _clientStatusReporting; std::shared_ptr _userStatusConnector; diff --git a/src/libsync/clientstatusreporting.cpp b/src/libsync/clientstatusreporting.cpp index 617405047c5ec..1930ce167f69b 100644 --- a/src/libsync/clientstatusreporting.cpp +++ b/src/libsync/clientstatusreporting.cpp @@ -52,7 +52,6 @@ void ClientStatusReporting::init() { Q_ASSERT(!_isInitialized); if (_isInitialized) { - qCDebug(lcClientStatusReporting) << "Double call to init"; return; } @@ -82,30 +81,12 @@ void ClientStatusReporting::init() return; } - if (!query.prepare(QStringLiteral("CREATE INDEX IF NOT EXISTS name ON clientstatusreporting(name);")) || !query.exec()) { - qCDebug(lcClientStatusReporting) << "Could not create index on clientstatusreporting table:" << query.lastError().text(); - return; - } - if (!query.prepare(QStringLiteral("CREATE TABLE IF NOT EXISTS keyvalue(key VARCHAR(4096), value VARCHAR(4096), PRIMARY KEY(key))")) || !query.exec()) { qCDebug(lcClientStatusReporting) << "Could not setup client keyvalue table:" << query.lastError().text(); return; } - // prevent issues in case enum gets changed in future, hash its value and clean the db in case there was a change - QByteArray statusNamesContatenated; - for (int i = 0; i < ClientStatusReporting::Status::Count; ++i) { - statusNamesContatenated += statusStringFromNumber(static_cast(i)); - } - statusNamesContatenated += QByteArray::number(ClientStatusReporting::Status::Count); - const auto statusNamesHashCurrent = QCryptographicHash::hash(statusNamesContatenated, QCryptographicHash::Md5).toHex(); - const auto statusNamesHashFromDb = getStatusNamesHash(); - - if (statusNamesHashCurrent != statusNamesHashFromDb) { - deleteClientStatusReportingRecords(); - setStatusNamesHash(statusNamesHashCurrent); - } - // + updateStatusNamesHash(); _clientStatusReportingSendTimer.setInterval(clientStatusReportingTrySendTimerInterval); connect(&_clientStatusReportingSendTimer, &QTimer::timeout, this, &ClientStatusReporting::sendReportToServer); @@ -212,7 +193,6 @@ void ClientStatusReporting::sendReportToServer() const auto report = prepareReport(); if (report.isEmpty()) { - qCDebug(lcClientStatusReporting) << "Failed to generate report. Report is empty."; return; } @@ -250,6 +230,22 @@ QString ClientStatusReporting::makeDbPath() const return ConfigFile().configPath() + QStringLiteral(".userdata_%1.db").arg(QString::fromLatin1(databaseIdHash.left(6).toHex())); } +void ClientStatusReporting::updateStatusNamesHash() +{ + QByteArray statusNamesContatenated; + for (int i = 0; i < ClientStatusReporting::Status::Count; ++i) { + statusNamesContatenated += statusStringFromNumber(static_cast(i)); + } + statusNamesContatenated += QByteArray::number(ClientStatusReporting::Status::Count); + const auto statusNamesHashCurrent = QCryptographicHash::hash(statusNamesContatenated, QCryptographicHash::Md5).toHex(); + const auto statusNamesHashFromDb = getStatusNamesHash(); + + if (statusNamesHashCurrent != statusNamesHashFromDb) { + deleteClientStatusReportingRecords(); + setStatusNamesHash(statusNamesHashCurrent); + } +} + quint64 ClientStatusReporting::getLastSentReportTimestamp() const { QMutexLocker locker(&_mutex); diff --git a/src/libsync/clientstatusreporting.h b/src/libsync/clientstatusreporting.h index 70b7508c028b0..38198e18af893 100644 --- a/src/libsync/clientstatusreporting.h +++ b/src/libsync/clientstatusreporting.h @@ -16,15 +16,18 @@ #include "owncloudlib.h" #include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include namespace OCC { @@ -52,6 +55,7 @@ class OWNCLOUDSYNC_EXPORT ClientStatusReporting : public QObject UploadError_Virus_Detected, Count, }; + Q_ENUM(Status); explicit ClientStatusReporting(Account *account, QObject *parent = nullptr); ~ClientStatusReporting() override; @@ -78,6 +82,8 @@ class OWNCLOUDSYNC_EXPORT ClientStatusReporting : public QObject [[nodiscard]] QString makeDbPath() const; + void updateStatusNamesHash(); + private slots: void sendReportToServer(); diff --git a/src/libsync/clientstatusreportingrecord.cpp b/src/libsync/clientstatusreportingrecord.cpp deleted file mode 100644 index 4d795de64b3eb..0000000000000 --- a/src/libsync/clientstatusreportingrecord.cpp +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright (C) 2023 by Oleksandr Zolotov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY - * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * for more details. - */ - -#include "clientstatusreportingrecord.h" - -namespace OCC -{ - -bool ClientStatusReportingRecord::isValid() const -{ - return _status >= 0 && !_name.isEmpty() && _lastOccurence > 0; -} -} diff --git a/src/libsync/clientstatusreportingrecord.h b/src/libsync/clientstatusreportingrecord.h index 6595177ec0995..94f190a0ee210 100644 --- a/src/libsync/clientstatusreportingrecord.h +++ b/src/libsync/clientstatusreportingrecord.h @@ -30,6 +30,9 @@ struct OWNCLOUDSYNC_EXPORT ClientStatusReportingRecord { quint64 _numOccurences = 1; quint64 _lastOccurence = 0; - [[nodiscard]] bool isValid() const; + [[nodiscard]] inline bool isValid() const + { + return _status >= 0 && !_name.isEmpty() && _lastOccurence > 0; + } }; } diff --git a/src/libsync/owncloudpropagator_p.h b/src/libsync/owncloudpropagator_p.h index 7d0c121bb4af6..cc905c2d4c3a8 100644 --- a/src/libsync/owncloudpropagator_p.h +++ b/src/libsync/owncloudpropagator_p.h @@ -63,7 +63,7 @@ inline QByteArray getEtagFromReply(QNetworkReply *reply) return ret; } -inline QPair getExceptionFromReply(QNetworkReply *reply) +inline QPair getExceptionFromReply(QNetworkReply * const reply) { Q_ASSERT(reply); if (!reply) {