Skip to content

Commit 1c8340b

Browse files
committed
feat: implement sendReplyIfNeeded() and shouldSendReply() with full test coverage
- Extracted reply decision logic into `shouldSendReply()` - Added `sendReplyIfNeeded()` as the central method for conditional reply sending - Command-line reply packet now correctly takes precedence over settings - Refactored buildReplyPacket() response data handling - Made sendOutgoingPacket() virtual in BaseTcpThread - Added proper consoleMode handling - Improved testability with getSettings() temporary file support for unit tests - Added comprehensive unit tests for shouldSendReply() (data-driven) and sendReplyIfNeeded() - Added test helpers: normalizeReplyPacket, setupThreadToSendReply, buildExpectedReplyPacket This improves separation of concerns and makes reply behavior much more testable and maintainable.
1 parent be3c5b2 commit 1c8340b

11 files changed

Lines changed: 394 additions & 25 deletions

src/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ set(
2424
mainwindow.h
2525
panel.h
2626
globals.h
27+
settingnames.h
2728
panelgenerator.h
2829
persistentconnection.h
2930
persistenthttp.h

src/basetcpthread.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class BaseTcpThread : public QThread
5656
[[nodiscard]] virtual QAbstractSocket::SocketState getSocketState() const;
5757
[[nodiscard]] virtual QByteArray readSocketData();
5858

59-
void sendOutgoingPacket(Packet& packet);
59+
virtual void sendOutgoingPacket(Packet& packet);
6060
virtual void closeConnection() = 0;
6161
virtual void sleep(unsigned long usec);
6262

@@ -67,6 +67,13 @@ class BaseTcpThread : public QThread
6767
bool closeRequest = false;
6868
bool insidePersistent = false;
6969

70+
#ifdef CONSOLE_MODE
71+
bool consoleMode = true;
72+
#else
73+
bool consoleMode = false;
74+
#endif
75+
76+
7077
private:
7178
std::unique_ptr<QSslSocket> socket;
7279
// protected slots:

src/globals.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
*/
1010
#pragma once
1111

12+
#include <QCoreApplication>
13+
#include <QSettings>
14+
#include <QTemporaryFile>
15+
#include <QStandardPaths>
16+
1217
//BEGIN SW VERSION
1318
#define SW_VERSION "8.10.4"
1419
//END SW VERSION
@@ -54,6 +59,32 @@
5459
#define KEYFILE ((QFile::exists("key.pem") || QFile::exists("portablemode.txt") ) ? ("key.pem") : ((SETTINGSPATH) + "key.pem"))
5560
#define PANELSFILE ((QFile::exists("ps_panels.json") || QFile::exists("portablemode.txt") ) ? ("ps_panels.json") : ((SETTINGSPATH) + "ps_panels.json"))
5661

62+
63+
// globals.cpp
64+
inline QSettings& getSettings()
65+
{
66+
// === Unit Test Path ===
67+
if (QCoreApplication::applicationName().contains("unittest", Qt::CaseInsensitive)) {
68+
69+
static QTemporaryFile* testSettingsFile = nullptr;
70+
71+
if (!testSettingsFile) {
72+
testSettingsFile = new QTemporaryFile();
73+
if (testSettingsFile->open())
74+
{
75+
testSettingsFile->close(); // Important
76+
}
77+
}
78+
79+
static QSettings testSettings(testSettingsFile->fileName(), QSettings::IniFormat);
80+
return testSettings;
81+
}
82+
83+
// === Normal Application Path ===
84+
static QSettings normalSettings(SETTINGSFILE, QSettings::IniFormat);
85+
return normalSettings;
86+
}
87+
5788
#define NAMEINIKEY "NAMES"
5889
#define DTLSSENDICON ":icons/tx_dtls.png"
5990
#define DTLSRXICON ":icons/rx_dtls.png"

src/outgoingtcpthread.cpp

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
#include <QMetaEnum>
88

9+
#include "settingnames.h"
10+
911
OutgoingTcpThread::OutgoingTcpThread(QSslSocket* socket, const Packet& packetToSend, QObject* parent)
1012
:BaseTcpThread(socket, parent), sendPacket(packetToSend)
1113
{
@@ -113,14 +115,18 @@ Packet OutgoingTcpThread::buildReplyPacket(const Packet& receivedPacket,
113115

114116
reply.fromPort = getSocket() ? getSocket()->localPort() : 0;
115117

116-
reply.tcpOrUdp = sendPacket.tcpOrUdp;
118+
reply.tcpOrUdp = receivedPacket.tcpOrUdp;
117119
if (isSocketEncrypted()) {
118120
reply.tcpOrUdp = "SSL";
119121
}
120122

121123
// Response content
122124
if (!responseData.isEmpty()) {
123125
reply.hexString = Packet::byteArrayToHex(responseData);
126+
} else
127+
{
128+
const QSettings& settings = getSettings();
129+
reply.hexString = settings.value(RESPONSE_HEX).toString();
124130
}
125131

126132
// Macro expansion
@@ -270,15 +276,57 @@ void OutgoingTcpThread::waitForAndProcessIncomingData()
270276
// Optionally: auto-send a response here if configured
271277
}
272278

273-
void OutgoingTcpThread::persistentConnectionLoop()
279+
bool OutgoingTcpThread::shouldSendReply() const
274280
{
275-
QDEBUG() << "Entering persistent connection loop for" << sendPacket.toIP << ":" << sendPacket.port;
281+
const QSettings &settings = getSettings();
276282

277-
if (shouldStopPersistentConnectionLoop()) {
278-
qDebug() << "Early exit from persistent loop due to close request";
283+
bool basicResponseEnabled = settings.value(SEND_RESPONSE, false).toBool();
284+
bool smartResponseEnabled = settings.value(SMART_RESPONSES_ENABLED, false).toBool();
285+
bool hasCommandLineReply = !commandLineReplyPacket.hexString.isEmpty();
286+
287+
if (consoleMode)
288+
{
289+
return hasCommandLineReply;
290+
}
291+
292+
// If either basic response or smart responses are enabled
293+
return basicResponseEnabled || smartResponseEnabled || hasCommandLineReply;
294+
}
295+
296+
void OutgoingTcpThread::sendReplyIfNeeded(const Packet& receivedPacket)
297+
{
298+
if (!shouldSendReply()) {
299+
QDEBUG() << "No reply configured - skipping";
279300
return;
280301
}
281302

303+
QDEBUG() << "shouldSendReply() == true → building reply";
304+
305+
// Smart response data will go here in the future
306+
QByteArray responseData;
307+
308+
Packet reply = buildReplyPacket(receivedPacket, responseData);
309+
310+
// Command-line reply has highest priority
311+
if (!commandLineReplyPacket.hexString.isEmpty()) {
312+
reply = commandLineReplyPacket;
313+
QDEBUG() << "Using command-line reply packet instead";
314+
}
315+
316+
// Don't send empty replies (this was in the old writeResponse)
317+
if (reply.hexString.isEmpty()) {
318+
QDEBUG() << "Reply has no data - skipping send";
319+
return;
320+
}
321+
322+
// === Actual send via Base class ===
323+
BaseTcpThread::sendOutgoingPacket(reply);
324+
}
325+
326+
void OutgoingTcpThread::persistentConnectionLoop()
327+
{
328+
QDEBUG() << "Entering persistent connection loop for" << sendPacket.toIP << ":" << sendPacket.port;
329+
282330
while (shouldContinuePersistentLoop()) {
283331
insidePersistent = true; // keeping for now, we can remove later if unused
284332

src/outgoingtcpthread.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,12 @@ class OutgoingTcpThread : public BaseTcpThread
5353
virtual Packet buildReceivedPacket();
5454
virtual void processIncomingData();
5555
virtual void waitForAndProcessIncomingData();
56+
virtual bool shouldSendReply() const;
57+
virtual void sendReplyIfNeeded(const Packet& receivedPacket);
5658
void persistentConnectionLoop();
5759

5860
Packet sendPacket;
59-
Packet replyPacket;
61+
Packet commandLineReplyPacket;
6062

6163
bool consoleMode = false;
6264
bool persistent = false;

src/settingnames.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//
2+
// Created by Tomas Gallucci on 5/25/26.
3+
//
4+
5+
#ifndef SETTINGNAMES_H
6+
#define SETTINGNAMES_H
7+
8+
// SEND_RESPONSE's value has been misspelled in the code base for a long time
9+
#define SEND_RESPONSE "sendReponse"
10+
#define SMART_RESPONSES_ENABLED "smartResponseEnableCheck"
11+
#define RESPONSE_HEX "responseHex"
12+
13+
#endif //SETTINGNAMES_H

src/tests/unit/outgoingtcpthreadpersistentconnectionlooptests.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,7 @@ void OutgoingTcpThreadPersistentConnectionLoopTests::testPersistentConnectionLoo
299299
OutgoingTcpThreadTestDouble thread(mockSock, TestUtils::createPacketForTest());
300300

301301
thread.callPersistentConnectionLoop();
302-
303-
constexpr int beforeLoopCallCount = 1;
304-
constexpr int insideLoopCallCount = 1;
305-
QCOMPARE(thread.shouldStopPersistentConnectionLoopCallCount, beforeLoopCallCount + insideLoopCallCount);
302+
QCOMPARE(thread.shouldStopPersistentConnectionLoopCallCount, 1);
306303
}
307304

308305
void OutgoingTcpThreadPersistentConnectionLoopTests::testPersistentConnectionLoop_callsIdleHandlerWhenNoData()
@@ -339,15 +336,15 @@ void OutgoingTcpThreadPersistentConnectionLoopTests::testPersistentConnectionLoo
339336
QCOMPARE(thread.handlePersistentIdleCaseCallCount, 0);
340337
}
341338

342-
void OutgoingTcpThreadPersistentConnectionLoopTests::testPersistentConnectionLoop_doesNotEnterLoopIfAlreadyStopped()
339+
void OutgoingTcpThreadPersistentConnectionLoopTests::testPersistentConnectionLoop_exitsLoopEarlyIfAlreadyStopped()
343340
{
344341
auto* mockSock = TestUtils::createMockSocketForTest();
345342
OutgoingTcpThreadTestDouble thread(mockSock, TestUtils::createPacketForTest());
346343

347344
thread.stop(); // stop before calling
348345

349346
thread.callPersistentConnectionLoop();
350-
QCOMPARE(thread.shouldContinuePersistentConnectionLoopCallCount, 0); // never entered while()
347+
QCOMPARE(thread.shouldContinuePersistentConnectionLoopCallCount, 1); // never entered while()
351348
}
352349

353350
void OutgoingTcpThreadPersistentConnectionLoopTests::testPersistentConnectionLoop_exitsWhenSocketDisconnects()

src/tests/unit/outgoingtcpthreadpersistentconnectionlooptests.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ private slots:
4949
void testPersistentConnectionLoop_callsShouldStopPersistentLoop();
5050
void testPersistentConnectionLoop_callsIdleHandlerWhenNoData();
5151
void testPersistentConnectionLoop_skipsIdleWhenHasDataToSend();
52-
void testPersistentConnectionLoop_doesNotEnterLoopIfAlreadyStopped();
52+
void testPersistentConnectionLoop_exitsLoopEarlyIfAlreadyStopped();
5353
void testPersistentConnectionLoop_exitsWhenSocketDisconnects();
5454
void testPersistentConnectionLoop_emitsIdleStatusOnlyEveryTwoSeconds();
5555
void testPersistentConnectionLoop_callsProcessIncomingData();

0 commit comments

Comments
 (0)