Merge pull request #55 from Fxrh/kitsune-timeline-items

2 files changed, 195 insertions, 95 deletions

diff --git a/room.cpp b/room.cpp
index 3d5ede4d..1cedab1e 100644
--- a/room.cpp
+++ b/room.cpp
@@ -47,7 +47,7 @@ class Room::Private
     public:
         /** Map of user names to users. User names potentially duplicate, hence a multi-hashmap. */
         typedef QMultiHash<QString, User*> members_map_t;
-        typedef Timeline::const_reverse_iterator rev_iter_t;
+        typedef std::pair<rev_iter_t, rev_iter_t> rev_iter_pair_t;
 
         Private(Connection* c, const QString& id_)
             : q(nullptr), connection(c), id(id_), joinState(JoinState::Join)
@@ -64,6 +64,7 @@ class Room::Private
 
         Connection* connection;
         Timeline timeline;
+        QHash<QString, TimelineItem::index_t> eventsIndex;
         QString id;
         QStringList aliases;
         QString canonicalAlias;
@@ -94,47 +95,40 @@ class Room::Private
 
         void getPreviousContent(int limit = 10);
 
-        bool isEventNotable(const Event* e) const;
+        bool isEventNotable(const Event* e) const
+        {
+            return e->senderId() != connection->userId() &&
+                    e->type() == EventType::RoomMessage;
+        }
 
         void appendEvent(Event* e)
         {
-            insertEvent(e, timeline.end());
+            insertEvent(e, timeline.end(),
+                        timeline.empty() ? 0 : q->maxTimelineIndex() + 1);
         }
         void prependEvent(Event* e)
         {
-            insertEvent(e, timeline.begin());
+            insertEvent(e, timeline.begin(),
+                        timeline.empty() ? 0 : q->minTimelineIndex() - 1);
         }
 
         /**
          * Removes events from the passed container that are already in the timeline
          */
-        void dropDuplicateEvents(Events& events) const
-        {
-            // Collect all duplicate events at the end of the container
-            auto dupsBegin =
-                std::stable_partition(events.begin(), events.end(),
-                    [&] (Event* e) { return !eventsIndex.contains(e->id()); });
-            // Dispose of those dups
-            std::for_each(dupsBegin, events.end(), [] (Event* e) { delete e; });
-            events.erase(dupsBegin, events.end());
-        }
+        void dropDuplicateEvents(Events& events) const;
 
-        rev_iter_t readMarker(const User* user) const
-        {
-            return eventsIndex.value(lastReadEventIds[user], timeline.crend());
-        }
-        std::pair<rev_iter_t, rev_iter_t> promoteReadMarker(User* u, QString eventId);
+        void setLastReadEvent(User* u, QString eventId);
+        rev_iter_pair_t promoteReadMarker(User* u, rev_iter_t newMarker);
 
     private:
-        QHash<QString, rev_iter_t> eventsIndex;
-
         QString calculateDisplayname() const;
         QString roomNameFromMemberNames(const QList<User*>& userlist) const;
 
         void insertMemberIntoMap(User* u);
         void removeMemberFromMap(QString username, User* u);
 
-        void insertEvent(Event* e, Timeline::iterator where);
+        void insertEvent(Event* e, Timeline::iterator where,
+                         TimelineItem::index_t index);
 };
 
 Room::Room(Connection* connection, QString id)
@@ -202,52 +196,45 @@ void Room::setJoinState(JoinState state)
     emit joinStateChanged(oldState, state);
 }
 
-void Room::setLastReadEvent(User* user, Event* event)
+void Room::Private::setLastReadEvent(User* u, QString eventId)
 {
-    d->lastReadEventIds.insert(user, event->id());
-    emit lastReadEventChanged(user);
-    if (user == d->connection->user())
-        emit readMarkerPromoted();
+    lastReadEventIds.insert(u, eventId);
+    emit q->lastReadEventChanged(u);
+    if (u == connection->user())
+        emit q->readMarkerMoved();
 }
 
-std::pair<Room::Private::rev_iter_t, Room::Private::rev_iter_t>
-Room::Private::promoteReadMarker(User* u, QString eventId)
+Room::Private::rev_iter_pair_t
+Room::Private::promoteReadMarker(User* u, Room::rev_iter_t newMarker)
 {
-    Q_ASSERT_X (!eventId.isEmpty(), __FUNCTION__,
-        "Attempt to promote a read marker to an event with no id");
+    Q_ASSERT_X(u, __FUNCTION__, "User* should not be nullptr");
+    Q_ASSERT(newMarker >= timeline.crbegin() && newMarker <= timeline.crend());
 
-    // Find the event by its id and position the marker at it. If the event
-    // is not found (most likely, because it's not fetched from the server),
-    // or if it's older than the current marker position, keep the marker intact.
-    const auto newMarker = eventsIndex.value(eventId, timeline.crend());
-    const auto prevMarker = readMarker(u);
+    const auto prevMarker = q->readMarker(u);
     if (prevMarker <= newMarker) // Remember, we deal with reverse iterators
         return { prevMarker, prevMarker };
 
-    using namespace std;
+    Q_ASSERT(newMarker < timeline.crend());
 
     // 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(),
-                 [=](Event* e) { return e->senderId() != u->id(); });
-    if (eagerMarker > timeline.begin())
-        q->setLastReadEvent(u, *(eagerMarker - 1));
+          [=](const TimelineItem& ti) { return ti->senderId() != u->id(); });
 
+    setLastReadEvent(u, (*(eagerMarker - 1))->id());
     if (u == connection->user() && unreadMessages)
     {
         auto stillUnreadMessagesCount =
             count_if(eagerMarker, timeline.cend(),
-                     [=](Event* e) { return isEventNotable(e); });
+                 [=](const TimelineItem& ti) { return isEventNotable(ti.event()); });
 
         if (stillUnreadMessagesCount == 0)
         {
             unreadMessages = false;
-            qDebug() << "Room" << q->displayName()
-                     << "has no more unread messages";
+            qDebug() << "Room" << displayname << "has no more unread messages";
             emit q->unreadMessagesChanged(q);
-        }
-        else
-            qDebug() << "Room" << q->displayName() << "still has"
+        } else
+            qDebug() << "Room" << displayname << "still has"
                      << stillUnreadMessagesCount << "unread message(s)";
     }
 
@@ -258,17 +245,21 @@ Room::Private::promoteReadMarker(User* u, QString eventId)
 
 void Room::markMessagesAsRead(QString uptoEventId)
 {
-    auto markers = d->promoteReadMarker(connection()->user(), uptoEventId);
+    User* localUser = connection()->user();
+    Private::rev_iter_pair_t markers =
+            d->promoteReadMarker(localUser, findInTimeline(uptoEventId));
+    if (markers.first != markers.second)
+        qDebug() << "Marked messages as read until" << *readMarker();
 
     // 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 (; markers.second < markers.first; ++markers.second)
     {
-        if ((*markers.second)->senderId() != connection()->userId())
+        if ((*markers.second)->senderId() != localUser->id())
         {
             connection()->callApi<PostReceiptJob>(this->id(),
-                                                     (*markers.second)->id());
+                                                  (*markers.second)->id());
             break;
         }
     }
@@ -279,6 +270,52 @@ bool Room::hasUnreadMessages()
     return d->unreadMessages;
 }
 
+Room::rev_iter_t Room::timelineEdge() const
+{
+    return d->timeline.crend();
+}
+
+TimelineItem::index_t Room::minTimelineIndex() const
+{
+    return d->timeline.empty() ? 0 : d->timeline.front().index();
+}
+
+TimelineItem::index_t Room::maxTimelineIndex() const
+{
+    return d->timeline.empty() ? 0 : d->timeline.back().index();
+}
+
+bool Room::isValidIndex(TimelineItem::index_t timelineIndex) const
+{
+    return !d->timeline.empty() &&
+           timelineIndex >= minTimelineIndex() &&
+           timelineIndex <= maxTimelineIndex();
+}
+
+Room::rev_iter_t Room::findInTimeline(TimelineItem::index_t index) const
+{
+    return timelineEdge() -
+            (isValidIndex(index) ? index - minTimelineIndex() + 1 : 0);
+}
+
+Room::rev_iter_t Room::findInTimeline(QString evtId) const
+{
+    if (!d->timeline.empty() && d->eventsIndex.contains(evtId))
+        return findInTimeline(d->eventsIndex.value(evtId));
+    return timelineEdge();
+}
+
+Room::rev_iter_t Room::readMarker(const User* user) const
+{
+    Q_ASSERT(user);
+    return findInTimeline(d->lastReadEventIds.value(user));
+}
+
+Room::rev_iter_t Room::readMarker() const
+{
+    return readMarker(connection()->user());
+}
+
 QString Room::readMarkerEventId() const
 {
     return d->lastReadEventIds.value(d->connection->user());
@@ -350,24 +387,30 @@ void Room::Private::removeMemberFromMap(QString username, User* u)
     updateDisplayname();
 }
 
-inline QByteArray makeErrorStr(Event* e, const char* msg)
+inline QByteArray makeErrorStr(const Event* e, const char* msg)
 {
     return QString("%1; event dump follows:\n%2")
             .arg(msg, e->originalJson()).toUtf8();
 }
 
-void Room::Private::insertEvent(Event* e, Room::Timeline::iterator where)
+void Room::Private::insertEvent(Event* e, Timeline::iterator where,
+                                TimelineItem::index_t index)
 {
     Q_ASSERT_X(e, __FUNCTION__, "Attempt to add nullptr to timeline");
     Q_ASSERT_X(!e->id().isEmpty(), __FUNCTION__,
-               makeErrorStr(e,
-                            "Event with empty id cannot be in the timeline"));
-    Q_ASSERT_X(!eventsIndex.contains(e->id()), __FUNCTION__,
-               makeErrorStr(e,
-                            "Event is already in the timeline (use dropDuplicateEvents())"));
-    Q_ASSERT(where >= timeline.begin() && where <= timeline.end());
-    eventsIndex.insert(e->id(), rev_iter_t(timeline.insert(where, e) + 1));
-    Q_ASSERT((*eventsIndex.value(e->id()))->id() == e->id());
+               makeErrorStr(e, "Event with empty id cannot be in the timeline"));
+    Q_ASSERT_X(where == timeline.end() || where == timeline.begin(), __FUNCTION__,
+               "Events can only be appended or prepended to the timeline");
+    if (eventsIndex.contains(e->id()))
+    {
+        qWarning() << "Event" << e->id() << "is already in the timeline.";
+        qWarning() << "Either dropDuplicateEvents() wasn't called or duplicate "
+                      "events within the same batch arrived from the server.";
+        return;
+    }
+    timeline.emplace(where, e, index);
+    eventsIndex.insert(e->id(), index);
+    Q_ASSERT(q->findInTimeline(e->id())->event() == e);
 }
 
 void Room::Private::addMember(User *u)
@@ -519,6 +562,17 @@ void Room::Private::getPreviousContent(int limit)
     }
 }
 
+void Room::Private::dropDuplicateEvents(Events& events) const
+{
+    // Collect all duplicate events at the end of the container
+    auto dupsBegin =
+            std::stable_partition(events.begin(), events.end(),
+                                  [&] (Event* e) { return !eventsIndex.contains(e->id()); });
+    // Dispose of those dups
+    std::for_each(dupsBegin, events.end(), [] (Event* e) { delete e; });
+    events.erase(dupsBegin, events.end());
+}
+
 Connection* Room::connection() const
 {
     return d->connection;
@@ -534,38 +588,29 @@ void Room::addNewMessageEvents(Events events)
     emit addedMessages();
 }
 
-bool Room::Private::isEventNotable(const Event* e) const
-{
-    return e->senderId() != connection->userId() &&
-            e->type() == EventType::RoomMessage;
-}
-
 void Room::doAddNewMessageEvents(const Events& events)
 {
     Q_ASSERT(!events.isEmpty());
-    Timeline::size_type newUnreadMessages = 0;
-
-    // The first event in the batch defines whose read marker we can
-    // automatically promote any further (if this author has read the whole
-    // timeline by then). Others will need explicit read receipts
-    // from the server (or, for the local user, markMessagesAsRead() invocation)
-    // to promote their read markers over the new message events.
-    User* firstWriter = connection()->user(events.front()->senderId());
-    bool canAutoPromote = d->readMarker(firstWriter) == d->timeline.crbegin();
 
+    Timeline::size_type newUnreadMessages = 0;
     for (auto e: events)
     {
         d->appendEvent(e);
         newUnreadMessages += d->isEventNotable(e);
     }
-    qDebug() << "Added" << events.size()
+    qDebug() << "Room" << displayName() << "received" << events.size()
              << "(with" << newUnreadMessages << "notable)"
-             << "new events to room" << displayName();
-    if (canAutoPromote)
-    {
-        qDebug() << "Auto-promoting" << firstWriter->id() << "over own events";
-        d->promoteReadMarker(firstWriter, events.front()->id());
-    }
+             << "new events; the last event is now" << d->timeline.back();
+
+    // The first event in the batch defines whose read marker 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 the new message events.
+    User* firstWriter = connection()->user(events.front()->senderId());
+    d->promoteReadMarker(firstWriter, findInTimeline(events.front()->id()));
+    qDebug() << "Auto-promoted read marker for" << firstWriter->id()
+             << "to" << *readMarker(firstWriter);
 
     if( !d->unreadMessages && newUnreadMessages > 0)
     {
@@ -591,8 +636,8 @@ void Room::doAddHistoricalMessageEvents(const Events& events)
     // Historical messages arrive in newest-to-oldest order
     for (auto e: events)
         d->prependEvent(e);
-    qDebug() << "Added" << events.size() << "historical events to room"
-             << displayName();
+    qDebug() << "Room" << displayName() << "received" << events.size()
+             << "past events; the oldest event is now" << d->timeline.front();
 }
 
 void Room::processStateEvents(const Events& events)
@@ -674,9 +719,25 @@ void Room::processEphemeralEvent(Event* event)
                 else
                     qd << receipts.size() << "users";
             }
-            for( const Receipt& r: receipts )
-                if (auto m = d->member(r.userId))
-                    d->promoteReadMarker(m, eventId);
+            if (d->eventsIndex.contains(eventId))
+            {
+                const auto newMarker = findInTimeline(eventId);
+                for( const Receipt& r: receipts )
+                    if (auto m = d->member(r.userId))
+                        d->promoteReadMarker(m, newMarker);
+            } else
+            {
+                qDebug() << "Event" << eventId
+                         << "not found; saving read markers 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: receipts )
+                    if (auto m = d->member(r.userId))
+                        if (readMarker(m) == timelineEdge())
+                            d->setLastReadEvent(m, eventId);
+            }
         }
     }
 }
@@ -782,3 +843,4 @@ bool MemberSorter::operator()(User *u1, User *u2) const
         n2.remove(0, 1);
     return n1 < n2;
 }
+
diff --git a/room.h b/room.h
index ff76b25a..582b89c4 100644
--- a/room.h
+++ b/room.h
@@ -18,6 +18,9 @@
 
 #pragma once
 
+#include <memory>
+#include <deque>
+
 #include <QtCore/QList>
 #include <QtCore/QStringList>
 #include <QtCore/QObject>
@@ -26,8 +29,6 @@
 #include "jobs/syncjob.h"
 #include "joinstate.h"
 
-#include <deque>
-
 namespace QMatrixClient
 {
     class Event;
@@ -35,18 +36,42 @@ namespace QMatrixClient
     class User;
     class MemberSorter;
 
+    class TimelineItem
+    {
+        public:
+            // For compatibility with Qt containers, even though we use
+            // a std:: container now
+            using index_t = int;
+
+            TimelineItem(Event* e, index_t number) : evt(e), idx(number) { }
+
+            Event* event() const { return evt.get(); }
+            Event* operator->() const { return event(); } //< Synonym for event()
+            index_t index() const { return idx; }
+
+        private:
+            std::unique_ptr<Event> evt;
+            index_t idx;
+    };
+    inline QDebug& operator<<(QDebug& d, const TimelineItem& ti)
+    {
+        QDebugStateSaver dss(d);
+        d.nospace() << "(" << ti.index() << "|" << ti->id() << ")";
+        return d;
+    }
+
     class Room: public QObject
     {
             Q_OBJECT
-            Q_PROPERTY(QString readMarkerEventId READ readMarkerEventId WRITE markMessagesAsRead NOTIFY readMarkerPromoted)
+            Q_PROPERTY(QString readMarkerEventId READ readMarkerEventId WRITE markMessagesAsRead NOTIFY readMarkerMoved)
         public:
-            using Timeline = Owning< std::deque<Event*> >;
+            using Timeline = std::deque<TimelineItem>;
+            using rev_iter_t = Timeline::const_reverse_iterator;
 
             Room(Connection* connection, QString id);
             virtual ~Room();
 
             Q_INVOKABLE QString id() const;
-            Q_INVOKABLE const Timeline& messageEvents() const;
             Q_INVOKABLE QString name() const;
             Q_INVOKABLE QStringList aliases() const;
             Q_INVOKABLE QString canonicalAlias() const;
@@ -72,6 +97,21 @@ namespace QMatrixClient
             Q_INVOKABLE void updateData(SyncRoomData& data );
             Q_INVOKABLE void setJoinState( JoinState state );
 
+            const Timeline& messageEvents() const;
+            /**
+             * A convenience method returning the read marker to the before-oldest
+             * message
+             */
+            rev_iter_t timelineEdge() const;
+            Q_INVOKABLE TimelineItem::index_t minTimelineIndex() const;
+            Q_INVOKABLE TimelineItem::index_t maxTimelineIndex() const;
+            Q_INVOKABLE bool isValidIndex(TimelineItem::index_t timelineIndex) const;
+
+            rev_iter_t findInTimeline(TimelineItem::index_t index) const;
+            rev_iter_t findInTimeline(QString evtId) const;
+
+            rev_iter_t readMarker(const User* user) const;
+            rev_iter_t readMarker() const;
             QString readMarkerEventId() const;
             /**
              * @brief Mark the event with uptoEventId as read
@@ -81,7 +121,7 @@ namespace QMatrixClient
              * for this message or, if it's from the local user, for
              * the nearest non-local message before. uptoEventId must be non-empty.
              */
-            Q_INVOKABLE void markMessagesAsRead(QString uptoEventId);
+            void markMessagesAsRead(QString uptoEventId);
 
             Q_INVOKABLE bool hasUnreadMessages();
 
@@ -119,7 +159,7 @@ namespace QMatrixClient
             void highlightCountChanged(Room* room);
             void notificationCountChanged(Room* room);
             void lastReadEventChanged(User* user);
-            void readMarkerPromoted();
+            void readMarkerMoved();
             void unreadMessagesChanged(Room* room);
 
         protected:
@@ -135,8 +175,6 @@ namespace QMatrixClient
 
             void addNewMessageEvents(Events events);
             void addHistoricalMessageEvents(Events events);
-
-            void setLastReadEvent(User* user, Event* event);
     };
 
     class MemberSorter