1 files changed, 241 insertions, 172 deletions

diff --git a/lib/room.cpp b/lib/room.cpp
index a923bf9a..a2b99039 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -121,15 +121,15 @@ public:
     int notificationCount = 0;
     members_map_t membersMap;
     QList<User*> usersTyping;
-    QMultiHash<QString, User*> eventIdReadUsers;
+    QHash<QString, QSet<User*>> eventIdReadUsers;
     QList<User*> usersInvited;
     QList<User*> membersLeft;
     int unreadMessages = 0;
     bool displayed = false;
     QString firstDisplayedEventId;
     QString lastDisplayedEventId;
-    QHash<const User*, QString> lastReadEventIds;
-    QString serverReadMarker;
+    QHash<const User*, ReadReceipt> lastReadReceipts;
+    QString fullyReadUntilEventId;
     TagsMap tags;
     UnorderedMap<QString, EventPtr> accountData;
     QString prevBatch;
@@ -292,11 +292,12 @@ public:
      */
     void dropDuplicateEvents(RoomEvents& events) const;
 
-    Changes setLastReadEvent(User* u, QString eventId);
-    void updateUnreadCount(const rev_iter_t& from, const rev_iter_t& to);
-    Changes promoteReadMarker(User* u, const rev_iter_t& newMarker, bool force = false);
-
-    Changes markMessagesAsRead(rev_iter_t upToMarker);
+    void setLastReadReceipt(User* u, rev_iter_t newMarker,
+                            ReadReceipt newReceipt = {});
+    Changes setFullyReadMarker(const QString &eventId);
+    Changes updateUnreadCount(const rev_iter_t& from, const rev_iter_t& to);
+    Changes recalculateUnreadCount(bool force = false);
+    void markMessagesAsRead(const rev_iter_t &upToMarker);
 
     void getAllMembers();
 
@@ -619,139 +620,189 @@ void Room::setJoinState(JoinState state)
     emit joinStateChanged(oldState, state);
 }
 
-Room::Changes Room::Private::setLastReadEvent(User* u, QString eventId)
+void Room::Private::setLastReadReceipt(User* u, rev_iter_t newMarker,
+                                       ReadReceipt newReceipt)
 {
-    auto& storedId = lastReadEventIds[u];
-    if (storedId == eventId)
-        return Change::NoChange;
-    eventIdReadUsers.remove(storedId, u);
-    eventIdReadUsers.insert(eventId, u);
-    swap(storedId, eventId);
-    emit q->lastReadEventChanged(u);
-    emit q->readMarkerForUserMoved(u, eventId, storedId);
-    if (isLocalUser(u)) {
-        if (storedId != serverReadMarker)
-            connection->callApi<SetReadMarkerJob>(BackgroundRequest, id,
-                                                  storedId);
-        emit q->readMarkerMoved(eventId, storedId);
-        return Change::ReadMarkerChange;
+    if (!u) {
+        Q_ASSERT(u != nullptr); // For Debug builds
+        qCCritical(MAIN) << "Empty user, skipping read receipt registration";
+        return; // For Release builds
+    }
+    if (!q->isMember(u->id())) {
+        qCWarning(EPHEMERAL)
+            << "Won't record read receipt for non-member" << u->id();
+        return;
     }
-    return Change::NoChange;
+
+    auto& storedReceipt = lastReadReceipts[u];
+    if (newMarker == timeline.crend() && !newReceipt.eventId.isEmpty())
+        newMarker = q->findInTimeline(newReceipt.eventId);
+    if (newMarker != timeline.crend()) {
+        // NB: with reverse iterators, timeline history >= sync edge
+        if (newMarker >= q->findInTimeline(storedReceipt.eventId)) {
+            qCDebug(EPHEMERAL) << "The new read receipt for" << u->id()
+                               << "is at or behind the old one, skipping";
+            return;
+        }
+
+        // Try to auto-promote the read marker over the user's own messages
+        // (switch to direct iterators for that).
+        const auto eagerMarker = find_if(newMarker.base(), timeline.cend(),
+                                         [=](const TimelineItem& ti) {
+                                             return ti->senderId() != u->id();
+                                         })
+                                 - 1;
+        newReceipt.eventId = (*eagerMarker)->id();
+        if (eagerMarker != newMarker.base() - 1) // &*(rIt.base() - 1) === &*rIt
+            qCDebug(EPHEMERAL) << "Auto-promoted read receipt for" << u->id()
+                               << "to" << newReceipt.eventId;
+    }
+
+    if (storedReceipt == newReceipt)
+        return;
+    // Finally make the change
+    auto& oldEventReadUsers = eventIdReadUsers[storedReceipt.eventId];
+    oldEventReadUsers.remove(u);
+    if (oldEventReadUsers.isEmpty())
+        eventIdReadUsers.remove(storedReceipt.eventId);
+    eventIdReadUsers[newReceipt.eventId].insert(u);
+    swap(storedReceipt, newReceipt); // Now newReceipt actually stores the old receipt
+    qCDebug(EPHEMERAL) << "The new read receipt for" << u->id() << "is at"
+                       << storedReceipt.eventId;
+    emit q->lastReadEventChanged(u);
+    // TODO: remove in 0.8
+    if (!isLocalUser(u))
+        emit q->readMarkerForUserMoved(u, newReceipt.eventId,
+                                       storedReceipt.eventId);
 }
 
-void Room::Private::updateUnreadCount(const rev_iter_t& from,
-                                      const rev_iter_t& to)
+Room::Changes Room::Private::updateUnreadCount(const rev_iter_t& from,
+                                               const rev_iter_t& to)
 {
     Q_ASSERT(from >= timeline.crbegin() && from <= timeline.crend());
     Q_ASSERT(to >= from && to <= timeline.crend());
 
-    // Catch a special case when the last read event id refers to an event
-    // that has just arrived. In this case we should recalculate
-    // unreadMessages and might need to promote the read marker further
-    // over local-origin messages.
-    auto readMarker = q->readMarker();
-    if (readMarker == historyEdge() && q->allHistoryLoaded())
-        --readMarker; // Read marker not found in the timeline, initialise it
-    if (readMarker >= from && readMarker < to) {
-        promoteReadMarker(q->localUser(), readMarker, true);
-        return;
-    }
-
-    Q_ASSERT(to <= readMarker);
+    auto fullyReadMarker = q->fullyReadMarker();
+    if (fullyReadMarker < from)
+        return NoChange; // What's arrived is already fully read
+
+    if (fullyReadMarker == historyEdge() && q->allHistoryLoaded())
+        --fullyReadMarker; // No read marker in the whole room, initialise it
+    if (fullyReadMarker < to) {
+        // Catch a special case when the last fully read event id refers to an
+        // event that has just arrived. In this case we should recalculate
+        // unreadMessages to get an exact number instead of an estimation
+        // (see https://github.com/quotient-im/libQuotient/wiki/unread_count).
+        // For the same reason (switching from the estimation to the exact
+        // number) this branch always emits unreadMessagesChanged() and returns
+        // UnreadNotifsChange, even if the estimation luckily matched the exact
+        // result.
+        return recalculateUnreadCount(true);
+    }
+
+    // Fully read marker is somewhere beyond the most historical message from
+    // the arrived batch - add up newly arrived messages to the current counter,
+    // instead of a complete recalculation.
+    Q_ASSERT(to <= fullyReadMarker);
 
     QElapsedTimer et;
     et.start();
     const auto newUnreadMessages =
-        count_if(from, to, std::bind(&Room::Private::isEventNotable, this, _1));
+        count_if(from, to,
+                 std::bind(&Room::Private::isEventNotable, this, _1));
     if (et.nsecsElapsed() > profilerMinNsecs() / 10)
         qCDebug(PROFILER) << "Counting gained unread messages took" << et;
 
-    if (newUnreadMessages > 0) {
-        // See https://github.com/quotient-im/libQuotient/wiki/unread_count
-        if (unreadMessages < 0)
-            unreadMessages = 0;
-
-        unreadMessages += newUnreadMessages;
-        qCDebug(MESSAGES) << "Room" << q->objectName() << "has gained"
-                          << newUnreadMessages << "unread message(s),"
-                          << (q->readMarker() == timeline.crend()
-                                  ? "in total at least"
-                                  : "in total")
-                          << unreadMessages << "unread message(s)";
-        emit q->unreadMessagesChanged(q);
-    }
-}
+    if (newUnreadMessages == 0)
+        return NoChange;
 
-Room::Changes Room::Private::promoteReadMarker(User* u,
-                                               const rev_iter_t& newMarker,
-                                               bool force)
-{
-    Q_ASSERT_X(u, __FUNCTION__, "User* should not be nullptr");
-    Q_ASSERT(newMarker >= timeline.crbegin() && newMarker <= timeline.crend());
+    // See https://github.com/quotient-im/libQuotient/wiki/unread_count
+    if (unreadMessages < 0)
+        unreadMessages = 0;
+
+    unreadMessages += newUnreadMessages;
+    qCDebug(MESSAGES) << "Room" << q->objectName() << "has gained"
+                      << newUnreadMessages << "unread message(s),"
+                      << (q->fullyReadMarker() == timeline.crend()
+                              ? "in total at least"
+                              : "in total")
+                      << unreadMessages << "unread message(s)";
+    emit q->unreadMessagesChanged(q);
+    return UnreadNotifsChange;
+}
+
+Room::Changes Room::Private::recalculateUnreadCount(bool force)
+{
+    // The recalculation logic assumes that the fully read marker points at
+    // a specific position in the timeline
+    Q_ASSERT(q->fullyReadMarker() != timeline.crend());
+    const auto oldUnreadCount = unreadMessages;
+    QElapsedTimer et;
+    et.start();
+    unreadMessages =
+        int(count_if(timeline.crbegin(), q->fullyReadMarker(),
+                     [this](const auto& ti) { return isEventNotable(ti); }));
+    if (et.nsecsElapsed() > profilerMinNsecs() / 10)
+        qCDebug(PROFILER) << "Recounting unread messages took" << et;
 
-    const auto prevMarker = q->readMarker(u);
-    if (!force && prevMarker <= newMarker) // Remember, we deal with reverse
-                                           // iterators
-        return Change::NoChange;
+    // See https://github.com/quotient-im/libQuotient/wiki/unread_count
+    if (unreadMessages == 0)
+        unreadMessages = -1;
 
-    Q_ASSERT(newMarker < historyEdge());
+    if (!force && unreadMessages == oldUnreadCount)
+        return NoChange;
 
-    // Try to auto-promote the read marker over the user's own messages
-    // (switch to direct iterators for that).
-    auto eagerMarker =
-        find_if(newMarker.base(), timeline.cend(), [=](const TimelineItem& ti) {
-            return ti->senderId() != u->id();
-        });
+    if (unreadMessages == -1)
+        qCDebug(MESSAGES)
+            << "Room" << displayname << "has no more unread messages";
+    else
+        qCDebug(MESSAGES) << "Room" << displayname << "still has"
+                          << unreadMessages << "unread message(s)";
+    emit q->unreadMessagesChanged(q);
+    return UnreadNotifsChange;
+}
 
-    auto changes = setLastReadEvent(u, (*(eagerMarker - 1))->id());
-    if (isLocalUser(u)) {
-        const auto oldUnreadCount = unreadMessages;
-        QElapsedTimer et;
-        et.start();
-        unreadMessages =
-            int(count_if(eagerMarker, syncEdge(),
-                         [this](const auto& ti) { return isEventNotable(ti); }));
-        if (et.nsecsElapsed() > profilerMinNsecs() / 10)
-            qCDebug(PROFILER) << "Recounting unread messages took" << et;
+Room::Changes Room::Private::setFullyReadMarker(const QString& eventId)
+{
+    if (fullyReadUntilEventId == eventId)
+        return NoChange;
 
-        // See https://github.com/quotient-im/libQuotient/wiki/unread_count
-        if (unreadMessages == 0)
-            unreadMessages = -1;
+    const auto prevFullyReadId = std::exchange(fullyReadUntilEventId, eventId);
+    qCDebug(MESSAGES) << "Fully read marker in" << q->objectName() //
+                      << "moved to" << fullyReadUntilEventId;
+    emit q->fullyReadMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
+    // TODO: Remove in 0.8
+    emit q->readMarkerMoved(prevFullyReadId, fullyReadUntilEventId);
 
-        if (force || unreadMessages != oldUnreadCount) {
-            if (unreadMessages == -1) {
-                qCDebug(MESSAGES)
-                    << "Room" << displayname << "has no more unread messages";
-            } else
-                qCDebug(MESSAGES) << "Room" << displayname << "still has"
-                                  << unreadMessages << "unread message(s)";
-            emit q->unreadMessagesChanged(q);
-            changes |= Change::UnreadNotifsChange;
-        }
+    Changes changes = ReadMarkerChange;
+    if (const auto rm = q->fullyReadMarker(); rm != historyEdge()) {
+        // Pull read receipt if it's behind
+        setLastReadReceipt(q->localUser(), rm);
+        changes |= recalculateUnreadCount(); // TODO: updateUnreadCount()?
     }
     return changes;
 }
 
-Room::Changes Room::Private::markMessagesAsRead(rev_iter_t upToMarker)
+void Room::setReadReceipt(const QString& atEventId)
 {
-    const auto prevMarker = q->readMarker();
-    auto changes = promoteReadMarker(q->localUser(), upToMarker);
-    if (prevMarker != upToMarker)
-        qCDebug(MESSAGES) << "Marked messages as read until" << *q->readMarker();
+    d->setLastReadReceipt(localUser(), historyEdge(),
+                          { atEventId, QDateTime::currentDateTime() });
+    connection()->callApi<PostReceiptJob>(BackgroundRequest, id(),
+                                          QStringLiteral("m.read"),
+                                          QUrl::toPercentEncoding(atEventId));
+}
 
-    // We shouldn't send read receipts for the local user's own messages - so
-    // search earlier messages for the latest message not from the local user
-    // until the previous last-read message, whichever comes first.
-    for (; upToMarker < prevMarker; ++upToMarker) {
-        if ((*upToMarker)->senderId() != q->localUser()->id()) {
-            connection->callApi<PostReceiptJob>(BackgroundRequest,
-                                                id, QStringLiteral("m.read"),
-                                                QUrl::toPercentEncoding(
-                                                    (*upToMarker)->id()));
-            break;
-        }
+void Room::Private::markMessagesAsRead(const rev_iter_t &upToMarker)
+{
+    if (upToMarker < q->fullyReadMarker()) {
+        setFullyReadMarker((*upToMarker)->id());
+        // Assuming that if a read receipt was sent on a newer event, it will
+        // stay there instead of "un-reading" notifications/mentions from
+        // m.fully_read to m.read
+        connection->callApi<SetReadMarkerJob>(BackgroundRequest, id,
+                                              fullyReadUntilEventId,
+                                              fullyReadUntilEventId);
     }
-    return changes;
 }
 
 void Room::markMessagesAsRead(QString uptoEventId)
@@ -882,11 +933,8 @@ void Room::setDisplayed(bool displayed)
 
     d->displayed = displayed;
     emit displayedChanged(displayed);
-    if (displayed) {
-        resetHighlightCount();
-        resetNotificationCount();
+    if (displayed)
         d->getAllMembers();
-    }
 }
 
 QString Room::firstDisplayedEventId() const { return d->firstDisplayedEventId; }
@@ -947,19 +995,28 @@ void Room::setLastDisplayedEvent(TimelineItem::index_t index)
 Room::rev_iter_t Room::readMarker(const User* user) const
 {
     Q_ASSERT(user);
-    return findInTimeline(d->lastReadEventIds.value(user));
+    return findInTimeline(lastReadReceipt(user->id()).eventId);
 }
 
-Room::rev_iter_t Room::readMarker() const { return readMarker(localUser()); }
+Room::rev_iter_t Room::readMarker() const { return fullyReadMarker(); }
+
+QString Room::readMarkerEventId() const { return lastFullyReadEventId(); }
 
-QString Room::readMarkerEventId() const
+ReadReceipt Room::lastReadReceipt(const QString& userId) const
 {
-    return d->lastReadEventIds.value(localUser());
+    return d->lastReadReceipts.value(user(userId));
 }
 
-QList<User*> Room::usersAtEventId(const QString& eventId)
+QString Room::lastFullyReadEventId() const { return d->fullyReadUntilEventId; }
+
+Room::rev_iter_t Room::fullyReadMarker() const
+{
+    return findInTimeline(d->fullyReadUntilEventId);
+}
+
+QSet<User*> Room::usersAtEventId(const QString& eventId)
 {
-    return d->eventIdReadUsers.values(eventId);
+    return d->eventIdReadUsers.value(eventId);
 }
 
 int Room::notificationCount() const { return d->notificationCount; }
@@ -2437,24 +2494,23 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events)
                        << timeline.back();
 
         // The first event in the just-added batch (referred to by `from`)
-        // defines whose read marker can possibly be promoted any further over
+        // defines whose read receipt can possibly be promoted any further over
         // the same author's events newly arrived. Others will need explicit
-        // read receipts from the server (or, for the local user,
-        // markMessagesAsRead() invocation) to promote their read markers over
+        // read receipts from the server - or, for the local user, calling
+        // setLastDisplayedEventId() - to promote their read receipts over
         // the new message events.
-        if (const auto senderId = (*from)->senderId(); !senderId.isEmpty()) {
-            auto* const firstWriter = q->user(senderId);
-            if (q->readMarker(firstWriter) != historyEdge()) {
-                roomChanges |=
-                    promoteReadMarker(firstWriter, rev_iter_t(from) - 1);
-                qCDebug(MESSAGES)
-                    << "Auto-promoted read marker for" << senderId
-                    << "to" << *q->readMarker(firstWriter);
-            }
+        auto* const firstWriter = q->user((*from)->senderId());
+        setLastReadReceipt(firstWriter, rev_iter_t(from + 1));
+        if (firstWriter == q->localUser()
+            && q->fullyReadMarker().base() == from) //
+        {
+            // If the local user's message(s) is/are first in the batch
+            // and the fully read marker was right before it, promote
+            // the fully read marker to the same event as the read receipt.
+            roomChanges |=
+                setFullyReadMarker(lastReadReceipts.value(firstWriter).eventId);
         }
-
-        updateUnreadCount(timeline.crbegin(), rev_iter_t(from));
-        roomChanges |= Change::UnreadNotifsChange;
+        roomChanges |= updateUnreadCount(timeline.crbegin(), rev_iter_t(from));
     }
 
     Q_ASSERT(timeline.size() == timelineSize + totalInserted);
@@ -2499,8 +2555,11 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events)
             emit q->updatedEvent(relation.eventId);
         }
     }
-    if (from <= q->readMarker())
-        updateUnreadCount(from, historyEdge());
+    updateUnreadCount(from, historyEdge());
+    // When there are no unread messages and the read marker is within the
+    // known timeline, unreadMessages == -1
+    // (see https://github.com/quotient-im/libQuotient/wiki/unread_count).
+    Q_ASSERT(unreadMessages != 0 || q->fullyReadMarker() == historyEdge());
 
     Q_ASSERT(timeline.size() == timelineSize + insertedSize);
     if (insertedSize > 9 || et.nsecsElapsed() >= profilerMinNsecs())
@@ -2746,31 +2805,21 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event)
                                        << p.receipts.size() << "users";
             }
             const auto newMarker = findInTimeline(p.evtId);
-            if (newMarker != historyEdge()) {
-                for (const Receipt& r : p.receipts) {
-                    if (r.userId == connection()->userId())
-                        continue; // FIXME, #185
-                    if (isMember(r.userId))
-                        changes |=
-                            d->promoteReadMarker(user(r.userId), newMarker);
+            if (newMarker == historyEdge())
+                qCDebug(EPHEMERAL) << "Event of the read receipt(s) is not "
+                                      "found; saving them anyway";
+            for (const Receipt& r : p.receipts)
+                if (isMember(r.userId)) {
+                    // If the event is not found (most likely, because it's
+                    // too old and hasn't been fetched from the server yet)
+                    // but there is a previous marker for a user, keep
+                    // the previous marker because read receipts are not
+                    // supposed to move backwards. Otherwise, blindly
+                    // store the event id for this user and update the read
+                    // marker when/if the event is fetched later on.
+                    d->setLastReadReceipt(user(r.userId), newMarker,
+                                          { p.evtId, r.timestamp });
                 }
-            } else {
-                qCDebug(EPHEMERAL) << "Event" << p.evtId
-                                   << "not found; saving read receipts anyway";
-                // If the event is not found (most likely, because it's too old
-                // and hasn't been fetched from the server yet), but there is
-                // a previous marker for a user, keep the previous marker.
-                // Otherwise, blindly store the event id for this user.
-                for (const Receipt& r : p.receipts) {
-                    if (r.userId == connection()->userId())
-                        continue; // FIXME, #185
-                    if (!isMember(r.userId))
-                        continue;
-                    auto u = user(r.userId);
-                    if (readMarker(u) == historyEdge())
-                        changes |= d->setLastReadEvent(u, p.evtId);
-                }
-            }
         }
         if (eventsWithReceipts.size() > 3 || totalReceipts > 10
             || et.nsecsElapsed() >= profilerMinNsecs())
@@ -2790,15 +2839,9 @@ Room::Changes Room::processAccountDataEvent(EventPtr&& event)
         changes |= Change::TagsChange;
     }
 
-    if (auto* evt = eventCast<ReadMarkerEvent>(event)) {
-        auto readEventId = evt->event_id();
-        qCDebug(STATE) << "Server-side read marker at" << readEventId;
-        d->serverReadMarker = readEventId;
-        const auto newMarker = findInTimeline(readEventId);
-        changes |= newMarker != historyEdge()
-                       ? d->markMessagesAsRead(newMarker)
-                       : d->setLastReadEvent(localUser(), readEventId);
-    }
+    if (auto* evt = eventCast<const ReadMarkerEvent>(event))
+        changes |= d->setFullyReadMarker(evt->event_id());
+
     // For all account data events
     auto& currentData = d->accountData[event->matrixType()];
     // A polymorphic event-specific comparison might be a bit more
@@ -2979,6 +3022,32 @@ QJsonObject Room::Private::toJson() const
                           { QStringLiteral("events"), accountDataEvents } });
     }
 
+    if (const auto& readReceiptEventId =
+            lastReadReceipts.value(q->localUser()).eventId;
+        !readReceiptEventId.isEmpty()) //
+    {
+        // Okay, that's a mouthful; but basically, it's simply placing an m.read
+        // event in the 'ephemeral' section of the cached sync payload.
+        // See also receiptevent.* and m.read example in the spec.
+        // Only the local user's read receipt is saved - others' are really
+        // considered ephemeral but this one is useful in understanding where
+        // the user is in the timeline before any history is loaded.
+        result.insert(
+            QStringLiteral("ephemeral"),
+            QJsonObject {
+                { QStringLiteral("events"),
+                  QJsonArray { QJsonObject {
+                      { TypeKey, ReceiptEvent::matrixTypeId() },
+                      { ContentKey,
+                        QJsonObject {
+                            { readReceiptEventId,
+                              QJsonObject {
+                                  { QStringLiteral("m.read"),
+                                    QJsonObject {
+                                        { connection->userId(),
+                                          QJsonObject {} } } } } } } } } } } });
+    }
+
     QJsonObject unreadNotifObj { { SyncRoomData::UnreadCountKey,
                                    unreadMessages } };