From 999399a70f4f741390fa8ed113ea9ef06ad14b46 Mon Sep 17 00:00:00 2001 From: Benjamin Grosse Date: Mon, 18 Dec 2023 20:55:04 -0800 Subject: [PATCH] Fix not showing display names in already synced rooms (#171) Fixes #149 --- src/base.rs | 55 +++++++++++++-- src/windows/mod.rs | 3 + src/windows/room/scrollback.rs | 87 ++++++++++++++--------- src/worker.rs | 124 ++++++++++++++++++++++----------- 4 files changed, 188 insertions(+), 81 deletions(-) diff --git a/src/base.rs b/src/base.rs index ca45181..cfc1eac 100644 --- a/src/base.rs +++ b/src/base.rs @@ -2,6 +2,7 @@ //! //! The types defined here get used throughout iamb. use std::borrow::Cow; +use std::collections::hash_map::IntoIter; use std::collections::{HashMap, HashSet}; use std::convert::TryFrom; use std::fmt::{self, Display}; @@ -1042,6 +1043,38 @@ pub struct SyncInfo { pub dms: Vec)>>, } +bitflags::bitflags! { + /// Load-needs + #[derive(Debug, Default, PartialEq)] + pub struct Need: u32 { + const EMPTY = 0b00000000; + const MESSAGES = 0b00000001; + const MEMBERS = 0b00000010; + } +} + +/// Things that need loading for different rooms. +#[derive(Default)] +pub struct RoomNeeds { + needs: HashMap, +} + +impl RoomNeeds { + /// Mark a room for needing something to be loaded. + pub fn insert(&mut self, room_id: OwnedRoomId, need: Need) { + self.needs.entry(room_id).or_default().insert(need); + } +} + +impl IntoIterator for RoomNeeds { + type Item = (OwnedRoomId, Need); + type IntoIter = IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.needs.into_iter() + } +} + /// The main application state. pub struct ChatStore { /// `:`-commands @@ -1066,7 +1099,7 @@ pub struct ChatStore { pub settings: ApplicationSettings, /// Set of rooms that need more messages loaded in their scrollback. - pub need_load: HashSet, + pub need_load: RoomNeeds, /// [CompletionMap] of Emoji shortcodes. pub emojis: CompletionMap, @@ -1113,11 +1146,6 @@ impl ChatStore { .unwrap_or_else(|| "Untitled Matrix Room".to_string()) } - /// Mark a room for loading more scrollback. - pub fn mark_for_load(&mut self, room_id: OwnedRoomId) { - self.need_load.insert(room_id); - } - /// Get the [RoomInfo] for a given room identifier. pub fn get_room_info(&mut self, room_id: OwnedRoomId) -> &mut RoomInfo { self.rooms.get_or_default(room_id) @@ -1659,6 +1687,21 @@ pub mod tests { ); } + #[test] + fn test_need_load() { + let room_id = TEST_ROOM1_ID.clone(); + + let mut need_load = RoomNeeds::default(); + + need_load.insert(room_id.clone(), Need::MESSAGES); + need_load.insert(room_id.clone(), Need::MEMBERS); + + assert_eq!(need_load.into_iter().collect::>(), vec![( + room_id, + Need::MESSAGES | Need::MEMBERS, + )],); + } + #[tokio::test] async fn test_complete_msgbar() { let store = mock_store().await; diff --git a/src/windows/mod.rs b/src/windows/mod.rs index eb77f62..73c7579 100644 --- a/src/windows/mod.rs +++ b/src/windows/mod.rs @@ -77,6 +77,7 @@ use crate::base::{ IambInfo, IambResult, MessageAction, + Need, ProgramAction, ProgramContext, ProgramStore, @@ -659,6 +660,7 @@ impl Window for IambWindow { let (room, name, tags) = store.application.worker.get_room(room_id)?; let room = RoomState::new(room, name, tags, store); + store.application.need_load.insert(room.id().to_owned(), Need::MEMBERS); return Ok(room.into()); }, IambId::DirectList => { @@ -710,6 +712,7 @@ impl Window for IambWindow { let (room, name, tags) = store.application.worker.get_room(room_id)?; let room = RoomState::new(room, name, tags, store); + store.application.need_load.insert(room.id().to_owned(), Need::MEMBERS); Ok(room.into()) } } diff --git a/src/windows/room/scrollback.rs b/src/windows/room/scrollback.rs index da0573a..3ecf369 100644 --- a/src/windows/room/scrollback.rs +++ b/src/windows/room/scrollback.rs @@ -1,5 +1,4 @@ //! Message scrollback -use std::collections::HashSet; use ratatui_image::FixedImage; use regex::Regex; @@ -73,7 +72,16 @@ use modalkit::editing::{ }; use crate::{ - base::{IambBufferId, IambInfo, IambResult, ProgramContext, ProgramStore, RoomFocus, RoomInfo}, + base::{ + IambBufferId, + IambInfo, + IambResult, + Need, + ProgramContext, + ProgramStore, + RoomFocus, + RoomInfo, + }, config::ApplicationSettings, message::{Message, MessageCursor, MessageKey, Messages}, }; @@ -468,8 +476,7 @@ impl ScrollbackState { needle: &Regex, mut count: usize, info: &RoomInfo, - need_load: &mut HashSet, - ) -> Option { + ) -> (Option, bool) { let mut mc = None; for (key, msg) in info.messages.range(..&end).rev() { @@ -483,11 +490,7 @@ impl ScrollbackState { } } - if count > 0 { - need_load.insert(self.room_id.clone()); - } - - return mc; + return (mc, count > 0); } fn find_message( @@ -497,11 +500,10 @@ impl ScrollbackState { needle: &Regex, count: usize, info: &RoomInfo, - need_load: &mut HashSet, - ) -> Option { + ) -> (Option, bool) { match dir { - MoveDir1D::Next => self.find_message_next(key, needle, count, info), - MoveDir1D::Previous => self.find_message_prev(key, needle, count, info, need_load), + MoveDir1D::Next => (self.find_message_next(key, needle, count, info), false), + MoveDir1D::Previous => self.find_message_prev(key, needle, count, info), } } } @@ -628,14 +630,14 @@ impl EditorActions for ScrollbackState { }, }; - self.find_message( - key, - dir, - &needle, - count, - info, - &mut store.application.need_load, - ) + let (mc, needs_load) = self.find_message(key, dir, &needle, count, info); + if needs_load { + store + .application + .need_load + .insert(self.room_id.clone(), Need::MESSAGES); + } + mc }, EditTarget::Search(SearchType::Word(_, _), _, _) => { let msg = "Cannot perform word search in a list"; @@ -714,15 +716,15 @@ impl EditorActions for ScrollbackState { }, }; - self.find_message( - key, - dir, - &needle, - count, - info, - &mut store.application.need_load, - ) - .map(|c| self._range_to(c)) + let (mc, needs_load) = self.find_message(key, dir, &needle, count, info); + if needs_load { + store + .application + .need_load + .insert(self.room_id.to_owned(), Need::MESSAGES); + } + + mc.map(|c| self._range_to(c)) }, EditTarget::Search(SearchType::Word(_, _), _, _) => { let msg = "Cannot perform word search in a list"; @@ -1288,7 +1290,10 @@ impl<'a> StatefulWidget for Scrollback<'a> { let cursor_key = if let Some(k) = cursor.to_key(info) { k } else { - self.store.application.mark_for_load(state.room_id.clone()); + self.store + .application + .need_load + .insert(state.room_id.to_owned(), Need::MESSAGES); return; }; @@ -1389,7 +1394,10 @@ impl<'a> StatefulWidget for Scrollback<'a> { let first_key = info.messages.first_key_value().map(|(k, _)| k.clone()); if first_key == state.viewctx.corner.timestamp { // If the top of the screen is the older message, load more. - self.store.application.mark_for_load(state.room_id.clone()); + self.store + .application + .need_load + .insert(state.room_id.to_owned(), Need::MESSAGES); } } } @@ -1427,12 +1435,23 @@ mod tests { // Search backwards to MSG2. scrollback.search(prev.clone(), 1.into(), &ctx, &mut store).unwrap(); assert_eq!(scrollback.cursor, MSG2_KEY.clone().into()); - assert_eq!(store.application.need_load.contains(&room_id), false); + assert_eq!( + std::mem::take(&mut store.application.need_load) + .into_iter() + .collect::>() + .is_empty(), + true, + ); // Can't go any further; need_load now contains the room ID. scrollback.search(prev.clone(), 1.into(), &ctx, &mut store).unwrap(); assert_eq!(scrollback.cursor, MSG2_KEY.clone().into()); - assert_eq!(store.application.need_load.contains(&room_id), true); + assert_eq!( + std::mem::take(&mut store.application.need_load) + .into_iter() + .collect::>(), + vec![(room_id.clone(), Need::MESSAGES)] + ); // Search forward twice to MSG1. scrollback.search(next.clone(), 2.into(), &ctx, &mut store).unwrap(); diff --git a/src/worker.rs b/src/worker.rs index f485028..6ad2d61 100644 --- a/src/worker.rs +++ b/src/worker.rs @@ -78,6 +78,7 @@ use matrix_sdk::{ use modalkit::editing::action::{EditInfo, InfoMessage, UIError}; +use crate::base::Need; use crate::{ base::{ AsyncProgramStore, @@ -187,34 +188,64 @@ async fn update_event_receipts(info: &mut RoomInfo, room: &MatrixRoom, event_id: } } -async fn load_plan(store: &AsyncProgramStore) -> HashMap> { +#[derive(Debug)] +enum Plan { + Messages(OwnedRoomId, Option), + Members(OwnedRoomId), +} + +async fn load_plans(store: &AsyncProgramStore) -> Vec { let mut locked = store.lock().await; let ChatStore { need_load, rooms, .. } = &mut locked.application; - let mut plan = HashMap::new(); + let mut plan = vec![]; - for room_id in std::mem::take(need_load).into_iter() { - let info = rooms.get_or_default(room_id.clone()); + for (room_id, mut need) in std::mem::take(need_load).into_iter() { + if need.contains(Need::MESSAGES) { + let info = rooms.get_or_default(room_id.clone()); - if info.recently_fetched() || info.fetching { - need_load.insert(room_id); - continue; - } else { - info.fetch_last = Instant::now().into(); - info.fetching = true; + if !info.recently_fetched() && !info.fetching { + info.fetch_last = Instant::now().into(); + info.fetching = true; + + let fetch_id = match &info.fetch_id { + RoomFetchStatus::Done => continue, + RoomFetchStatus::HaveMore(fetch_id) => Some(fetch_id.clone()), + RoomFetchStatus::NotStarted => None, + }; + + plan.push(Plan::Messages(room_id.to_owned(), fetch_id)); + need.remove(Need::MESSAGES); + } + } + if need.contains(Need::MEMBERS) { + plan.push(Plan::Members(room_id.to_owned())); + need.remove(Need::MEMBERS); + } + if !need.is_empty() { + need_load.insert(room_id, need); } - - let fetch_id = match &info.fetch_id { - RoomFetchStatus::Done => continue, - RoomFetchStatus::HaveMore(fetch_id) => Some(fetch_id.clone()), - RoomFetchStatus::NotStarted => None, - }; - - plan.insert(room_id, fetch_id); } return plan; } +async fn run_plan(client: &Client, store: &AsyncProgramStore, plan: Plan) { + match plan { + Plan::Messages(room_id, fetch_id) => { + let limit = MIN_MSG_LOAD; + let client = client.clone(); + let store = store.clone(); + + let res = load_older_one(&client, &room_id, fetch_id, limit).await; + load_insert(room_id, res, store).await; + }, + Plan::Members(room_id) => { + let res = members_load(client, &room_id).await; + members_insert(room_id, res, store).await + }, + } +} + async fn load_older_one( client: &Client, room_id: &RoomId, @@ -246,15 +277,7 @@ async fn load_older_one( async fn load_insert(room_id: OwnedRoomId, res: MessageFetchResult, store: AsyncProgramStore) { let mut locked = store.lock().await; - let ChatStore { - need_load, - presences, - rooms, - worker, - picker, - settings, - .. - } = &mut locked.application; + let ChatStore { presences, rooms, worker, picker, settings, .. } = &mut locked.application; let info = rooms.get_or_default(room_id.clone()); info.fetching = false; let client = &worker.client; @@ -296,33 +319,52 @@ async fn load_insert(room_id: OwnedRoomId, res: MessageFetchResult, store: Async warn!(room_id = room_id.as_str(), err = e.to_string(), "Failed to load older messages"); // Wait and try again. - need_load.insert(room_id); + locked.application.need_load.insert(room_id, Need::MESSAGES); }, } } async fn load_older(client: &Client, store: &AsyncProgramStore) -> usize { - let limit = MIN_MSG_LOAD; - - // Fetch each room separately, so they don't block each other. - load_plan(store) + // Plans are run in parallel. Any room *may* have several plans. + load_plans(store) .await .into_iter() - .map(|(room_id, fetch_id)| { - let store = store.clone(); - - async move { - let res = load_older_one(client, room_id.as_ref(), fetch_id, limit).await; - load_insert(room_id, res, store).await; - } - }) + .map(|plan| run_plan(client, store, plan)) .collect::>() .count() .await } +async fn members_load(client: &Client, room_id: &RoomId) -> IambResult> { + if let Some(room) = client.get_room(room_id) { + Ok(room.members_no_sync().await.map_err(IambError::from)?) + } else { + Err(IambError::UnknownRoom(room_id.to_owned()).into()) + } +} + +async fn members_insert( + room_id: OwnedRoomId, + res: IambResult>, + store: &AsyncProgramStore, +) { + if let Ok(members) = res { + let mut locked = store.lock().await; + let ChatStore { rooms, .. } = &mut locked.application; + let info = rooms.get_or_default(room_id.clone()); + + for member in members { + let user_id = member.user_id(); + let display_name = + member.display_name().map_or(user_id.to_string(), |str| str.to_string()); + info.display_names.insert(user_id.to_owned(), display_name); + } + } + // else ??? +} + async fn load_older_forever(client: &Client, store: &AsyncProgramStore) { - // Load older messages every 2 seconds. + // Load any pending older messages or members every 2 seconds. let mut interval = tokio::time::interval(Duration::from_secs(2)); loop {