[fractal] Refactor message handling into MessageList

[Alejandro Domínguez <aledomu src gnome org>]

commit 9f686d1428a025d6afeb98faab935fee170ef0ff
Author: Kai A. Hiller <V02460 gmail com>
Date:   Sat Jan 9 22:27:15 2021 +0100

    Refactor message handling into MessageList

 fractal-gtk/src/appop/message.rs        | 36 ++++++++++--------
 fractal-gtk/src/appop/sync.rs           |  7 +++-
 fractal-gtk/src/model/message_list.rs   | 65 +++++++++++++++++++++++++++++++++
 fractal-gtk/src/model/mod.rs            |  1 +
 fractal-gtk/src/model/room.rs           | 39 +++++++++++---------
 fractal-gtk/src/widgets/media_viewer.rs |  3 +-
 fractal-gtk/src/widgets/roomlist.rs     |  4 +-
 7 files changed, 117 insertions(+), 38 deletions(-)
---
diff --git a/fractal-gtk/src/appop/message.rs b/fractal-gtk/src/appop/message.rs
index 3ae44e32..ebc3fbe5 100644
--- a/fractal-gtk/src/appop/message.rs
+++ b/fractal-gtk/src/appop/message.rs
@@ -136,17 +136,24 @@ impl AppOp {
             let active_room_id = self.active_room.as_ref()?;
             let room = self.rooms.get_mut(active_room_id)?;
             let uid = login_data.uid.clone();
-            room.messages.iter_mut().for_each(|msg| {
-                if msg.receipt.contains_key(&uid) {
-                    msg.receipt.remove(&uid);
-                }
-            });
-            let last_message = room.messages.last_mut()?;
+
+            let dirty_msgs: Vec<_> = room
+                .messages
+                .iter()
+                .filter(|m| m.receipt.contains_key(&uid))
+                .cloned()
+                .collect();
+            for mut msg in dirty_msgs {
+                msg.receipt.remove(&uid);
+                room.take_new_message(msg);
+            }
+            let mut last_message = room.messages.iter().last()?.clone();
+            let event_id = last_message.id.clone()?;
+            let room_id = last_message.room.clone();
             last_message.receipt.insert(uid, 0);
+            room.take_new_message(last_message);
 
             let session_client = login_data.session_client;
-            let room_id = last_message.room.clone();
-            let event_id = last_message.id.clone()?;
             RUNTIME.spawn(async move {
                 match room::mark_as_read(session_client, room_id, event_id).await {
                     Ok((r, _)) => {
@@ -353,10 +360,10 @@ impl AppOp {
 
         for msg in newmsgs.iter() {
             if let Some(r) = self.rooms.get_mut(&msg.room) {
-                if !r.messages.contains(msg) {
-                    r.messages.push(msg.clone());
+                if !r.messages.contains(msg.id.as_ref().unwrap()) {
                     msgs.push(msg.clone());
                 }
+                r.take_new_message(msg.clone());
             }
         }
 
@@ -421,7 +428,7 @@ impl AppOp {
             }
 
             if let Some(r) = self.rooms.get_mut(&item.room) {
-                r.messages.insert(0, item.clone());
+                r.take_new_message(item.clone());
             }
         }
 
@@ -433,12 +440,11 @@ impl AppOp {
     pub fn remove_message(&mut self, room_id: RoomId, id: EventId) -> Option<()> {
         let message = self.get_message_by_id(&room_id, &id);
 
-        if let Some(msg) = message {
+        if let Some(mut msg) = message {
             self.remove_room_message(msg.clone());
             if let Some(ref mut room) = self.rooms.get_mut(&msg.room) {
-                if let Some(ref mut message) = room.messages.iter_mut().find(|e| e.id == msg.id) {
-                    message.redacted = true;
-                }
+                msg.redacted = true;
+                room.take_new_message(msg);
             }
         }
         None
diff --git a/fractal-gtk/src/appop/sync.rs b/fractal-gtk/src/appop/sync.rs
index 17f282aa..87ba0000 100644
--- a/fractal-gtk/src/appop/sync.rs
+++ b/fractal-gtk/src/appop/sync.rs
@@ -50,8 +50,11 @@ impl AppOp {
                         let clear_room_list = sync_ret.updates.is_none();
                         if let Some(updates) = sync_ret.updates {
                             let rooms = sync_ret.rooms;
-                            let msgs: Vec<_> =
-                                rooms.iter().flat_map(|r| &r.messages).cloned().collect();
+                            let msgs: Vec<_> = rooms
+                                .iter()
+                                .flat_map(|r| r.messages.iter())
+                                .cloned()
+                                .collect();
                             APPOP!(set_rooms, (rooms, clear_room_list));
                             APPOP!(show_room_messages, (msgs));
                             let typing_events_as_rooms = updates.typing_events_as_rooms;
diff --git a/fractal-gtk/src/model/message_list.rs b/fractal-gtk/src/model/message_list.rs
new file mode 100644
index 00000000..ac476449
--- /dev/null
+++ b/fractal-gtk/src/model/message_list.rs
@@ -0,0 +1,65 @@
+use crate::model::message::Message;
+use matrix_sdk::identifiers::EventId;
+use std::iter::FromIterator;
+use std::slice::Iter;
+
+/// Contains an ordered list of messages and their relations.
+#[derive(Debug, Default, Clone)]
+pub struct MessageList {
+    messages: Vec<Message>,
+}
+
+impl MessageList {
+    pub fn new() -> Self {
+        Default::default()
+    }
+
+    /// Returns the message with the given event ID.
+    pub fn get(&self, event_id: &EventId) -> Option<&Message> {
+        self.messages
+            .iter()
+            .find(|m| m.id.as_ref() == Some(event_id))
+    }
+
+    /// Whether the message with the given id is in the room.
+    pub fn contains(&self, msg_id: &EventId) -> bool {
+        self.get(msg_id).is_some()
+    }
+
+    /// Returns an iterator over all messages.
+    pub fn iter(&self) -> Iter<Message> {
+        self.messages.iter()
+    }
+
+    /// Inserts the message at the correct position replacing its older version.
+    pub fn add(&mut self, msg: Message) {
+        assert!(msg.id.is_some());
+
+        // Deduplication only happens for messages with the same date, so we have
+        // to manually go through the message list and remove possible duplicates.
+        //
+        // This is necessary due to the special case of just-sent messages.
+        // They don't contain the “official” timestamp from the server, but the
+        // time they were sent from Fractal. Due to this circumstance, we might
+        // end up with two messages having the same id, but different dates. We
+        // brute-force-fix this by searching all messages for duplicates.
+        self.messages.retain(|m| m.id != msg.id);
+
+        match self.messages.binary_search(&msg) {
+            Ok(idx) => self.messages[idx] = msg,
+            Err(idx) => self.messages.insert(idx, msg),
+        }
+        // TODO: Use is_sorted (https://github.com/rust-lang/rust/issues/53485)
+        // debug_assert!(self.messages.is_sorted());
+    }
+}
+
+impl FromIterator<Message> for MessageList {
+    fn from_iter<I: IntoIterator<Item = Message>>(messages: I) -> Self {
+        let mut message_list = Self::new();
+        for m in messages {
+            message_list.add(m);
+        }
+        message_list
+    }
+}
diff --git a/fractal-gtk/src/model/mod.rs b/fractal-gtk/src/model/mod.rs
index 6d9eedcb..1781d7e4 100644
--- a/fractal-gtk/src/model/mod.rs
+++ b/fractal-gtk/src/model/mod.rs
@@ -1,4 +1,5 @@
 pub mod fileinfo;
 pub mod member;
 pub mod message;
+pub mod message_list;
 pub mod room;
diff --git a/fractal-gtk/src/model/room.rs b/fractal-gtk/src/model/room.rs
index b852b5bd..7a508541 100644
--- a/fractal-gtk/src/model/room.rs
+++ b/fractal-gtk/src/model/room.rs
@@ -1,6 +1,7 @@
 use crate::model::member::Member;
 use crate::model::member::MemberList;
 use crate::model::message::Message;
+use crate::model::message_list::MessageList;
 use anyhow::anyhow;
 use chrono::DateTime;
 use chrono::Utc;
@@ -123,7 +124,7 @@ pub struct Room {
     pub members: MemberList,
     pub notifications: u64,
     pub highlight: u64,
-    pub messages: Vec<Message>,
+    pub messages: MessageList,
     pub membership: RoomMembership,
     pub direct: bool,
     pub prev_batch: Option<String>,
@@ -324,30 +325,27 @@ impl Room {
                 })
                 .collect();
 
-            r.messages
-                .iter_mut()
+            let changed_msgs: Vec<_> = r
+                .messages
+                .iter()
                 .filter_map(|msg| {
-                    let receipt = msg
-                        .id
-                        .as_ref()
-                        .and_then(|evid| receipts.get(evid))
-                        .cloned()?;
-                    Some((msg, receipt))
+                    let receipt = msg.id.as_ref().and_then(|evid| receipts.get(evid))?;
+                    Some((msg.clone(), receipt.clone()))
                 })
-                .for_each(|(msg, receipt)| msg.set_receipt(receipt));
+                .collect();
+            for (mut msg, receipt) in changed_msgs {
+                msg.set_receipt(receipt);
+                r.take_new_message(msg);
+            }
 
             if let Some(event_id) = room.ephemeral.events.iter().find_map(|event| match event {
                 AnySyncEphemeralRoomEvent::FullyRead(ev) => Some(ev.content.event_id.clone()),
                 _ => None,
             }) {
-                let event_id = Some(event_id);
-
-                r.messages
-                    .iter_mut()
-                    .filter(|msg| msg.id == event_id)
-                    .for_each(|msg| {
-                        msg.receipt.insert(user_id.clone(), 0);
-                    });
+                if let Some(mut msg) = r.messages.get(&event_id).cloned() {
+                    msg.receipt.insert(user_id.clone(), 0);
+                    r.take_new_message(msg);
+                }
             }
 
             r
@@ -452,6 +450,11 @@ impl Room {
             .chain(invited_rooms)
             .collect()
     }
+
+    /// Inserts the given message into the room.
+    pub fn take_new_message(&mut self, msg: Message) {
+        self.messages.add(msg);
+    }
 }
 
 impl TryFrom<PublicRoomsChunk> for Room {
diff --git a/fractal-gtk/src/widgets/media_viewer.rs b/fractal-gtk/src/widgets/media_viewer.rs
index 3f74ccaa..f697e93c 100644
--- a/fractal-gtk/src/widgets/media_viewer.rs
+++ b/fractal-gtk/src/widgets/media_viewer.rs
@@ -594,7 +594,8 @@ impl MediaViewer {
 
         let media_list: Vec<Message> = room
             .messages
-            .clone()
+            .iter()
+            .cloned()
             .into_iter()
             .filter(|msg| msg.mtype == "m.image" || msg.mtype == "m.video")
             .collect();
diff --git a/fractal-gtk/src/widgets/roomlist.rs b/fractal-gtk/src/widgets/roomlist.rs
index e1606703..42b5a4f0 100644
--- a/fractal-gtk/src/widgets/roomlist.rs
+++ b/fractal-gtk/src/widgets/roomlist.rs
@@ -23,7 +23,7 @@ pub struct RoomUpdated {
 
 impl RoomUpdated {
     pub fn new(room: Room) -> RoomUpdated {
-        let updated = match room.messages.last() {
+        let updated = match room.messages.iter().last() {
             Some(l) => l.date,
             None => Local.ymd(1970, 1, 1).and_hms(0, 0, 0),
         };
@@ -367,7 +367,7 @@ impl RoomListGroup {
     }
 
     pub fn add_rooms(&mut self, mut array: Vec<Room>) {
-        array.sort_by_key(|ref x| match x.messages.last() {
+        array.sort_by_key(|ref x| match x.messages.iter().last() {
             Some(l) => l.date,
             None => Local.ymd(1970, 1, 1).and_hms(0, 0, 0),
         });