client: Fix duplicated query quit messages
authorShane Synan <digitalcircuit36939@gmail.com>
Tue, 7 Aug 2018 17:01:39 +0000 (12:01 -0500)
committerManuel Nickschas <sputnick@quassel-irc.org>
Tue, 28 Aug 2018 19:55:21 +0000 (21:55 +0200)
Replace the .contains() check with std::binary_search on message
timestamps, passing a custom comparator that provides +/- 1000 ms of
fuzzy matching.  If a quit message is received and doesn't exist
within this time range, it gets forwarded to the query, otherwise
discarded.

Remove the QMultiHash method that tracked quit nicknames.  Each query
gets a unique MessageFilter, so there's no need to store and filter
that.  This will need modified if e.g. adding an option for quit
messages in the Chat Monitor.

This fixes an existing bug made much worse with millisecond timestamp
precision.  Before the 64-bit time changes, quit messages would only
be duplicated if they got split over the turn of a second, a rare
occurrence.  Afterwards, they'd get duplicated if split over the turn
of a millisecond, a much more common occurrence.

Fixes regression from commit 6a63070246d89aa2a2474e3a9a1035fa889ad77e

src/client/messagefilter.cpp
src/client/messagefilter.h

index c501c61..67a873a 100644 (file)
@@ -20,6 +20,8 @@
 
 #include "messagefilter.h"
 
+#include <algorithm>
+
 #include "buffersettings.h"
 #include "client.h"
 #include "buffermodel.h"
@@ -81,7 +83,7 @@ void MessageFilter::messageTypeFilterChanged()
 
     if (_messageTypeFilter != newFilter) {
         _messageTypeFilter = newFilter;
-        _filteredQuitMsgs.clear();
+        _filteredQuitMsgTime.clear();
         invalidateFilter();
     }
 }
@@ -221,17 +223,30 @@ bool MessageFilter::filterAcceptsRow(int sourceRow, const QModelIndex &sourcePar
         if (myNetworkId != msgNetworkId)
             return false;
 
+        // Extract timestamp and nickname from the new quit message
         qint64 messageTimestamp = sourceModel()->data(sourceIdx, MessageModel::TimestampRole)
                 .value<QDateTime>().toMSecsSinceEpoch();
-        QString quiter = sourceModel()->data(sourceIdx, Qt::DisplayRole).toString().section(' ', 0, 0, QString::SectionSkipEmpty).toLower();
+        QString quiter = nickFromMask(sourceModel()->data(sourceIdx, MessageModel::MessageRole)
+                                      .value<Message>().sender()).toLower();
+
+        // Check that nickname matches query name
         if (quiter != bufferName().toLower())
             return false;
 
-        if (_filteredQuitMsgs.contains(quiter, messageTimestamp))
+        // Check if a quit message was already forwarded within +/- 1000 ms
+        static constexpr qint64 MAX_QUIT_DELTA_MS = 1 * 1000;
+        // No need to check if it's the appropriate buffer, each query has a unique message filter
+        if (std::binary_search(_filteredQuitMsgTime.begin(), _filteredQuitMsgTime.end(),
+                               messageTimestamp,
+                               [](qint64 a, qint64 b) { return ((a + MAX_QUIT_DELTA_MS) < b); } )) {
+            // New element is less than if at least 1000 ms older/newer
+            // Match found, no need to forward another quit message
             return false;
+        }
 
+        // Mark query as having a quit message inserted
         MessageFilter *that = const_cast<MessageFilter *>(this);
-        that->_filteredQuitMsgs.insert(quiter,  messageTimestamp);
+        that->_filteredQuitMsgTime.insert(messageTimestamp);
         return true;
     }
 }
index b6180fa..81fa3ff 100644 (file)
@@ -22,6 +22,7 @@
 #define MESSAGEFILTER_H_
 
 #include <QSortFilterProxyModel>
+#include <set>
 
 #include "bufferinfo.h"
 #include "client.h"
@@ -62,7 +63,7 @@ private:
     void init();
 
     QSet<BufferId> _validBuffers;
-    QMultiHash<QString, qint64> _filteredQuitMsgs;
+    std::set<qint64> _filteredQuitMsgTime; ///< Timestamps (ms) of already forwarded quit messages
     int _messageTypeFilter;
 
     int _userNoticesTarget;