Refactor packet sending: Add SendSuccess/SendError

7 files changed, 90 insertions, 36 deletions

diff --git a/src/base/QXmppPacket.cpp b/src/base/QXmppPacket.cpp
index bd29106f..3676c269 100644
--- a/src/base/QXmppPacket.cpp
+++ b/src/base/QXmppPacket.cpp
@@ -29,7 +29,7 @@
 
 /// \cond
 QXmppPacket::QXmppPacket(const QXmppNonza &nonza)
-    : m_interface(new QFutureInterface<QXmpp::PacketState>(QFutureInterfaceBase::Started)),
+    : m_interface(std::make_shared<QFutureInterface<QXmpp::SendResult>>(QFutureInterfaceBase::Started)),
       m_isXmppStanza(nonza.isXmppStanza())
 {
     QXmlStreamWriter xmlStream(&m_data);
@@ -46,7 +46,7 @@ bool QXmppPacket::isXmppStanza() const
     return m_isXmppStanza;
 }
 
-QFuture<QXmpp::PacketState> QXmppPacket::future()
+QFuture<QXmpp::SendResult> QXmppPacket::future()
 {
     return m_interface->future();
 }
@@ -56,7 +56,7 @@ void QXmppPacket::reportFinished()
     m_interface->reportFinished();
 }
 
-void QXmppPacket::reportResult(QXmpp::PacketState result)
+void QXmppPacket::reportResult(const QXmpp::SendResult &result)
 {
     m_interface->reportResult(result);
 }
diff --git a/src/base/QXmppPacket_p.h b/src/base/QXmppPacket_p.h
index ea5baca3..a99752bb 100644
--- a/src/base/QXmppPacket_p.h
+++ b/src/base/QXmppPacket_p.h
@@ -25,9 +25,11 @@
 #define QXMPPPACKET_H
 
 #include "QXmppGlobal.h"
+#include "QXmppSendResult.h"
+
+#include <memory>
 
 #include <QFutureInterface>
-#include <QSharedPointer>
 
 class QXmppNonza;
 
@@ -39,13 +41,13 @@ public:
     QByteArray data() const;
     bool isXmppStanza() const;
 
-    QFuture<QXmpp::PacketState> future();
+    QFuture<QXmpp::SendResult> future();
 
     void reportFinished();
-    void reportResult(QXmpp::PacketState);
+    void reportResult(const QXmpp::SendResult &);
 
 private:
-    QSharedPointer<QFutureInterface<QXmpp::PacketState>> m_interface;
+    std::shared_ptr<QFutureInterface<QXmpp::SendResult>> m_interface;
     QByteArray m_data;
     bool m_isXmppStanza;
 };
diff --git a/src/base/QXmppSendResult.h b/src/base/QXmppSendResult.h
new file mode 100644
index 00000000..15b84b05
--- /dev/null
+++ b/src/base/QXmppSendResult.h
@@ -0,0 +1,48 @@
+#ifndef QXMPPSENDRESULT_H
+#define QXMPPSENDRESULT_H
+
+#include "QXmppGlobal.h"
+
+#include <variant>
+
+namespace QXmpp {
+
+///
+/// A struct containing a packet send error type and error message.
+///
+/// \since QXmpp 1.5
+///
+struct SendError
+{
+    /// Describes the type of an error.
+    enum Type : uint8_t {
+        SocketWriteError, ///< The packet was written to the socket with no success (only happens when Stream Management is disabled).
+        Disconnected,     ///< The packet couldn't be sent because the connection hasn't been (re)established.
+        EncryptionError,  ///< The packet couldn't be sent because prior encryption failed.
+    };
+
+    /// Text describing the error.
+    QString text;
+    /// Type of the occured error.
+    Type type;
+};
+
+///
+/// A struct indicating success when sending packets
+///
+/// \since QXmpp 1.5
+///
+struct SendSuccess 
+{
+    /// Indicates whether the packet has been acknowledged by the other peer.
+    bool acknowledged = false;
+};
+
+///
+/// A variant containing either a SendSuccess object or a SendError.
+///
+using SendResult = std::variant<SendSuccess, SendError>;
+
+}
+
+#endif // QXMPPSENDRESULT_H
diff --git a/src/base/QXmppStream.cpp b/src/base/QXmppStream.cpp
index 28a26963..0ede6b18 100644
--- a/src/base/QXmppStream.cpp
+++ b/src/base/QXmppStream.cpp
@@ -176,8 +176,9 @@ bool QXmppStream::sendData(const QByteArray &data)
 ///
 bool QXmppStream::sendPacket(const QXmppNonza &stanza)
 {
-    // the first result is always reported immediately
-    return send(stanza).resultAt(0) != QXmpp::NotSent;
+    bool success;
+    send(stanza, success);
+    return success;
 }
 
 ///
@@ -185,13 +186,19 @@ bool QXmppStream::sendPacket(const QXmppNonza &stanza)
 ///
 /// \since QXmpp 1.5
 ///
-QFuture<QXmpp::PacketState> QXmppStream::send(const QXmppNonza &stanza)
+QFuture<QXmpp::SendResult> QXmppStream::send(const QXmppNonza &nonza)
 {
-    QXmppPacket packet(stanza);
-    sendPacket(packet);
+    bool success;
+    return send(nonza, success);
+}
+
+QFuture<QXmpp::SendResult> QXmppStream::send(const QXmppNonza &nonza, bool &writtenToSocket)
+{
+    QXmppPacket packet(nonza);
+    writtenToSocket = sendData(packet.data());
 
     // handle stream management
-    d->streamManager.handlePacketSent(packet);
+    d->streamManager.handlePacketSent(packet, writtenToSocket);
 
     return packet.future();
 }
@@ -222,13 +229,13 @@ QFuture<QXmppStream::IqResult> QXmppStream::sendIq(const QXmppIq &iq)
 
     auto sendFuture = send(iq);
     if (sendFuture.isFinished()) {
-        if (sendFuture.result() == QXmpp::NotSent) {
+        if (std::holds_alternative<QXmpp::SendError>(sendFuture.result())) {
             // early exit (saves QFutureWatcher)
             return makeReadyFuture<IqResult>(QXmpp::NotSent);
         }
     } else {
-        awaitLast(sendFuture, this, [this, id = iq.id()](QXmpp::PacketState result) {
-            if (result == QXmpp::NotSent) {
+        awaitLast(sendFuture, this, [this, id = iq.id()](QXmpp::SendResult result) {
+            if (std::holds_alternative<QXmpp::SendError>(result)) {
                 if (auto itr = d->runningIqs.find(id); itr != d->runningIqs.end()) {
                     itr.value().reportResult(QXmpp::NotSent);
                     itr.value().reportFinished();
@@ -427,15 +434,6 @@ void QXmppStream::processData(const QString &data)
     }
 }
 
-void QXmppStream::sendPacket(QXmppPacket &packet)
-{
-    if (sendData(packet.data())) {
-        packet.reportResult(QXmpp::Sent);
-    } else {
-        packet.reportResult(QXmpp::NotSent);
-    }
-}
-
 bool QXmppStream::handleIqResponse(const QDomElement &stanza)
 {
     if (stanza.tagName() != QStringLiteral("iq")) {
diff --git a/src/base/QXmppStream.h b/src/base/QXmppStream.h
index 62558aa0..dd327a92 100644
--- a/src/base/QXmppStream.h
+++ b/src/base/QXmppStream.h
@@ -26,6 +26,7 @@
 #define QXMPPSTREAM_H
 
 #include "QXmppLogger.h"
+#include "QXmppSendResult.h"
 
 #include <variant>
 
@@ -56,7 +57,7 @@ public:
     virtual bool isConnected() const;
 
     bool sendPacket(const QXmppNonza &);
-    QFuture<QXmpp::PacketState> send(const QXmppNonza &);
+    QFuture<QXmpp::SendResult> send(const QXmppNonza &);
 
     using IqResult = std::variant<QDomElement, QXmpp::PacketState>;
     QFuture<IqResult> sendIq(const QXmppIq &);
@@ -109,8 +110,8 @@ private:
     friend class tst_QXmppStream;
     friend class TestClient;
 
+    QFuture<QXmpp::SendResult> send(const QXmppNonza &, bool &);
     void processData(const QString &data);
-    void sendPacket(QXmppPacket &packet);
     bool handleIqResponse(const QDomElement &);
 
     QXmppStreamPrivate *const d;
diff --git a/src/base/QXmppStreamManagement.cpp b/src/base/QXmppStreamManagement.cpp
index 4d5f3540..39bfbf32 100644
--- a/src/base/QXmppStreamManagement.cpp
+++ b/src/base/QXmppStreamManagement.cpp
@@ -351,9 +351,6 @@ unsigned int QXmppStreamManager::lastIncomingSequenceNumber() const
 void QXmppStreamManager::handleDisconnect()
 {
     m_enabled = false;
-    for (auto &packet : m_unacknowledgedStanzas) {
-        packet.reportResult(QXmpp::NotSent);
-    }
 }
 
 void QXmppStreamManager::handleStart()
@@ -361,12 +358,20 @@ void QXmppStreamManager::handleStart()
     m_enabled = false;
 }
 
-void QXmppStreamManager::handlePacketSent(QXmppPacket &packet)
+void QXmppStreamManager::handlePacketSent(QXmppPacket &packet, bool sentData)
 {
     if (m_enabled && packet.isXmppStanza()) {
         m_unacknowledgedStanzas.insert(++m_lastOutgoingSequenceNumber, packet);
         sendAcknowledgementRequest();
     } else {
+        if (sentData) {
+            packet.reportResult(QXmpp::SendSuccess { false });
+        } else {
+            packet.reportResult(QXmpp::SendError {
+                QStringLiteral("Couldn't write data to socket. No stream management enabled."),
+                QXmpp::SendError::SocketWriteError
+            });
+        }
         packet.reportFinished();
     }
 }
@@ -405,7 +410,7 @@ void QXmppStreamManager::enableStreamManagement(bool resetSequenceNumber)
 
             for (auto &packet : oldUnackedStanzas) {
                 m_unacknowledgedStanzas.insert(++m_lastOutgoingSequenceNumber, packet);
-                stream->sendPacket(packet);
+                stream->sendData(packet.data());
             }
 
             sendAcknowledgementRequest();
@@ -414,7 +419,7 @@ void QXmppStreamManager::enableStreamManagement(bool resetSequenceNumber)
         // resend unacked stanzas
         if (!m_unacknowledgedStanzas.isEmpty()) {
             for (auto &packet : m_unacknowledgedStanzas) {
-                stream->sendPacket(packet);
+                stream->sendData(packet.data());
             }
 
             sendAcknowledgementRequest();
@@ -426,7 +431,7 @@ void QXmppStreamManager::setAcknowledgedSequenceNumber(unsigned int sequenceNumb
 {
     for (auto it = m_unacknowledgedStanzas.begin(); it != m_unacknowledgedStanzas.end();) {
         if (it.key() <= sequenceNumber) {
-            it->reportResult(QXmpp::Acknowledged);
+            it->reportResult(QXmpp::SendSuccess { true });
             it->reportFinished();
             it = m_unacknowledgedStanzas.erase(it);
         } else {
@@ -477,7 +482,7 @@ void QXmppStreamManager::sendAcknowledgementRequest()
 void QXmppStreamManager::resetCache()
 {
     for (auto &packet : m_unacknowledgedStanzas) {
-        packet.reportResult(QXmpp::NotSent);
+        packet.reportResult(QXmpp::SendError { QStringLiteral("Disconnected"), QXmpp::SendError::Disconnected });
         packet.reportFinished();
     }
 
diff --git a/src/base/QXmppStreamManagement_p.h b/src/base/QXmppStreamManagement_p.h
index 1a528c0c..9f940494 100644
--- a/src/base/QXmppStreamManagement_p.h
+++ b/src/base/QXmppStreamManagement_p.h
@@ -197,7 +197,7 @@ public:
 
     void handleDisconnect();
     void handleStart();
-    void handlePacketSent(QXmppPacket &packet);
+    void handlePacketSent(QXmppPacket &packet, bool sentData);
     bool handleStanza(const QDomElement &stanza);
 
     void resetCache();