Refactor packet sending: Add SendSuccess/SendError

12 files changed, 102 insertions, 65 deletions

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b21e198a..e5357ad2 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -68,6 +68,7 @@ set(INSTALL_HEADER_FILES
     base/QXmppResultSet.h
     base/QXmppRosterIq.h
     base/QXmppRpcIq.h
+    base/QXmppSendResult.h
     base/QXmppSessionIq.h
     base/QXmppSocks.h
     base/QXmppStanza.h
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();
diff --git a/src/client/QXmppClient.cpp b/src/client/QXmppClient.cpp
index 4d1efe7d..e6655bd0 100644
--- a/src/client/QXmppClient.cpp
+++ b/src/client/QXmppClient.cpp
@@ -326,32 +326,12 @@ bool QXmppClient::sendPacket(const QXmppNonza &packet)
 ///
 /// Sends a packet and reports the result via QFuture.
 ///
-/// The QFuture might have multiple results. The first result of the future is
-/// reported immediately (it's safe to access resultAt(0)). If writing the data
-/// to the socket succeeds (that does not mean the server received it),
-/// QXmpp::Sent is reported. Otherwise QXmpp::NotSent is reported.
-///
 /// If stream management is enabled the future continues to be active until the
-/// server acknowledges the packet. On success QXmpp::Acknowledged is reported
-/// and the future finishes.
+/// server acknowledges the packet. On success QXmpp::SendSuccess with
+/// acknowledged == true is reported and the future finishes.
 ///
 /// If connection errors occur the packet is resent if possible. If reconnecting
-/// is not possible, QXmpp::NotSent is reported.
-///
-/// For you the most important result is the last one. QXmpp::Sent means the
-/// packet has been sent without stream management (no acknowledgment).
-/// QXmpp::Acknowledged means the packet has been sent and has been received by
-/// the server, QXmpp::NotSent means no success.
-///
-/// \note <b>TL;DR</b>: The future might have multiple results, so don't do:
-/// \code
-/// send().result();
-/// \endcode
-/// Instead do the following to handle the final result:
-/// \code
-/// send().results().last();
-/// \endcode
-/// QXmpp::Sent or QXmpp::Acknowledged mean success.
+/// is not possible, an error is reported.
 ///
 /// \warning THIS API IS NOT FINALIZED YET!
 ///
@@ -359,7 +339,7 @@ bool QXmppClient::sendPacket(const QXmppNonza &packet)
 /// You can use QFutureWatcher in Qt 5 and QFuture::then() in Qt 6 to handle the
 /// results.
 ///
-QFuture<QXmpp::PacketState> QXmppClient::send(const QXmppStanza &stanza)
+QFuture<QXmpp::SendResult> QXmppClient::send(const QXmppStanza &stanza)
 {
     return d->stream->send(stanza);
 }
diff --git a/src/client/QXmppClient.h b/src/client/QXmppClient.h
index 9a977074..3daf629c 100644
--- a/src/client/QXmppClient.h
+++ b/src/client/QXmppClient.h
@@ -27,6 +27,7 @@
 #include "QXmppConfiguration.h"
 #include "QXmppLogger.h"
 #include "QXmppPresence.h"
+#include "QXmppSendResult.h"
 
 #include <variant>
 
@@ -230,7 +231,7 @@ public:
     State state() const;
     QXmppStanza::Error::Condition xmppStreamError();
 
-    QFuture<QXmpp::PacketState> send(const QXmppStanza &);
+    QFuture<QXmpp::SendResult> send(const QXmppStanza &);
     QFuture<IqResult> sendIq(const QXmppIq &);
     QFuture<EmptyResult> sendGenericIq(const QXmppIq &iq);
 
diff --git a/src/client/QXmppRosterManager.cpp b/src/client/QXmppRosterManager.cpp
index 0f1f0df1..0405056e 100644
--- a/src/client/QXmppRosterManager.cpp
+++ b/src/client/QXmppRosterManager.cpp
@@ -351,7 +351,7 @@ QFuture<QXmppRosterManager::Result> QXmppRosterManager::renameRosterItem(const Q
 ///
 /// \since QXmpp 1.5
 ///
-QFuture<QXmpp::PacketState> QXmppRosterManager::subscribeTo(const QString &bareJid, const QString &reason)
+QFuture<QXmpp::SendResult> QXmppRosterManager::subscribeTo(const QString &bareJid, const QString &reason)
 {
     QXmppPresence packet;
     packet.setTo(QXmppUtils::jidToBareJid(bareJid));
@@ -368,7 +368,7 @@ QFuture<QXmpp::PacketState> QXmppRosterManager::subscribeTo(const QString &bareJ
 ///
 /// \since QXmpp 1.5
 ///
-QFuture<QXmpp::PacketState> QXmppRosterManager::unsubscribeFrom(const QString &bareJid, const QString &reason)
+QFuture<QXmpp::SendResult> QXmppRosterManager::unsubscribeFrom(const QString &bareJid, const QString &reason)
 {
     QXmppPresence packet;
     packet.setTo(QXmppUtils::jidToBareJid(bareJid));
diff --git a/src/client/QXmppRosterManager.h b/src/client/QXmppRosterManager.h
index 09c32ea0..6d872343 100644
--- a/src/client/QXmppRosterManager.h
+++ b/src/client/QXmppRosterManager.h
@@ -29,6 +29,7 @@
 #include "QXmppClientExtension.h"
 #include "QXmppPresence.h"
 #include "QXmppRosterIq.h"
+#include "QXmppSendResult.h"
 
 #include <variant>
 
@@ -94,8 +95,8 @@ public:
     QFuture<Result> addRosterItem(const QString &bareJid, const QString &name = {}, const QSet<QString> &groups = {});
     QFuture<Result> removeRosterItem(const QString &bareJid);
     QFuture<Result> renameRosterItem(const QString &bareJid, const QString &name);
-    QFuture<QXmpp::PacketState> subscribeTo(const QString &bareJid, const QString &reason = {});
-    QFuture<QXmpp::PacketState> unsubscribeFrom(const QString &bareJid, const QString &reason = {});
+    QFuture<QXmpp::SendResult> subscribeTo(const QString &bareJid, const QString &reason = {});
+    QFuture<QXmpp::SendResult> unsubscribeFrom(const QString &bareJid, const QString &reason = {});
 
     /// \cond
     bool handleStanza(const QDomElement &element) override;