fix(receipts): Prevent double message send for received receipt

Fixes #2726
Register for receipt handling only once, cache receipts that are received before message is writen to history and mark a message as sent once both its receipt has been received and it has been writen to history

src/nexus.cpp

@@ -218,7 +218,6 @@ void Nexus::showMainGUI()
     connect(core, &Core::friendStatusMessageChanged, widget, &Widget::onFriendStatusMessageChanged);
     connect(core, &Core::friendRequestReceived, widget, &Widget::onFriendRequestReceived);
     connect(core, &Core::friendMessageReceived, widget, &Widget::onFriendMessageReceived);
-    connect(core, &Core::receiptRecieved, widget, &Widget::onReceiptRecieved);
     connect(core, &Core::groupInviteReceived, widget, &Widget::onGroupInviteReceived);
     connect(core, &Core::groupMessageReceived, widget, &Widget::onGroupMessageReceived);
     connect(core, &Core::groupNamelistChanged, widget, &Widget::onGroupNamelistChanged);

src/persistence/offlinemsgengine.cpp

@@ -25,6 +25,7 @@
 #include "src/persistence/settings.h"
 #include <QMutexLocker>
 #include <QTimer>
+#include <QCoreApplication>
 
 /**
  * @var static const int OfflineMsgEngine::offlineTimeout
@@ -52,28 +53,31 @@ void OfflineMsgEngine::dischargeReceipt(int receipt)
 {
     QMutexLocker ml(&mutex);
 
-    Profile* profile = Nexus::getProfile();
     auto it = receipts.find(receipt);
-    if (it != receipts.end()) {
-        int mID = it.value();
-        auto msgIt = undeliveredMsgs.find(mID);
-        if (msgIt != undeliveredMsgs.end()) {
-            if (profile->isHistoryEnabled())
-                profile->getHistory()->markAsSent(mID);
-            msgIt.value().msg->markAsSent(QDateTime::currentDateTime());
-            undeliveredMsgs.erase(msgIt);
-        }
-        receipts.erase(it);
+    if (it == receipts.end()) {
+        it = receipts.insert(receipt, Receipt());
+    } else if (it->bRecepitReceived) {
+        qWarning() << "Received duplicate receipt";
     }
+    it->bRecepitReceived = true;
+    processReceipt(receipt);
 }
 
 void OfflineMsgEngine::registerReceipt(int receipt, int64_t messageID, ChatMessage::Ptr msg,
                                        const QDateTime& timestamp)
 {
     QMutexLocker ml(&mutex);
 
-    receipts[receipt] = messageID;
+    auto it = receipts.find(receipt);
+    if (it == receipts.end()) {
+        it = receipts.insert(receipt, Receipt());
+    } else if (it->bRowValid && receipt != 0 /* offline receipt */) {
+        qWarning() << "Received duplicate registration of receipt";
+    }
+    it->rowId = messageID;
+    it->bRowValid = true;
     undeliveredMsgs[messageID] = {msg, timestamp, receipt};
+    processReceipt(receipt);
 }
 
 void OfflineMsgEngine::deliverOfflineMsgs()
@@ -119,3 +123,43 @@ void OfflineMsgEngine::removeAllReceipts()
 
     receipts.clear();
 }
+
+void OfflineMsgEngine::updateTimestamp(int receiptId)
+{
+    QMutexLocker ml(&mutex);
+
+    auto receipt = receipts.find(receiptId);
+    const auto msg = undeliveredMsgs.constFind(receipt->rowId);
+    if (msg == undeliveredMsgs.end()) {
+        // this should never occur as registerReceipt adds the msg before processReceipt calls updateTimestamp
+        qCritical() << "Message was not in undeliveredMsgs map when attempting to update its timestamp!";
+        return;
+    }
+    msg->msg->markAsSent(QDateTime::currentDateTime());
+    undeliveredMsgs.remove(receipt->rowId);
+    receipts.erase(receipt);
+}
+
+void OfflineMsgEngine::processReceipt(int receiptId)
+{
+    const auto receipt = receipts.constFind(receiptId);
+    if (receipt == receipts.end()) {
+        // this should never occur as callers ensure receipts contains receiptId
+        qCritical() << "Receipt was not added to map prior to attempting to process it!";
+        return;
+    }
+
+    if (!receipt->bRecepitReceived || !receipt->bRowValid)
+        return;
+
+    Profile* const profile = Nexus::getProfile();
+    if (profile->isHistoryEnabled()) {
+        profile->getHistory()->markAsSent(receipt->rowId);
+    }
+
+    if (QThread::currentThread() == QCoreApplication::instance()->thread()) {
+        updateTimestamp(receiptId);
+    } else {
+        QMetaObject::invokeMethod(this, "updateTimestamp", Qt::QueuedConnection, Q_ARG(int, receiptId));
+    }
+}

src/persistence/offlinemsgengine.h

@@ -45,18 +45,26 @@ class OfflineMsgEngine : public QObject
 public slots:
     void deliverOfflineMsgs();
     void removeAllReceipts();
+    void updateTimestamp(int receiptId);
 
 private:
+    void processReceipt(int receiptId);
+    struct Receipt
+    {
+        bool bRowValid{false};
+        int64_t rowId{0};
+        bool bRecepitReceived{false};
+    };
+
     struct MsgPtr
     {
         ChatMessage::Ptr msg;
         QDateTime timestamp;
         int receipt;
     };
-
     QMutex mutex;
     Friend* f;
-    QHash<int, int64_t> receipts;
+    QHash<int, Receipt> receipts;
     QMap<int64_t, MsgPtr> undeliveredMsgs;
 
     static const int offlineTimeout;