From 9f686d1428a025d6afeb98faab935fee170ef0ff Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Sat, 9 Jan 2021 22:27:15 +0100 Subject: [PATCH] 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(-) create mode 100644 fractal-gtk/src/model/message_list.rs 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, +} + +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 { + 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 for MessageList { + fn from_iter>(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, + pub messages: MessageList, pub membership: RoomMembership, pub direct: bool, pub prev_batch: Option, @@ -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 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 = 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) { - 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), });