From ad10d4bb9c87fb24e548b412c5409fda6858a748 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 28 Sep 2022 11:37:37 +0200 Subject: [PATCH 1/3] remove support for skipping an update: hard to understand for users skiping an update is probably there for historical reasons to work around broken updates in the past this can lead to users not getting an update they should be getting this can elad to user confusion and the current user interaction is broken if you do not go read the settings file with a text editor Signed-off-by: Matthieu Gallien --- src/gui/updater/ocupdater.cpp | 31 ++++++------------------------- src/gui/updater/ocupdater.h | 1 - 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/src/gui/updater/ocupdater.cpp b/src/gui/updater/ocupdater.cpp index 6c056024f..4c036321f 100644 --- a/src/gui/updater/ocupdater.cpp +++ b/src/gui/updater/ocupdater.cpp @@ -29,12 +29,12 @@ namespace OCC { -static const char updateAvailableC[] = "Updater/updateAvailable"; -static const char updateTargetVersionC[] = "Updater/updateTargetVersion"; -static const char updateTargetVersionStringC[] = "Updater/updateTargetVersionString"; -static const char seenVersionC[] = "Updater/seenVersion"; -static const char autoUpdateAttemptedC[] = "Updater/autoUpdateAttempted"; - +namespace { +const auto updateAvailableC = QStringLiteral("Updater/updateAvailable"); +const auto updateTargetVersionC = QStringLiteral("Updater/updateTargetVersion"); +const auto updateTargetVersionStringC = QStringLiteral("Updater/updateTargetVersionString"); +const auto autoUpdateAttemptedC = QStringLiteral("Updater/autoUpdateAttempted"); +} UpdaterScheduler::UpdaterScheduler(QObject *parent) : QObject(parent) @@ -347,12 +347,9 @@ void NSISUpdater::versionInfoArrived(const UpdateInfo &info) ConfigFile cfg; QSettings settings(cfg.configFile(), QSettings::IniFormat); qint64 infoVersion = Helper::stringVersionToInt(info.version()); - auto seenString = settings.value(seenVersionC).toString(); - qint64 seenVersion = Helper::stringVersionToInt(seenString); qint64 currVersion = Helper::currentVersionToInt(); qCInfo(lcUpdater) << "Version info arrived:" << "Your version:" << currVersion - << "Skipped version:" << seenVersion << seenString << "Available version:" << infoVersion << info.version() << "Available version string:" << info.versionString() << "Web url:" << info.web() @@ -423,15 +420,12 @@ void NSISUpdater::showNoUrlDialog(const UpdateInfo &info) hlayout->addWidget(lbl); auto *bb = new QDialogButtonBox; - QPushButton *skip = bb->addButton(tr("Skip this version"), QDialogButtonBox::ResetRole); QPushButton *reject = bb->addButton(tr("Skip this time"), QDialogButtonBox::AcceptRole); QPushButton *getupdate = bb->addButton(tr("Get update"), QDialogButtonBox::AcceptRole); - connect(skip, &QAbstractButton::clicked, msgBox, &QDialog::reject); connect(reject, &QAbstractButton::clicked, msgBox, &QDialog::reject); connect(getupdate, &QAbstractButton::clicked, msgBox, &QDialog::accept); - connect(skip, &QAbstractButton::clicked, this, &NSISUpdater::slotSetSeenVersion); connect(getupdate, &QAbstractButton::clicked, this, &NSISUpdater::slotOpenUpdateUrl); layout->addWidget(bb); @@ -473,20 +467,14 @@ void NSISUpdater::showUpdateErrorDialog(const QString &targetVersion) hlayout->addWidget(lbl); auto bb = new QDialogButtonBox; - auto skip = bb->addButton(tr("Skip this version"), QDialogButtonBox::ResetRole); auto askagain = bb->addButton(tr("Ask again later"), QDialogButtonBox::ResetRole); auto retry = bb->addButton(tr("Restart and update"), QDialogButtonBox::AcceptRole); auto getupdate = bb->addButton(tr("Update manually"), QDialogButtonBox::AcceptRole); - connect(skip, &QAbstractButton::clicked, msgBox, &QDialog::reject); connect(askagain, &QAbstractButton::clicked, msgBox, &QDialog::reject); connect(retry, &QAbstractButton::clicked, msgBox, &QDialog::accept); connect(getupdate, &QAbstractButton::clicked, msgBox, &QDialog::accept); - connect(skip, &QAbstractButton::clicked, this, [this]() { - wipeUpdateData(); - slotSetSeenVersion(); - }); // askagain: do nothing connect(retry, &QAbstractButton::clicked, this, [this]() { slotStartInstaller(); @@ -531,13 +519,6 @@ bool NSISUpdater::handleStartup() return false; } -void NSISUpdater::slotSetSeenVersion() -{ - ConfigFile cfg; - QSettings settings(cfg.configFile(), QSettings::IniFormat); - settings.setValue(seenVersionC, updateInfo().version()); -} - //////////////////////////////////////////////////////////////////////// PassiveUpdateNotifier::PassiveUpdateNotifier(const QUrl &url) diff --git a/src/gui/updater/ocupdater.h b/src/gui/updater/ocupdater.h index f857bb832..1bbf8b859 100644 --- a/src/gui/updater/ocupdater.h +++ b/src/gui/updater/ocupdater.h @@ -156,7 +156,6 @@ public: explicit NSISUpdater(const QUrl &url); bool handleStartup() override; private slots: - void slotSetSeenVersion(); void slotDownloadFinished(); void slotWriteFile(); From 146bd44b09d98be5df7bcb5a878ea55a94bf3b65 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 27 Sep 2022 18:55:29 +0200 Subject: [PATCH 2/3] use correct version copmparison on NSIS updater: fix update from rc Signed-off-by: Matthieu Gallien --- src/gui/updater/ocupdater.cpp | 44 ++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/gui/updater/ocupdater.cpp b/src/gui/updater/ocupdater.cpp index 4c036321f..7454f62a1 100644 --- a/src/gui/updater/ocupdater.cpp +++ b/src/gui/updater/ocupdater.cpp @@ -259,6 +259,7 @@ bool OCUpdater::updateSucceeded() const void OCUpdater::slotVersionInfoArrived() { + qCInfo(lcUpdater()) << "received a reply"; _timeoutWatchdog->stop(); auto *reply = qobject_cast(sender()); reply->deleteLater(); @@ -358,28 +359,32 @@ void NSISUpdater::versionInfoArrived(const UpdateInfo &info) { qCInfo(lcUpdater) << "No version information available at the moment"; setDownloadState(UpToDate); - } else if (infoVersion <= currVersion - || infoVersion <= seenVersion) { - qCInfo(lcUpdater) << "Client is on latest version!"; - setDownloadState(UpToDate); } else { - QString url = info.downloadUrl(); - if (url.isEmpty()) { - showNoUrlDialog(info); + qint64 currentVer = Helper::currentVersionToInt(); + qint64 remoteVer = Helper::stringVersionToInt(info.version()); + + if (info.version().isEmpty() || currentVer >= remoteVer) { + qCInfo(lcUpdater) << "Client is on latest version!"; + setDownloadState(UpToDate); } else { - _targetFile = cfg.configPath() + url.mid(url.lastIndexOf('/')+1); - if (QFile(_targetFile).exists()) { - setDownloadState(DownloadComplete); + QString url = info.downloadUrl(); + if (url.isEmpty()) { + showNoUrlDialog(info); } else { - auto request = QNetworkRequest(QUrl(url)); - request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy); - QNetworkReply *reply = qnam()->get(request); - connect(reply, &QIODevice::readyRead, this, &NSISUpdater::slotWriteFile); - connect(reply, &QNetworkReply::finished, this, &NSISUpdater::slotDownloadFinished); - setDownloadState(Downloading); - _file.reset(new QTemporaryFile); - _file->setAutoRemove(true); - _file->open(); + _targetFile = cfg.configPath() + url.mid(url.lastIndexOf('/')+1); + if (QFile(_targetFile).exists()) { + setDownloadState(DownloadComplete); + } else { + auto request = QNetworkRequest(QUrl(url)); + request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy); + QNetworkReply *reply = qnam()->get(request); + connect(reply, &QIODevice::readyRead, this, &NSISUpdater::slotWriteFile); + connect(reply, &QNetworkReply::finished, this, &NSISUpdater::slotDownloadFinished); + setDownloadState(Downloading); + _file.reset(new QTemporaryFile); + _file->setAutoRemove(true); + _file->open(); + } } } } @@ -554,6 +559,7 @@ void PassiveUpdateNotifier::versionInfoArrived(const UpdateInfo &info) qCInfo(lcUpdater) << "Client is on latest version!"; setDownloadState(UpToDate); } else { + qCInfo(lcUpdater) << "Client is on older version. We will update!"; setDownloadState(UpdateOnlyAvailableThroughSystem); } } From a36672c1a4c13d50017c2c5629d3a4e5e55131b9 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 28 Sep 2022 11:41:23 +0200 Subject: [PATCH 3/3] fix review comments Signed-off-by: Matthieu Gallien --- src/gui/updater/ocupdater.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gui/updater/ocupdater.cpp b/src/gui/updater/ocupdater.cpp index 7454f62a1..b44a0d18a 100644 --- a/src/gui/updater/ocupdater.cpp +++ b/src/gui/updater/ocupdater.cpp @@ -259,7 +259,7 @@ bool OCUpdater::updateSucceeded() const void OCUpdater::slotVersionInfoArrived() { - qCInfo(lcUpdater()) << "received a reply"; + qCDebug(lcUpdater) << "received a reply"; _timeoutWatchdog->stop(); auto *reply = qobject_cast(sender()); reply->deleteLater(); @@ -360,14 +360,14 @@ void NSISUpdater::versionInfoArrived(const UpdateInfo &info) qCInfo(lcUpdater) << "No version information available at the moment"; setDownloadState(UpToDate); } else { - qint64 currentVer = Helper::currentVersionToInt(); - qint64 remoteVer = Helper::stringVersionToInt(info.version()); + const auto currentVer = Helper::currentVersionToInt(); + const auto remoteVer = Helper::stringVersionToInt(info.version()); if (info.version().isEmpty() || currentVer >= remoteVer) { qCInfo(lcUpdater) << "Client is on latest version!"; setDownloadState(UpToDate); } else { - QString url = info.downloadUrl(); + const auto url = info.downloadUrl(); if (url.isEmpty()) { showNoUrlDialog(info); } else {