From cca6f80cb53ae3c773a575f1b7c8f90d5b81af6d Mon Sep 17 00:00:00 2001 From: sithlord48 Date: Fri, 28 Nov 2025 13:37:01 -0500 Subject: [PATCH] refactor: ClientConnection, use signal to ask for dialog move ClientConnectionTests to Qt Tests track if the dialog is open in the gui track suppression in clientConnection based on connect / disconnect message --- src/lib/gui/MainWindow.cpp | 14 +- src/lib/gui/MainWindow.h | 8 ++ src/lib/gui/core/ClientConnection.cpp | 35 ++--- src/lib/gui/core/ClientConnection.h | 22 +-- src/unittests/gui/core/CMakeLists.txt | 7 + .../gui/core/ClientConnectionTests.cpp | 130 ++++++++++++++++++ .../gui/core/ClientConnectionTests.h | 27 ++++ .../gui/core/ClientConnectionTests.cpp | 124 ----------------- 8 files changed, 197 insertions(+), 170 deletions(-) create mode 100644 src/unittests/gui/core/ClientConnectionTests.cpp create mode 100644 src/unittests/gui/core/ClientConnectionTests.h delete mode 100644 src/unittests/legacytests/legacytests/gui/core/ClientConnectionTests.cpp diff --git a/src/lib/gui/MainWindow.cpp b/src/lib/gui/MainWindow.cpp index 1940eeae0..2a39f4675 100644 --- a/src/lib/gui/MainWindow.cpp +++ b/src/lib/gui/MainWindow.cpp @@ -295,7 +295,7 @@ void MainWindow::connectSlots() connect(&m_serverConnection, &ServerConnection::clientsChanged, this, &MainWindow::serverClientsChanged); connect(&m_serverConnection, &ServerConnection::messageShowing, this, &MainWindow::showAndActivate); - connect(&m_clientConnection, &ClientConnection::messageShowing, this, &MainWindow::showAndActivate); + connect(&m_clientConnection, &ClientConnection::requestShowError, this, &MainWindow::showClientError); connect(ui->btnToggleCore, &QPushButton::clicked, m_actionStartCore, &QAction::trigger, Qt::UniqueConnection); connect(ui->btnRestartCore, &QPushButton::clicked, this, &MainWindow::resetCore); @@ -411,7 +411,6 @@ void MainWindow::coreProcessError(CoreProcess::Error error) void MainWindow::startCore() { - m_clientConnection.setShowMessage(); m_coreProcess.start(); m_actionStartCore->setVisible(false); m_actionRestartCore->setVisible(true); @@ -484,7 +483,6 @@ void MainWindow::openSettings() void MainWindow::resetCore() { - m_clientConnection.setShowMessage(); m_coreProcess.restart(); } @@ -1229,3 +1227,13 @@ void MainWindow::remoteHostChanged(const QString &newRemoteHost) Settings::setValue(Settings::Client::RemoteHost, newRemoteHost); } } + +void MainWindow::showClientError(deskflow::client::ErrorType error, const QString &address) +{ + if (m_clientErrorVisible) + return; + m_clientErrorVisible = true; + deskflow::gui::messages::showClientConnectError(this, error, address); + showAndActivate(); + m_clientErrorVisible = false; +} diff --git a/src/lib/gui/MainWindow.h b/src/lib/gui/MainWindow.h index 1b3701e20..a0c3240fe 100644 --- a/src/lib/gui/MainWindow.h +++ b/src/lib/gui/MainWindow.h @@ -152,6 +152,13 @@ private: void toggleCanRunCore(bool enableButtons); void remoteHostChanged(const QString &newRemoteHost); + /** + * @brief showClientError + * @param error Error Type + * @param address + */ + void showClientError(deskflow::client::ErrorType error, const QString &address); + /** * @brief trustedFingerprintDatabase get the FingerprintDatabase for the trusted clients or trusted servers. * @return The path to the trusted fingerprint file @@ -174,6 +181,7 @@ private: VersionChecker m_versionChecker; bool m_secureSocket = false; bool m_saveOnExit = true; + bool m_clientErrorVisible = false; deskflow::gui::core::WaylandWarnings m_waylandWarnings; ServerConfig m_serverConfig; deskflow::gui::CoreProcess m_coreProcess; diff --git a/src/lib/gui/core/ClientConnection.cpp b/src/lib/gui/core/ClientConnection.cpp index 4721ce953..355496a18 100644 --- a/src/lib/gui/core/ClientConnection.cpp +++ b/src/lib/gui/core/ClientConnection.cpp @@ -1,12 +1,12 @@ /* * Deskflow -- mouse and keyboard sharing utility + * SPDX-FileCopyrightText: (C) 2025 Deskflow Developers * SPDX-FileCopyrightText: (C) 2021 Symless Ltd. * SPDX-License-Identifier: GPL-2.0-only WITH LicenseRef-OpenSSL-Exception */ #include "ClientConnection.h" -#include "Messages.h" #include "common/Settings.h" #include @@ -14,41 +14,30 @@ namespace deskflow::gui { -// -// ClientConnection::Deps -// - -void ClientConnection::Deps::showError(QWidget *parent, deskflow::client::ErrorType error, const QString &address) const -{ - messages::showClientConnectError(parent, error, address); -} - -// -// ClientConnection -// - void ClientConnection::handleLogLine(const QString &logLine) { + if (logLine.contains("disconnected from server")) { + m_supressMessage = false; + return; + } + + if (logLine.contains("connected to server")) { + m_supressMessage = true; + return; + } if (logLine.contains("failed to connect to server")) { - - if (!m_showMessage) { + if (m_supressMessage) { qDebug("message already shown, skipping"); return; } - - m_showMessage = false; - // ignore the message if it's about the server refusing by name as // this will trigger the server to show an 'add client' dialog. if (logLine.contains("server refused client with our name")) { qDebug("ignoring client name refused message"); return; } - showMessage(logLine); - } else if (logLine.contains("connected to server")) { - m_showMessage = false; } } @@ -76,8 +65,6 @@ void ClientConnection::showMessage(const QString &logLine) return; Q_EMIT requestShowError(error, address); - Q_EMIT messageShowing(); - m_deps->showError(m_pParent, error, address); } } // namespace deskflow::gui diff --git a/src/lib/gui/core/ClientConnection.h b/src/lib/gui/core/ClientConnection.h index 205ebe7ed..d31b17aa5 100644 --- a/src/lib/gui/core/ClientConnection.h +++ b/src/lib/gui/core/ClientConnection.h @@ -1,5 +1,6 @@ /* * Deskflow -- mouse and keyboard sharing utility + * SPDX-FileCopyrightText: (C) 2025 Deskflow Developers * SPDX-FileCopyrightText: (C) 2021 Symless Ltd. * SPDX-License-Identifier: GPL-2.0-only WITH LicenseRef-OpenSSL-Exception */ @@ -7,12 +8,9 @@ #pragma once #include "common/Enums.h" -#include "gui/Messages.h" -#include #include #include -#include class QWidget; @@ -23,24 +21,12 @@ class ClientConnection : public QObject Q_OBJECT public: - struct Deps - { - virtual ~Deps() = default; - virtual void showError(QWidget *parent, deskflow::client::ErrorType error, const QString &address) const; - }; - - explicit ClientConnection(QWidget *parent, std::shared_ptr deps = std::make_shared()) - : m_pParent(parent), - m_deps(deps) + explicit ClientConnection(QWidget *parent) : m_pParent(parent) { // do nothing } void handleLogLine(const QString &line); - void setShowMessage() - { - m_showMessage = true; - } Q_SIGNALS: /** @@ -50,14 +36,12 @@ Q_SIGNALS: * @param address of the host */ void requestShowError(deskflow::client::ErrorType error, const QString &address); - void messageShowing(); private: void showMessage(const QString &logLine); QWidget *m_pParent; - std::shared_ptr m_deps; - bool m_showMessage = true; + bool m_supressMessage = false; }; } // namespace deskflow::gui diff --git a/src/unittests/gui/core/CMakeLists.txt b/src/unittests/gui/core/CMakeLists.txt index 06b23dc93..15abad75e 100644 --- a/src/unittests/gui/core/CMakeLists.txt +++ b/src/unittests/gui/core/CMakeLists.txt @@ -7,3 +7,10 @@ create_test( SOURCE CommandProcessTests.cpp WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/src/lib/gui" ) + +create_test( + NAME ClientConnectionTests + DEPENDS gui + SOURCE ClientConnectionTests.cpp + WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/src/lib/gui" +) diff --git a/src/unittests/gui/core/ClientConnectionTests.cpp b/src/unittests/gui/core/ClientConnectionTests.cpp new file mode 100644 index 000000000..7c25fe90d --- /dev/null +++ b/src/unittests/gui/core/ClientConnectionTests.cpp @@ -0,0 +1,130 @@ +/* + * Deskflow -- mouse and keyboard sharing utility + * SPDX-FileCopyrightText: (C) 2025 Chris Rizzitello + * SPDX-FileCopyrightText: (C) 2024 Symless Ltd. + * SPDX-License-Identifier: GPL-2.0-only WITH LicenseRef-OpenSSL-Exception + */ + +#include "ClientConnectionTests.h" + +#include "gui/core/ClientConnection.h" +#include + +#include + +using namespace deskflow::gui; + +void ClientConnectionTests::initTestCase() +{ + QDir dir; + QVERIFY(dir.mkpath(m_settingsPath)); + + QFile oldSettings(m_settingsFile); + if (oldSettings.exists()) + oldSettings.remove(); + + Settings::setSettingsFile(m_settingsFile); + Settings::setStateFile(m_stateFile); +} + +void ClientConnectionTests::handleLogLine_alreadyConnected_showError() +{ + ClientConnection clientConnection(nullptr); + const auto serverName = QStringLiteral("test server"); + Settings::setValue(Settings::Client::RemoteHost, serverName); + + QSignalSpy spy(&clientConnection, &ClientConnection::requestShowError); + QVERIFY(spy.isValid()); + + clientConnection.handleLogLine( + "failed to connect to server\n" + "server already has a connected client with our name" + ); + + QCOMPARE(spy.count(), 1); +} + +void ClientConnectionTests::handleLogLine_withHostname_showError() +{ + ClientConnection clientConnection(nullptr); + const auto serverName = QStringLiteral("test server"); + Settings::setValue(Settings::Client::RemoteHost, serverName); + + QSignalSpy spy(&clientConnection, &ClientConnection::requestShowError); + QVERIFY(spy.isValid()); + + clientConnection.handleLogLine("failed to connect to server"); + + QCOMPARE(spy.count(), 1); +} + +void ClientConnectionTests::handleLogLine_withIpAddress_showError() +{ + ClientConnection clientConnection(nullptr); + const auto serverName = QStringLiteral("1.1.1.1"); + Settings::setValue(Settings::Client::RemoteHost, serverName); + + QSignalSpy spy(&clientConnection, &ClientConnection::requestShowError); + QVERIFY(spy.isValid()); + + clientConnection.handleLogLine("failed to connect to server"); + + QCOMPARE(spy.count(), 1); +} + +void ClientConnectionTests::handleLogLine_serverRefusedClient_shouldNotShowError() +{ + ClientConnection clientConnection(nullptr); + + QSignalSpy spy(&clientConnection, &ClientConnection::requestShowError); + QVERIFY(spy.isValid()); + + clientConnection.handleLogLine( + "failed to connect to server\n" + "server refused client with our name" + ); + + QCOMPARE(spy.count(), 0); +} + +void ClientConnectionTests::handleLogLine_connected_shouldPreventFutureError() +{ + ClientConnection clientConnection(nullptr); + clientConnection.handleLogLine("connected to server"); + + QSignalSpy spy(&clientConnection, &ClientConnection::requestShowError); + QVERIFY(spy.isValid()); + + clientConnection.handleLogLine("failed to connect to server"); + + QCOMPARE(spy.count(), 0); +} + +void ClientConnectionTests::handleLogLine_connectToggled_showAfterDisconnect() +{ + ClientConnection clientConnection(nullptr); + clientConnection.handleLogLine("connected to server"); + + QSignalSpy spy(&clientConnection, &ClientConnection::requestShowError); + QVERIFY(spy.isValid()); + + clientConnection.handleLogLine("failed to connect to server"); + clientConnection.handleLogLine("disconnected from server"); + clientConnection.handleLogLine("failed to connect to server"); + + QCOMPARE(spy.count(), 1); +} + +void ClientConnectionTests::handleLogLine_otherMessage_shouldNotShowError() +{ + ClientConnection clientConnection(nullptr); + + QSignalSpy spy(&clientConnection, &ClientConnection::requestShowError); + QVERIFY(spy.isValid()); + + clientConnection.handleLogLine("hello world"); + + QCOMPARE(spy.count(), 0); +} + +QTEST_MAIN(ClientConnectionTests) diff --git a/src/unittests/gui/core/ClientConnectionTests.h b/src/unittests/gui/core/ClientConnectionTests.h new file mode 100644 index 000000000..33ff0e3cc --- /dev/null +++ b/src/unittests/gui/core/ClientConnectionTests.h @@ -0,0 +1,27 @@ +/* + * Deskflow -- mouse and keyboard sharing utility + * SPDX-FileCopyrightText: (C) 2025 Chris Rizzitello + * SPDX-License-Identifier: GPL-2.0-only WITH LicenseRef-OpenSSL-Exception + */ + +#include + +class ClientConnectionTests : public QObject +{ + Q_OBJECT +private Q_SLOTS: + // Test are run in order top to bottom + void initTestCase(); + void handleLogLine_alreadyConnected_showError(); + void handleLogLine_withHostname_showError(); + void handleLogLine_withIpAddress_showError(); + void handleLogLine_serverRefusedClient_shouldNotShowError(); + void handleLogLine_connected_shouldPreventFutureError(); + void handleLogLine_connectToggled_showAfterDisconnect(); + void handleLogLine_otherMessage_shouldNotShowError(); + +private: + inline static const QString m_settingsPath = QStringLiteral("tmp/test"); + inline static const QString m_settingsFile = QStringLiteral("%1/Deskflow.conf").arg(m_settingsPath); + inline static const QString m_stateFile = QStringLiteral("%1/Deskflow.state").arg(m_settingsPath); +}; diff --git a/src/unittests/legacytests/legacytests/gui/core/ClientConnectionTests.cpp b/src/unittests/legacytests/legacytests/gui/core/ClientConnectionTests.cpp deleted file mode 100644 index ab25e02fc..000000000 --- a/src/unittests/legacytests/legacytests/gui/core/ClientConnectionTests.cpp +++ /dev/null @@ -1,124 +0,0 @@ -/* - * Deskflow -- mouse and keyboard sharing utility - * SPDX-FileCopyrightText: (C) 2024 Symless Ltd. - * SPDX-License-Identifier: GPL-2.0-only WITH LicenseRef-OpenSSL-Exception - */ - -#include "common/Settings.h" -#include "gui/core/ClientConnection.h" - -#include -#include - -class QWidget; - -using testing::_; -using testing::NiceMock; -using namespace deskflow::gui; -using enum deskflow::client::ErrorType; - -namespace { - -struct DepsMock : public ClientConnection::Deps -{ - MOCK_METHOD( - void, showError, (QWidget * parent, deskflow::client::ErrorType error, const QString &address), (const, override) - ); -}; - -} // namespace - -class ClientConnectionTests : public testing::Test -{ -public: - ClientConnectionTests() - { - Settings::setValue(Settings::Client::RemoteHost, stub); - } - - std::shared_ptr m_pDeps = std::make_shared>(); - -private: - const QString stub = "stub"; -}; - -TEST_F(ClientConnectionTests, handleLogLine_alreadyConnected_showError) -{ - ClientConnection clientConnection(nullptr, m_pDeps); - - const QString serverName = "test server"; - Settings::setValue(Settings::Client::RemoteHost, serverName); - - EXPECT_CALL(*m_pDeps, showError(_, AlreadyConnected, serverName)); - - clientConnection.handleLogLine( - "failed to connect to server\n" - "server already has a connected client with our name" - ); -} - -TEST_F(ClientConnectionTests, handleLogLine_withHostname_showError) -{ - ClientConnection clientConnection(nullptr, m_pDeps); - - const QString serverName = "test hostname"; - Settings::setValue(Settings::Client::RemoteHost, serverName); - - EXPECT_CALL(*m_pDeps, showError(_, HostnameError, serverName)); - - clientConnection.handleLogLine("failed to connect to server"); -} - -TEST_F(ClientConnectionTests, handleLogLine_withIpAddress_showError) -{ - ClientConnection clientConnection(nullptr, m_pDeps); - - const QString serverName = "1.1.1.1"; - Settings::setValue(Settings::Client::RemoteHost, serverName); - - EXPECT_CALL(*m_pDeps, showError(_, GenericError, serverName)); - - clientConnection.handleLogLine("failed to connect to server"); -} - -TEST_F(ClientConnectionTests, handleLogLine_messageShown_shouldNotShowAgain) -{ - ClientConnection clientConnection(nullptr, m_pDeps); - - clientConnection.handleLogLine("failed to connect to server"); - - EXPECT_CALL(*m_pDeps, showError(_, _, _)).Times(0); - - clientConnection.handleLogLine("failed to connect to server"); -} - -TEST_F(ClientConnectionTests, handleLogLine_serverRefusedClient_shouldNotShowError) -{ - ClientConnection clientConnection(nullptr, m_pDeps); - - EXPECT_CALL(*m_pDeps, showError(_, _, _)).Times(0); - - clientConnection.handleLogLine( - "failed to connect to server\n" - "server refused client with our name" - ); -} - -TEST_F(ClientConnectionTests, handleLogLine_connected_shouldPreventFutureError) -{ - ClientConnection clientConnection(nullptr, m_pDeps); - clientConnection.handleLogLine("connected to server"); - - EXPECT_CALL(*m_pDeps, showError(_, _, _)).Times(0); - - clientConnection.handleLogLine("failed to connect to server"); -} - -TEST_F(ClientConnectionTests, handleLogLine_otherMessage_shouldNotShowError) -{ - ClientConnection clientConnection(nullptr, m_pDeps); - - EXPECT_CALL(*m_pDeps, showError(_, _, _)).Times(0); - - clientConnection.handleLogLine("hello world"); -}