Review fixes

CMakeLists.txt

@@ -49,7 +49,6 @@ option(WITH_XC_YUBIKEY "Include YubiKey support." OFF)
 option(WITH_XC_SSHAGENT "Include SSH agent support." OFF)
 option(WITH_XC_KEESHARE "Sharing integration with KeeShare" OFF)
 option(WITH_XC_KEESHARE_SECURE "Sharing integration with secured KeeShare containers" OFF)
-option(WITH_XC_UPDATECHECK "Include update checking feature (requires networking)." OFF)
 if(APPLE)
     option(WITH_XC_TOUCHID "Include TouchID support for macOS." OFF)
 endif()
@@ -62,12 +61,9 @@ if(WITH_XC_ALL)
     set(WITH_XC_YUBIKEY ON)
     set(WITH_XC_SSHAGENT ON)
     set(WITH_XC_KEESHARE ON)
-    set(WITH_XC_UPDATECHECK ON)
     if(APPLE)
         set(WITH_XC_TOUCHID ON)
     endif()
-elseif(WITH_XC_UPDATECHECK)
-    set(WITH_XC_NETWORKING ON)
 endif()
 
 if(WITH_XC_KEESHARE_SECURE)

INSTALL.md

@@ -101,7 +101,6 @@ These steps place the compiled KeePassXC binary inside the `./build/src/` direct
 	  -DWITH_XC_SSHAGENT=[ON|OFF] Enable/Disable SSHAgent support (default: OFF)
 	  -DWITH_XC_KEESHARE=[ON|OFF] Enable/Disable KeeShare group syncronization extension (default: OFF)
 	  -DWITH_XC_TOUCHID=[ON|OFF] (macOS Only) Enable/Disable Touch ID unlock (default:OFF)
-	  -DWITH_XC_UPDATECHECK=[ON|OFF] Enable/Disable checking for updates when KeePassXC starts. (default: OFF)
 	  -DWITH_XC_ALL=[ON|OFF] Enable/Disable compiling all plugins above (default: OFF)
 	  -DWITH_XC_KEESHARE_SECURE=[ON|OFF] Enable/Disable KeeShare secure containers, requires libquazip5 (default: OFF)
 	  -DWITH_TESTS=[ON|OFF] Enable/Disable building of unit tests (default: ON)

src/CMakeLists.txt

@@ -205,7 +205,6 @@ add_feature_info(SSHAgent WITH_XC_SSHAGENT "SSH agent integration compatible wit
 add_feature_info(KeeShare WITH_XC_KEESHARE "Sharing integration with KeeShare")
 add_feature_info(KeeShare-Secure WITH_XC_KEESHARE_SECURE "Sharing integration with KeeShare with secure sources")
 add_feature_info(YubiKey WITH_XC_YUBIKEY "YubiKey HMAC-SHA1 challenge-response")
-add_feature_info(Update-Check WITH_XC_UPDATECHECK "Check for updates on application startup")
 if(APPLE)
     add_feature_info(TouchID WITH_XC_TOUCHID "TouchID integration")
 endif()
@@ -258,8 +257,8 @@ else()
     list(APPEND keepassx_SOURCES keys/drivers/YubiKey.h keys/drivers/YubiKeyStub.cpp)
 endif()
 
-if(WITH_XC_UPDATECHECK AND WITH_XC_NETWORKING)
-    list(APPEND keepassx_SOURCES updatecheck/UpdateCheck.cpp gui/UpdateCheckDialog.cpp)
+if(WITH_XC_NETWORKING)
+    list(APPEND keepassx_SOURCES updatecheck/UpdateChecker.cpp gui/UpdateCheckDialog.cpp)
 endif()
 
 if(WITH_XC_TOUCHID)

src/config-keepassx.h.cmake

@@ -21,7 +21,6 @@
 #cmakedefine WITH_XC_KEESHARE_INSECURE
 #cmakedefine WITH_XC_KEESHARE_SECURE
 #cmakedefine WITH_XC_TOUCHID
-#cmakedefine WITH_XC_UPDATECHECK
 
 #cmakedefine KEEPASSXC_BUILD_TYPE "@KEEPASSXC_BUILD_TYPE@"
 #cmakedefine KEEPASSXC_BUILD_TYPE_RELEASE

src/gui/ApplicationSettingsWidget.cpp

@@ -91,11 +91,8 @@ ApplicationSettingsWidget::ApplicationSettingsWidget(QWidget* parent)
             m_secUi->touchIDResetSpinBox, SLOT(setEnabled(bool)));
     // clang-format on
 
-#ifndef WITH_XC_UPDATECHECK
-    m_generalUi->checkForUpdatesOnStartupCheckBox->setVisible(false);
-#endif
-
 #ifndef WITH_XC_NETWORKING
+    m_generalUi->checkForUpdatesOnStartupCheckBox->setVisible(false);
     m_secUi->privacy->setVisible(false);
 #endif
 

src/gui/MainWindow.cpp

@@ -41,8 +41,8 @@
 #include "keys/FileKey.h"
 #include "keys/PasswordKey.h"
 
-#ifdef WITH_XC_UPDATECHECK
-#include "updatecheck/UpdateCheck.h"
+#ifdef WITH_XC_NETWORKING
+#include "updatecheck/UpdateChecker.h"
 #include "gui/MessageBox.h"
 #include "gui/UpdateCheckDialog.h"
 #endif
@@ -372,8 +372,9 @@ MainWindow::MainWindow()
     setUnifiedTitleAndToolBarOnMac(true);
 #endif
 
-#ifdef WITH_XC_UPDATECHECK
+#ifdef WITH_XC_NETWORKING
     connect(m_ui->actionCheckForUpdates, SIGNAL(triggered()), SLOT(showUpdateCheckDialog()));
+    connect(UpdateChecker::instance(), SIGNAL(updateCheckFinished(bool, QString)), SLOT(hasUpdateAvailable(bool, QString)));
     QTimer::singleShot(3000, this, SLOT(showUpdateCheckStartup()));
 #else
     m_ui->actionCheckForUpdates->setVisible(false);
@@ -678,7 +679,7 @@ void MainWindow::showAboutDialog()
 
 void MainWindow::showUpdateCheckStartup()
 {
-#ifdef WITH_XC_UPDATECHECK
+#ifdef WITH_XC_NETWORKING
     if (!config()->get("UpdateCheckMessageShown", false).toBool()) {
         auto result = MessageBox::question(this,
                                            tr("Check for updates on startup?"),
@@ -689,19 +690,18 @@ void MainWindow::showUpdateCheckStartup()
 
         config()->set("GUI/CheckForUpdates", (result == MessageBox::Yes));
         config()->set("UpdateCheckMessageShown", true);
-    } else if (config()->get("GUI/CheckForUpdates", false).toBool()) {
+    }
+
+    if (config()->get("GUI/CheckForUpdates", false).toBool()) {
         updateCheck()->checkForUpdates();
     }
 
-    connect(UpdateCheck::instance(), SIGNAL(updateCheckFinished(bool, QString)), this, SLOT(hasUpdateAvailable(bool, QString)));
 #endif
 }
 
 void MainWindow::hasUpdateAvailable(bool hasUpdate, const QString& version)
 {
-#ifdef WITH_XC_UPDATECHECK
-    disconnect(UpdateCheck::instance(), nullptr, this, nullptr);
-
+#ifdef WITH_XC_NETWORKING
     if (hasUpdate) {
         auto* updateCheckDialog = new UpdateCheckDialog(this);
         updateCheckDialog->showUpdateCheckResponse(hasUpdate, version);
@@ -712,7 +712,7 @@ void MainWindow::hasUpdateAvailable(bool hasUpdate, const QString& version)
 
 void MainWindow::showUpdateCheckDialog()
 {
-#ifdef WITH_XC_UPDATECHECK
+#ifdef WITH_XC_NETWORKING
     updateCheck()->checkForUpdates();
     auto* updateCheckDialog = new UpdateCheckDialog(this);
     updateCheckDialog->show();

src/gui/UpdateCheckDialog.cpp

@@ -17,12 +17,12 @@
 
 #include "UpdateCheckDialog.h"
 #include "ui_UpdateCheckDialog.h"
-#include "updatecheck/UpdateCheck.h"
+#include "updatecheck/UpdateChecker.h"
 #include "core/FilePath.h"
 
 UpdateCheckDialog::UpdateCheckDialog(QWidget* parent)
-: QDialog(parent)
-, m_ui(new Ui::UpdateCheckDialog())
+    : QDialog(parent)
+    , m_ui(new Ui::UpdateCheckDialog())
 {
     m_ui->setupUi(this);
     setWindowFlags(Qt::Window);
@@ -31,8 +31,7 @@ UpdateCheckDialog::UpdateCheckDialog(QWidget* parent)
     m_ui->iconLabel->setPixmap(filePath()->applicationIcon().pixmap(48));
 
     connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(close()));
-    connect(UpdateCheck::instance(), SIGNAL(updateCheckFinished(bool, QString)), this, SLOT(
-            showUpdateCheckResponse(bool, QString)));
+    connect(UpdateChecker::instance(), SIGNAL(updateCheckFinished(bool, QString)), SLOT(close()));
 }
 
 void UpdateCheckDialog::showUpdateCheckResponse(bool status, const QString& version) {

src/gui/UpdateCheckDialog.h

@@ -24,7 +24,7 @@
 #include "gui/MessageBox.h"
 #include "config-keepassx.h"
 #include "core/Global.h"
-#include "updatecheck/UpdateCheck.h"
+#include "updatecheck/UpdateChecker.h"
 
 namespace Ui
 {
@@ -40,7 +40,7 @@ Q_OBJECT
     ~UpdateCheckDialog() override;
 
 public slots:
-    void showUpdateCheckResponse(bool status, const QString &version);
+    void showUpdateCheckResponse(bool status, const QString& version);
 
 private:
     QScopedPointer<Ui::UpdateCheckDialog> m_ui;

src/updatecheck/UpdateCheck.cpp → src/updatecheck/UpdateChecker.cpp

@@ -15,24 +15,26 @@
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "UpdateCheck.h"
+#include "UpdateChecker.h"
 #include "config-keepassx.h"
 #include <QJsonObject>
+#include <QtNetwork>
+#include <QNetworkAccessManager>
 
-UpdateCheck* UpdateCheck::m_instance(nullptr);
+UpdateChecker* UpdateChecker::m_instance(nullptr);
 
-UpdateCheck::UpdateCheck(QObject* parent)
+UpdateChecker::UpdateChecker(QObject* parent)
     : QObject(parent)
     , m_netMgr(new QNetworkAccessManager(this))
     , m_reply(nullptr)
 {
 }
 
-UpdateCheck::~UpdateCheck()
+UpdateChecker::~UpdateChecker()
 {
 }
 
-void UpdateCheck::checkForUpdates()
+void UpdateChecker::checkForUpdates()
 {
     m_bytesReceived.clear();
 
@@ -41,16 +43,16 @@ void UpdateCheck::checkForUpdates()
 
     m_reply = m_netMgr->get(request);
 
-    connect(m_reply, &QNetworkReply::finished, this, &UpdateCheck::fetchFinished);
-    connect(m_reply, &QIODevice::readyRead, this, &UpdateCheck::fetchReadyRead);
+    connect(m_reply, &QNetworkReply::finished, this, &UpdateChecker::fetchFinished);
+    connect(m_reply, &QIODevice::readyRead, this, &UpdateChecker::fetchReadyRead);
 }
 
-void UpdateCheck::fetchReadyRead()
+void UpdateChecker::fetchReadyRead()
 {
     m_bytesReceived += m_reply->readAll();
 }
 
-void UpdateCheck::fetchFinished()
+void UpdateChecker::fetchFinished()
 {
     bool error = (m_reply->error() != QNetworkReply::NoError);
     bool hasNewVersion = false;
@@ -75,7 +77,7 @@ void UpdateCheck::fetchFinished()
     emit updateCheckFinished(hasNewVersion, version);
 }
 
-bool UpdateCheck::compareVersions(const QString& remoteVersion, const QString& localVersion)
+bool UpdateChecker::compareVersions(const QString& remoteVersion, const QString& localVersion)
 {
     if (localVersion == remoteVersion) {
         return false; // Currently using updated version
@@ -121,10 +123,10 @@ bool UpdateCheck::compareVersions(const QString& remoteVersion, const QString& l
     return false; // Invalid version string
 }
 
-UpdateCheck* UpdateCheck::instance()
+UpdateChecker* UpdateChecker::instance()
 {
     if (!m_instance) {
-        m_instance = new UpdateCheck();
+        m_instance = new UpdateChecker();
     }
 
     return m_instance;

src/updatecheck/UpdateCheck.h → src/updatecheck/UpdateChecker.h

@@ -19,22 +19,20 @@
 #define KEEPASSXC_UPDATECHECK_H
 #include <QString>
 #include <QObject>
-#include <QtNetwork>
-#include <QNetworkAccessManager>
 
 class QNetworkAccessManager;
 class QNetworkReply;
 
-class UpdateCheck : public QObject
+class UpdateChecker : public QObject
 {
     Q_OBJECT
 public:
-    UpdateCheck(QObject* parent = nullptr);
-    ~UpdateCheck();
+    UpdateChecker(QObject* parent = nullptr);
+    ~UpdateChecker() override;
 
     void checkForUpdates();
     static bool compareVersions(const QString& remoteVersion, const QString& localVersion);
-    static UpdateCheck* instance();
+    static UpdateChecker* instance();
 
 signals:
     void updateCheckFinished(bool hasNewVersion, QString version);
@@ -48,14 +46,14 @@ private slots:
     QNetworkReply* m_reply;
     QByteArray m_bytesReceived;
 
-    static UpdateCheck* m_instance;
+    static UpdateChecker* m_instance;
 
-    Q_DISABLE_COPY(UpdateCheck)
+    Q_DISABLE_COPY(UpdateChecker)
 };
 
-inline UpdateCheck* updateCheck()
+inline UpdateChecker* updateCheck()
 {
-    return UpdateCheck::instance();
+    return UpdateChecker::instance();
 }
 
 #endif //KEEPASSXC_UPDATECHECK_H

tests/CMakeLists.txt

@@ -162,7 +162,7 @@ add_unit_test(NAME testkeepass1reader SOURCES TestKeePass1Reader.cpp
 add_unit_test(NAME testwildcardmatcher SOURCES TestWildcardMatcher.cpp
         LIBS ${TEST_LIBRARIES})
 
-if(WITH_XC_UPDATECHECK)
+if(WITH_XC_NETWORKING)
     add_unit_test(NAME testupdatecheck SOURCES TestUpdateCheck.cpp
             LIBS ${TEST_LIBRARIES})
 endif()

tests/TestUpdateCheck.cpp

@@ -17,7 +17,7 @@
 
 #include "TestUpdateCheck.h"
 #include "TestGlobal.h"
-#include "updatecheck/UpdateCheck.h"
+#include "updatecheck/UpdateChecker.h"
 #include "crypto/Crypto.h"
 
 QTEST_GUILESS_MAIN(TestUpdateCheck)
@@ -30,12 +30,12 @@ void TestUpdateCheck::initTestCase()
 void TestUpdateCheck::testCompareVersion()
 {
                                        // Remote Version , Installed Version
-    QCOMPARE(UpdateCheck::compareVersions(QString("2.4.0"), QString("2.3.4")), true);
-    QCOMPARE(UpdateCheck::compareVersions(QString("2.3.0"), QString("2.4.0")), false);
-    QCOMPARE(UpdateCheck::compareVersions(QString("2.3.0"), QString("2.3.0")), false);
-    QCOMPARE(UpdateCheck::compareVersions(QString("2.3.0"), QString("2.3.0-beta1")), true);
-    QCOMPARE(UpdateCheck::compareVersions(QString("2.3.0-beta2"), QString("2.3.0-beta1")), true);
-    QCOMPARE(UpdateCheck::compareVersions(QString("2.3.4"), QString("2.4.0-snapshot")), false);
-    QCOMPARE(UpdateCheck::compareVersions(QString("invalid"), QString("2.4.0")), false);
-    QCOMPARE(UpdateCheck::compareVersions(QString(""), QString("2.4.0")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("2.3.4")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.4.0")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.0")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.0-beta1")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta2"), QString("2.3.0-beta1")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.4"), QString("2.4.0-snapshot")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString("invalid"), QString("2.4.0")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString(""), QString("2.4.0")), false);
 }