Add autoKeepAlive to QMqttClient

In certain scenarios, a user wants to manually handle the keepAlive.
Introduce autoKeepAlive as a property. If set to false, then the user
needs to handle the keepAlive by invoking requestPing.

The default for this property is true and keeps the current behavior.

Task-number: QTBUG-76782
Change-Id: I073874e6bdacf3c8d128c20431606affe1d9eeb1
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
Reviewed-by: Karsten Heimrich <karsten.heimrich@qt.io>

6 files changed, 151 insertions, 8 deletions

diff --git a/src/mqtt/qmqttclient.cpp b/src/mqtt/qmqttclient.cpp
index bd5bb12..a13ce1e 100644
--- a/src/mqtt/qmqttclient.cpp
+++ b/src/mqtt/qmqttclient.cpp
@@ -94,6 +94,8 @@ Q_LOGGING_CATEGORY(lcMqttClient, "qt.mqtt.client")
 
     If the broker does not respond within a grace period the connection will be
     closed.
+
+    \sa autoKeepAlive(), requestPing(), pingResponseReceived()
 */
 
 /*!
@@ -157,6 +159,24 @@ Q_LOGGING_CATEGORY(lcMqttClient, "qt.mqtt.client")
 */
 
 /*!
+    \property QMqttClient::autoKeepAlive
+    \since 5.14
+    \brief This property holds whether the client will automatically manage
+    keep alive messages to the server.
+
+    If this property is \c true, then the client will automatically send a
+    ping message to the server at the keepAlive interval.
+
+    Otherwise, a user will have to manually invoke requestPing
+    within the specified interval of the connection. If no ping has been
+    sent within the interval, the server will disconnect.
+
+    The default of this property is \c true.
+
+    \sa keepAlive(), requestPing(), serverConnectionProperties(), pingResponseReceived()
+*/
+
+/*!
     \enum QMqttClient::TransportType
 
     This enum type specifies the connection method to be used to instantiate a
@@ -471,19 +491,25 @@ qint32 QMqttClient::publish(const QMqttTopicName &topic, const QMqttPublishPrope
 }
 
 /*!
-    Sends a ping message to the broker and expects a reply. If the connection
-    is active, the MQTT client will automatically send a ping message at
-    \l keepAlive intervals.
+    Sends a ping message to the broker and expects a reply.
+
+    If the connection is active and \l autoKeepAlive is \c true, then calling this
+    function will fail as the client is responsible for managing this process.
+
+    Using \c requestPing() manually requires a call every time within the \l keepAlive
+    interval as long as the connection is active.
 
     To check whether the ping is successful, connect to the
     \l pingResponseReceived() signal.
 
     Returns \c true if the ping request could be sent.
+
+    \sa pingResponseReceived(), autoKeepAlive(), keepAlive()
  */
 bool QMqttClient::requestPing()
 {
     Q_D(QMqttClient);
-    return d->m_connection.sendControlPingRequest();
+    return d->m_connection.sendControlPingRequest(false);
 }
 
 QString QMqttClient::hostname() const
@@ -642,6 +668,12 @@ bool QMqttClient::willRetain() const
     return d->m_willRetain;
 }
 
+bool QMqttClient::autoKeepAlive() const
+{
+    Q_D(const QMqttClient);
+    return d->m_autoKeepAlive;
+}
+
 /*!
     \since 5.12
 
@@ -985,6 +1017,22 @@ void QMqttClient::setWillRetain(bool willRetain)
     emit willRetainChanged(willRetain);
 }
 
+void QMqttClient::setAutoKeepAlive(bool autoKeepAlive)
+{
+    Q_D(QMqttClient);
+
+    if (state() != QMqttClient::Disconnected) {
+        qCDebug(lcMqttClient) << "Changing autoKeepAlive while connected is not possible.";
+        return;
+    }
+
+    if (d->m_autoKeepAlive == autoKeepAlive)
+        return;
+
+    d->m_autoKeepAlive = autoKeepAlive;
+    emit autoKeepAliveChanged(d->m_autoKeepAlive);
+}
+
 void QMqttClient::setError(ClientError e)
 {
     Q_D(QMqttClient);
diff --git a/src/mqtt/qmqttclient.h b/src/mqtt/qmqttclient.h
index 478c7e8..2d650a8 100644
--- a/src/mqtt/qmqttclient.h
+++ b/src/mqtt/qmqttclient.h
@@ -101,6 +101,7 @@ private:
     Q_PROPERTY(QByteArray willMessage READ willMessage WRITE setWillMessage NOTIFY willMessageChanged)
     Q_PROPERTY(quint8 willQoS READ willQoS WRITE setWillQoS NOTIFY willQoSChanged)
     Q_PROPERTY(bool willRetain READ willRetain WRITE setWillRetain NOTIFY willRetainChanged)
+    Q_PROPERTY(bool autoKeepAlive READ autoKeepAlive WRITE setAutoKeepAlive NOTIFY autoKeepAliveChanged)
 public:
     explicit QMqttClient(QObject *parent = nullptr);
 
@@ -148,6 +149,7 @@ public:
     quint8 willQoS() const;
     QByteArray willMessage() const;
     bool willRetain() const;
+    bool autoKeepAlive() const;
 
     void setConnectionProperties(const QMqttConnectionProperties &prop);
     QMqttConnectionProperties connectionProperties() const;
@@ -182,6 +184,7 @@ Q_SIGNALS:
     void willQoSChanged(quint8 willQoS);
     void willMessageChanged(QByteArray willMessage);
     void willRetainChanged(bool willRetain);
+    void autoKeepAliveChanged(bool autoKeepAlive);
 
     void authenticationRequested(const QMqttAuthenticationProperties &p);
     void authenticationFinished(const QMqttAuthenticationProperties &p);
@@ -201,6 +204,7 @@ public Q_SLOTS:
     void setWillQoS(quint8 willQoS);
     void setWillMessage(const QByteArray &willMessage);
     void setWillRetain(bool willRetain);
+    void setAutoKeepAlive(bool autoKeepAlive);
 
 private:
     void connectToHost(bool encrypted, const QString &sslPeerName);
diff --git a/src/mqtt/qmqttclient_p.h b/src/mqtt/qmqttclient_p.h
index 2ce9f48..b2343c3 100644
--- a/src/mqtt/qmqttclient_p.h
+++ b/src/mqtt/qmqttclient_p.h
@@ -71,6 +71,7 @@ public:
     QByteArray m_willMessage;
     quint8 m_willQoS{0};
     bool m_willRetain{false};
+    bool m_autoKeepAlive{true};
     QString m_username;
     QString m_password;
     bool m_cleanSession{true};
diff --git a/src/mqtt/qmqttconnection.cpp b/src/mqtt/qmqttconnection.cpp
index 4923e8d..af87609 100644
--- a/src/mqtt/qmqttconnection.cpp
+++ b/src/mqtt/qmqttconnection.cpp
@@ -587,20 +587,27 @@ bool QMqttConnection::sendControlUnsubscribe(const QMqttTopicFilter &topic, cons
     return true;
 }
 
-bool QMqttConnection::sendControlPingRequest()
+bool QMqttConnection::sendControlPingRequest(bool isAuto)
 {
     qCDebug(lcMqttConnection) << Q_FUNC_INFO;
 
     if (m_internalState != QMqttConnection::BrokerConnected)
         return false;
 
+
+    if (!isAuto && m_clientPrivate->m_autoKeepAlive) {
+        qCDebug(lcMqttConnection) << "Requesting a manual ping while autoKeepAlive is enabled "
+                                  << "is not allowed.";
+        return false;
+    }
+
     // 3.1.2.10 If a Client does not receive a PINGRESP packet within a reasonable amount of time
     // after it has sent a PINGREQ, it SHOULD close the Network Connection to the Server
-    // Consider two pending PINGRESP as reasonable.
     if (m_pingTimeout > 1) {
         closeConnection(QMqttClient::ServerUnavailable);
         return false;
     }
+
     const QMqttControlPacket packet(QMqttControlPacket::PINGREQ);
     if (!writePacketToTransport(packet)) {
         qCDebug(lcMqttConnection) << "Failed to write PINGREQ to transport.";
@@ -1473,7 +1480,8 @@ void QMqttConnection::finalize_connack()
     m_internalState = BrokerConnected;
     m_clientPrivate->setStateAndError(QMqttClient::Connected);
 
-    m_pingTimer.start(m_clientPrivate->m_keepAlive * 1000, this);
+    if (m_clientPrivate->m_autoKeepAlive)
+        m_pingTimer.start(m_clientPrivate->m_keepAlive * 1000, this);
 }
 
 void QMqttConnection::finalize_suback()
diff --git a/src/mqtt/qmqttconnection_p.h b/src/mqtt/qmqttconnection_p.h
index 91c6b77..456cd5a 100644
--- a/src/mqtt/qmqttconnection_p.h
+++ b/src/mqtt/qmqttconnection_p.h
@@ -86,7 +86,7 @@ public:
     bool sendControlPublishComp(quint16 id);
     QMqttSubscription *sendControlSubscribe(const QMqttTopicFilter &topic, quint8 qos, const QMqttSubscriptionProperties &properties);
     bool sendControlUnsubscribe(const QMqttTopicFilter &topic, const QMqttUnsubscriptionProperties &properties);
-    bool sendControlPingRequest();
+    bool sendControlPingRequest(bool isAuto = true);
     bool sendControlDisconnect();
 
     void setClientPrivate(QMqttClientPrivate *clientPrivate);
diff --git a/tests/auto/qmqttclient/tst_qmqttclient.cpp b/tests/auto/qmqttclient/tst_qmqttclient.cpp
index 21bd512..9563e7d 100644
--- a/tests/auto/qmqttclient/tst_qmqttclient.cpp
+++ b/tests/auto/qmqttclient/tst_qmqttclient.cpp
@@ -74,6 +74,8 @@ private Q_SLOTS:
     void messageStatusReceive_data();
     void messageStatusReceive();
     void subscriptionIdsOverlap();
+    void keepAlive_data();
+    void keepAlive();
 private:
     QProcess m_brokerProcess;
     QString m_testBroker;
@@ -147,6 +149,9 @@ void Tst_QMqttClient::getSetCheck()
     QCOMPARE(client.willMessage(), QByteArray());
     QCOMPARE(client.willQoS(), quint8(0));
     QCOMPARE(client.willRetain(), false);
+    QCOMPARE(client.autoKeepAlive(), true);
+    client.setAutoKeepAlive(false);
+    QCOMPARE(client.autoKeepAlive(), false);
 }
 
 void Tst_QMqttClient::sendReceive_data()
@@ -827,6 +832,83 @@ void Tst_QMqttClient::subscriptionIdsOverlap()
     QTRY_VERIFY2(receiveACounter == 1, "Did not receive non-overlapping message.");
 }
 
+DefaultVersionTestData(Tst_QMqttClient::keepAlive_data)
+
+void Tst_QMqttClient::keepAlive()
+{
+    QFETCH(QMqttClient::ProtocolVersion, mqttVersion);
+    const quint16 keepAlive = 1;
+
+    VersionClient(mqttVersion, client);
+    client.setHostname(m_testBroker);
+    client.setPort(m_port);
+
+    client.setKeepAlive(keepAlive);
+    QCOMPARE(client.keepAlive(), keepAlive);
+    QCOMPARE(client.autoKeepAlive(), true); // default
+
+    client.connectToHost();
+    QTRY_COMPARE(client.state(), QMqttClient::Connected);
+
+    if (client.protocolVersion() == QMqttClient::MQTT_5_0) {
+        const auto serverProps = client.serverConnectionProperties();
+        if (serverProps.isValid() && serverProps.availableProperties() & QMqttServerConnectionProperties::ServerKeepAlive) {
+            if (serverProps.serverKeepAlive() == 0 || serverProps.serverKeepAlive() > keepAlive) {
+                qDebug() << "Server specifies keepAlive which cannot be used for this test";
+                return;
+            }
+        }
+    }
+
+    // Changing autoKeepAlive is not possible while connected
+    client.setAutoKeepAlive(false);
+    QCOMPARE(client.autoKeepAlive(), true);
+
+    // Sending a manual ping request with autoKeepAlive is not allowed
+    // and will be suppressed.
+    const bool sent = client.requestPing();
+    QCOMPARE(sent, false);
+
+    // Verify auto keep alive works
+    QSignalSpy spy(&client, &QMqttClient::pingResponseReceived);
+    QTRY_VERIFY_WITH_TIMEOUT(spy.count() == 5, keepAlive * 1000 * 5 * 2);
+    spy.clear();
+
+    client.disconnectFromHost();
+    QTRY_COMPARE(client.state(), QMqttClient::Disconnected);
+
+    // Set manual keepAlive
+    client.setAutoKeepAlive(false);
+    QCOMPARE(client.autoKeepAlive(), false);
+    client.setKeepAlive(keepAlive);
+
+    client.connectToHost();
+    QTRY_COMPARE(client.state(), QMqttClient::Connected);
+
+    // Check for timeout / disconnect when not ping sent
+    QTRY_COMPARE_WITH_TIMEOUT(client.state(), QMqttClient::Disconnected, keepAlive * 1000 * 5 * 2);
+
+    // Check manual ping
+    QTimer t;
+    t.setInterval(keepAlive * 1000);
+    t.setSingleShot(false);
+    t.connect(&t, &QTimer::timeout, [&client]() {
+        client.requestPing();
+    });
+
+    connect(&client, &QMqttClient::connected, [&t]() {
+        t.start();
+    });
+
+    client.connectToHost();
+    QTRY_COMPARE(client.state(), QMqttClient::Connected);
+
+    QTRY_VERIFY_WITH_TIMEOUT(spy.count() == 5, keepAlive * 1000 * 5 * 2);
+
+    client.disconnectFromHost();
+    QTRY_COMPARE(client.state(), QMqttClient::Disconnected);
+}
+
 QTEST_MAIN(Tst_QMqttClient)
 
 #include "tst_qmqttclient.moc"