From b52696b6b0b86760dfe2a7329ae886c6242ab2e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Kunz?= Date: Tue, 15 Oct 2024 15:36:41 +0200 Subject: [PATCH] KDESKTOP-1130 - Fix locked version management --- src/gui/clientgui.cpp | 5 +- src/gui/guirequests.cpp | 23 +++---- src/gui/guirequests.h | 2 +- src/gui/synthesispopover.cpp | 91 +++++++++++--------------- src/gui/synthesispopover.h | 8 ++- src/gui/versionwidget.cpp | 3 +- src/libcommon/comm.h | 6 +- src/libcommon/utility/types.h | 1 - src/server/appserver.cpp | 13 ++-- src/server/updater/abstractupdater.cpp | 13 ++-- src/server/updater/abstractupdater.h | 6 +- src/server/updater/sparkleupdater.h | 2 - src/server/updater/sparkleupdater.mm | 10 --- src/server/updater/updatemanager.cpp | 13 ++-- src/server/updater/updatemanager.h | 4 +- 15 files changed, 87 insertions(+), 113 deletions(-) diff --git a/src/gui/clientgui.cpp b/src/gui/clientgui.cpp index 6b4151d1f..2a3a657ed 100644 --- a/src/gui/clientgui.cpp +++ b/src/gui/clientgui.cpp @@ -307,7 +307,7 @@ void ClientGui::showSynthesisDialog() { _synthesisPopover->setPosition(trayIconRect); raiseDialog(_synthesisPopover.get()); } - _synthesisPopover->onUpdateAvailabalityChange(); + _synthesisPopover->refreshLockedStatus(); } } @@ -322,8 +322,7 @@ int ClientGui::driveErrorsCount(int driveDbId, bool unresolved) const { } const QString ClientGui::folderPath(int syncDbId, const QString &filePath) const { - QString fullFilePath = QString(); - + QString fullFilePath; const auto syncInfoIt = _syncInfoMap.find(syncDbId); if (syncInfoIt != _syncInfoMap.end()) { fullFilePath = syncInfoIt->second.localPath() + dirSeparator + filePath; diff --git a/src/gui/guirequests.cpp b/src/gui/guirequests.cpp index 6dec3e5cd..bb3d53bc5 100644 --- a/src/gui/guirequests.cpp +++ b/src/gui/guirequests.cpp @@ -1209,8 +1209,7 @@ ExitCode GuiRequests::changeDistributionChannel(const DistributionChannel channe QDataStream paramsStream(¶ms, QIODevice::WriteOnly); paramsStream << channel; - QByteArray results; - if (!CommClient::instance()->execute(RequestNum::UPDATER_CHANGE_CHANNEL, params, results)) { + if (QByteArray results; !CommClient::instance()->execute(RequestNum::UPDATER_CHANGE_CHANNEL, params, results)) { return ExitCode::SystemError; } return ExitCode::Ok; @@ -1239,8 +1238,7 @@ ExitCode GuiRequests::updateState(UpdateState &state) { } ExitCode GuiRequests::startInstaller() { - QByteArray results; - if (!CommClient::instance()->execute(RequestNum::UPDATER_START_INSTALLER, QByteArray(), results)) { + if (QByteArray results; !CommClient::instance()->execute(RequestNum::UPDATER_START_INSTALLER, QByteArray(), results)) { return ExitCode::SystemError; } return ExitCode::Ok; @@ -1251,19 +1249,18 @@ ExitCode GuiRequests::skipUpdate(const std::string &version) { QDataStream paramsStream(¶ms, QIODevice::WriteOnly); paramsStream << QString::fromStdString(version); - QByteArray results; - if (!CommClient::instance()->execute(RequestNum::UPDATER_SKIP_VERSION, params, results)) { + if (QByteArray results; !CommClient::instance()->execute(RequestNum::UPDATER_SKIP_VERSION, params, results)) { return ExitCode::SystemError; } return ExitCode::Ok; } -ExitCode GuiRequests::unskipUpdate() { - QByteArray results; - if (!CommClient::instance()->execute(RequestNum::RECONSIDER_SKIPPED_UPDATE, QByteArray(), results)) { - return ExitCode::SystemError; - } - return ExitCode::Ok; -} +// ExitCode GuiRequests::unskipUpdate() { +// QByteArray results; +// if (!CommClient::instance()->execute(RequestNum::RECONSIDER_SKIPPED_UPDATE, QByteArray(), results)) { +// return ExitCode::SystemError; +// } +// return ExitCode::Ok; +// } } // namespace KDC diff --git a/src/gui/guirequests.h b/src/gui/guirequests.h index a3fc6c6e3..f64229122 100644 --- a/src/gui/guirequests.h +++ b/src/gui/guirequests.h @@ -133,6 +133,6 @@ struct GuiRequests { static ExitCode updateState(UpdateState &state); static ExitCode startInstaller(); static ExitCode skipUpdate(const std::string &version); - static ExitCode unskipUpdate(); + // static ExitCode unskipUpdate(); }; } // namespace KDC diff --git a/src/gui/synthesispopover.cpp b/src/gui/synthesispopover.cpp index c16d6207f..1ad909058 100644 --- a/src/gui/synthesispopover.cpp +++ b/src/gui/synthesispopover.cpp @@ -199,6 +199,11 @@ void SynthesisPopover::forceRedraw() { }); #endif } +void SynthesisPopover::refreshLockedStatus() { + auto updateState = UpdateState::Unknown; + GuiRequests::updateState(updateState); + onUpdateAvailabalityChange(updateState); +} void SynthesisPopover::changeEvent(QEvent *event) { QDialog::changeEvent(event); @@ -214,7 +219,7 @@ void SynthesisPopover::changeEvent(QEvent *event) { } void SynthesisPopover::paintEvent(QPaintEvent *event) { - Q_UNUSED(event); + Q_UNUSED(event) QScreen *screen = QGuiApplication::screenAt(_sysTrayIconRect.center()); if (!screen) { @@ -604,10 +609,10 @@ void SynthesisPopover::initUI() { connect(_statusBarWidget, &StatusBarWidget::resumeSync, this, &SynthesisPopover::onResumeSync); connect(_statusBarWidget, &StatusBarWidget::linkActivated, this, &SynthesisPopover::onLinkActivated); connect(_buttonsBarWidget, &ButtonsBarWidget::buttonToggled, this, &SynthesisPopover::onButtonBarToggled); - // TODO : put back for locked versions - // connect(UpdaterClient::instance(), &UpdaterClient::downloadStateChanged, this, - // &SynthesisPopover::onUpdateAvailabalityChange, - // Qt::UniqueConnection); + + connect(_lockedAppUpdateButton, &QPushButton::clicked, this, &SynthesisPopover::onStartInstaller, Qt::UniqueConnection); + connect(_gui.get(), &ClientGui::updateStateChanged, this, &SynthesisPopover::onUpdateAvailabalityChange, + Qt::UniqueConnection); } QUrl SynthesisPopover::syncUrl(int syncDbId, const QString &filePath) { @@ -1062,52 +1067,34 @@ void SynthesisPopover::onUpdateSynchronizedListWidget() { } } -void SynthesisPopover::onUpdateAvailabalityChange() { - // if (!_lockedAppUpdateButton || !_lockedAppUpdateOptionalLabel) return; - // if (_lockedAppVersionWidget->isHidden()) return; - // QString statusString; - // UpdateState updateState = UpdateState::Unknown; - // // try { - // // if (!UpdaterClient::instance()->isSparkleUpdater()) { - // // statusString = UpdaterClient::instance()->statusString(); - // // updateState = UpdaterClient::instance()->updateState(); - // // } else { - // // updateState = UpdateState::Ready; // On macOS, we just start the installer (Sparkle does the rest) - // // } - // // } catch (std::exception const &) { - // // return; - // // } - // if (GuiRequests::updateState(updateState) != ExitCode::Ok) { - // qCWarning(lcSynthesisPopover()) << "Failed to fetch update state."; - // } - // - // _lockedAppUpdateButton->setEnabled(updateState == UpdateState::Ready); - // _lockedAppUpdateOptionalLabel->setVisible(updateState != UpdateState::Ready && updateState != UpdateState::Downloading); - // switch (updateState) { - // case UpdateState::Ready: - // _lockedAppUpdateButton->setText(tr("Update")); - // break; - // case UpdateState::Downloading: - // _lockedAppUpdateButton->setText(tr("Update download in progress")); - // break; - // case UpdateState::Skipped: - // UpdaterClient::instance()->unskipUpdate(); - // case UpdateState::Checking: - // _lockedAppUpdateButton->setText(tr("Looking for update...")); - // break; - // case UpdateState::ManualOnly: - // _lockedAppUpdateButton->setText(tr("Manual update")); - // //_lockedAppUpdateOptionalLabel->setText(statusString); - // break; - // default: - // _lockedAppUpdateButton->setText(tr("Unavailable")); - // //_lockedAppUpdateOptionalLabel->setText(statusString); - // SentryHandler::instance()->captureMessage( - // SentryLevel::Fatal, "AppLocked", - // "406 uError received but unable to fetch an pdate: " + statusString.toStdString()); - // break; - // } - // connect(_lockedAppUpdateButton, &QPushButton::clicked, this, &SynthesisPopover::onStartInstaller, Qt::UniqueConnection); +void SynthesisPopover::onUpdateAvailabalityChange(const UpdateState updateState) { + if (!_lockedAppUpdateButton || !_lockedAppUpdateOptionalLabel) return; + if (_lockedAppVersionWidget->isHidden()) return; + + _lockedAppUpdateButton->setEnabled(updateState == UpdateState::Ready || updateState == UpdateState::Available); + _lockedAppUpdateOptionalLabel->setVisible(updateState != UpdateState::Ready && updateState != UpdateState::Downloading); + switch (updateState) { + case UpdateState::Ready: + case UpdateState::Available: + _lockedAppUpdateButton->setText(tr("Update")); + break; + case UpdateState::Downloading: + _lockedAppUpdateButton->setText(tr("Update download in progress")); + break; + case UpdateState::Checking: + _lockedAppUpdateButton->setText(tr("Looking for update...")); + break; + case UpdateState::ManualUpdateAvailable: + _lockedAppUpdateButton->setText(tr("Manual update")); + //_lockedAppUpdateOptionalLabel->setText(statusString); + break; + default: + _lockedAppUpdateButton->setText(tr("Unavailable")); + //_lockedAppUpdateOptionalLabel->setText(statusString); + SentryHandler::instance()->captureMessage(SentryLevel::Fatal, "AppLocked", + "406 uError received but unable to fetch an update"); + break; + } } void SynthesisPopover::onStartInstaller() noexcept { @@ -1124,7 +1111,7 @@ void SynthesisPopover::onAppVersionLocked(bool currentVersionLocked) { _lockedAppVersionWidget->show(); setFixedSize(lockedWindowSize); _gui->closeAllExcept(this); - onUpdateAvailabalityChange(); + refreshLockedStatus(); } else if (!currentVersionLocked && _mainWidget->isHidden()) { _lockedAppVersionWidget->hide(); _mainWidget->show(); diff --git a/src/gui/synthesispopover.h b/src/gui/synthesispopover.h index b8ddfb3d5..71cab98ec 100644 --- a/src/gui/synthesispopover.h +++ b/src/gui/synthesispopover.h @@ -63,6 +63,8 @@ class SynthesisPopover : public QDialog { void setPosition(const QRect &sysTrayIconRect); void forceRedraw(); + void refreshLockedStatus(); + signals: void showParametersDialog(int syncDbId = 0, bool errorPage = false); void exit(); @@ -87,7 +89,7 @@ class SynthesisPopover : public QDialog { void onRefreshErrorList(int driveDbId); void onAppVersionLocked(bool currentVersionLocked); void onRefreshStatusNeeded(); - void onUpdateAvailabalityChange(); + void onUpdateAvailabalityChange(UpdateState updateState); private: std::shared_ptr _gui; @@ -127,8 +129,8 @@ class SynthesisPopover : public QDialog { void paintEvent(QPaintEvent *event) override; bool event(QEvent *event) override; - inline QColor backgroundMainColor() const { return _backgroundMainColor; } - inline void setBackgroundMainColor(const QColor &value) { _backgroundMainColor = value; } + [[nodiscard]] QColor backgroundMainColor() const { return _backgroundMainColor; } + void setBackgroundMainColor(const QColor &value) { _backgroundMainColor = value; } void initUI(); QUrl syncUrl(int syncDbId, const QString &filePath); diff --git a/src/gui/versionwidget.cpp b/src/gui/versionwidget.cpp index a4fe48b80..7b6d28d9a 100644 --- a/src/gui/versionwidget.cpp +++ b/src/gui/versionwidget.cpp @@ -147,8 +147,7 @@ void VersionWidget::refresh(UpdateState state /*= UpdateState::Unknown*/) const break; } case UpdateState::Available: - case UpdateState::Ready: - case UpdateState::Skipped: { + case UpdateState::Ready: { statusString = tr("An update is available: %1").arg(versionStr); showReleaseNote = true; showUpdateButton = true; diff --git a/src/libcommon/comm.h b/src/libcommon/comm.h index 379cd30ad..6f1d085ef 100644 --- a/src/libcommon/comm.h +++ b/src/libcommon/comm.h @@ -117,7 +117,7 @@ enum class RequestNum { UPDATER_STATE, UPDATER_START_INSTALLER, UPDATER_SKIP_VERSION, - RECONSIDER_SKIPPED_UPDATE, + // RECONSIDER_SKIPPED_UPDATE, UTILITY_CRASH, UTILITY_QUIT, }; @@ -270,8 +270,8 @@ inline std::string toString(RequestNum e) { return "UPDATER_START_INSTALLER"; case RequestNum::UPDATER_SKIP_VERSION: return "UPDATER_SKIP_VERSION"; - case RequestNum::RECONSIDER_SKIPPED_UPDATE: - return "RECONSIDER_SKIPPED_UPDATE"; + // case RequestNum::RECONSIDER_SKIPPED_UPDATE: + // return "RECONSIDER_SKIPPED_UPDATE"; case RequestNum::UTILITY_CRASH: return "UTILITY_CRASH"; case RequestNum::UTILITY_QUIT: diff --git a/src/libcommon/utility/types.h b/src/libcommon/utility/types.h index beb5987eb..98df15d8e 100644 --- a/src/libcommon/utility/types.h +++ b/src/libcommon/utility/types.h @@ -423,7 +423,6 @@ enum class UpdateState { ManualUpdateAvailable, Downloading, Ready, - Skipped, CheckError, DownloadError, UpdateError, diff --git a/src/server/appserver.cpp b/src/server/appserver.cpp index 4ff671b9c..a74d3b1c3 100644 --- a/src/server/appserver.cpp +++ b/src/server/appserver.cpp @@ -1983,10 +1983,10 @@ void AppServer::onRequestReceived(int id, RequestNum num, const QByteArray ¶ _updateManager->skipVersion(tmp.toStdString()); break; } - case RequestNum::RECONSIDER_SKIPPED_UPDATE: { - _updateManager->unskipVersion(); - break; - } + // case RequestNum::RECONSIDER_SKIPPED_UPDATE: { + // _updateManager->unskipVersion(); + // break; + // } case RequestNum::UTILITY_CRASH: { resultStream << ExitCode::Ok; QTimer::singleShot(QUIT_DELAY, []() { CommonUtility::crash(); }); @@ -3906,13 +3906,14 @@ void AppServer::addError(const Error &error) { // Decrease JobManager pool capacity JobManager::instance()->decreasePoolCapacity(); + } else if (error.exitCode() == ExitCode::UpdateRequired) { + UpdateManager::unskipVersion(); } if (!ServerRequests::isAutoResolvedError(error)) { // Send error to sentry only for technical errors SentryUser sentryUser(user.email().c_str(), user.name().c_str(), std::to_string(user.userId()).c_str()); - SentryHandler::instance()->captureMessage(SentryLevel::Warning, "AppServer::addError", error.errorString().c_str(), - sentryUser); + SentryHandler::instance()->captureMessage(SentryLevel::Warning, "AppServer::addError", error.errorString(), sentryUser); } } diff --git a/src/server/updater/abstractupdater.cpp b/src/server/updater/abstractupdater.cpp index cc7bd18e1..84dbed7c2 100644 --- a/src/server/updater/abstractupdater.cpp +++ b/src/server/updater/abstractupdater.cpp @@ -55,16 +55,19 @@ void AbstractUpdater::onAppVersionReceived() { } } -void AbstractUpdater::skipVersion(const std::string& skippedVersion) const { +void AbstractUpdater::skipVersion(const std::string& skippedVersion) { ParametersCache::instance()->parameters().setSeenVersion(skippedVersion); ParametersCache::instance()->save(); } -void AbstractUpdater::unskipVersion() const { - ParametersCache::instance()->parameters().setSeenVersion(""); - ParametersCache::instance()->save(); + +void AbstractUpdater::unskipVersion() { + if (!ParametersCache::instance()->parameters().seenVersion().empty()) { + ParametersCache::instance()->parameters().setSeenVersion(""); + ParametersCache::instance()->save(); + } } -bool AbstractUpdater::isVersionSkipped(const std::string& version) const { +bool AbstractUpdater::isVersionSkipped(const std::string& version) { if (version.empty()) return false; const auto seenVerison = ParametersCache::instance()->parameters().seenVersion(); diff --git a/src/server/updater/abstractupdater.h b/src/server/updater/abstractupdater.h index f4992901b..6e5d00817 100644 --- a/src/server/updater/abstractupdater.h +++ b/src/server/updater/abstractupdater.h @@ -63,9 +63,9 @@ class AbstractUpdater { virtual void setQuitCallback(const std::function &quitCallback) { /* Redefined in child class if necessary */ } void setStateChangeCallback(const std::function &stateChangeCallback); - void skipVersion(const std::string &skippedVersion) const; - void unskipVersion() const; - [[nodiscard]] bool isVersionSkipped(const std::string &version) const; + static void skipVersion(const std::string &skippedVersion); + static void unskipVersion(); + [[nodiscard]] static bool isVersionSkipped(const std::string &version); protected: void setState(UpdateState newState); diff --git a/src/server/updater/sparkleupdater.h b/src/server/updater/sparkleupdater.h index 0c569932d..e5a01bed4 100644 --- a/src/server/updater/sparkleupdater.h +++ b/src/server/updater/sparkleupdater.h @@ -22,8 +22,6 @@ namespace KDC { -enum DownloadState { Unknown = 0, FindValidUpdate, DidNotFindUpdate, AbortWithError }; // TODO : useful?? - class SparkleUpdater final : public AbstractUpdater { public: explicit SparkleUpdater(); diff --git a/src/server/updater/sparkleupdater.mm b/src/server/updater/sparkleupdater.mm index 0065cda26..5dee612d4 100644 --- a/src/server/updater/sparkleupdater.mm +++ b/src/server/updater/sparkleupdater.mm @@ -33,7 +33,6 @@ // DelegateUpdaterObject class @interface DelegateUpdaterObject : NSObject { @protected - KDC::DownloadState _state; NSString *_availableVersion; NSString *_feedUrl; std::function _quitCallback; @@ -42,7 +41,6 @@ @interface DelegateUpdaterObject : NSObject { - (BOOL)updaterMayCheckForUpdates:(SPUUpdater *)updater; - (BOOL)updaterShouldRelaunchApplication:(SPUUpdater *)updater; - (void)updaterWillRelaunchApplication:(SPUUpdater *)updater; -- (KDC::DownloadState)downloadState; - (NSString *)availableVersion; - (void)setQuitCallback:(std::function)quitCallback; - (void)setSkipCallback:(std::function)skipCallback; @@ -53,7 +51,6 @@ @implementation DelegateUpdaterObject //(SUUpdaterDelegateInformalProtocol) - (instancetype)init { self = [super init]; if (self) { - _state = KDC::Unknown; _availableVersion = @""; _feedUrl = @""; _quitCallback = nullptr; @@ -98,10 +95,6 @@ - (void)updaterWillRelaunchApplication:(SPUUpdater *)updater { } } -- (KDC::DownloadState)downloadState { - return _state; -} - - (NSString *)availableVersion { return _availableVersion; } @@ -129,7 +122,6 @@ - (void)updater:(SPUUpdater *)updater userDidMakeChoice:(SPUUserUpdateChoice)cho - (void)updater:(SPUUpdater *)updater didFindValidUpdate:(SUAppcastItem *)updateItem { (void)updater; LOG_DEBUG(KDC::Log::instance()->getLogger(), "Version: " << [updateItem.versionString UTF8String]); - _state = KDC::FindValidUpdate; _availableVersion = [updateItem.versionString copy]; } @@ -137,7 +129,6 @@ - (void)updater:(SPUUpdater *)updater didFindValidUpdate:(SUAppcastItem *)update - (void)updaterDidNotFindUpdate:(SPUUpdater *)update { (void)update; LOG_DEBUG(KDC::Log::instance()->getLogger(), "No valid update found"); - _state = KDC::DidNotFindUpdate; } // Sent immediately before installing the specified update. @@ -150,7 +141,6 @@ - (void)updater:(SPUUpdater *)updater didAbortWithError:(NSError *)error { (void)updater; if ([error code] != SUNoUpdateError) { LOG_WARN(KDC::Log::instance()->getLogger(), "Error: " << [error code]); - _state = KDC::AbortWithError; } } diff --git a/src/server/updater/updatemanager.cpp b/src/server/updater/updatemanager.cpp index 81288d57b..57357d772 100644 --- a/src/server/updater/updatemanager.cpp +++ b/src/server/updater/updatemanager.cpp @@ -52,12 +52,12 @@ void UpdateManager::startInstaller() const { _updater->startInstaller(); } -void UpdateManager::skipVersion(const std::string &skippedVersion) const { - _updater->skipVersion(skippedVersion); +void UpdateManager::skipVersion(const std::string &skippedVersion) { + AbstractUpdater::skipVersion(skippedVersion); } -void UpdateManager::unskipVersion() const { - _updater->unskipVersion(); +void UpdateManager::unskipVersion() { + AbstractUpdater::unskipVersion(); } void UpdateManager::slotTimerFired() const { @@ -69,8 +69,7 @@ void UpdateManager::slotUpdateStateChanged(const UpdateState newState) { switch (newState) { case UpdateState::UpToDate: case UpdateState::Checking: - case UpdateState::Downloading: - case UpdateState::Skipped: { + case UpdateState::Downloading: { // Nothing to do break; } @@ -85,7 +84,7 @@ void UpdateManager::slotUpdateStateChanged(const UpdateState newState) { break; } case UpdateState::Ready: { - if (_updater->isVersionSkipped(_updater->versionInfo().fullVersion())) break; + if (AbstractUpdater::isVersionSkipped(_updater->versionInfo().fullVersion())) break; // The new version is ready to be installed #if defined(_WIN32) emit showUpdateDialog(); diff --git a/src/server/updater/updatemanager.h b/src/server/updater/updatemanager.h index 15c7c225b..4d5bf39bc 100644 --- a/src/server/updater/updatemanager.h +++ b/src/server/updater/updatemanager.h @@ -47,8 +47,8 @@ class UpdateManager final : public QObject { [[nodiscard]] const UpdateState &state() const { return _updater->state(); } void startInstaller() const; - void skipVersion(const std::string &skippedVersion) const; - void unskipVersion() const; + static void skipVersion(const std::string &skippedVersion); + static void unskipVersion(); void setQuitCallback(const std::function &quitCallback) const { _updater->setQuitCallback(quitCallback); }