From c1355858b64b9edc5f2597ddbf62ad5729f86339 Mon Sep 17 00:00:00 2001 From: Andrew Baldwin Date: Wed, 31 Jul 2024 18:40:24 -0700 Subject: [PATCH 01/29] Initial [`read-marker`](https://ircv3.net/specs/extensions/read-marker) implementation. --- CHANGELOG.md | 2 +- README.md | 1 + book/src/README.md | 1 + data/src/client.rs | 48 +++++++ data/src/history.rs | 273 ++++++++++++++++++++++++++++++------ data/src/history/manager.rs | 119 ++++++++++++++-- data/src/message.rs | 5 +- irc/proto/src/command.rs | 5 + src/main.rs | 98 ++++++++++++- src/screen/dashboard.rs | 53 +++++++ 10 files changed, 547 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 662b448f4..12b94fb67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ Added: - Ability to include or exclude channels for server messages (join, part, etc.). See [configuration](https://halloy.squidowl.org/configuration/buffer/server_messages/index.html). - Ability to color nicknames within channel messages. See [configuration](https://halloy.squidowl.org/configuration/buffer/channel/message.html#nickname_color) - Ability to define a shell command for loading a server password. See [configuration](https://halloy.squidowl.org/configuration/servers/index.html#password_command) -- Enable support for IRCv3 `msgid` +- Enable support for IRCv3 `msgid` and `read-marker` Fixed: diff --git a/README.md b/README.md index 04ea3d52e..1c2c43d4f 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,7 @@ Halloy is also available from [Flathub](https://flathub.org/apps/org.squidowl.ha * [Monitor](https://ircv3.net/specs/extensions/monitor) * [msgid](https://ircv3.net/specs/extensions/message-ids) * [multi-prefix](https://ircv3.net/specs/extensions/multi-prefix) + * [read-marker](https://ircv3.net/specs/extensions/read-marker) * [sasl-3.1](https://ircv3.net/specs/extensions/sasl-3.1) * [server-time](https://ircv3.net/specs/extensions/server-time) * [userhost-in-names](https://ircv3.net/specs/extensions/userhost-in-names) diff --git a/book/src/README.md b/book/src/README.md index 69f4387fc..ce91ca6e2 100644 --- a/book/src/README.md +++ b/book/src/README.md @@ -20,6 +20,7 @@ * [Monitor](https://ircv3.net/specs/extensions/monitor) * [msgid](https://ircv3.net/specs/extensions/message-ids) * [multi-prefix](https://ircv3.net/specs/extensions/multi-prefix) + * [read-marker](https://ircv3.net/specs/extensions/read-marker) * [sasl-3.1](https://ircv3.net/specs/extensions/sasl-3.1) * [server-time](https://ircv3.net/specs/extensions/server-time) * [userhost-in-names](https://ircv3.net/specs/extensions/userhost-in-names) diff --git a/data/src/client.rs b/data/src/client.rs index 8ba5e3ee7..d5081e10c 100644 --- a/data/src/client.rs +++ b/data/src/client.rs @@ -6,6 +6,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt; use std::time::{Duration, Instant}; +use crate::history::read_marker_to_string; use crate::message::server_time; use crate::time::Posix; use crate::user::{Nick, NickRef}; @@ -79,6 +80,8 @@ pub enum Event { Broadcast(Broadcast), Notification(message::Encoded, Nick, Notification), FileTransferRequest(file_transfer::ReceiveRequest), + LoadReadMarker(String), + UpdateReadMarker(String, Option>), } pub struct Client { @@ -99,6 +102,7 @@ pub struct Client { supports_away_notify: bool, supports_account_notify: bool, supports_extended_join: bool, + supports_read_marker: bool, highlight_blackout: HighlightBlackout, registration_required_channels: Vec, isupport: HashMap, @@ -151,6 +155,7 @@ impl Client { supports_away_notify: false, supports_account_notify: false, supports_extended_join: false, + supports_read_marker: false, highlight_blackout: HighlightBlackout::Blackout(Instant::now()), registration_required_channels: vec![], isupport: HashMap::new(), @@ -367,6 +372,9 @@ impl Client { if contains("multi-prefix") { requested.push("multi-prefix"); } + if contains("draft/read-marker") { + requested.push("draft/read-marker"); + } if !requested.is_empty() { // Request @@ -399,6 +407,9 @@ impl Client { if caps.contains(&"extended-join") { self.supports_extended_join = true; } + if caps.contains(&"draft/read-marker") { + self.supports_read_marker = true; + } let supports_sasl = caps.iter().any(|cap| cap.contains("sasl")); @@ -479,6 +490,9 @@ impl Client { if newly_contains("multi-prefix") { requested.push("multi-prefix"); } + if newly_contains("draft/read-marker") { + requested.push("draft/read-marker"); + } if !requested.is_empty() { // Request @@ -506,6 +520,9 @@ impl Client { if del_caps.contains(&"extended-join") { self.supports_extended_join = false; } + if del_caps.contains(&"draft/read-marker") { + self.supports_read_marker = false; + } self.listed_caps .retain(|cap| !del_caps.iter().any(|del_cap| del_cap == cap)); @@ -1225,12 +1242,32 @@ impl Client { Command::Numeric(RPL_ENDOFMONLIST, _) => { return None; } + Command::MARKREAD(target, Some(timestamp)) => { + let timestamp = timestamp.strip_prefix("timestamp=").and_then(|timestamp| { + DateTime::parse_from_rfc3339(timestamp) + .ok() + .map(|dt| dt.with_timezone(&Utc)) + }); + return Some(vec![Event::UpdateReadMarker(target.clone(), timestamp)]); + } _ => {} } Some(vec![Event::Single(message, self.nickname().to_owned())]) } + pub fn send_markread(&mut self, target: &str, read_marker: Option>) { + if self.supports_read_marker { + if let Some(read_marker) = read_marker { + let _ = self.handle.try_send(command!( + "MARKREAD", + target.to_string(), + format!("timestamp={}", read_marker_to_string(&Some(read_marker))), + )); + } + } + } + fn sync(&mut self) { self.channels = self.chanmap.keys().cloned().collect(); self.users = self @@ -1430,6 +1467,17 @@ impl Map { } } + pub fn send_markread( + &mut self, + server: &Server, + target: &str, + read_marker: Option>, + ) { + if let Some(client) = self.client_mut(server) { + client.send_markread(target, read_marker); + } + } + pub fn join(&mut self, server: &Server, channels: &[String]) { if let Some(client) = self.client_mut(server) { client.join(channels); diff --git a/data/src/history.rs b/data/src/history.rs index 6c1dc2c83..5d9c840e0 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -1,3 +1,5 @@ +use chrono::{format::SecondsFormat, DateTime, Utc}; +use irc::proto; use std::path::PathBuf; use std::time::Duration; use std::{fmt, io}; @@ -8,9 +10,8 @@ use tokio::fs; use tokio::time::Instant; pub use self::manager::{Manager, Resource}; -use crate::time::Posix; use crate::user::Nick; -use crate::{compression, environment, message, server, Message}; +use crate::{compression, environment, message, server, Message, Server}; pub mod manager; @@ -49,28 +50,74 @@ impl From for Kind { } } -pub async fn load(server: &server::Server, kind: &Kind) -> Result, Error> { - let path = path(server, kind).await?; +impl From for Kind { + fn from(target: String) -> Self { + Kind::from(target.as_ref()) + } +} + +impl From<&str> for Kind { + fn from(target: &str) -> Self { + if proto::is_channel(target) { + Kind::Channel(target.to_string()) + } else { + Kind::Query(target.to_string().into()) + } + } +} - Ok(read_all(&path).await.unwrap_or_default()) +pub async fn load_messages(server: &server::Server, kind: &Kind) -> Result, Error> { + let path = messages_path(server, kind).await?; + + Ok(read_messages(&path).await.unwrap_or_default()) +} + +pub async fn load_read_marker( + server: server::Server, + kind: Kind, +) -> Result>, Error> { + let path = read_marker_path(&server, &kind).await?; + + if let Ok(bytes) = fs::read(path).await { + Ok(serde_json::from_slice(&bytes).unwrap_or_default()) + } else { + Ok(None) + } } pub async fn overwrite( server: &server::Server, kind: &Kind, messages: &[Message], + read_marker: &Option>, ) -> Result<(), Error> { if messages.is_empty() { - return Ok(()); + return overwrite_read_marker(server, kind, read_marker).await; } let latest = &messages[messages.len().saturating_sub(MAX_MESSAGES)..]; - let path = path(server, kind).await?; + let path = messages_path(server, kind).await?; let compressed = compression::compress(&latest)?; fs::write(path, &compressed).await?; + overwrite_read_marker(server, kind, read_marker).await?; + + Ok(()) +} + +pub async fn overwrite_read_marker( + server: &server::Server, + kind: &Kind, + read_marker: &Option>, +) -> Result<(), Error> { + let bytes = serde_json::to_vec(&read_marker)?; + + let path = read_marker_path(server, kind).await?; + + fs::write(path, &bytes).await?; + Ok(()) } @@ -78,40 +125,61 @@ pub async fn append( server: &server::Server, kind: &Kind, messages: Vec, + read_marker: &Option>, ) -> Result<(), Error> { if messages.is_empty() { - return Ok(()); + return overwrite_read_marker(server, kind, read_marker).await; } - let mut all_messages = load(server, kind).await?; + let mut all_messages = load_messages(server, kind).await?; all_messages.extend(messages); - overwrite(server, kind, &all_messages).await + overwrite(server, kind, &all_messages, read_marker).await } -async fn read_all(path: &PathBuf) -> Result, Error> { +async fn read_messages(path: &PathBuf) -> Result, Error> { let bytes = fs::read(path).await?; Ok(compression::decompress(&bytes)?) } -async fn path(server: &server::Server, kind: &Kind) -> Result { +async fn dir_path() -> Result { let data_dir = environment::data_dir(); - // TODO: Is this stable enough? What if user's nickname changes + let history_dir = data_dir.join("history"); + + if !history_dir.exists() { + fs::create_dir_all(&history_dir).await?; + } + + Ok(history_dir) +} + +async fn messages_path(server: &server::Server, kind: &Kind) -> Result { + let dir = dir_path().await?; + let name = match kind { Kind::Server => format!("{server}"), Kind::Channel(channel) => format!("{server}channel{channel}"), Kind::Query(nick) => format!("{server}nickname{}", nick), }; + let hashed_name = seahash::hash(name.as_bytes()); - let parent = data_dir.join("history"); + Ok(dir.join(format!("{hashed_name}.json.gz"))) +} - if !parent.exists() { - fs::create_dir_all(&parent).await?; - } +async fn read_marker_path(server: &server::Server, kind: &Kind) -> Result { + let dir = dir_path().await?; - Ok(parent.join(format!("{hashed_name}.json.gz"))) + let name = match kind { + Kind::Server => format!("{server}_read_marker"), + Kind::Channel(channel) => format!("{server}channel{channel}_read_marker"), + Kind::Query(nick) => format!("{server}nickname{}_read_marker", nick), + }; + + let hashed_name = seahash::hash(name.as_bytes()); + + Ok(dir.join(format!("{hashed_name}.json"))) } #[derive(Debug)] @@ -122,52 +190,59 @@ pub enum History { messages: Vec, last_received_at: Option, unread_message_count: usize, - opened_at: Posix, + read_marker: Option>, }, Full { server: server::Server, kind: Kind, messages: Vec, last_received_at: Option, - opened_at: Posix, + read_marker: Option>, }, } impl History { - fn partial(server: server::Server, kind: Kind, opened_at: Posix) -> Self { + fn partial(server: server::Server, kind: Kind, read_marker: Option>) -> Self { Self::Partial { server, kind, messages: vec![], last_received_at: None, unread_message_count: 0, - opened_at, + read_marker, + } + } + + pub fn inc_unread_count(&mut self, increment: usize) { + if let History::Partial { + unread_message_count, + .. + } = self + { + *unread_message_count += increment; } } fn add_message(&mut self, message: Message) { - match self { + if match self { History::Partial { messages, last_received_at, - unread_message_count, + read_marker, .. - } => { - if message.triggers_unread() { - *unread_message_count += 1; - } - - messages.push(message); - *last_received_at = Some(Instant::now()); } - History::Full { + | History::Full { messages, last_received_at, + read_marker, .. } => { - messages.push(message); *last_received_at = Some(Instant::now()); + + insert_message(messages, message, read_marker) } + } { + self.inc_unread_count(1); } } @@ -177,6 +252,7 @@ impl History { server, kind, messages, + read_marker, last_received_at, .. } => { @@ -186,10 +262,14 @@ impl History { if since >= FLUSH_AFTER_LAST_RECEIVED && !messages.is_empty() { let server = server.clone(); let kind = kind.clone(); + let read_marker = *read_marker; let messages = std::mem::take(messages); *last_received_at = None; - return Some(async move { append(&server, &kind, messages).await }.boxed()); + return Some( + async move { append(&server, &kind, messages, &read_marker).await } + .boxed(), + ); } } @@ -199,6 +279,7 @@ impl History { server, kind, messages, + read_marker, last_received_at, .. } => { @@ -208,6 +289,7 @@ impl History { if since >= FLUSH_AFTER_LAST_RECEIVED && !messages.is_empty() { let server = server.clone(); let kind = kind.clone(); + let read_marker = *read_marker; *last_received_at = None; if messages.len() > MAX_MESSAGES { @@ -217,7 +299,8 @@ impl History { let messages = messages.clone(); return Some( - async move { overwrite(&server, &kind, &messages).await }.boxed(), + async move { overwrite(&server, &kind, &messages, &read_marker).await } + .boxed(), ); } } @@ -234,15 +317,23 @@ impl History { server, kind, messages, + read_marker, .. } => { let server = server.clone(); let kind = kind.clone(); + let read_marker = messages + .iter() + .rev() + .find(|message| { + !matches!(message.target.source(), message::Source::Internal(_)) + }) + .map_or(*read_marker, |message| Some(message.server_time)); let messages = std::mem::take(messages); - *self = Self::partial(server.clone(), kind.clone(), Posix::now()); + *self = Self::partial(server.clone(), kind.clone(), read_marker); - Some(async move { overwrite(&server, &kind, &messages).await }) + Some(async move { overwrite(&server, &kind, &messages, &read_marker).await }) } } } @@ -253,16 +344,118 @@ impl History { server, kind, messages, + read_marker, .. - } => append(&server, &kind, messages).await, + } => append(&server, &kind, messages, &read_marker).await, History::Full { server, kind, messages, + read_marker, .. - } => overwrite(&server, &kind, &messages).await, + } => overwrite(&server, &kind, &messages, &read_marker).await, } } + + pub fn get_read_marker(&self) -> Option> { + match self { + History::Partial { read_marker, .. } => *read_marker, + History::Full { read_marker, .. } => *read_marker, + } + } + + pub fn update_read_marker(&mut self, read_marker: Option>) -> bool { + let history_read_marker = match self { + History::Partial { + read_marker: history_read_marker, + .. + } => history_read_marker, + History::Full { + read_marker: history_read_marker, + .. + } => history_read_marker, + }; + + if let Some(read_marker) = read_marker { + if let Some(history_read_marker) = history_read_marker { + if read_marker <= *history_read_marker { + return false; + } + } + + *history_read_marker = Some(read_marker); + } else { + return false; + } + + if let History::Partial { + messages, + unread_message_count, + read_marker: history_read_marker, + .. + } = self + { + *unread_message_count = 0; + + for message in messages { + if message.triggers_unread(history_read_marker) { + *unread_message_count += 1; + } + } + } + + true + } +} + +pub fn insert_message( + messages: &mut Vec, + message: Message, + read_marker: &Option>, +) -> bool { + let message_triggers_unread = message.triggers_unread(read_marker); + + messages.push(message); + + message_triggers_unread +} + +pub fn after_read_marker(message: &Message, read_marker: &Option>) -> bool { + read_marker.is_none() + || read_marker.is_some_and(|read_marker| message.server_time > read_marker) +} + +pub fn read_marker_to_string(read_marker: &Option>) -> String { + if let Some(read_marker) = read_marker { + read_marker.to_rfc3339_opts(SecondsFormat::Millis, true) + } else { + "*".to_string() + } +} + +pub async fn num_stored_unread_messages( + server: Server, + kind: Kind, + read_marker: Option>, +) -> usize { + let messages = load_messages(&server, &kind).await; + + if let Ok(messages) = messages { + messages + .into_iter() + .rev() + .map_while(|message| { + if after_read_marker(&message, &read_marker) { + Some(message) + } else { + None + } + }) + .filter(|message| message.triggers_unread(&read_marker)) + .count() + } else { + 0 + } } #[derive(Debug)] @@ -280,4 +473,6 @@ pub enum Error { Compression(#[from] compression::Error), #[error(transparent)] Io(#[from] io::Error), + #[error(transparent)] + SerdeJson(#[from] serde_json::Error), } diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 352d7027a..0ccf9f4d8 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -8,7 +8,6 @@ use tokio::time::Instant; use crate::history::{self, History}; use crate::message::{self, Limit}; -use crate::time::Posix; use crate::user::Nick; use crate::{config, input}; use crate::{server, Buffer, Config, Input, Server, User}; @@ -30,6 +29,10 @@ pub enum Message { Flushed(server::Server, history::Kind, Result<(), history::Error>), } +pub enum Event { + LoadReadMarker(server::Server, history::Kind), +} + #[derive(Debug, Default)] pub struct Manager { resources: HashSet, @@ -43,7 +46,7 @@ impl Manager { let added = added.into_iter().map(|resource| { async move { - history::load(&resource.server.clone(), &resource.kind.clone()) + history::load_messages(&resource.server.clone(), &resource.kind.clone()) .map(move |result| Message::Loaded(resource.server, resource.kind, result)) .await } @@ -190,6 +193,52 @@ impl Manager { ); } + pub fn get_read_marker(&self, server: &Server, kind: &history::Kind) -> Option> { + self.data.get_read_marker(server, kind) + } + + pub fn update_read_marker( + &mut self, + server: &Server, + kind: &history::Kind, + read_marker: Option>, + ) -> bool { + self.data.update_read_marker(server, kind, read_marker) + } + + pub fn inc_unread_count(&mut self, server: &Server, kind: &history::Kind, increment: usize) { + if let Some(ref mut history) = self + .data + .map + .get_mut(server) + .and_then(|map| map.get_mut(kind)) + { + history.inc_unread_count(increment); + } + } + + pub fn stored_messages_may_be_unread( + &self, + server: &Server, + kind: &history::Kind, + read_marker: Option>, + ) -> bool { + if let Some(ref mut history) = self.data.map.get(server).and_then(|map| map.get(kind)) { + match history { + History::Partial { messages, .. } => { + if let Some(message) = messages.iter().next() { + if !history::after_read_marker(message, &read_marker) { + return false; + } + } + } + History::Full { .. } => return false, + } + } + + true + } + pub fn get_channel_messages( &self, server: &Server, @@ -441,18 +490,18 @@ impl Data { History::Partial { messages: new_messages, last_received_at, - opened_at, + read_marker, .. } => { let last_received_at = *last_received_at; - let opened_at = *opened_at; + let read_marker = *read_marker; messages.extend(std::mem::take(new_messages)); entry.insert(History::Full { server, kind, messages, last_received_at, - opened_at, + read_marker, }); } _ => { @@ -461,7 +510,7 @@ impl Data { kind, messages, last_received_at: None, - opened_at: Posix::now(), + read_marker: None, }); } }, @@ -471,7 +520,7 @@ impl Data { kind, messages, last_received_at: None, - opened_at: Posix::now(), + read_marker: None, }); } } @@ -486,7 +535,7 @@ impl Data { ) -> Option { let History::Full { messages, - opened_at, + read_marker, .. } = self.map.get(server)?.get(kind)? else { @@ -602,10 +651,13 @@ impl Data { let limited = with_limit(limit, filtered.into_iter()); - let split_at = limited - .iter() - .position(|message| message.received_at >= *opened_at) - .unwrap_or(limited.len()); + let split_at = read_marker.map_or(0, |read_marker| { + limited + .iter() + .rev() + .position(|message| message.server_time <= read_marker) + .map_or(limited.len(), |position| limited.len() - position) + }); let (old, new) = limited.split_at(split_at); @@ -623,13 +675,50 @@ impl Data { server: server::Server, kind: history::Kind, message: crate::Message, - ) { + ) -> Option { + use std::collections::hash_map; + + match self + .map + .entry(server.clone()) + .or_default() + .entry(kind.clone()) + { + hash_map::Entry::Occupied(mut entry) => { + entry.get_mut().add_message(message); + + None + } + hash_map::Entry::Vacant(entry) => { + entry + .insert(History::partial(server.clone(), kind.clone(), None)) + .add_message(message); + + Some(Event::LoadReadMarker(server.clone(), kind.clone())) + } + } + } + + fn get_read_marker( + &self, + server: &server::Server, + kind: &history::Kind, + ) -> Option> { + self.map.get(server)?.get(kind)?.get_read_marker() + } + + fn update_read_marker( + &mut self, + server: &server::Server, + kind: &history::Kind, + read_marker: Option>, + ) -> bool { self.map .entry(server.clone()) .or_default() .entry(kind.clone()) - .or_insert_with(|| History::partial(server, kind, message.received_at)) - .add_message(message) + .or_insert_with(|| History::partial(server.clone(), kind.clone(), None)) + .update_read_marker(read_marker) } fn untrack( diff --git a/data/src/message.rs b/data/src/message.rs index b85e3dae6..a4334fdc2 100644 --- a/data/src/message.rs +++ b/data/src/message.rs @@ -14,6 +14,7 @@ pub use self::formatting::Formatting; pub use self::source::Source; use crate::config::buffer::UsernameFormat; +use crate::history::after_read_marker; use crate::time::{self, Posix}; use crate::user::{Nick, NickRef}; use crate::{ctcp, Config, User}; @@ -152,7 +153,7 @@ pub struct Message { } impl Message { - pub fn triggers_unread(&self) -> bool { + pub fn triggers_unread(&self, read_marker: &Option>) -> bool { matches!(self.direction, Direction::Received) && match self.target.source() { Source::User(_) => true, @@ -166,6 +167,7 @@ impl Message { } _ => false, } + && after_read_marker(self, read_marker) } pub fn received<'a>( @@ -687,6 +689,7 @@ fn target( | Command::CNOTICE(_, _, _) | Command::CPRIVMSG(_, _, _) | Command::KNOCK(_, _) + | Command::MARKREAD(_, _) | Command::MONITOR(_, _) | Command::TAGMSG(_) | Command::USERIP(_) diff --git a/irc/proto/src/command.rs b/irc/proto/src/command.rs index a7ad32aa4..12de0d7d6 100644 --- a/irc/proto/src/command.rs +++ b/irc/proto/src/command.rs @@ -107,6 +107,8 @@ pub enum Command { CPRIVMSG(String, String, String), /// [] KNOCK(String, Option), + /// [] + MARKREAD(String, Option), /// [] MONITOR(String, Option), /// @@ -205,6 +207,7 @@ impl Command { "CNOTICE" if len > 2 => CNOTICE(req!(), req!(), req!()), "CPRIVMSG" if len > 2 => CPRIVMSG(req!(), req!(), req!()), "KNOCK" if len > 0 => KNOCK(req!(), opt!()), + "MARKREAD" if len > 0 => MARKREAD(req!(), opt!()), "MONITOR" if len > 0 => MONITOR(req!(), opt!()), "TAGMSG" if len > 0 => TAGMSG(req!()), "USERIP" if len > 0 => USERIP(req!()), @@ -265,6 +268,7 @@ impl Command { Command::CNOTICE(a, b, c) => vec![a, b, c], Command::CPRIVMSG(a, b, c) => vec![a, b, c], Command::KNOCK(a, b) => std::iter::once(a).chain(b).collect(), + Command::MARKREAD(a, b) => std::iter::once(a).chain(b).collect(), Command::MONITOR(a, b) => std::iter::once(a).chain(b).collect(), Command::TAGMSG(a) => vec![a], Command::USERIP(a) => vec![a], @@ -324,6 +328,7 @@ impl Command { CNOTICE(_, _, _) => "CNOTICE".to_string(), CPRIVMSG(_, _, _) => "CPRIVMSG".to_string(), KNOCK(_, _) => "KNOCK".to_string(), + MARKREAD(_, _) => "MARKREAD".to_string(), MONITOR(_, _) => "MONITOR".to_string(), TAGMSG(_) => "TAGMSG".to_string(), USERIP(_) => "USERIP".to_string(), diff --git a/src/main.rs b/src/main.rs index 51122f276..be36c50ca 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,10 +19,11 @@ mod window; use std::env; use std::time::{Duration, Instant}; -use chrono::Utc; +use chrono::{DateTime, Utc}; use data::config::{self, Config}; +use data::history::{self, read_marker_to_string}; use data::version::Version; -use data::{environment, history, server, version, Url, User}; +use data::{environment, server, version, Url, User}; use iced::widget::{column, container}; use iced::{padding, Length, Subscription, Task}; use screen::{dashboard, help, migration, welcome}; @@ -217,6 +218,7 @@ pub enum Message { RouteReceived(String), Window(window::Id, window::Event), WindowSettingsSaved(Result<(), window::Error>), + UpdateReadMarker(server::Server, history::Kind, Option>), } impl Halloy { @@ -644,6 +646,57 @@ impl Halloy { commands.push(command.map(Message::Dashboard)); } } + data::client::Event::LoadReadMarker(target) => { + let server = server.clone(); + let kind = history::Kind::from(target); + + commands.push(Task::perform( + history::load_read_marker(server.clone(), kind.clone()), + move |read_marker| { + Message::UpdateReadMarker( + server.clone(), + kind.clone(), + read_marker.ok().flatten(), + ) + }, + )); + } + data::client::Event::UpdateReadMarker(target, read_marker) => { + let server = server.clone(); + let kind = history::Kind::from(target); + + if dashboard.update_read_marker(&server, &kind, read_marker) + { + log::debug!( + "[{server}] updated read-marker for {kind} to {}", + read_marker_to_string(&read_marker), + ); + + if dashboard.stored_messages_may_be_unread( + &server, + &kind, + read_marker, + ) { + commands.push(Task::perform( + history::num_stored_unread_messages( + server.clone(), + kind.clone(), + read_marker, + ), + move |num_stored_unread_messages| { + Message::Dashboard( + dashboard::Message::IncrementUnread( + server.clone(), + kind.clone(), + read_marker, + num_stored_unread_messages, + ), + ) + }, + )); + } + } + } } } @@ -788,6 +841,47 @@ impl Halloy { log::error!("window settings failed to save: {:?}", err) } + Task::none() + } + Message::UpdateReadMarker(server, kind, read_marker) => { + if let Screen::Dashboard(dashboard) = &mut self.screen { + if dashboard.update_read_marker(&server, &kind, read_marker) { + log::debug!( + "[{server}] updated read-marker for {kind} to {}", + read_marker_to_string(&read_marker), + ); + + match kind.clone() { + history::Kind::Server => (), + history::Kind::Channel(channel) => { + self.clients.send_markread(&server, &channel, read_marker) + } + history::Kind::Query(nick) => { + self.clients + .send_markread(&server, nick.as_ref(), read_marker) + } + } + + if dashboard.stored_messages_may_be_unread(&server, &kind, read_marker) { + return Task::perform( + history::num_stored_unread_messages( + server.clone(), + kind.clone(), + read_marker, + ), + move |num_stored_unread_messages| { + Message::Dashboard(dashboard::Message::IncrementUnread( + server.clone(), + kind.clone(), + read_marker, + num_stored_unread_messages, + )) + }, + ); + } + } + } + Task::none() } } diff --git a/src/screen/dashboard.rs b/src/screen/dashboard.rs index 2cee0f1d2..54722948b 100644 --- a/src/screen/dashboard.rs +++ b/src/screen/dashboard.rs @@ -59,6 +59,8 @@ pub enum Message { CloseContextMenu(bool), ThemeEditor(theme_editor::Message), ConfigReloaded(Result), + IncrementUnread(Server, history::Kind, Option>, usize), + UpdateReadMarker(Server, history::Kind, Option>), } #[derive(Debug)] @@ -494,6 +496,22 @@ impl Dashboard { } } Message::History(message) => { + if let history::manager::Message::Closed(ref server, ref kind, Ok(_)) = message { + match kind { + history::Kind::Server => (), + history::Kind::Channel(channel) => clients.send_markread( + server, + channel, + self.get_read_marker(server, kind), + ), + history::Kind::Query(nick) => clients.send_markread( + server, + nick.as_ref(), + self.get_read_marker(server, kind), + ), + } + } + self.history.update(message); } Message::DashboardSaved(Ok(_)) => { @@ -796,6 +814,14 @@ impl Dashboard { Message::ConfigReloaded(config) => { return (Task::none(), Some(Event::ConfigReloaded(config))); } + Message::IncrementUnread(server, kind, read_marker, increment) => { + if self.get_read_marker(&server, &kind) == read_marker { + self.inc_unread_count(&server, &kind, increment); + } + } + Message::UpdateReadMarker(server, kind, read_marker) => { + self.update_read_marker(&server, &kind, read_marker); + } } (Task::none(), None) @@ -1323,6 +1349,33 @@ impl Dashboard { ); } + pub fn get_read_marker(&self, server: &Server, kind: &history::Kind) -> Option> { + self.history.get_read_marker(server, kind) + } + + pub fn update_read_marker( + &mut self, + server: &Server, + kind: &history::Kind, + read_marker: Option>, + ) -> bool { + self.history.update_read_marker(server, kind, read_marker) + } + + pub fn inc_unread_count(&mut self, server: &Server, kind: &history::Kind, increment: usize) { + self.history.inc_unread_count(server, kind, increment); + } + + pub fn stored_messages_may_be_unread( + &self, + server: &Server, + kind: &history::Kind, + read_marker: Option>, + ) -> bool { + self.history + .stored_messages_may_be_unread(server, kind, read_marker) + } + fn get_focused_mut( &mut self, main_window: &Window, From f3720f67558e6312e9a03c40b1c3deec7719483a Mon Sep 17 00:00:00 2001 From: Andrew Baldwin Date: Tue, 17 Sep 2024 11:29:15 -0700 Subject: [PATCH 02/29] Restructuring `read_marker` to be a field of a `Metadata` struct. Disallow calling `send_markread` with a value-less `read_marker`. --- data/src/client.rs | 25 +++--- data/src/history.rs | 153 +++++++++++------------------------ data/src/history/manager.rs | 30 +++---- data/src/history/metadata.rs | 64 +++++++++++++++ src/main.rs | 30 ++++--- src/screen/dashboard.rs | 20 ++--- 6 files changed, 164 insertions(+), 158 deletions(-) create mode 100644 data/src/history/metadata.rs diff --git a/data/src/client.rs b/data/src/client.rs index d5081e10c..158b4afae 100644 --- a/data/src/client.rs +++ b/data/src/client.rs @@ -6,7 +6,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt; use std::time::{Duration, Instant}; -use crate::history::read_marker_to_string; +use crate::history::metadata::read_marker_to_string; use crate::message::server_time; use crate::time::Posix; use crate::user::{Nick, NickRef}; @@ -80,7 +80,7 @@ pub enum Event { Broadcast(Broadcast), Notification(message::Encoded, Nick, Notification), FileTransferRequest(file_transfer::ReceiveRequest), - LoadReadMarker(String), + LoadHistoryMetadata(String), UpdateReadMarker(String, Option>), } @@ -1256,15 +1256,13 @@ impl Client { Some(vec![Event::Single(message, self.nickname().to_owned())]) } - pub fn send_markread(&mut self, target: &str, read_marker: Option>) { + pub fn send_markread(&mut self, target: &str, read_marker: DateTime) { if self.supports_read_marker { - if let Some(read_marker) = read_marker { - let _ = self.handle.try_send(command!( - "MARKREAD", - target.to_string(), - format!("timestamp={}", read_marker_to_string(&Some(read_marker))), - )); - } + let _ = self.handle.try_send(command!( + "MARKREAD", + target.to_string(), + format!("timestamp={}", read_marker_to_string(&Some(read_marker))), + )); } } @@ -1467,12 +1465,7 @@ impl Map { } } - pub fn send_markread( - &mut self, - server: &Server, - target: &str, - read_marker: Option>, - ) { + pub fn send_markread(&mut self, server: &Server, target: &str, read_marker: DateTime) { if let Some(client) = self.client_mut(server) { client.send_markread(target, read_marker); } diff --git a/data/src/history.rs b/data/src/history.rs index 5d9c840e0..223d8f9b3 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -1,19 +1,21 @@ -use chrono::{format::SecondsFormat, DateTime, Utc}; -use irc::proto; +use chrono::{DateTime, Utc}; use std::path::PathBuf; use std::time::Duration; use std::{fmt, io}; use futures::future::BoxFuture; use futures::{Future, FutureExt}; +use irc::proto; use tokio::fs; use tokio::time::Instant; pub use self::manager::{Manager, Resource}; +pub use self::metadata::{after_read_marker, Metadata}; use crate::user::Nick; use crate::{compression, environment, message, server, Message, Server}; pub mod manager; +pub mod metadata; // TODO: Make this configurable? /// Max # messages to persist @@ -67,56 +69,29 @@ impl From<&str> for Kind { } pub async fn load_messages(server: &server::Server, kind: &Kind) -> Result, Error> { - let path = messages_path(server, kind).await?; - - Ok(read_messages(&path).await.unwrap_or_default()) -} - -pub async fn load_read_marker( - server: server::Server, - kind: Kind, -) -> Result>, Error> { - let path = read_marker_path(&server, &kind).await?; + let path = path(server, kind).await?; - if let Ok(bytes) = fs::read(path).await { - Ok(serde_json::from_slice(&bytes).unwrap_or_default()) - } else { - Ok(None) - } + Ok(read_all(&path).await.unwrap_or_default()) } pub async fn overwrite( server: &server::Server, kind: &Kind, messages: &[Message], - read_marker: &Option>, + metadata: &Metadata, ) -> Result<(), Error> { if messages.is_empty() { - return overwrite_read_marker(server, kind, read_marker).await; + return metadata::overwrite(server, kind, metadata).await; } let latest = &messages[messages.len().saturating_sub(MAX_MESSAGES)..]; - let path = messages_path(server, kind).await?; + let path = path(server, kind).await?; let compressed = compression::compress(&latest)?; fs::write(path, &compressed).await?; - overwrite_read_marker(server, kind, read_marker).await?; - - Ok(()) -} - -pub async fn overwrite_read_marker( - server: &server::Server, - kind: &Kind, - read_marker: &Option>, -) -> Result<(), Error> { - let bytes = serde_json::to_vec(&read_marker)?; - - let path = read_marker_path(server, kind).await?; - - fs::write(path, &bytes).await?; + metadata::overwrite(server, kind, metadata).await?; Ok(()) } @@ -125,24 +100,24 @@ pub async fn append( server: &server::Server, kind: &Kind, messages: Vec, - read_marker: &Option>, + metadata: &Metadata, ) -> Result<(), Error> { if messages.is_empty() { - return overwrite_read_marker(server, kind, read_marker).await; + return metadata::overwrite(server, kind, metadata).await; } let mut all_messages = load_messages(server, kind).await?; all_messages.extend(messages); - overwrite(server, kind, &all_messages, read_marker).await + overwrite(server, kind, &all_messages, metadata).await } -async fn read_messages(path: &PathBuf) -> Result, Error> { +async fn read_all(path: &PathBuf) -> Result, Error> { let bytes = fs::read(path).await?; Ok(compression::decompress(&bytes)?) } -async fn dir_path() -> Result { +pub async fn dir_path() -> Result { let data_dir = environment::data_dir(); let history_dir = data_dir.join("history"); @@ -154,7 +129,7 @@ async fn dir_path() -> Result { Ok(history_dir) } -async fn messages_path(server: &server::Server, kind: &Kind) -> Result { +async fn path(server: &server::Server, kind: &Kind) -> Result { let dir = dir_path().await?; let name = match kind { @@ -168,20 +143,6 @@ async fn messages_path(server: &server::Server, kind: &Kind) -> Result Result { - let dir = dir_path().await?; - - let name = match kind { - Kind::Server => format!("{server}_read_marker"), - Kind::Channel(channel) => format!("{server}channel{channel}_read_marker"), - Kind::Query(nick) => format!("{server}nickname{}_read_marker", nick), - }; - - let hashed_name = seahash::hash(name.as_bytes()); - - Ok(dir.join(format!("{hashed_name}.json"))) -} - #[derive(Debug)] pub enum History { Partial { @@ -190,26 +151,26 @@ pub enum History { messages: Vec, last_received_at: Option, unread_message_count: usize, - read_marker: Option>, + metadata: Metadata, }, Full { server: server::Server, kind: Kind, messages: Vec, last_received_at: Option, - read_marker: Option>, + metadata: Metadata, }, } impl History { - fn partial(server: server::Server, kind: Kind, read_marker: Option>) -> Self { + fn partial(server: server::Server, kind: Kind, metadata: Metadata) -> Self { Self::Partial { server, kind, messages: vec![], last_received_at: None, unread_message_count: 0, - read_marker, + metadata, } } @@ -228,18 +189,18 @@ impl History { History::Partial { messages, last_received_at, - read_marker, + metadata, .. } | History::Full { messages, last_received_at, - read_marker, + metadata, .. } => { *last_received_at = Some(Instant::now()); - insert_message(messages, message, read_marker) + insert_message(messages, message, &metadata.read_marker) } } { self.inc_unread_count(1); @@ -252,7 +213,7 @@ impl History { server, kind, messages, - read_marker, + metadata, last_received_at, .. } => { @@ -262,12 +223,12 @@ impl History { if since >= FLUSH_AFTER_LAST_RECEIVED && !messages.is_empty() { let server = server.clone(); let kind = kind.clone(); - let read_marker = *read_marker; + let metadata = metadata.clone(); let messages = std::mem::take(messages); *last_received_at = None; return Some( - async move { append(&server, &kind, messages, &read_marker).await } + async move { append(&server, &kind, messages, &metadata).await } .boxed(), ); } @@ -279,7 +240,7 @@ impl History { server, kind, messages, - read_marker, + metadata, last_received_at, .. } => { @@ -289,7 +250,7 @@ impl History { if since >= FLUSH_AFTER_LAST_RECEIVED && !messages.is_empty() { let server = server.clone(); let kind = kind.clone(); - let read_marker = *read_marker; + let metadata = metadata.clone(); *last_received_at = None; if messages.len() > MAX_MESSAGES { @@ -299,7 +260,7 @@ impl History { let messages = messages.clone(); return Some( - async move { overwrite(&server, &kind, &messages, &read_marker).await } + async move { overwrite(&server, &kind, &messages, &metadata).await } .boxed(), ); } @@ -317,7 +278,7 @@ impl History { server, kind, messages, - read_marker, + metadata, .. } => { let server = server.clone(); @@ -328,12 +289,13 @@ impl History { .find(|message| { !matches!(message.target.source(), message::Source::Internal(_)) }) - .map_or(*read_marker, |message| Some(message.server_time)); + .map_or(metadata.read_marker, |message| Some(message.server_time)); + let metadata = Metadata { read_marker }; let messages = std::mem::take(messages); - *self = Self::partial(server.clone(), kind.clone(), read_marker); + *self = Self::partial(server.clone(), kind.clone(), metadata.clone()); - Some(async move { overwrite(&server, &kind, &messages, &read_marker).await }) + Some(async move { overwrite(&server, &kind, &messages, &metadata).await }) } } } @@ -344,46 +306,40 @@ impl History { server, kind, messages, - read_marker, + metadata, .. - } => append(&server, &kind, messages, &read_marker).await, + } => append(&server, &kind, messages, &metadata).await, History::Full { server, kind, messages, - read_marker, + metadata, .. - } => overwrite(&server, &kind, &messages, &read_marker).await, + } => overwrite(&server, &kind, &messages, &metadata).await, } } pub fn get_read_marker(&self) -> Option> { match self { - History::Partial { read_marker, .. } => *read_marker, - History::Full { read_marker, .. } => *read_marker, + History::Partial { metadata, .. } => metadata.read_marker, + History::Full { metadata, .. } => metadata.read_marker, } } pub fn update_read_marker(&mut self, read_marker: Option>) -> bool { - let history_read_marker = match self { - History::Partial { - read_marker: history_read_marker, - .. - } => history_read_marker, - History::Full { - read_marker: history_read_marker, - .. - } => history_read_marker, + let metadata = match self { + History::Partial { metadata, .. } => metadata, + History::Full { metadata, .. } => metadata, }; if let Some(read_marker) = read_marker { - if let Some(history_read_marker) = history_read_marker { - if read_marker <= *history_read_marker { + if let Some(history_read_marker) = metadata.read_marker { + if read_marker <= history_read_marker { return false; } } - *history_read_marker = Some(read_marker); + metadata.read_marker = Some(read_marker); } else { return false; } @@ -391,14 +347,14 @@ impl History { if let History::Partial { messages, unread_message_count, - read_marker: history_read_marker, + metadata, .. } = self { *unread_message_count = 0; for message in messages { - if message.triggers_unread(history_read_marker) { + if message.triggers_unread(&metadata.read_marker) { *unread_message_count += 1; } } @@ -420,19 +376,6 @@ pub fn insert_message( message_triggers_unread } -pub fn after_read_marker(message: &Message, read_marker: &Option>) -> bool { - read_marker.is_none() - || read_marker.is_some_and(|read_marker| message.server_time > read_marker) -} - -pub fn read_marker_to_string(read_marker: &Option>) -> String { - if let Some(read_marker) = read_marker { - read_marker.to_rfc3339_opts(SecondsFormat::Millis, true) - } else { - "*".to_string() - } -} - pub async fn num_stored_unread_messages( server: Server, kind: Kind, diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 0ccf9f4d8..1fa79c2b7 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -6,7 +6,7 @@ use futures::{future, Future, FutureExt}; use itertools::Itertools; use tokio::time::Instant; -use crate::history::{self, History}; +use crate::history::{self, metadata::Metadata, History}; use crate::message::{self, Limit}; use crate::user::Nick; use crate::{config, input}; @@ -30,7 +30,7 @@ pub enum Message { } pub enum Event { - LoadReadMarker(server::Server, history::Kind), + LoadHistoryMetadata(server::Server, history::Kind), } #[derive(Debug, Default)] @@ -490,18 +490,18 @@ impl Data { History::Partial { messages: new_messages, last_received_at, - read_marker, + metadata, .. } => { + let metadata = metadata.clone(); let last_received_at = *last_received_at; - let read_marker = *read_marker; messages.extend(std::mem::take(new_messages)); entry.insert(History::Full { server, kind, messages, last_received_at, - read_marker, + metadata, }); } _ => { @@ -510,7 +510,7 @@ impl Data { kind, messages, last_received_at: None, - read_marker: None, + metadata: Metadata::default(), }); } }, @@ -520,7 +520,7 @@ impl Data { kind, messages, last_received_at: None, - read_marker: None, + metadata: Metadata::default(), }); } } @@ -534,9 +534,7 @@ impl Data { buffer_config: &config::Buffer, ) -> Option { let History::Full { - messages, - read_marker, - .. + messages, metadata, .. } = self.map.get(server)?.get(kind)? else { return None; @@ -651,7 +649,7 @@ impl Data { let limited = with_limit(limit, filtered.into_iter()); - let split_at = read_marker.map_or(0, |read_marker| { + let split_at = metadata.read_marker.map_or(0, |read_marker| { limited .iter() .rev() @@ -691,10 +689,14 @@ impl Data { } hash_map::Entry::Vacant(entry) => { entry - .insert(History::partial(server.clone(), kind.clone(), None)) + .insert(History::partial( + server.clone(), + kind.clone(), + Metadata::default(), + )) .add_message(message); - Some(Event::LoadReadMarker(server.clone(), kind.clone())) + Some(Event::LoadHistoryMetadata(server.clone(), kind.clone())) } } } @@ -717,7 +719,7 @@ impl Data { .entry(server.clone()) .or_default() .entry(kind.clone()) - .or_insert_with(|| History::partial(server.clone(), kind.clone(), None)) + .or_insert_with(|| History::partial(server.clone(), kind.clone(), Metadata::default())) .update_read_marker(read_marker) } diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs new file mode 100644 index 000000000..7d9c9e76e --- /dev/null +++ b/data/src/history/metadata.rs @@ -0,0 +1,64 @@ +use chrono::{format::SecondsFormat, DateTime, Utc}; +use std::path::PathBuf; + +use serde::{Deserialize, Serialize}; +use tokio::fs; + +use crate::history::{dir_path, Error, Kind}; +use crate::{server, Message}; + +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct Metadata { + pub read_marker: Option>, +} + +pub async fn load(server: server::Server, kind: Kind) -> Result { + let path = path(&server, &kind).await?; + + if let Ok(bytes) = fs::read(path).await { + Ok(serde_json::from_slice(&bytes).unwrap_or_default()) + } else { + Ok(Metadata::default()) + } +} + +pub async fn overwrite( + server: &server::Server, + kind: &Kind, + metadata: &Metadata, +) -> Result<(), Error> { + let bytes = serde_json::to_vec(metadata)?; + + let path = path(server, kind).await?; + + fs::write(path, &bytes).await?; + + Ok(()) +} + +async fn path(server: &server::Server, kind: &Kind) -> Result { + let dir = dir_path().await?; + + let name = match kind { + Kind::Server => format!("{server}-metadata"), + Kind::Channel(channel) => format!("{server}channel{channel}-metadata"), + Kind::Query(nick) => format!("{server}nickname{}-metadata", nick), + }; + + let hashed_name = seahash::hash(name.as_bytes()); + + Ok(dir.join(format!("{hashed_name}.json"))) +} + +pub fn after_read_marker(message: &Message, read_marker: &Option>) -> bool { + read_marker.is_none() + || read_marker.is_some_and(|read_marker| message.server_time > read_marker) +} + +pub fn read_marker_to_string(read_marker: &Option>) -> String { + if let Some(read_marker) = read_marker { + read_marker.to_rfc3339_opts(SecondsFormat::Millis, true) + } else { + "*".to_string() + } +} diff --git a/src/main.rs b/src/main.rs index be36c50ca..1fa20e123 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,7 +21,7 @@ use std::time::{Duration, Instant}; use chrono::{DateTime, Utc}; use data::config::{self, Config}; -use data::history::{self, read_marker_to_string}; +use data::history::{self, metadata::read_marker_to_string}; use data::version::Version; use data::{environment, server, version, Url, User}; use iced::widget::{column, container}; @@ -646,17 +646,19 @@ impl Halloy { commands.push(command.map(Message::Dashboard)); } } - data::client::Event::LoadReadMarker(target) => { + data::client::Event::LoadHistoryMetadata(target) => { let server = server.clone(); let kind = history::Kind::from(target); commands.push(Task::perform( - history::load_read_marker(server.clone(), kind.clone()), - move |read_marker| { + history::metadata::load(server.clone(), kind.clone()), + move |metadata| { + let metadata = metadata.unwrap_or_default(); + Message::UpdateReadMarker( server.clone(), kind.clone(), - read_marker.ok().flatten(), + metadata.read_marker, ) }, )); @@ -851,14 +853,16 @@ impl Halloy { read_marker_to_string(&read_marker), ); - match kind.clone() { - history::Kind::Server => (), - history::Kind::Channel(channel) => { - self.clients.send_markread(&server, &channel, read_marker) - } - history::Kind::Query(nick) => { - self.clients - .send_markread(&server, nick.as_ref(), read_marker) + if let Some(read_marker) = read_marker { + match kind.clone() { + history::Kind::Server => (), + history::Kind::Channel(channel) => { + self.clients.send_markread(&server, &channel, read_marker) + } + history::Kind::Query(nick) => { + self.clients + .send_markread(&server, nick.as_ref(), read_marker) + } } } diff --git a/src/screen/dashboard.rs b/src/screen/dashboard.rs index 54722948b..fe60790e9 100644 --- a/src/screen/dashboard.rs +++ b/src/screen/dashboard.rs @@ -499,16 +499,16 @@ impl Dashboard { if let history::manager::Message::Closed(ref server, ref kind, Ok(_)) = message { match kind { history::Kind::Server => (), - history::Kind::Channel(channel) => clients.send_markread( - server, - channel, - self.get_read_marker(server, kind), - ), - history::Kind::Query(nick) => clients.send_markread( - server, - nick.as_ref(), - self.get_read_marker(server, kind), - ), + history::Kind::Channel(channel) => { + if let Some(read_marker) = self.get_read_marker(server, kind) { + clients.send_markread(server, channel, read_marker) + } + } + history::Kind::Query(nick) => { + if let Some(read_marker) = self.get_read_marker(server, kind) { + clients.send_markread(server, nick.as_ref(), read_marker) + } + } } } From 1559b6dcd864974e9ee81b28e7c5ee07aa89c0de Mon Sep 17 00:00:00 2001 From: Andrew Baldwin Date: Tue, 17 Sep 2024 12:23:33 -0700 Subject: [PATCH 03/29] Chunk capability requests. --- data/src/client.rs | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/data/src/client.rs b/data/src/client.rs index 158b4afae..04e0a63f1 100644 --- a/data/src/client.rs +++ b/data/src/client.rs @@ -379,9 +379,10 @@ impl Client { if !requested.is_empty() { // Request self.registration_step = RegistrationStep::Req; - let _ = self - .handle - .try_send(command!("CAP", "REQ", requested.join(" "))); + + for message in group_capability_requests(&requested) { + let _ = self.handle.try_send(message); + } } else { // If none requested, end negotiation self.registration_step = RegistrationStep::End; @@ -495,10 +496,9 @@ impl Client { } if !requested.is_empty() { - // Request - let _ = self - .handle - .try_send(command!("CAP", "REQ", requested.join(" "))); + for message in group_capability_requests(&requested) { + let _ = self.handle.try_send(message); + } } self.listed_caps.extend(new_caps); @@ -1701,6 +1701,26 @@ pub enum WhoStatus { Done(Instant), } +fn group_capability_requests<'a>( + capabilities: &'a [&'a str], +) -> impl Iterator + 'a { + const MAX_LEN: usize = proto::format::BYTE_LIMIT - b"CAP REQ :\r\n".len(); + + capabilities + .iter() + .scan(0, |count, capability| { + // Capability + a space + *count += capability.len() + 1; + + let chunk = *count / MAX_LEN; + + Some((chunk, capability)) + }) + .into_group_map() + .into_values() + .map(|capabilities| command!("CAP", "REQ", capabilities.into_iter().join(" "))) +} + /// Group channels together into as few JOIN messages as possible fn group_joins<'a>( channels: &'a [String], @@ -1773,5 +1793,5 @@ fn group_monitors( }) .into_group_map() .into_values() - .map(|targets| command!("MONITOR", "+", targets.into_iter().join(","),)) + .map(|targets| command!("MONITOR", "+", targets.into_iter().join(","))) } From 92fd5c50b57c7d01edab97873bc4934d3c5d5c96 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Tue, 17 Sep 2024 12:45:07 -0700 Subject: [PATCH 04/29] Always load metadata w/ messages --- data/src/history.rs | 21 ++++++++++++--------- data/src/history/manager.rs | 20 ++++++++++++-------- data/src/history/metadata.rs | 6 +----- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 223d8f9b3..930c7a986 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -68,10 +68,13 @@ impl From<&str> for Kind { } } -pub async fn load_messages(server: &server::Server, kind: &Kind) -> Result, Error> { - let path = path(server, kind).await?; +pub async fn load(server: server::Server, kind: Kind) -> Result<(Vec, Metadata), Error> { + let path = path(&server, &kind).await?; + + let messages = read_all(&path).await.unwrap_or_default(); + let metadata = metadata::load(server, kind).await.unwrap_or_default(); - Ok(read_all(&path).await.unwrap_or_default()) + Ok((messages, metadata)) } pub async fn overwrite( @@ -81,7 +84,7 @@ pub async fn overwrite( metadata: &Metadata, ) -> Result<(), Error> { if messages.is_empty() { - return metadata::overwrite(server, kind, metadata).await; + return metadata::save(server, kind, metadata).await; } let latest = &messages[messages.len().saturating_sub(MAX_MESSAGES)..]; @@ -91,7 +94,7 @@ pub async fn overwrite( fs::write(path, &compressed).await?; - metadata::overwrite(server, kind, metadata).await?; + metadata::save(server, kind, metadata).await?; Ok(()) } @@ -103,10 +106,10 @@ pub async fn append( metadata: &Metadata, ) -> Result<(), Error> { if messages.is_empty() { - return metadata::overwrite(server, kind, metadata).await; + return metadata::save(server, kind, metadata).await; } - let mut all_messages = load_messages(server, kind).await?; + let (mut all_messages, _) = load(server.clone(), kind.clone()).await?; all_messages.extend(messages); overwrite(server, kind, &all_messages, metadata).await @@ -381,9 +384,9 @@ pub async fn num_stored_unread_messages( kind: Kind, read_marker: Option>, ) -> usize { - let messages = load_messages(&server, &kind).await; + let result = load(server, kind).await; - if let Ok(messages) = messages { + if let Ok((messages, _)) = result { messages .into_iter() .rev() diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 1fa79c2b7..e45a06f0e 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -23,7 +23,7 @@ pub enum Message { Loaded( server::Server, history::Kind, - Result, history::Error>, + Result<(Vec, history::Metadata), history::Error>, ), Closed(server::Server, history::Kind, Result<(), history::Error>), Flushed(server::Server, history::Kind, Result<(), history::Error>), @@ -46,7 +46,7 @@ impl Manager { let added = added.into_iter().map(|resource| { async move { - history::load_messages(&resource.server.clone(), &resource.kind.clone()) + history::load(resource.server.clone(), resource.kind.clone()) .map(move |result| Message::Loaded(resource.server, resource.kind, result)) .await } @@ -71,12 +71,12 @@ impl Manager { pub fn update(&mut self, message: Message) { match message { - Message::Loaded(server, kind, Ok(messages)) => { + Message::Loaded(server, kind, Ok((messages, metadata))) => { log::debug!( "loaded history for {kind} on {server}: {} messages", messages.len() ); - self.data.loaded(server, kind, messages); + self.data.loaded(server, kind, messages, metadata); } Message::Loaded(server, kind, Err(error)) => { log::warn!("failed to load history for {kind} on {server}: {error}"); @@ -477,6 +477,7 @@ impl Data { server: server::Server, kind: history::Kind, mut messages: Vec, + metadata: Metadata, ) { use std::collections::hash_map; @@ -490,10 +491,13 @@ impl Data { History::Partial { messages: new_messages, last_received_at, - metadata, + metadata: partial_metadata, .. } => { - let metadata = metadata.clone(); + // In-memory metadata will always be more up-to-date than disk + // since we might not have flushed this yet + let metadata = partial_metadata.clone(); + let last_received_at = *last_received_at; messages.extend(std::mem::take(new_messages)); entry.insert(History::Full { @@ -510,7 +514,7 @@ impl Data { kind, messages, last_received_at: None, - metadata: Metadata::default(), + metadata, }); } }, @@ -520,7 +524,7 @@ impl Data { kind, messages, last_received_at: None, - metadata: Metadata::default(), + metadata, }); } } diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index 7d9c9e76e..218560505 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -22,11 +22,7 @@ pub async fn load(server: server::Server, kind: Kind) -> Result } } -pub async fn overwrite( - server: &server::Server, - kind: &Kind, - metadata: &Metadata, -) -> Result<(), Error> { +pub async fn save(server: &server::Server, kind: &Kind, metadata: &Metadata) -> Result<(), Error> { let bytes = serde_json::to_vec(metadata)?; let path = path(server, kind).await?; From 15b46095997ab77ec0efb3b0079ecd3a08f33494 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Tue, 17 Sep 2024 15:01:17 -0700 Subject: [PATCH 05/29] Encapsulate read marker logic / loading in history module --- data/src/client.rs | 23 ++-- data/src/history.rs | 210 +++++++++++++++------------------ data/src/history/manager.rs | 220 +++++++++++++++++++---------------- data/src/history/metadata.rs | 99 +++++++++++++--- data/src/message.rs | 4 +- src/main.rs | 104 ++--------------- src/screen/dashboard.rs | 100 +++++++--------- 7 files changed, 357 insertions(+), 403 deletions(-) diff --git a/data/src/client.rs b/data/src/client.rs index 04e0a63f1..18a0f28a1 100644 --- a/data/src/client.rs +++ b/data/src/client.rs @@ -6,7 +6,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt; use std::time::{Duration, Instant}; -use crate::history::metadata::read_marker_to_string; +use crate::history::ReadMarker; use crate::message::server_time; use crate::time::Posix; use crate::user::{Nick, NickRef}; @@ -80,8 +80,7 @@ pub enum Event { Broadcast(Broadcast), Notification(message::Encoded, Nick, Notification), FileTransferRequest(file_transfer::ReceiveRequest), - LoadHistoryMetadata(String), - UpdateReadMarker(String, Option>), + UpdateReadMarker(String, ReadMarker), } pub struct Client { @@ -1243,12 +1242,12 @@ impl Client { return None; } Command::MARKREAD(target, Some(timestamp)) => { - let timestamp = timestamp.strip_prefix("timestamp=").and_then(|timestamp| { - DateTime::parse_from_rfc3339(timestamp) - .ok() - .map(|dt| dt.with_timezone(&Utc)) - }); - return Some(vec![Event::UpdateReadMarker(target.clone(), timestamp)]); + if let Some(read_marker) = timestamp + .strip_prefix("timestamp=") + .and_then(|timestamp| timestamp.parse::().ok()) + { + return Some(vec![Event::UpdateReadMarker(target.clone(), read_marker)]); + } } _ => {} } @@ -1256,12 +1255,12 @@ impl Client { Some(vec![Event::Single(message, self.nickname().to_owned())]) } - pub fn send_markread(&mut self, target: &str, read_marker: DateTime) { + pub fn send_markread(&mut self, target: &str, read_marker: ReadMarker) { if self.supports_read_marker { let _ = self.handle.try_send(command!( "MARKREAD", target.to_string(), - format!("timestamp={}", read_marker_to_string(&Some(read_marker))), + format!("timestamp={read_marker}"), )); } } @@ -1465,7 +1464,7 @@ impl Map { } } - pub fn send_markread(&mut self, server: &Server, target: &str, read_marker: DateTime) { + pub fn send_markread(&mut self, server: &Server, target: &str, read_marker: ReadMarker) { if let Some(client) = self.client_mut(server) { client.send_markread(target, read_marker); } diff --git a/data/src/history.rs b/data/src/history.rs index 930c7a986..8e460458a 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -1,8 +1,8 @@ -use chrono::{DateTime, Utc}; use std::path::PathBuf; use std::time::Duration; use std::{fmt, io}; +use chrono::{DateTime, Utc}; use futures::future::BoxFuture; use futures::{Future, FutureExt}; use irc::proto; @@ -10,9 +10,9 @@ use tokio::fs; use tokio::time::Instant; pub use self::manager::{Manager, Resource}; -pub use self::metadata::{after_read_marker, Metadata}; +pub use self::metadata::{Metadata, ReadMarker}; use crate::user::Nick; -use crate::{compression, environment, message, server, Message, Server}; +use crate::{compression, environment, message, server, Message}; pub mod manager; pub mod metadata; @@ -32,6 +32,16 @@ pub enum Kind { Query(Nick), } +impl Kind { + pub fn target(&self) -> Option<&str> { + match self { + Kind::Server => None, + Kind::Channel(channel) => Some(channel), + Kind::Query(nick) => Some(nick.as_ref()), + } + } +} + impl fmt::Display for Kind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -68,13 +78,19 @@ impl From<&str> for Kind { } } -pub async fn load(server: server::Server, kind: Kind) -> Result<(Vec, Metadata), Error> { +#[derive(Debug)] +pub struct Loaded { + pub messages: Vec, + pub metadata: Metadata, +} + +pub async fn load(server: server::Server, kind: Kind) -> Result { let path = path(&server, &kind).await?; let messages = read_all(&path).await.unwrap_or_default(); let metadata = metadata::load(server, kind).await.unwrap_or_default(); - Ok((messages, metadata)) + Ok(Loaded { messages, metadata }) } pub async fn overwrite( @@ -109,7 +125,9 @@ pub async fn append( return metadata::save(server, kind, metadata).await; } - let (mut all_messages, _) = load(server.clone(), kind.clone()).await?; + let loaded = load(server.clone(), kind.clone()).await?; + + let mut all_messages = loaded.messages; all_messages.extend(messages); overwrite(server, kind, &all_messages, metadata).await @@ -152,61 +170,79 @@ pub enum History { server: server::Server, kind: Kind, messages: Vec, - last_received_at: Option, - unread_message_count: usize, + last_updated_at: Option, metadata: Metadata, + last_on_disk: Option>, }, Full { server: server::Server, kind: Kind, messages: Vec, - last_received_at: Option, + last_updated_at: Option, metadata: Metadata, }, } impl History { - fn partial(server: server::Server, kind: Kind, metadata: Metadata) -> Self { + fn partial(server: server::Server, kind: Kind) -> Self { Self::Partial { server, kind, messages: vec![], - last_received_at: None, - unread_message_count: 0, - metadata, + last_updated_at: None, + metadata: Metadata::default(), + last_on_disk: None, } } - pub fn inc_unread_count(&mut self, increment: usize) { - if let History::Partial { - unread_message_count, + pub fn update_partial(&mut self, loaded: Loaded) { + if let Self::Partial { + metadata, + last_on_disk, .. } = self { - *unread_message_count += increment; + *metadata = metadata.merge(loaded.metadata); + *last_on_disk = loaded.messages.last().map(|message| message.server_time); } } - fn add_message(&mut self, message: Message) { - if match self { + fn has_unread(&self) -> bool { + match self { History::Partial { messages, - last_received_at, metadata, + last_on_disk, + .. + } => { + // Read marker is prior to last message on disk + // or prior to unflushed messages in memory + metadata.read_marker.is_none() + || last_on_disk + .zip(metadata.read_marker) + .map_or(false, |(a, b)| a > b.date_time()) + || metadata.unread_count(messages) > 0 + } + History::Full { .. } => false, + } + } + + fn add_message(&mut self, message: Message) { + match self { + History::Partial { + messages, + last_updated_at, .. } | History::Full { messages, - last_received_at, - metadata, + last_updated_at, .. } => { - *last_received_at = Some(Instant::now()); + *last_updated_at = Some(Instant::now()); - insert_message(messages, message, &metadata.read_marker) + messages.push(message); } - } { - self.inc_unread_count(1); } } @@ -217,18 +253,22 @@ impl History { kind, messages, metadata, - last_received_at, + last_updated_at, + last_on_disk, .. } => { - if let Some(last_received) = *last_received_at { + if let Some(last_received) = *last_updated_at { let since = now.duration_since(last_received); - if since >= FLUSH_AFTER_LAST_RECEIVED && !messages.is_empty() { + if since >= FLUSH_AFTER_LAST_RECEIVED { let server = server.clone(); let kind = kind.clone(); - let metadata = metadata.clone(); + let metadata = *metadata; let messages = std::mem::take(messages); - *last_received_at = None; + + *last_updated_at = None; + + *last_on_disk = messages.last().map(|message| message.server_time); return Some( async move { append(&server, &kind, messages, &metadata).await } @@ -244,17 +284,17 @@ impl History { kind, messages, metadata, - last_received_at, + last_updated_at, .. } => { - if let Some(last_received) = *last_received_at { + if let Some(last_received) = *last_updated_at { let since = now.duration_since(last_received); if since >= FLUSH_AFTER_LAST_RECEIVED && !messages.is_empty() { let server = server.clone(); let kind = kind.clone(); - let metadata = metadata.clone(); - *last_received_at = None; + let metadata = *metadata; + *last_updated_at = None; if messages.len() > MAX_MESSAGES { messages.drain(0..messages.len() - (MAX_MESSAGES - TRUNC_COUNT)); @@ -274,7 +314,7 @@ impl History { } } - fn make_partial(&mut self) -> Option>> { + fn make_partial(&mut self) -> Option, Error>>> { match self { History::Partial { .. } => None, History::Full { @@ -286,19 +326,24 @@ impl History { } => { let server = server.clone(); let kind = kind.clone(); - let read_marker = messages - .iter() - .rev() - .find(|message| { - !matches!(message.target.source(), message::Source::Internal(_)) - }) - .map_or(metadata.read_marker, |message| Some(message.server_time)); - let metadata = Metadata { read_marker }; let messages = std::mem::take(messages); - *self = Self::partial(server.clone(), kind.clone(), metadata.clone()); - - Some(async move { overwrite(&server, &kind, &messages, &metadata).await }) + let metadata = metadata.updated(&messages); + + *self = Self::Partial { + server: server.clone(), + kind: kind.clone(), + messages: vec![], + last_updated_at: None, + metadata, + last_on_disk: messages.last().map(|message| message.server_time), + }; + + Some(async move { + overwrite(&server, &kind, &messages, &metadata) + .await + .map(|_| metadata.read_marker) + }) } } } @@ -322,85 +367,20 @@ impl History { } } - pub fn get_read_marker(&self) -> Option> { + pub fn get_read_marker(&self) -> Option { match self { History::Partial { metadata, .. } => metadata.read_marker, History::Full { metadata, .. } => metadata.read_marker, } } - pub fn update_read_marker(&mut self, read_marker: Option>) -> bool { + pub fn update_read_marker(&mut self, read_marker: ReadMarker) { let metadata = match self { History::Partial { metadata, .. } => metadata, History::Full { metadata, .. } => metadata, }; - if let Some(read_marker) = read_marker { - if let Some(history_read_marker) = metadata.read_marker { - if read_marker <= history_read_marker { - return false; - } - } - - metadata.read_marker = Some(read_marker); - } else { - return false; - } - - if let History::Partial { - messages, - unread_message_count, - metadata, - .. - } = self - { - *unread_message_count = 0; - - for message in messages { - if message.triggers_unread(&metadata.read_marker) { - *unread_message_count += 1; - } - } - } - - true - } -} - -pub fn insert_message( - messages: &mut Vec, - message: Message, - read_marker: &Option>, -) -> bool { - let message_triggers_unread = message.triggers_unread(read_marker); - - messages.push(message); - - message_triggers_unread -} - -pub async fn num_stored_unread_messages( - server: Server, - kind: Kind, - read_marker: Option>, -) -> usize { - let result = load(server, kind).await; - - if let Ok((messages, _)) = result { - messages - .into_iter() - .rev() - .map_while(|message| { - if after_read_marker(&message, &read_marker) { - Some(message) - } else { - None - } - }) - .filter(|message| message.triggers_unread(&read_marker)) - .count() - } else { - 0 + metadata.update_read_marker(read_marker); } } diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index e45a06f0e..5305cc499 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -6,7 +6,7 @@ use futures::{future, Future, FutureExt}; use itertools::Itertools; use tokio::time::Instant; -use crate::history::{self, metadata::Metadata, History}; +use crate::history::{self, History}; use crate::message::{self, Limit}; use crate::user::Nick; use crate::{config, input}; @@ -20,17 +20,26 @@ pub struct Resource { #[derive(Debug)] pub enum Message { - Loaded( + LoadFull( server::Server, history::Kind, - Result<(Vec, history::Metadata), history::Error>, + Result, + ), + UpdatePartial( + server::Server, + history::Kind, + Result, + ), + Closed( + server::Server, + history::Kind, + Result, history::Error>, ), - Closed(server::Server, history::Kind, Result<(), history::Error>), Flushed(server::Server, history::Kind, Result<(), history::Error>), } pub enum Event { - LoadHistoryMetadata(server::Server, history::Kind), + Closed(server::Server, history::Kind, Option), } #[derive(Debug, Default)] @@ -47,7 +56,7 @@ impl Manager { let added = added.into_iter().map(|resource| { async move { history::load(resource.server.clone(), resource.kind.clone()) - .map(move |result| Message::Loaded(resource.server, resource.kind, result)) + .map(move |result| Message::LoadFull(resource.server, resource.kind, result)) .await } .boxed() @@ -69,20 +78,21 @@ impl Manager { tasks } - pub fn update(&mut self, message: Message) { + pub fn update(&mut self, message: Message) -> Option { match message { - Message::Loaded(server, kind, Ok((messages, metadata))) => { + Message::LoadFull(server, kind, Ok(loaded)) => { log::debug!( "loaded history for {kind} on {server}: {} messages", - messages.len() + loaded.messages.len() ); - self.data.loaded(server, kind, messages, metadata); + self.data.load_full(server, kind, loaded); } - Message::Loaded(server, kind, Err(error)) => { + Message::LoadFull(server, kind, Err(error)) => { log::warn!("failed to load history for {kind} on {server}: {error}"); } - Message::Closed(server, kind, Ok(_)) => { + Message::Closed(server, kind, Ok(read_marker)) => { log::debug!("closed history for {kind} on {server}",); + return Some(Event::Closed(server, kind, read_marker)); } Message::Closed(server, kind, Err(error)) => { log::warn!("failed to close history for {kind} on {server}: {error}") @@ -93,7 +103,16 @@ impl Manager { Message::Flushed(server, kind, Err(error)) => { log::warn!("failed to flush history for {kind} on {server}: {error}") } + Message::UpdatePartial(server, kind, Ok(loaded)) => { + log::debug!("loaded history metadata for {kind} on {server}"); + self.data.update_partial(server, kind, loaded); + } + Message::UpdatePartial(server, kind, Err(error)) => { + log::warn!("failed to load history metadata for {kind} on {server}: {error}"); + } } + + None } pub fn tick(&mut self, now: Instant) -> Vec> { @@ -169,76 +188,52 @@ impl Manager { } } - pub fn record_input(&mut self, input: Input, user: User, channel_users: &[User]) { + pub fn record_input( + &mut self, + input: Input, + user: User, + channel_users: &[User], + ) -> Vec> { + let mut tasks = vec![]; + if let Some(messages) = input.messages(user, channel_users) { for message in messages { - self.record_message(input.server(), message); + tasks.extend(self.record_message(input.server(), message)); } } if let Some(text) = input.raw() { self.data.input.record(input.buffer(), text.to_string()); } + + tasks } pub fn record_draft(&mut self, draft: input::Draft) { self.data.input.store_draft(draft); } - pub fn record_message(&mut self, server: &Server, message: crate::Message) { + pub fn record_message( + &mut self, + server: &Server, + message: crate::Message, + ) -> Option> { self.data.add_message( server.clone(), history::Kind::from(message.target.clone()), message, - ); - } - - pub fn get_read_marker(&self, server: &Server, kind: &history::Kind) -> Option> { - self.data.get_read_marker(server, kind) + ) } pub fn update_read_marker( &mut self, - server: &Server, - kind: &history::Kind, - read_marker: Option>, - ) -> bool { + server: Server, + kind: history::Kind, + read_marker: history::ReadMarker, + ) -> Option> { self.data.update_read_marker(server, kind, read_marker) } - pub fn inc_unread_count(&mut self, server: &Server, kind: &history::Kind, increment: usize) { - if let Some(ref mut history) = self - .data - .map - .get_mut(server) - .and_then(|map| map.get_mut(kind)) - { - history.inc_unread_count(increment); - } - } - - pub fn stored_messages_may_be_unread( - &self, - server: &Server, - kind: &history::Kind, - read_marker: Option>, - ) -> bool { - if let Some(ref mut history) = self.data.map.get(server).and_then(|map| map.get(kind)) { - match history { - History::Partial { messages, .. } => { - if let Some(message) = messages.iter().next() { - if !history::after_read_marker(message, &read_marker) { - return false; - } - } - } - History::Full { .. } => return false, - } - } - - true - } - pub fn get_channel_messages( &self, server: &Server, @@ -301,15 +296,7 @@ impl Manager { .map .get(server) .and_then(|map| map.get(kind)) - .map(|history| { - matches!( - history, - History::Partial { - unread_message_count, - .. - } if *unread_message_count > 0 - ) - }) + .map(|history| history.has_unread()) .unwrap_or_default() } @@ -472,15 +459,14 @@ struct Data { } impl Data { - fn loaded( - &mut self, - server: server::Server, - kind: history::Kind, - mut messages: Vec, - metadata: Metadata, - ) { + fn load_full(&mut self, server: server::Server, kind: history::Kind, data: history::Loaded) { use std::collections::hash_map; + let history::Loaded { + mut messages, + metadata, + } = data; + match self .map .entry(server.clone()) @@ -490,21 +476,21 @@ impl Data { hash_map::Entry::Occupied(mut entry) => match entry.get_mut() { History::Partial { messages: new_messages, - last_received_at, + last_updated_at, metadata: partial_metadata, .. } => { // In-memory metadata will always be more up-to-date than disk // since we might not have flushed this yet - let metadata = partial_metadata.clone(); + let metadata = *partial_metadata; - let last_received_at = *last_received_at; + let last_updated_at = *last_updated_at; messages.extend(std::mem::take(new_messages)); entry.insert(History::Full { server, kind, messages, - last_received_at, + last_updated_at, metadata, }); } @@ -513,7 +499,7 @@ impl Data { server, kind, messages, - last_received_at: None, + last_updated_at: None, metadata, }); } @@ -523,13 +509,24 @@ impl Data { server, kind, messages, - last_received_at: None, + last_updated_at: None, metadata, }); } } } + fn update_partial( + &mut self, + server: server::Server, + kind: history::Kind, + data: history::Loaded, + ) { + if let Some(history) = self.map.get_mut(&server).and_then(|map| map.get_mut(&kind)) { + history.update_partial(data); + } + } + fn history_view( &self, server: &server::Server, @@ -657,7 +654,7 @@ impl Data { limited .iter() .rev() - .position(|message| message.server_time <= read_marker) + .position(|message| message.server_time <= read_marker.date_time()) .map_or(limited.len(), |position| limited.len() - position) }); @@ -677,7 +674,7 @@ impl Data { server: server::Server, kind: history::Kind, message: crate::Message, - ) -> Option { + ) -> Option> { use std::collections::hash_map; match self @@ -693,45 +690,62 @@ impl Data { } hash_map::Entry::Vacant(entry) => { entry - .insert(History::partial( - server.clone(), - kind.clone(), - Metadata::default(), - )) + .insert(History::partial(server.clone(), kind.clone())) .add_message(message); - Some(Event::LoadHistoryMetadata(server.clone(), kind.clone())) + Some( + async move { + let loaded = history::load(server.clone(), kind.clone()).await; + + Message::UpdatePartial(server, kind, loaded) + } + .boxed(), + ) } } } - fn get_read_marker( - &self, - server: &server::Server, - kind: &history::Kind, - ) -> Option> { - self.map.get(server)?.get(kind)?.get_read_marker() - } - fn update_read_marker( &mut self, - server: &server::Server, - kind: &history::Kind, - read_marker: Option>, - ) -> bool { - self.map + server: server::Server, + kind: history::Kind, + read_marker: history::ReadMarker, + ) -> Option> { + use std::collections::hash_map; + + match self + .map .entry(server.clone()) .or_default() .entry(kind.clone()) - .or_insert_with(|| History::partial(server.clone(), kind.clone(), Metadata::default())) - .update_read_marker(read_marker) + { + hash_map::Entry::Occupied(mut entry) => { + entry.get_mut().update_read_marker(read_marker); + + None + } + hash_map::Entry::Vacant(entry) => { + entry + .insert(History::partial(server.clone(), kind.clone())) + .update_read_marker(read_marker); + + Some( + async move { + let loaded = history::load(server.clone(), kind.clone()).await; + + Message::UpdatePartial(server, kind, loaded) + } + .boxed(), + ) + } + } } fn untrack( &mut self, server: &server::Server, kind: &history::Kind, - ) -> Option>> { + ) -> Option, history::Error>>> { self.map .get_mut(server) .and_then(|map| map.get_mut(kind).and_then(History::make_partial)) diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index 218560505..accc3a770 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -1,15 +1,95 @@ use chrono::{format::SecondsFormat, DateTime, Utc}; +use std::fmt; use std::path::PathBuf; +use std::str::FromStr; use serde::{Deserialize, Serialize}; use tokio::fs; use crate::history::{dir_path, Error, Kind}; -use crate::{server, Message}; +use crate::{message, server, Message}; -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Debug, Clone, Copy, Default, Deserialize, Serialize)] pub struct Metadata { - pub read_marker: Option>, + pub read_marker: Option, +} + +impl Metadata { + pub fn merge(self, other: Self) -> Self { + Self { + read_marker: self + .read_marker + .zip(other.read_marker) + .map(|(a, b)| a.max(b)) + .or(self.read_marker) + .or(other.read_marker), + } + } + + pub fn update_read_marker(&mut self, read_marker: ReadMarker) { + self.read_marker = Some( + self.read_marker + .map_or(read_marker, |marker| marker.max(read_marker)), + ); + } + + pub fn updated(self, messages: &[Message]) -> Self { + Self { + read_marker: ReadMarker::latest(messages).or(self.read_marker), + } + } + + pub fn unread_count(&self, messages: &[Message]) -> usize { + messages + .iter() + .filter(|message| self.triggers_unread(message)) + .count() + } + + pub fn triggers_unread(&self, message: &Message) -> bool { + message.triggers_unread() && self.after_read_marker(message) + } + + pub fn after_read_marker(&self, message: &Message) -> bool { + self.read_marker.is_none() + || self + .read_marker + .is_some_and(|read_marker| message.server_time > read_marker.0) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default, Deserialize, Serialize)] +pub struct ReadMarker(DateTime); + +impl ReadMarker { + pub fn latest(messages: &[Message]) -> Option { + messages + .iter() + .rev() + .find(|message| !matches!(message.target.source(), message::Source::Internal(_))) + .map(|message| message.server_time) + .map(Self) + } + + pub fn date_time(&self) -> DateTime { + self.0 + } +} + +impl FromStr for ReadMarker { + type Err = chrono::ParseError; + + fn from_str(s: &str) -> Result { + DateTime::parse_from_rfc3339(s) + .map(|dt| dt.with_timezone(&Utc)) + .map(Self) + } +} + +impl fmt::Display for ReadMarker { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.to_rfc3339_opts(SecondsFormat::Millis, true).fmt(f) + } } pub async fn load(server: server::Server, kind: Kind) -> Result { @@ -45,16 +125,3 @@ async fn path(server: &server::Server, kind: &Kind) -> Result { Ok(dir.join(format!("{hashed_name}.json"))) } - -pub fn after_read_marker(message: &Message, read_marker: &Option>) -> bool { - read_marker.is_none() - || read_marker.is_some_and(|read_marker| message.server_time > read_marker) -} - -pub fn read_marker_to_string(read_marker: &Option>) -> String { - if let Some(read_marker) = read_marker { - read_marker.to_rfc3339_opts(SecondsFormat::Millis, true) - } else { - "*".to_string() - } -} diff --git a/data/src/message.rs b/data/src/message.rs index a4334fdc2..d730aa20e 100644 --- a/data/src/message.rs +++ b/data/src/message.rs @@ -14,7 +14,6 @@ pub use self::formatting::Formatting; pub use self::source::Source; use crate::config::buffer::UsernameFormat; -use crate::history::after_read_marker; use crate::time::{self, Posix}; use crate::user::{Nick, NickRef}; use crate::{ctcp, Config, User}; @@ -153,7 +152,7 @@ pub struct Message { } impl Message { - pub fn triggers_unread(&self, read_marker: &Option>) -> bool { + pub fn triggers_unread(&self) -> bool { matches!(self.direction, Direction::Received) && match self.target.source() { Source::User(_) => true, @@ -167,7 +166,6 @@ impl Message { } _ => false, } - && after_read_marker(self, read_marker) } pub fn received<'a>( diff --git a/src/main.rs b/src/main.rs index 1fa20e123..c6f477a65 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,9 +19,9 @@ mod window; use std::env; use std::time::{Duration, Instant}; -use chrono::{DateTime, Utc}; +use chrono::Utc; use data::config::{self, Config}; -use data::history::{self, metadata::read_marker_to_string}; +use data::history; use data::version::Version; use data::{environment, server, version, Url, User}; use iced::widget::{column, container}; @@ -218,7 +218,6 @@ pub enum Message { RouteReceived(String), Window(window::Id, window::Event), WindowSettingsSaved(Result<(), window::Error>), - UpdateReadMarker(server::Server, history::Kind, Option>), } impl Halloy { @@ -646,58 +645,18 @@ impl Halloy { commands.push(command.map(Message::Dashboard)); } } - data::client::Event::LoadHistoryMetadata(target) => { - let server = server.clone(); + data::client::Event::UpdateReadMarker(target, read_marker) => { let kind = history::Kind::from(target); - commands.push(Task::perform( - history::metadata::load(server.clone(), kind.clone()), - move |metadata| { - let metadata = metadata.unwrap_or_default(); - - Message::UpdateReadMarker( + commands.push( + dashboard + .update_read_marker( server.clone(), - kind.clone(), - metadata.read_marker, + kind, + read_marker, ) - }, - )); - } - data::client::Event::UpdateReadMarker(target, read_marker) => { - let server = server.clone(); - let kind = history::Kind::from(target); - - if dashboard.update_read_marker(&server, &kind, read_marker) - { - log::debug!( - "[{server}] updated read-marker for {kind} to {}", - read_marker_to_string(&read_marker), - ); - - if dashboard.stored_messages_may_be_unread( - &server, - &kind, - read_marker, - ) { - commands.push(Task::perform( - history::num_stored_unread_messages( - server.clone(), - kind.clone(), - read_marker, - ), - move |num_stored_unread_messages| { - Message::Dashboard( - dashboard::Message::IncrementUnread( - server.clone(), - kind.clone(), - read_marker, - num_stored_unread_messages, - ), - ) - }, - )); - } - } + .map(Message::Dashboard), + ); } } } @@ -843,49 +802,6 @@ impl Halloy { log::error!("window settings failed to save: {:?}", err) } - Task::none() - } - Message::UpdateReadMarker(server, kind, read_marker) => { - if let Screen::Dashboard(dashboard) = &mut self.screen { - if dashboard.update_read_marker(&server, &kind, read_marker) { - log::debug!( - "[{server}] updated read-marker for {kind} to {}", - read_marker_to_string(&read_marker), - ); - - if let Some(read_marker) = read_marker { - match kind.clone() { - history::Kind::Server => (), - history::Kind::Channel(channel) => { - self.clients.send_markread(&server, &channel, read_marker) - } - history::Kind::Query(nick) => { - self.clients - .send_markread(&server, nick.as_ref(), read_marker) - } - } - } - - if dashboard.stored_messages_may_be_unread(&server, &kind, read_marker) { - return Task::perform( - history::num_stored_unread_messages( - server.clone(), - kind.clone(), - read_marker, - ), - move |num_stored_unread_messages| { - Message::Dashboard(dashboard::Message::IncrementUnread( - server.clone(), - kind.clone(), - read_marker, - num_stored_unread_messages, - )) - }, - ); - } - } - } - Task::none() } } diff --git a/src/screen/dashboard.rs b/src/screen/dashboard.rs index fe60790e9..b6f74b255 100644 --- a/src/screen/dashboard.rs +++ b/src/screen/dashboard.rs @@ -1,5 +1,6 @@ use chrono::{DateTime, Utc}; use data::environment::RELEASE_WEBSITE; +use data::history::ReadMarker; use std::collections::HashMap; use std::path::PathBuf; use std::slice; @@ -59,8 +60,6 @@ pub enum Message { CloseContextMenu(bool), ThemeEditor(theme_editor::Message), ConfigReloaded(Result), - IncrementUnread(Server, history::Kind, Option>, usize), - UpdateReadMarker(Server, history::Kind, Option>), } #[derive(Debug)] @@ -156,12 +155,12 @@ impl Dashboard { config, ); - let command = command.map(move |message| { + let task = command.map(move |message| { Message::Pane(window, pane::Message::Buffer(id, message)) }); let Some(event) = event else { - return (command, None); + return (task, None); }; match event { @@ -172,11 +171,11 @@ impl Dashboard { mode, ) => { let Some(buffer) = pane.buffer.data() else { - return (command, None); + return (task, None); }; let Some(target) = buffer.target() else { - return (command, None); + return (task, None); }; let command = data::Command::Mode( @@ -228,12 +227,23 @@ impl Dashboard { if let Some(messages) = input.messages(user, channel_users) { + let mut tasks = vec![task]; + for message in messages { - self.history.record_message( - input.server(), - message, - ); + if let Some(task) = + self.history.record_message( + input.server(), + message, + ) + { + tasks.push(Task::perform( + task, + Message::History, + )); + } } + + return (Task::batch(tasks), None); } } } @@ -246,7 +256,7 @@ impl Dashboard { ); return ( Task::batch(vec![ - command, + task, self.open_buffer( buffer, config.buffer.clone().into(), @@ -261,12 +271,12 @@ impl Dashboard { let Some((_, pane, history)) = self.get_focused_with_history_mut(main_window) else { - return (command, None); + return (task, None); }; return ( Task::batch(vec![ - command, + task, pane.buffer .insert_user_to_input(nick, history) .map(move |message| { @@ -287,7 +297,7 @@ impl Dashboard { return ( Task::batch(vec![ - command, + task, Task::perform( async move { rfd::AsyncFileDialog::new() @@ -321,7 +331,7 @@ impl Dashboard { { return ( Task::batch(vec![ - command, + task, self.open_channel( server, channel, @@ -336,7 +346,7 @@ impl Dashboard { } } - return (command, None); + return (task, None); } } pane::Message::ToggleShowUserList => { @@ -496,23 +506,15 @@ impl Dashboard { } } Message::History(message) => { - if let history::manager::Message::Closed(ref server, ref kind, Ok(_)) = message { - match kind { - history::Kind::Server => (), - history::Kind::Channel(channel) => { - if let Some(read_marker) = self.get_read_marker(server, kind) { - clients.send_markread(server, channel, read_marker) - } - } - history::Kind::Query(nick) => { - if let Some(read_marker) = self.get_read_marker(server, kind) { - clients.send_markread(server, nick.as_ref(), read_marker) + if let Some(event) = self.history.update(message) { + match event { + history::manager::Event::Closed(server, kind, read_marker) => { + if let Some((target, read_marker)) = kind.target().zip(read_marker) { + clients.send_markread(&server, target, read_marker); } } } } - - self.history.update(message); } Message::DashboardSaved(Ok(_)) => { log::info!("dashboard saved"); @@ -814,14 +816,6 @@ impl Dashboard { Message::ConfigReloaded(config) => { return (Task::none(), Some(Event::ConfigReloaded(config))); } - Message::IncrementUnread(server, kind, read_marker, increment) => { - if self.get_read_marker(&server, &kind) == read_marker { - self.inc_unread_count(&server, &kind, increment); - } - } - Message::UpdateReadMarker(server, kind, read_marker) => { - self.update_read_marker(&server, &kind, read_marker); - } } (Task::none(), None) @@ -1349,31 +1343,17 @@ impl Dashboard { ); } - pub fn get_read_marker(&self, server: &Server, kind: &history::Kind) -> Option> { - self.history.get_read_marker(server, kind) - } - pub fn update_read_marker( &mut self, - server: &Server, - kind: &history::Kind, - read_marker: Option>, - ) -> bool { - self.history.update_read_marker(server, kind, read_marker) - } - - pub fn inc_unread_count(&mut self, server: &Server, kind: &history::Kind, increment: usize) { - self.history.inc_unread_count(server, kind, increment); - } - - pub fn stored_messages_may_be_unread( - &self, - server: &Server, - kind: &history::Kind, - read_marker: Option>, - ) -> bool { - self.history - .stored_messages_may_be_unread(server, kind, read_marker) + server: Server, + kind: history::Kind, + read_marker: ReadMarker, + ) -> Task { + if let Some(task) = self.history.update_read_marker(server, kind, read_marker) { + Task::perform(task, Message::History) + } else { + Task::none() + } } fn get_focused_mut( From 45b4eebbe0f75469e393d10d920535c4d0571016 Mon Sep 17 00:00:00 2001 From: Andrew Baldwin Date: Tue, 17 Sep 2024 23:53:09 -0700 Subject: [PATCH 06/29] Limit `last_on_disk` calculation to messages that trigger unread. --- data/src/history.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 8e460458a..9b013127e 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -203,7 +203,11 @@ impl History { } = self { *metadata = metadata.merge(loaded.metadata); - *last_on_disk = loaded.messages.last().map(|message| message.server_time); + *last_on_disk = loaded + .messages + .iter() + .rev() + .find_map(|message| message.triggers_unread().then_some(message.server_time)); } } @@ -268,7 +272,9 @@ impl History { *last_updated_at = None; - *last_on_disk = messages.last().map(|message| message.server_time); + *last_on_disk = messages.iter().rev().find_map(|message| { + message.triggers_unread().then_some(message.server_time) + }); return Some( async move { append(&server, &kind, messages, &metadata).await } @@ -336,7 +342,9 @@ impl History { messages: vec![], last_updated_at: None, metadata, - last_on_disk: messages.last().map(|message| message.server_time), + last_on_disk: messages.iter().rev().find_map(|message| { + message.triggers_unread().then_some(message.server_time) + }), }; Some(async move { From fee53fde0c949a0344dd54c1bb65ac27a69a883b Mon Sep 17 00:00:00 2001 From: Andrew Baldwin Date: Wed, 18 Sep 2024 10:45:07 -0700 Subject: [PATCH 07/29] Require that at least one unread message exists in memory to count as `has_unread`. --- data/src/history.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 9b013127e..57d02ee4b 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -221,10 +221,9 @@ impl History { } => { // Read marker is prior to last message on disk // or prior to unflushed messages in memory - metadata.read_marker.is_none() - || last_on_disk - .zip(metadata.read_marker) - .map_or(false, |(a, b)| a > b.date_time()) + last_on_disk + .zip(metadata.read_marker) + .map_or(false, |(a, b)| a > b.date_time()) || metadata.unread_count(messages) > 0 } History::Full { .. } => false, From e8fdf521fc2bc23b7af1534d4cc77faa45239f2d Mon Sep 17 00:00:00 2001 From: Andrew Baldwin Date: Wed, 18 Sep 2024 11:35:19 -0700 Subject: [PATCH 08/29] Indicate unread if there is a message on disk and there is no specified read marker. --- data/src/history.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 57d02ee4b..b0a198882 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -221,10 +221,10 @@ impl History { } => { // Read marker is prior to last message on disk // or prior to unflushed messages in memory - last_on_disk - .zip(metadata.read_marker) - .map_or(false, |(a, b)| a > b.date_time()) - || metadata.unread_count(messages) > 0 + last_on_disk.zip(metadata.read_marker).map_or( + last_on_disk.is_some() && metadata.read_marker.is_none(), + |(a, b)| a > b.date_time(), + ) || metadata.unread_count(messages) > 0 } History::Full { .. } => false, } From ed6c44266c1c71110b8c424bde0305331929eb09 Mon Sep 17 00:00:00 2001 From: Andrew Baldwin Date: Wed, 18 Sep 2024 11:04:44 -0700 Subject: [PATCH 09/29] Keep existing `last_on_disk` if no messages trigger unread. --- data/src/history.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index b0a198882..4f061f852 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -164,6 +164,13 @@ async fn path(server: &server::Server, kind: &Kind) -> Result { Ok(dir.join(format!("{hashed_name}.json.gz"))) } +fn last_in_messages(messages: &[Message]) -> Option> { + messages + .iter() + .rev() + .find_map(|message| message.triggers_unread().then_some(message.server_time)) +} + #[derive(Debug)] pub enum History { Partial { @@ -203,11 +210,7 @@ impl History { } = self { *metadata = metadata.merge(loaded.metadata); - *last_on_disk = loaded - .messages - .iter() - .rev() - .find_map(|message| message.triggers_unread().then_some(message.server_time)); + *last_on_disk = last_in_messages(&loaded.messages).or(*last_on_disk); } } @@ -271,9 +274,7 @@ impl History { *last_updated_at = None; - *last_on_disk = messages.iter().rev().find_map(|message| { - message.triggers_unread().then_some(message.server_time) - }); + *last_on_disk = last_in_messages(&messages).or(*last_on_disk); return Some( async move { append(&server, &kind, messages, &metadata).await } @@ -341,9 +342,7 @@ impl History { messages: vec![], last_updated_at: None, metadata, - last_on_disk: messages.iter().rev().find_map(|message| { - message.triggers_unread().then_some(message.server_time) - }), + last_on_disk: last_in_messages(&messages), }; Some(async move { From d3271800676810fdd5f4bd1ad21d96f07a89a545 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 14:00:13 -0700 Subject: [PATCH 10/29] Update metadadata when closing --- data/src/history.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 4f061f852..f62eb9c11 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -362,14 +362,20 @@ impl History { messages, metadata, .. - } => append(&server, &kind, messages, &metadata).await, + } => { + let metadata = metadata.updated(&messages); + append(&server, &kind, messages, &metadata).await + } History::Full { server, kind, messages, metadata, .. - } => overwrite(&server, &kind, &messages, &metadata).await, + } => { + let metadata = metadata.updated(&messages); + overwrite(&server, &kind, &messages, &metadata).await + } } } From a24a09453c8e151a2ae1c0ec13879cf663b26305 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 14:06:04 -0700 Subject: [PATCH 11/29] Merge loaded w/ in-memory metadata when loading full history --- data/src/history/manager.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 5305cc499..7d0e6b042 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -480,9 +480,7 @@ impl Data { metadata: partial_metadata, .. } => { - // In-memory metadata will always be more up-to-date than disk - // since we might not have flushed this yet - let metadata = *partial_metadata; + let metadata = partial_metadata.merge(metadata); let last_updated_at = *last_updated_at; messages.extend(std::mem::take(new_messages)); From ae9aa40312ad47d70335f17e9a51114054451327 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 14:06:57 -0700 Subject: [PATCH 12/29] Remove dead code --- data/src/history.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index f62eb9c11..69958a09a 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -379,13 +379,6 @@ impl History { } } - pub fn get_read_marker(&self) -> Option { - match self { - History::Partial { metadata, .. } => metadata.read_marker, - History::Full { metadata, .. } => metadata.read_marker, - } - } - pub fn update_read_marker(&mut self, read_marker: ReadMarker) { let metadata = match self { History::Partial { metadata, .. } => metadata, From 686cbf14ac9450df5f2184f4f4ede24dce662135 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 14:21:12 -0700 Subject: [PATCH 13/29] Cleanup unread logic --- data/src/history.rs | 15 +++++++++++---- data/src/history/metadata.rs | 18 ------------------ 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 69958a09a..af8804184 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -224,10 +224,17 @@ impl History { } => { // Read marker is prior to last message on disk // or prior to unflushed messages in memory - last_on_disk.zip(metadata.read_marker).map_or( - last_on_disk.is_some() && metadata.read_marker.is_none(), - |(a, b)| a > b.date_time(), - ) || metadata.unread_count(messages) > 0 + if let Some(read_marker) = metadata.read_marker { + last_on_disk.is_some_and(|last| read_marker.date_time() < last) + || messages.iter().any(|message| { + read_marker.date_time() < message.server_time + && message.triggers_unread() + }) + } + // Default state == unread + else { + true + } } History::Full { .. } => false, } diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index accc3a770..3778ab98b 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -38,24 +38,6 @@ impl Metadata { read_marker: ReadMarker::latest(messages).or(self.read_marker), } } - - pub fn unread_count(&self, messages: &[Message]) -> usize { - messages - .iter() - .filter(|message| self.triggers_unread(message)) - .count() - } - - pub fn triggers_unread(&self, message: &Message) -> bool { - message.triggers_unread() && self.after_read_marker(message) - } - - pub fn after_read_marker(&self, message: &Message) -> bool { - self.read_marker.is_none() - || self - .read_marker - .is_some_and(|read_marker| message.server_time > read_marker.0) - } } #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default, Deserialize, Serialize)] From fcf4516d77537dc150f3fa2bd90c8540bc395fa3 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 14:29:14 -0700 Subject: [PATCH 14/29] Make max_triggers_unread stateful for all messages --- data/src/history.rs | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index af8804184..54bdae866 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -164,7 +164,7 @@ async fn path(server: &server::Server, kind: &Kind) -> Result { Ok(dir.join(format!("{hashed_name}.json.gz"))) } -fn last_in_messages(messages: &[Message]) -> Option> { +fn last_triggers_unread(messages: &[Message]) -> Option> { messages .iter() .rev() @@ -179,7 +179,7 @@ pub enum History { messages: Vec, last_updated_at: Option, metadata: Metadata, - last_on_disk: Option>, + max_triggers_unread: Option>, }, Full { server: server::Server, @@ -198,38 +198,32 @@ impl History { messages: vec![], last_updated_at: None, metadata: Metadata::default(), - last_on_disk: None, + max_triggers_unread: None, } } pub fn update_partial(&mut self, loaded: Loaded) { if let Self::Partial { metadata, - last_on_disk, + max_triggers_unread, .. } = self { *metadata = metadata.merge(loaded.metadata); - *last_on_disk = last_in_messages(&loaded.messages).or(*last_on_disk); + *max_triggers_unread = last_triggers_unread(&loaded.messages).max(*max_triggers_unread); } } fn has_unread(&self) -> bool { match self { History::Partial { - messages, metadata, - last_on_disk, + max_triggers_unread, .. } => { - // Read marker is prior to last message on disk - // or prior to unflushed messages in memory + // Read marker is prior to last known message which triggers unread if let Some(read_marker) = metadata.read_marker { - last_on_disk.is_some_and(|last| read_marker.date_time() < last) - || messages.iter().any(|message| { - read_marker.date_time() < message.server_time - && message.triggers_unread() - }) + max_triggers_unread.is_some_and(|max| read_marker.date_time() < max) } // Default state == unread else { @@ -241,6 +235,16 @@ impl History { } fn add_message(&mut self, message: Message) { + if message.triggers_unread() { + if let History::Partial { + max_triggers_unread, + .. + } = self + { + *max_triggers_unread = (*max_triggers_unread).max(Some(message.server_time)); + } + } + match self { History::Partial { messages, @@ -267,7 +271,6 @@ impl History { messages, metadata, last_updated_at, - last_on_disk, .. } => { if let Some(last_received) = *last_updated_at { @@ -281,8 +284,6 @@ impl History { *last_updated_at = None; - *last_on_disk = last_in_messages(&messages).or(*last_on_disk); - return Some( async move { append(&server, &kind, messages, &metadata).await } .boxed(), @@ -349,7 +350,7 @@ impl History { messages: vec![], last_updated_at: None, metadata, - last_on_disk: last_in_messages(&messages), + max_triggers_unread: last_triggers_unread(&messages), }; Some(async move { From f6028d11de68fb3c523ca2f958e06a448b021197 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 14:44:56 -0700 Subject: [PATCH 15/29] Cleanup merge logic --- data/src/history/metadata.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index 3778ab98b..9adeae59e 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -17,20 +17,12 @@ pub struct Metadata { impl Metadata { pub fn merge(self, other: Self) -> Self { Self { - read_marker: self - .read_marker - .zip(other.read_marker) - .map(|(a, b)| a.max(b)) - .or(self.read_marker) - .or(other.read_marker), + read_marker: self.read_marker.max(other.read_marker), } } pub fn update_read_marker(&mut self, read_marker: ReadMarker) { - self.read_marker = Some( - self.read_marker - .map_or(read_marker, |marker| marker.max(read_marker)), - ); + self.read_marker = self.read_marker.max(Some(read_marker)); } pub fn updated(self, messages: &[Message]) -> Self { From 444994437d460528ec792165048b431a39aa7523 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 14:48:02 -0700 Subject: [PATCH 16/29] Don't show unread indicator when there's no messages --- data/src/history.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 54bdae866..fa52f649f 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -225,9 +225,9 @@ impl History { if let Some(read_marker) = metadata.read_marker { max_triggers_unread.is_some_and(|max| read_marker.date_time() < max) } - // Default state == unread + // Default state == unread if theres messages that trigger indicator else { - true + max_triggers_unread.is_some() } } History::Full { .. } => false, From a6f734aff9fc4c571a98c6a8585279939feea49a Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 15:12:23 -0700 Subject: [PATCH 17/29] Send markread when closing --- data/src/client.rs | 4 ++++ data/src/history.rs | 12 +++++++--- data/src/history/manager.rs | 44 +++++++++++++------------------------ src/main.rs | 2 +- src/screen/dashboard.rs | 28 ++++++++++++++++++----- 5 files changed, 51 insertions(+), 39 deletions(-) diff --git a/data/src/client.rs b/data/src/client.rs index 18a0f28a1..cce7b0f88 100644 --- a/data/src/client.rs +++ b/data/src/client.rs @@ -1557,6 +1557,10 @@ impl Map { } }) } + + pub fn take(&mut self) -> Self { + Self(std::mem::take(&mut self.0)) + } } #[derive(Debug, Clone)] diff --git a/data/src/history.rs b/data/src/history.rs index fa52f649f..5343ae64b 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -362,7 +362,7 @@ impl History { } } - async fn close(self) -> Result<(), Error> { + async fn close(self) -> Result, Error> { match self { History::Partial { server, @@ -372,7 +372,10 @@ impl History { .. } => { let metadata = metadata.updated(&messages); - append(&server, &kind, messages, &metadata).await + + append(&server, &kind, messages, &metadata).await?; + + Ok(metadata.read_marker) } History::Full { server, @@ -382,7 +385,10 @@ impl History { .. } => { let metadata = metadata.updated(&messages); - overwrite(&server, &kind, &messages, &metadata).await + + overwrite(&server, &kind, &messages, &metadata).await?; + + Ok(metadata.read_marker) } } } diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 7d0e6b042..64cf3e1c4 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -104,7 +104,7 @@ impl Manager { log::warn!("failed to flush history for {kind} on {server}: {error}") } Message::UpdatePartial(server, kind, Ok(loaded)) => { - log::debug!("loaded history metadata for {kind} on {server}"); + log::debug!("updating partial history for {kind} on {server}"); self.data.update_partial(server, kind, loaded); } Message::UpdatePartial(server, kind, Err(error)) => { @@ -123,46 +123,26 @@ impl Manager { &mut self, server: Server, kind: history::Kind, - ) -> Option> { + ) -> Option)>> { let history = self.data.map.get_mut(&server)?.remove(&kind)?; Some(async move { match history.close().await { - Ok(_) => { + Ok(marker) => { log::debug!("closed history for {kind} on {server}",); + (server, kind, marker) } Err(error) => { log::warn!("failed to close history for {kind} on {server}: {error}"); + (server, kind, None) } } }) } - pub fn close_server(&mut self, server: Server) -> Option> { - let map = self.data.map.remove(&server)?; - - Some(async move { - let tasks = map.into_iter().map(move |(kind, state)| { - let server = server.clone(); - state.close().map(move |result| (server, kind, result)) - }); - - let results = future::join_all(tasks).await; - - for (server, kind, result) in results { - match result { - Ok(_) => { - log::debug!("closed history for {kind} on {server}",); - } - Err(error) => { - log::warn!("failed to close history for {kind} on {server}: {error}"); - } - } - } - }) - } - - pub fn close_all(&mut self) -> impl Future { + pub fn close_all( + &mut self, + ) -> impl Future)>> { let map = std::mem::take(&mut self.data).map; async move { @@ -175,16 +155,22 @@ impl Manager { let results = future::join_all(tasks).await; + let mut output = vec![]; + for (server, kind, result) in results { match result { - Ok(_) => { + Ok(marker) => { log::debug!("closed history for {kind} on {server}",); + output.push((server, kind, marker)); } Err(error) => { log::warn!("failed to close history for {kind} on {server}: {error}"); + output.push((server, kind, None)); } } } + + output } } diff --git a/src/main.rs b/src/main.rs index c6f477a65..fcddfb7b8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -778,7 +778,7 @@ impl Halloy { } window::Event::CloseRequested => { if let Screen::Dashboard(dashboard) = &mut self.screen { - return dashboard.exit().then(|_| iced::exit()); + return dashboard.exit(self.clients.take()).then(|_| iced::exit()); } else { return iced::exit(); } diff --git a/src/screen/dashboard.rs b/src/screen/dashboard.rs index b6f74b255..f44ab1c04 100644 --- a/src/screen/dashboard.rs +++ b/src/screen/dashboard.rs @@ -52,7 +52,7 @@ pub enum Message { SelectedText(Vec<(f32, String)>), History(history::manager::Message), DashboardSaved(Result<(), data::dashboard::Error>), - CloseHistory, + CloseHistory(Server, history::Kind, Option), Task(command_bar::Message), Shortcut(shortcut::Command), FileTransfer(file_transfer::task::Update), @@ -522,7 +522,11 @@ impl Dashboard { Message::DashboardSaved(Err(error)) => { log::warn!("error saving dashboard: {error}"); } - Message::CloseHistory => {} + Message::CloseHistory(server, kind, marker) => { + if let Some((target, marker)) = kind.target().zip(marker) { + clients.send_markread(&server, target, marker); + } + } Message::Task(message) => { let Some(command_bar) = &mut self.command_bar else { return (Task::none(), None); @@ -1173,7 +1177,11 @@ impl Dashboard { tasks.push( self.history .close(server, history::Kind::Channel(channel)) - .map(|task| Task::perform(task, |_| Message::CloseHistory)) + .map(|task| { + Task::perform(task, |(server, kind, marker)| { + Message::CloseHistory(server, kind, marker) + }) + }) .unwrap_or_else(Task::none), ); @@ -1183,7 +1191,11 @@ impl Dashboard { tasks.push( self.history .close(server, history::Kind::Query(nick)) - .map(|task| Task::perform(task, |_| Message::CloseHistory)) + .map(|task| { + Task::perform(task, |(server, kind, marker)| { + Message::CloseHistory(server, kind, marker) + }) + }) .unwrap_or_else(Task::none), ); @@ -1801,13 +1813,17 @@ impl Dashboard { } } - pub fn exit(&mut self) -> Task<()> { + pub fn exit(&mut self, mut clients: client::Map) -> Task<()> { let history = self.history.close_all(); let last_changed = self.last_changed; let dashboard = data::Dashboard::from(&*self); let task = async move { - history.await; + for (server, kind, marker) in history.await { + if let Some((target, marker)) = kind.target().zip(marker) { + clients.send_markread(&server, target, marker); + } + } if last_changed.is_some() { match dashboard.save().await { From 0fbcafe5c45dd371b3060af4017c1ad834e66911 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Wed, 18 Sep 2024 15:13:09 -0700 Subject: [PATCH 18/29] Don't clobber partial read marker --- data/src/history.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 5343ae64b..72641ac3e 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -371,8 +371,6 @@ impl History { metadata, .. } => { - let metadata = metadata.updated(&messages); - append(&server, &kind, messages, &metadata).await?; Ok(metadata.read_marker) From b164212b06e72d033c3b1874edfd0f5456da8931 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Mon, 23 Sep 2024 11:24:09 -0700 Subject: [PATCH 19/29] Hook up new history tasks to all code-paths --- data/src/history/manager.rs | 9 +- src/buffer.rs | 5 +- src/buffer/channel.rs | 6 +- src/buffer/input_view.rs | 15 ++- src/buffer/query.rs | 6 +- src/buffer/server.rs | 21 +++-- src/main.rs | 173 ++++++++++++++++++++++------------ src/screen/dashboard.rs | 179 +++++++----------------------------- 8 files changed, 182 insertions(+), 232 deletions(-) diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 64cf3e1c4..174b0d6cc 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -292,7 +292,7 @@ impl Manager { broadcast: Broadcast, config: &Config, sent_time: DateTime, - ) { + ) -> Vec> { let map = self.data.map.entry(server.clone()).or_default(); let channels = map @@ -410,9 +410,10 @@ impl Manager { } }; - messages.into_iter().for_each(|message| { - self.record_message(server, message); - }); + messages + .into_iter() + .filter_map(|message| self.record_message(server, message)) + .collect() } pub fn input<'a>(&'a self, buffer: &Buffer) -> input::Cache<'a> { diff --git a/src/buffer.rs b/src/buffer.rs index 1951cb021..d8f393e0a 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -37,10 +37,10 @@ pub enum Message { FileTransfers(file_transfers::Message), } -#[derive(Debug, Clone)] pub enum Event { UserContext(user_context::Event), OpenChannel(String), + History(Task), } impl Buffer { @@ -73,6 +73,7 @@ impl Buffer { let event = event.map(|event| match event { channel::Event::UserContext(event) => Event::UserContext(event), channel::Event::OpenChannel(channel) => Event::OpenChannel(channel), + channel::Event::History(task) => Event::History(task), }); (command.map(Message::Channel), event) @@ -83,6 +84,7 @@ impl Buffer { let event = event.map(|event| match event { server::Event::UserContext(event) => Event::UserContext(event), server::Event::OpenChannel(channel) => Event::OpenChannel(channel), + server::Event::History(task) => Event::History(task), }); (command.map(Message::Server), event) @@ -93,6 +95,7 @@ impl Buffer { let event = event.map(|event| match event { query::Event::UserContext(event) => Event::UserContext(event), query::Event::OpenChannel(channel) => Event::OpenChannel(channel), + query::Event::History(task) => Event::History(task), }); (command.map(Message::Query), event) diff --git a/src/buffer/channel.rs b/src/buffer/channel.rs index 7e196f254..3f8bb4a95 100644 --- a/src/buffer/channel.rs +++ b/src/buffer/channel.rs @@ -19,10 +19,10 @@ pub enum Message { Topic(topic::Message), } -#[derive(Debug, Clone)] pub enum Event { UserContext(user_context::Event), OpenChannel(String), + History(Task), } pub fn view<'a>( @@ -341,13 +341,13 @@ impl Channel { let command = command.map(Message::InputView); match event { - Some(input_view::Event::InputSent) => { + Some(input_view::Event::InputSent { history_task }) => { let command = Task::batch(vec![ command, self.scroll_view.scroll_to_end().map(Message::ScrollView), ]); - (command, None) + (command, Some(Event::History(history_task))) } None => (command, None), } diff --git a/src/buffer/input_view.rs b/src/buffer/input_view.rs index 6292bdd45..201d1a0b5 100644 --- a/src/buffer/input_view.rs +++ b/src/buffer/input_view.rs @@ -11,7 +11,9 @@ use crate::widget::{anchored_overlay, key_press, Element}; mod completion; pub enum Event { - InputSent, + InputSent { + history_task: Task, + }, } #[derive(Debug, Clone)] @@ -177,6 +179,8 @@ impl State { clients.send(buffer, encoded); } + let mut history_task = Task::none(); + if let Some(nick) = clients.nickname(buffer.server()) { let mut user = nick.to_owned().into(); let mut channel_users = &[][..]; @@ -192,10 +196,15 @@ impl State { } } - history.record_input(input, user, channel_users); + history_task = Task::batch( + history + .record_input(input, user, channel_users) + .into_iter() + .map(Task::future), + ); } - (Task::none(), Some(Event::InputSent)) + (Task::none(), Some(Event::InputSent { history_task })) } else { (Task::none(), None) } diff --git a/src/buffer/query.rs b/src/buffer/query.rs index abef293ff..9a6b8ebb4 100644 --- a/src/buffer/query.rs +++ b/src/buffer/query.rs @@ -13,10 +13,10 @@ pub enum Message { InputView(input_view::Message), } -#[derive(Debug, Clone)] pub enum Event { UserContext(user_context::Event), OpenChannel(String), + History(Task), } pub fn view<'a>( @@ -255,13 +255,13 @@ impl Query { let command = command.map(Message::InputView); match event { - Some(input_view::Event::InputSent) => { + Some(input_view::Event::InputSent { history_task }) => { let command = Task::batch(vec![ command, self.scroll_view.scroll_to_end().map(Message::ScrollView), ]); - (command, None) + (command, Some(Event::History(history_task))) } None => (command, None), } diff --git a/src/buffer/server.rs b/src/buffer/server.rs index d6d08369b..71a439356 100644 --- a/src/buffer/server.rs +++ b/src/buffer/server.rs @@ -12,10 +12,10 @@ pub enum Message { InputView(input_view::Message), } -#[derive(Debug, Clone)] pub enum Event { UserContext(user_context::Event), OpenChannel(String), + History(Task), } pub fn view<'a>( @@ -143,15 +143,16 @@ impl Server { .update(message, &self.buffer, clients, history, config); let command = command.map(Message::InputView); - let task = match event { - Some(input_view::Event::InputSent) => Task::batch(vec![ - command, - self.scroll_view.scroll_to_end().map(Message::ScrollView), - ]), - None => command, - }; - - (task, None) + match event { + Some(input_view::Event::InputSent { history_task }) => ( + Task::batch(vec![ + command, + self.scroll_view.scroll_to_end().map(Message::ScrollView), + ]), + Some(Event::History(history_task)), + ), + None => (command, None), + } } } } diff --git a/src/main.rs b/src/main.rs index fcddfb7b8..a5298425a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,6 +22,7 @@ use std::time::{Duration, Instant}; use chrono::Utc; use data::config::{self, Config}; use data::history; +use data::history::manager::Broadcast; use data::version::Version; use data::{environment, server, version, Url, User}; use iced::widget::{column, container}; @@ -404,14 +405,21 @@ impl Halloy { if is_initial { // Intial is sent when first trying to connect - dashboard.broadcast_connecting(&server, &self.config, sent_time); + dashboard + .broadcast(&server, &self.config, sent_time, Broadcast::Connecting) + .map(Message::Dashboard) } else { notification::disconnected(&self.config.notifications, &server); - dashboard.broadcast_disconnected(&server, error, &self.config, sent_time); + dashboard + .broadcast( + &server, + &self.config, + sent_time, + Broadcast::Disconnected { error }, + ) + .map(Message::Dashboard) } - - Task::none() } stream::Update::Connected { server, @@ -428,14 +436,16 @@ impl Halloy { if is_initial { notification::connected(&self.config.notifications, &server); - dashboard.broadcast_connected(&server, &self.config, sent_time); + dashboard + .broadcast(&server, &self.config, sent_time, Broadcast::Connected) + .map(Message::Dashboard) } else { notification::reconnected(&self.config.notifications, &server); - dashboard.broadcast_reconnected(&server, &self.config, sent_time); + dashboard + .broadcast(&server, &self.config, sent_time, Broadcast::Reconnected) + .map(Message::Dashboard) } - - Task::none() } stream::Update::ConnectionFailed { server, @@ -446,9 +456,14 @@ impl Halloy { return Task::none(); }; - dashboard.broadcast_connection_failed(&server, error, &self.config, sent_time); - - Task::none() + dashboard + .broadcast( + &server, + &self.config, + sent_time, + Broadcast::ConnectionFailed { error }, + ) + .map(Message::Dashboard) } stream::Update::MessagesReceived(server, messages) => { let Screen::Dashboard(dashboard) = &mut self.screen else { @@ -481,7 +496,11 @@ impl Halloy { resolve_user_attributes, channel_users, ) { - dashboard.record_message(&server, message); + commands.push( + dashboard + .record_message(&server, message) + .map(Message::Dashboard), + ); } } data::client::Event::WithTarget(encoded, our_nick, target) => { @@ -492,9 +511,13 @@ impl Halloy { resolve_user_attributes, channel_users, ) { - dashboard.record_message( - &server, - message.with_target(target), + commands.push( + dashboard + .record_message( + &server, + message.with_target(target), + ) + .map(Message::Dashboard), ); } } @@ -504,16 +527,20 @@ impl Halloy { comment, channels, sent_time, - } => { - dashboard.broadcast_quit( - &server, - user, - comment, - channels, - &self.config, - sent_time, - ); - } + } => commands.push( + dashboard + .broadcast( + &server, + &self.config, + sent_time, + Broadcast::Quit { + user, + comment, + user_channels: channels, + }, + ) + .map(Message::Dashboard), + ), data::client::Broadcast::Nickname { old_user, new_nick, @@ -523,14 +550,20 @@ impl Halloy { } => { let old_nick = old_user.nickname(); - dashboard.broadcast_nickname( - &server, - old_nick.to_owned(), - new_nick, - ourself, - channels, - &self.config, - sent_time, + commands.push( + dashboard + .broadcast( + &server, + &self.config, + sent_time, + Broadcast::Nickname { + old_nick: old_nick.to_owned(), + new_nick, + ourself, + user_channels: channels, + }, + ) + .map(Message::Dashboard), ); } data::client::Broadcast::Invite { @@ -541,13 +574,19 @@ impl Halloy { } => { let inviter = inviter.nickname(); - dashboard.broadcast_invite( - &server, - inviter.to_owned(), - channel, - user_channels, - &self.config, - sent_time, + commands.push( + dashboard + .broadcast( + &server, + &self.config, + sent_time, + Broadcast::Invite { + inviter: inviter.to_owned(), + channel, + user_channels, + }, + ) + .map(Message::Dashboard), ); } data::client::Broadcast::ChangeHost { @@ -558,15 +597,21 @@ impl Halloy { channels, sent_time, } => { - dashboard.broadcast_change_host( - &server, - old_user, - new_username, - new_hostname, - ourself, - channels, - &self.config, - sent_time, + commands.push( + dashboard + .broadcast( + &server, + &self.config, + sent_time, + Broadcast::ChangeHost { + old_user, + new_username, + new_hostname, + ourself, + user_channels: channels, + }, + ) + .map(Message::Dashboard), ); } }, @@ -582,7 +627,11 @@ impl Halloy { resolve_user_attributes, channel_users, ) { - dashboard.record_message(&server, message); + commands.push( + dashboard + .record_message(&server, message) + .map(Message::Dashboard), + ); } match notification { @@ -683,17 +732,21 @@ impl Halloy { let channels = client.channels().to_vec(); - dashboard.broadcast_quit( - &server, - user, - reason, - channels, - &self.config, - Utc::now(), - ); + dashboard + .broadcast( + &server, + &self.config, + Utc::now(), + Broadcast::Quit { + user, + comment: reason, + user_channels: channels, + }, + ) + .map(Message::Dashboard) + } else { + Task::none() } - - Task::none() } }, Message::Event(window, event) => { diff --git a/src/screen/dashboard.rs b/src/screen/dashboard.rs index f44ab1c04..f873a3059 100644 --- a/src/screen/dashboard.rs +++ b/src/screen/dashboard.rs @@ -10,7 +10,7 @@ use data::config; use data::file_transfer; use data::history::manager::Broadcast; use data::user::Nick; -use data::{client, environment, history, Config, Server, User, Version}; +use data::{client, environment, history, Config, Server, Version}; use iced::widget::pane_grid::{self, PaneGrid}; use iced::widget::{column, container, row, Space}; use iced::{clipboard, Length, Task, Vector}; @@ -344,6 +344,12 @@ impl Dashboard { ); } } + buffer::Event::History(history_task) => { + return ( + Task::batch(vec![task, history_task.map(Message::History)]), + None, + ) + } } return (task, None); @@ -1205,154 +1211,27 @@ impl Dashboard { } } - pub fn record_message(&mut self, server: &Server, message: data::Message) { - self.history.record_message(server, message); - } - - pub fn broadcast_quit( - &mut self, - server: &Server, - user: User, - comment: Option, - user_channels: Vec, - config: &Config, - sent_time: DateTime, - ) { - self.history.broadcast( - server, - Broadcast::Quit { - user, - comment, - user_channels, - }, - config, - sent_time, - ); - } - - pub fn broadcast_nickname( - &mut self, - server: &Server, - old_nick: Nick, - new_nick: Nick, - ourself: bool, - user_channels: Vec, - config: &Config, - sent_time: DateTime, - ) { - self.history.broadcast( - server, - Broadcast::Nickname { - new_nick, - old_nick, - ourself, - user_channels, - }, - config, - sent_time, - ); - } - - pub fn broadcast_invite( - &mut self, - server: &Server, - inviter: Nick, - channel: String, - user_channels: Vec, - config: &Config, - sent_time: DateTime, - ) { - self.history.broadcast( - server, - Broadcast::Invite { - inviter, - channel, - user_channels, - }, - config, - sent_time, - ); - } - - pub fn broadcast_change_host( - &mut self, - server: &Server, - old_user: User, - new_username: String, - new_hostname: String, - ourself: bool, - user_channels: Vec, - config: &Config, - sent_time: DateTime, - ) { - self.history.broadcast( - server, - Broadcast::ChangeHost { - old_user, - new_username, - new_hostname, - ourself, - user_channels, - }, - config, - sent_time, - ); - } - - pub fn broadcast_connecting( - &mut self, - server: &Server, - config: &Config, - sent_time: DateTime, - ) { - self.history - .broadcast(server, Broadcast::Connecting, config, sent_time); - } - - pub fn broadcast_connected( - &mut self, - server: &Server, - config: &Config, - sent_time: DateTime, - ) { - self.history - .broadcast(server, Broadcast::Connected, config, sent_time); - } - - pub fn broadcast_disconnected( - &mut self, - server: &Server, - error: Option, - config: &Config, - sent_time: DateTime, - ) { - self.history - .broadcast(server, Broadcast::Disconnected { error }, config, sent_time); - } - - pub fn broadcast_reconnected( - &mut self, - server: &Server, - config: &Config, - sent_time: DateTime, - ) { - self.history - .broadcast(server, Broadcast::Reconnected, config, sent_time); + pub fn record_message(&mut self, server: &Server, message: data::Message) -> Task { + if let Some(task) = self.history.record_message(server, message) { + Task::perform(task, Message::History) + } else { + Task::none() + } } - pub fn broadcast_connection_failed( + pub fn broadcast( &mut self, server: &Server, - error: String, config: &Config, sent_time: DateTime, - ) { - self.history.broadcast( - server, - Broadcast::ConnectionFailed { error }, - config, - sent_time, - ); + broadcast: Broadcast, + ) -> Task { + Task::batch( + self.history + .broadcast(server, broadcast, config, sent_time) + .into_iter() + .map(|task| Task::perform(task, Message::History)), + ) } pub fn update_read_marker( @@ -1660,32 +1539,36 @@ impl Dashboard { server: &Server, event: file_transfer::manager::Event, ) -> Task { + let mut tasks = vec![]; + match event { file_transfer::manager::Event::NewTransfer(transfer, task) => { match transfer.direction { file_transfer::Direction::Received => { - self.record_message( + tasks.push(self.record_message( server, data::Message::file_transfer_request_received( &transfer.remote_user, &transfer.filename, ), - ); + )); } file_transfer::Direction::Sent => { - self.record_message( + tasks.push(self.record_message( server, data::Message::file_transfer_request_sent( &transfer.remote_user, &transfer.filename, ), - ); + )); } } - Task::run(task, Message::FileTransfer) + tasks.push(Task::run(task, Message::FileTransfer)); } } + + Task::batch(tasks) } fn from_data( From cd241977494ae0ab11a7dec9e18a38b42ed41d75 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Mon, 23 Sep 2024 11:29:46 -0700 Subject: [PATCH 20/29] Give markreads a chance to send --- src/screen/dashboard.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/screen/dashboard.rs b/src/screen/dashboard.rs index f873a3059..ef40f8149 100644 --- a/src/screen/dashboard.rs +++ b/src/screen/dashboard.rs @@ -1701,13 +1701,16 @@ impl Dashboard { let last_changed = self.last_changed; let dashboard = data::Dashboard::from(&*self); - let task = async move { + Task::future(async move { for (server, kind, marker) in history.await { if let Some((target, marker)) = kind.target().zip(marker) { clients.send_markread(&server, target, marker); } } + // Give markread messages a chance to send + tokio::time::sleep(Duration::from_millis(500)).await; + if last_changed.is_some() { match dashboard.save().await { Ok(_) => { @@ -1718,9 +1721,7 @@ impl Dashboard { } } } - }; - - Task::perform(task, move |_| ()) + }) } fn open_popout_window(&mut self, main_window: &Window, pane: Pane) -> Task { From b06d6067c7ddc69279e0009b8d17686768bace77 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Mon, 23 Sep 2024 11:37:55 -0700 Subject: [PATCH 21/29] Load metadata for channel after join --- data/src/client.rs | 6 ++++++ data/src/history/manager.rs | 39 +++++++++++++++++++++++++++++++++++++ src/main.rs | 7 +++++++ src/screen/dashboard.rs | 8 ++++++++ 4 files changed, 60 insertions(+) diff --git a/data/src/client.rs b/data/src/client.rs index cce7b0f88..b6aa31af0 100644 --- a/data/src/client.rs +++ b/data/src/client.rs @@ -81,6 +81,7 @@ pub enum Event { Notification(message::Encoded, Nick, Notification), FileTransferRequest(file_transfer::ReceiveRequest), UpdateReadMarker(String, ReadMarker), + JoinedChannel(String), } pub struct Client { @@ -872,6 +873,11 @@ impl Client { channel.users.insert(user); } + + return Some(vec![ + Event::JoinedChannel(channel.clone()), + Event::Single(message, self.nickname().to_owned()), + ]); } Command::KICK(channel, victim, _) => { if victim == self.nickname().as_ref() { diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 174b0d6cc..10808db41 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -220,6 +220,14 @@ impl Manager { self.data.update_read_marker(server, kind, read_marker) } + pub fn channel_joined( + &mut self, + server: Server, + channel: String, + ) -> Option> { + self.data.channel_joined(server, channel) + } + pub fn get_channel_messages( &self, server: &Server, @@ -726,6 +734,37 @@ impl Data { } } + fn channel_joined( + &mut self, + server: server::Server, + channel: String, + ) -> Option> { + use std::collections::hash_map; + + let kind = history::Kind::Channel(channel); + + match self + .map + .entry(server.clone()) + .or_default() + .entry(kind.clone()) + { + hash_map::Entry::Occupied(_) => None, + hash_map::Entry::Vacant(entry) => { + entry.insert(History::partial(server.clone(), kind.clone())); + + Some( + async move { + let loaded = history::load(server.clone(), kind.clone()).await; + + Message::UpdatePartial(server, kind, loaded) + } + .boxed(), + ) + } + } + } + fn untrack( &mut self, server: &server::Server, diff --git a/src/main.rs b/src/main.rs index a5298425a..5441ec459 100644 --- a/src/main.rs +++ b/src/main.rs @@ -707,6 +707,13 @@ impl Halloy { .map(Message::Dashboard), ); } + data::client::Event::JoinedChannel(channel) => { + commands.push( + dashboard + .channel_joined(server.clone(), channel) + .map(Message::Dashboard), + ); + } } } diff --git a/src/screen/dashboard.rs b/src/screen/dashboard.rs index ef40f8149..b5851c909 100644 --- a/src/screen/dashboard.rs +++ b/src/screen/dashboard.rs @@ -1247,6 +1247,14 @@ impl Dashboard { } } + pub fn channel_joined(&mut self, server: Server, channel: String) -> Task { + if let Some(task) = self.history.channel_joined(server, channel) { + Task::perform(task, Message::History) + } else { + Task::none() + } + } + fn get_focused_mut( &mut self, main_window: &Window, From 07ba40fff9d59624e39bb5241f59999dfc070f93 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Mon, 23 Sep 2024 12:01:00 -0700 Subject: [PATCH 22/29] Persist last triggers so we only need to load metadata for partial --- data/src/history.rs | 84 +++++++++++++++++------------------- data/src/history/manager.rs | 28 ++++++------ data/src/history/metadata.rs | 50 ++++++++++----------- 3 files changed, 76 insertions(+), 86 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index 72641ac3e..4a9477364 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -97,10 +97,10 @@ pub async fn overwrite( server: &server::Server, kind: &Kind, messages: &[Message], - metadata: &Metadata, + read_marker: Option, ) -> Result<(), Error> { if messages.is_empty() { - return metadata::save(server, kind, metadata).await; + return metadata::save(server, kind, messages, read_marker).await; } let latest = &messages[messages.len().saturating_sub(MAX_MESSAGES)..]; @@ -110,7 +110,7 @@ pub async fn overwrite( fs::write(path, &compressed).await?; - metadata::save(server, kind, metadata).await?; + metadata::save(server, kind, latest, read_marker).await?; Ok(()) } @@ -119,10 +119,10 @@ pub async fn append( server: &server::Server, kind: &Kind, messages: Vec, - metadata: &Metadata, + read_marker: Option, ) -> Result<(), Error> { if messages.is_empty() { - return metadata::save(server, kind, metadata).await; + return metadata::save(server, kind, &messages, read_marker).await; } let loaded = load(server.clone(), kind.clone()).await?; @@ -130,7 +130,7 @@ pub async fn append( let mut all_messages = loaded.messages; all_messages.extend(messages); - overwrite(server, kind, &all_messages, metadata).await + overwrite(server, kind, &all_messages, read_marker).await } async fn read_all(path: &PathBuf) -> Result, Error> { @@ -164,13 +164,6 @@ async fn path(server: &server::Server, kind: &Kind) -> Result { Ok(dir.join(format!("{hashed_name}.json.gz"))) } -fn last_triggers_unread(messages: &[Message]) -> Option> { - messages - .iter() - .rev() - .find_map(|message| message.triggers_unread().then_some(message.server_time)) -} - #[derive(Debug)] pub enum History { Partial { @@ -178,15 +171,15 @@ pub enum History { kind: Kind, messages: Vec, last_updated_at: Option, - metadata: Metadata, max_triggers_unread: Option>, + read_marker: Option, }, Full { server: server::Server, kind: Kind, messages: Vec, last_updated_at: Option, - metadata: Metadata, + read_marker: Option, }, } @@ -197,32 +190,32 @@ impl History { kind, messages: vec![], last_updated_at: None, - metadata: Metadata::default(), max_triggers_unread: None, + read_marker: None, } } - pub fn update_partial(&mut self, loaded: Loaded) { + pub fn update_partial(&mut self, metadata: Metadata) { if let Self::Partial { - metadata, max_triggers_unread, + read_marker, .. } = self { - *metadata = metadata.merge(loaded.metadata); - *max_triggers_unread = last_triggers_unread(&loaded.messages).max(*max_triggers_unread); + *read_marker = (*read_marker).max(metadata.read_marker); + *max_triggers_unread = (*max_triggers_unread).max(metadata.last_triggers_unread); } } fn has_unread(&self) -> bool { match self { History::Partial { - metadata, max_triggers_unread, + read_marker, .. } => { // Read marker is prior to last known message which triggers unread - if let Some(read_marker) = metadata.read_marker { + if let Some(read_marker) = read_marker { max_triggers_unread.is_some_and(|max| read_marker.date_time() < max) } // Default state == unread if theres messages that trigger indicator @@ -269,8 +262,8 @@ impl History { server, kind, messages, - metadata, last_updated_at, + read_marker, .. } => { if let Some(last_received) = *last_updated_at { @@ -279,13 +272,13 @@ impl History { if since >= FLUSH_AFTER_LAST_RECEIVED { let server = server.clone(); let kind = kind.clone(); - let metadata = *metadata; let messages = std::mem::take(messages); + let read_marker = *read_marker; *last_updated_at = None; return Some( - async move { append(&server, &kind, messages, &metadata).await } + async move { append(&server, &kind, messages, read_marker).await } .boxed(), ); } @@ -297,8 +290,8 @@ impl History { server, kind, messages, - metadata, last_updated_at, + read_marker, .. } => { if let Some(last_received) = *last_updated_at { @@ -307,7 +300,7 @@ impl History { if since >= FLUSH_AFTER_LAST_RECEIVED && !messages.is_empty() { let server = server.clone(); let kind = kind.clone(); - let metadata = *metadata; + let read_marker = *read_marker; *last_updated_at = None; if messages.len() > MAX_MESSAGES { @@ -317,7 +310,7 @@ impl History { let messages = messages.clone(); return Some( - async move { overwrite(&server, &kind, &messages, &metadata).await } + async move { overwrite(&server, &kind, &messages, read_marker).await } .boxed(), ); } @@ -335,28 +328,29 @@ impl History { server, kind, messages, - metadata, + read_marker, .. } => { let server = server.clone(); let kind = kind.clone(); let messages = std::mem::take(messages); - let metadata = metadata.updated(&messages); + let read_marker = ReadMarker::latest(&messages).max(*read_marker); + let max_triggers_unread = metadata::find_latest_triggers(&messages); *self = Self::Partial { server: server.clone(), kind: kind.clone(), messages: vec![], last_updated_at: None, - metadata, - max_triggers_unread: last_triggers_unread(&messages), + read_marker, + max_triggers_unread, }; Some(async move { - overwrite(&server, &kind, &messages, &metadata) + overwrite(&server, &kind, &messages, read_marker) .await - .map(|_| metadata.read_marker) + .map(|_| read_marker) }) } } @@ -368,36 +362,36 @@ impl History { server, kind, messages, - metadata, + read_marker, .. } => { - append(&server, &kind, messages, &metadata).await?; + append(&server, &kind, messages, read_marker).await?; - Ok(metadata.read_marker) + Ok(read_marker) } History::Full { server, kind, messages, - metadata, + read_marker, .. } => { - let metadata = metadata.updated(&messages); + let read_marker = ReadMarker::latest(&messages).max(read_marker); - overwrite(&server, &kind, &messages, &metadata).await?; + overwrite(&server, &kind, &messages, read_marker).await?; - Ok(metadata.read_marker) + Ok(read_marker) } } } pub fn update_read_marker(&mut self, read_marker: ReadMarker) { - let metadata = match self { - History::Partial { metadata, .. } => metadata, - History::Full { metadata, .. } => metadata, + let stored = match self { + History::Partial { read_marker, .. } => read_marker, + History::Full { read_marker, .. } => read_marker, }; - metadata.update_read_marker(read_marker); + *stored = (*stored).max(Some(read_marker)); } } diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 10808db41..fe893aa92 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -28,7 +28,7 @@ pub enum Message { UpdatePartial( server::Server, history::Kind, - Result, + Result, ), Closed( server::Server, @@ -104,7 +104,7 @@ impl Manager { log::warn!("failed to flush history for {kind} on {server}: {error}") } Message::UpdatePartial(server, kind, Ok(loaded)) => { - log::debug!("updating partial history for {kind} on {server}"); + log::debug!("updating metadata for {kind} on {server}"); self.data.update_partial(server, kind, loaded); } Message::UpdatePartial(server, kind, Err(error)) => { @@ -472,10 +472,10 @@ impl Data { History::Partial { messages: new_messages, last_updated_at, - metadata: partial_metadata, + read_marker: partial_read_marker, .. } => { - let metadata = partial_metadata.merge(metadata); + let read_marker = (*partial_read_marker).max(metadata.read_marker); let last_updated_at = *last_updated_at; messages.extend(std::mem::take(new_messages)); @@ -484,7 +484,7 @@ impl Data { kind, messages, last_updated_at, - metadata, + read_marker, }); } _ => { @@ -493,7 +493,7 @@ impl Data { kind, messages, last_updated_at: None, - metadata, + read_marker: metadata.read_marker, }); } }, @@ -503,7 +503,7 @@ impl Data { kind, messages, last_updated_at: None, - metadata, + read_marker: metadata.read_marker, }); } } @@ -513,7 +513,7 @@ impl Data { &mut self, server: server::Server, kind: history::Kind, - data: history::Loaded, + data: history::Metadata, ) { if let Some(history) = self.map.get_mut(&server).and_then(|map| map.get_mut(&kind)) { history.update_partial(data); @@ -528,7 +528,9 @@ impl Data { buffer_config: &config::Buffer, ) -> Option { let History::Full { - messages, metadata, .. + messages, + read_marker, + .. } = self.map.get(server)?.get(kind)? else { return None; @@ -643,7 +645,7 @@ impl Data { let limited = with_limit(limit, filtered.into_iter()); - let split_at = metadata.read_marker.map_or(0, |read_marker| { + let split_at = read_marker.map_or(0, |read_marker| { limited .iter() .rev() @@ -688,7 +690,7 @@ impl Data { Some( async move { - let loaded = history::load(server.clone(), kind.clone()).await; + let loaded = history::metadata::load(server.clone(), kind.clone()).await; Message::UpdatePartial(server, kind, loaded) } @@ -724,7 +726,7 @@ impl Data { Some( async move { - let loaded = history::load(server.clone(), kind.clone()).await; + let loaded = history::metadata::load(server.clone(), kind.clone()).await; Message::UpdatePartial(server, kind, loaded) } @@ -755,7 +757,7 @@ impl Data { Some( async move { - let loaded = history::load(server.clone(), kind.clone()).await; + let loaded = history::metadata::load(server.clone(), kind.clone()).await; Message::UpdatePartial(server, kind, loaded) } diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index 9adeae59e..f23dcc8bd 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -7,29 +7,12 @@ use serde::{Deserialize, Serialize}; use tokio::fs; use crate::history::{dir_path, Error, Kind}; -use crate::{message, server, Message}; +use crate::{server, Message}; #[derive(Debug, Clone, Copy, Default, Deserialize, Serialize)] pub struct Metadata { pub read_marker: Option, -} - -impl Metadata { - pub fn merge(self, other: Self) -> Self { - Self { - read_marker: self.read_marker.max(other.read_marker), - } - } - - pub fn update_read_marker(&mut self, read_marker: ReadMarker) { - self.read_marker = self.read_marker.max(Some(read_marker)); - } - - pub fn updated(self, messages: &[Message]) -> Self { - Self { - read_marker: ReadMarker::latest(messages).or(self.read_marker), - } - } + pub last_triggers_unread: Option>, } #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default, Deserialize, Serialize)] @@ -37,15 +20,10 @@ pub struct ReadMarker(DateTime); impl ReadMarker { pub fn latest(messages: &[Message]) -> Option { - messages - .iter() - .rev() - .find(|message| !matches!(message.target.source(), message::Source::Internal(_))) - .map(|message| message.server_time) - .map(Self) + find_latest_triggers(messages).map(Self) } - pub fn date_time(&self) -> DateTime { + pub fn date_time(self) -> DateTime { self.0 } } @@ -66,6 +44,14 @@ impl fmt::Display for ReadMarker { } } +pub fn find_latest_triggers(messages: &[Message]) -> Option> { + messages + .iter() + .rev() + .find(|message| message.triggers_unread()) + .map(|message| message.server_time) +} + pub async fn load(server: server::Server, kind: Kind) -> Result { let path = path(&server, &kind).await?; @@ -76,8 +62,16 @@ pub async fn load(server: server::Server, kind: Kind) -> Result } } -pub async fn save(server: &server::Server, kind: &Kind, metadata: &Metadata) -> Result<(), Error> { - let bytes = serde_json::to_vec(metadata)?; +pub async fn save( + server: &server::Server, + kind: &Kind, + messages: &[Message], + read_marker: Option, +) -> Result<(), Error> { + let bytes = serde_json::to_vec(&Metadata { + read_marker, + last_triggers_unread: find_latest_triggers(messages), + })?; let path = path(server, kind).await?; From a3c89737d99db5502d8e546c22536c544716d155 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Mon, 23 Sep 2024 12:05:13 -0700 Subject: [PATCH 23/29] Dont send markread for partial history We'll never have updated the read marker so there's no reason to try and send this value back to the server --- data/src/history.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/src/history.rs b/data/src/history.rs index 4a9477364..eb0c2fa4f 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -367,7 +367,7 @@ impl History { } => { append(&server, &kind, messages, read_marker).await?; - Ok(read_marker) + Ok(None) } History::Full { server, From 9f1d346a7abdf459a98c5acad895a8f965247cd5 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Mon, 23 Sep 2024 12:11:10 -0700 Subject: [PATCH 24/29] Rename method --- data/src/history.rs | 2 +- data/src/history/metadata.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index eb0c2fa4f..ceb96b096 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -336,7 +336,7 @@ impl History { let messages = std::mem::take(messages); let read_marker = ReadMarker::latest(&messages).max(*read_marker); - let max_triggers_unread = metadata::find_latest_triggers(&messages); + let max_triggers_unread = metadata::latest_triggers_unread(&messages); *self = Self::Partial { server: server.clone(), diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index f23dcc8bd..d744e04c9 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -20,7 +20,7 @@ pub struct ReadMarker(DateTime); impl ReadMarker { pub fn latest(messages: &[Message]) -> Option { - find_latest_triggers(messages).map(Self) + latest_triggers_unread(messages).map(Self) } pub fn date_time(self) -> DateTime { @@ -44,7 +44,7 @@ impl fmt::Display for ReadMarker { } } -pub fn find_latest_triggers(messages: &[Message]) -> Option> { +pub fn latest_triggers_unread(messages: &[Message]) -> Option> { messages .iter() .rev() @@ -70,7 +70,7 @@ pub async fn save( ) -> Result<(), Error> { let bytes = serde_json::to_vec(&Metadata { read_marker, - last_triggers_unread: find_latest_triggers(messages), + last_triggers_unread: latest_triggers_unread(messages), })?; let path = path(server, kind).await?; From e38c9e2bbfc64cef6af734cd3265fa7b5b4cb715 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Mon, 23 Sep 2024 12:31:10 -0700 Subject: [PATCH 25/29] Use all messages when updating partial metadata --- data/src/history.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/data/src/history.rs b/data/src/history.rs index ceb96b096..d82828f26 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -121,10 +121,6 @@ pub async fn append( messages: Vec, read_marker: Option, ) -> Result<(), Error> { - if messages.is_empty() { - return metadata::save(server, kind, &messages, read_marker).await; - } - let loaded = load(server.clone(), kind.clone()).await?; let mut all_messages = loaded.messages; From 9208f1db33d915817014381f7bbed741786c9920 Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Mon, 23 Sep 2024 12:35:26 -0700 Subject: [PATCH 26/29] Fix read marker to calculate on all non-internal messages --- data/src/history/metadata.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index d744e04c9..61e714958 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; use tokio::fs; use crate::history::{dir_path, Error, Kind}; -use crate::{server, Message}; +use crate::{message, server, Message}; #[derive(Debug, Clone, Copy, Default, Deserialize, Serialize)] pub struct Metadata { @@ -20,7 +20,12 @@ pub struct ReadMarker(DateTime); impl ReadMarker { pub fn latest(messages: &[Message]) -> Option { - latest_triggers_unread(messages).map(Self) + messages + .iter() + .rev() + .find(|message| !matches!(message.target.source(), message::Source::Internal(_))) + .map(|message| message.server_time) + .map(Self) } pub fn date_time(self) -> DateTime { From bd717e2da9140d5476f4afab449c46c6a1fe258a Mon Sep 17 00:00:00 2001 From: Andrew Baldwin Date: Tue, 24 Sep 2024 00:44:25 -0700 Subject: [PATCH 27/29] Don't create a new history entry when updating a read marker due to `MARKREAD`; update on disk instead. --- data/src/history/manager.rs | 34 +++++++++++++++++++++------------- data/src/history/metadata.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index fe893aa92..7c358d0b6 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -30,6 +30,12 @@ pub enum Message { history::Kind, Result, ), + UpdateReadMarker( + server::Server, + history::Kind, + history::ReadMarker, + Result<(), history::Error>, + ), Closed( server::Server, history::Kind, @@ -110,6 +116,14 @@ impl Manager { Message::UpdatePartial(server, kind, Err(error)) => { log::warn!("failed to load history metadata for {kind} on {server}: {error}"); } + Message::UpdateReadMarker(server, kind, read_marker, Ok(_)) => { + log::debug!("updated read marker for {kind} on {server} to {read_marker}"); + } + Message::UpdateReadMarker(server, kind, read_marker, Err(error)) => { + log::warn!( + "failed to update read marker for {kind} on {server} to {read_marker}: {error}" + ); + } } None @@ -719,20 +733,14 @@ impl Data { None } - hash_map::Entry::Vacant(entry) => { - entry - .insert(History::partial(server.clone(), kind.clone())) - .update_read_marker(read_marker); + hash_map::Entry::Vacant(_) => Some( + async move { + let updated = history::metadata::update(&server, &kind, &read_marker).await; - Some( - async move { - let loaded = history::metadata::load(server.clone(), kind.clone()).await; - - Message::UpdatePartial(server, kind, loaded) - } - .boxed(), - ) - } + Message::UpdateReadMarker(server, kind, read_marker, updated) + } + .boxed(), + ), } } diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index 61e714958..e4b3e29f4 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -85,6 +85,32 @@ pub async fn save( Ok(()) } +pub async fn update( + server: &server::Server, + kind: &Kind, + read_marker: &ReadMarker, +) -> Result<(), Error> { + let metadata = load(server.clone(), kind.clone()).await?; + + if metadata + .read_marker + .is_some_and(|metadata_read_marker| metadata_read_marker >= *read_marker) + { + return Ok(()); + } + + let bytes = serde_json::to_vec(&Metadata { + read_marker: Some(*read_marker), + last_triggers_unread: metadata.last_triggers_unread, + })?; + + let path = path(server, kind).await?; + + fs::write(path, &bytes).await?; + + Ok(()) +} + async fn path(server: &server::Server, kind: &Kind) -> Result { let dir = dir_path().await?; From 78792733d1f0fd8665407f3d3034de3cd8a7388d Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Tue, 24 Sep 2024 10:19:15 -0700 Subject: [PATCH 28/29] Only return join event when we joined channel --- data/src/client.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/data/src/client.rs b/data/src/client.rs index b6aa31af0..b175f6a1a 100644 --- a/data/src/client.rs +++ b/data/src/client.rs @@ -862,6 +862,8 @@ impl Client { } log::debug!("[{}] {channel} - WHO requested", self.server); } + + return Some(vec![Event::JoinedChannel(channel.clone())]); } else if let Some(channel) = self.chanmap.get_mut(channel) { let user = if self.supports_extended_join { accountname.as_ref().map_or(user.clone(), |accountname| { @@ -873,11 +875,6 @@ impl Client { channel.users.insert(user); } - - return Some(vec![ - Event::JoinedChannel(channel.clone()), - Event::Single(message, self.nickname().to_owned()), - ]); } Command::KICK(channel, victim, _) => { if victim == self.nickname().as_ref() { From 8abfb9f380ebe176d33aa4b066d3435e144033ad Mon Sep 17 00:00:00 2001 From: Cory Forsstrom Date: Tue, 24 Sep 2024 11:28:17 -0700 Subject: [PATCH 29/29] Cleanup & proper exit once all streams close --- data/src/client.rs | 18 ++++++-- data/src/history.rs | 5 ++- data/src/history/manager.rs | 82 ++++++++++++++++++------------------ data/src/history/metadata.rs | 2 +- src/main.rs | 81 +++++++++++++++++++++-------------- src/screen/dashboard.rs | 71 ++++++++++++++----------------- 6 files changed, 142 insertions(+), 117 deletions(-) diff --git a/data/src/client.rs b/data/src/client.rs index b175f6a1a..846368c96 100644 --- a/data/src/client.rs +++ b/data/src/client.rs @@ -1485,6 +1485,20 @@ impl Map { } } + pub fn exit(&mut self) -> HashSet { + self.0 + .iter_mut() + .filter_map(|(server, state)| { + if let State::Ready(client) = state { + client.quit(None); + Some(server.clone()) + } else { + None + } + }) + .collect() + } + pub fn resolve_user_attributes<'a>( &'a self, server: &Server, @@ -1560,10 +1574,6 @@ impl Map { } }) } - - pub fn take(&mut self) -> Self { - Self(std::mem::take(&mut self.0)) - } } #[derive(Debug, Clone)] diff --git a/data/src/history.rs b/data/src/history.rs index d82828f26..be1a07760 100644 --- a/data/src/history.rs +++ b/data/src/history.rs @@ -9,11 +9,12 @@ use irc::proto; use tokio::fs; use tokio::time::Instant; -pub use self::manager::{Manager, Resource}; -pub use self::metadata::{Metadata, ReadMarker}; use crate::user::Nick; use crate::{compression, environment, message, server, Message}; +pub use self::manager::{Manager, Resource}; +pub use self::metadata::{Metadata, ReadMarker}; + pub mod manager; pub mod metadata; diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index 7c358d0b6..209842b33 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -42,10 +42,18 @@ pub enum Message { Result, history::Error>, ), Flushed(server::Server, history::Kind, Result<(), history::Error>), + Exited( + Vec<( + Server, + history::Kind, + Result, history::Error>, + )>, + ), } pub enum Event { Closed(server::Server, history::Kind, Option), + Exited(Vec<(Server, history::Kind, Option)>), } #[derive(Debug, Default)] @@ -109,12 +117,12 @@ impl Manager { Message::Flushed(server, kind, Err(error)) => { log::warn!("failed to flush history for {kind} on {server}: {error}") } - Message::UpdatePartial(server, kind, Ok(loaded)) => { - log::debug!("updating metadata for {kind} on {server}"); - self.data.update_partial(server, kind, loaded); + Message::UpdatePartial(server, kind, Ok(metadata)) => { + log::debug!("loaded metadata for {kind} on {server}"); + self.data.update_partial(server, kind, metadata); } Message::UpdatePartial(server, kind, Err(error)) => { - log::warn!("failed to load history metadata for {kind} on {server}: {error}"); + log::warn!("failed to load metadata for {kind} on {server}: {error}"); } Message::UpdateReadMarker(server, kind, read_marker, Ok(_)) => { log::debug!("updated read marker for {kind} on {server} to {read_marker}"); @@ -124,6 +132,24 @@ impl Manager { "failed to update read marker for {kind} on {server} to {read_marker}: {error}" ); } + Message::Exited(results) => { + let mut output = vec![]; + + for (server, kind, result) in results { + match result { + Ok(marker) => { + log::debug!("closed history for {kind} on {server}",); + output.push((server, kind, marker)); + } + Err(error) => { + log::warn!("failed to close history for {kind} on {server}: {error}"); + output.push((server, kind, None)); + } + } + } + + return Some(Event::Exited(output)); + } } None @@ -137,26 +163,17 @@ impl Manager { &mut self, server: Server, kind: history::Kind, - ) -> Option)>> { + ) -> Option> { let history = self.data.map.get_mut(&server)?.remove(&kind)?; - Some(async move { - match history.close().await { - Ok(marker) => { - log::debug!("closed history for {kind} on {server}",); - (server, kind, marker) - } - Err(error) => { - log::warn!("failed to close history for {kind} on {server}: {error}"); - (server, kind, None) - } - } - }) + Some( + history + .close() + .map(|result| Message::Closed(server, kind, result)), + ) } - pub fn close_all( - &mut self, - ) -> impl Future)>> { + pub fn exit(&mut self) -> impl Future { let map = std::mem::take(&mut self.data).map; async move { @@ -167,24 +184,7 @@ impl Manager { }) }); - let results = future::join_all(tasks).await; - - let mut output = vec![]; - - for (server, kind, result) in results { - match result { - Ok(marker) => { - log::debug!("closed history for {kind} on {server}",); - output.push((server, kind, marker)); - } - Err(error) => { - log::warn!("failed to close history for {kind} on {server}: {error}"); - output.push((server, kind, None)); - } - } - } - - output + Message::Exited(future::join_all(tasks).await) } } @@ -228,7 +228,7 @@ impl Manager { pub fn update_read_marker( &mut self, server: Server, - kind: history::Kind, + kind: impl Into, read_marker: history::ReadMarker, ) -> Option> { self.data.update_read_marker(server, kind, read_marker) @@ -717,11 +717,13 @@ impl Data { fn update_read_marker( &mut self, server: server::Server, - kind: history::Kind, + kind: impl Into, read_marker: history::ReadMarker, ) -> Option> { use std::collections::hash_map; + let kind = kind.into(); + match self .map .entry(server.clone()) diff --git a/data/src/history/metadata.rs b/data/src/history/metadata.rs index e4b3e29f4..892736820 100644 --- a/data/src/history/metadata.rs +++ b/data/src/history/metadata.rs @@ -1,8 +1,8 @@ -use chrono::{format::SecondsFormat, DateTime, Utc}; use std::fmt; use std::path::PathBuf; use std::str::FromStr; +use chrono::{format::SecondsFormat, DateTime, Utc}; use serde::{Deserialize, Serialize}; use tokio::fs; diff --git a/src/main.rs b/src/main.rs index 5441ec459..b9bf5dd2a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -16,15 +16,16 @@ mod url; mod widget; mod window; +use std::collections::HashSet; use std::env; use std::time::{Duration, Instant}; use chrono::Utc; use data::config::{self, Config}; -use data::history; use data::history::manager::Broadcast; use data::version::Version; use data::{environment, server, version, Url, User}; +use data::{history, Server}; use iced::widget::{column, container}; use iced::{padding, Length, Subscription, Task}; use screen::{dashboard, help, migration, welcome}; @@ -201,6 +202,7 @@ pub enum Screen { Help(screen::Help), Welcome(screen::Welcome), Migration(screen::Migration), + Exit { pending_exit: HashSet }, } #[derive(Debug)] @@ -339,6 +341,16 @@ impl Halloy { self.clients.quit(&server, None); Task::none() } + Some(dashboard::Event::Exit) => { + let pending_exit = self.clients.exit(); + + if pending_exit.is_empty() { + iced::exit() + } else { + self.screen = Screen::Exit { pending_exit }; + Task::none() + } + } None => Task::none(), }; @@ -695,13 +707,11 @@ impl Halloy { } } data::client::Event::UpdateReadMarker(target, read_marker) => { - let kind = history::Kind::from(target); - commands.push( dashboard .update_read_marker( server.clone(), - kind, + target, read_marker, ) .map(Message::Dashboard), @@ -727,34 +737,42 @@ impl Halloy { Task::batch(commands) } - stream::Update::Quit(server, reason) => { - let Screen::Dashboard(dashboard) = &mut self.screen else { - return Task::none(); - }; - - self.servers.remove(&server); - - if let Some(client) = self.clients.remove(&server) { - let user = client.nickname().to_owned().into(); - - let channels = client.channels().to_vec(); + stream::Update::Quit(server, reason) => match &mut self.screen { + Screen::Dashboard(dashboard) => { + self.servers.remove(&server); + + if let Some(client) = self.clients.remove(&server) { + let user = client.nickname().to_owned().into(); + + let channels = client.channels().to_vec(); + + dashboard + .broadcast( + &server, + &self.config, + Utc::now(), + Broadcast::Quit { + user, + comment: reason, + user_channels: channels, + }, + ) + .map(Message::Dashboard) + } else { + Task::none() + } + } + Screen::Exit { pending_exit } => { + pending_exit.remove(&server); - dashboard - .broadcast( - &server, - &self.config, - Utc::now(), - Broadcast::Quit { - user, - comment: reason, - user_channels: channels, - }, - ) - .map(Message::Dashboard) - } else { - Task::none() + if pending_exit.is_empty() { + iced::exit() + } else { + Task::none() + } } - } + _ => Task::none(), + }, }, Message::Event(window, event) => { // Events only enabled for main window @@ -838,7 +856,7 @@ impl Halloy { } window::Event::CloseRequested => { if let Screen::Dashboard(dashboard) = &mut self.screen { - return dashboard.exit(self.clients.take()).then(|_| iced::exit()); + return dashboard.exit().map(Message::Dashboard); } else { return iced::exit(); } @@ -882,6 +900,7 @@ impl Halloy { Screen::Help(help) => help.view().map(Message::Help), Screen::Welcome(welcome) => welcome.view().map(Message::Welcome), Screen::Migration(migration) => migration.view().map(Message::Migration), + Screen::Exit { .. } => column![].into(), }; let content = container(screen) diff --git a/src/screen/dashboard.rs b/src/screen/dashboard.rs index b5851c909..c80c4fb21 100644 --- a/src/screen/dashboard.rs +++ b/src/screen/dashboard.rs @@ -52,7 +52,6 @@ pub enum Message { SelectedText(Vec<(f32, String)>), History(history::manager::Message), DashboardSaved(Result<(), data::dashboard::Error>), - CloseHistory(Server, history::Kind, Option), Task(command_bar::Message), Shortcut(shortcut::Command), FileTransfer(file_transfer::task::Update), @@ -67,6 +66,7 @@ pub enum Event { ConfigReloaded(Result), ReloadThemes, QuitServer(Server), + Exit, } impl Dashboard { @@ -519,6 +519,16 @@ impl Dashboard { clients.send_markread(&server, target, read_marker); } } + history::manager::Event::Exited(results) => { + for (server, kind, read_marker) in results { + if let Some((target, read_marker)) = kind.target().zip(read_marker) + { + clients.send_markread(&server, target, read_marker); + } + } + + return (Task::none(), Some(Event::Exit)); + } } } } @@ -528,11 +538,6 @@ impl Dashboard { Message::DashboardSaved(Err(error)) => { log::warn!("error saving dashboard: {error}"); } - Message::CloseHistory(server, kind, marker) => { - if let Some((target, marker)) = kind.target().zip(marker) { - clients.send_markread(&server, target, marker); - } - } Message::Task(message) => { let Some(command_bar) = &mut self.command_bar else { return (Task::none(), None); @@ -1183,11 +1188,7 @@ impl Dashboard { tasks.push( self.history .close(server, history::Kind::Channel(channel)) - .map(|task| { - Task::perform(task, |(server, kind, marker)| { - Message::CloseHistory(server, kind, marker) - }) - }) + .map(|task| Task::perform(task, Message::History)) .unwrap_or_else(Task::none), ); @@ -1197,11 +1198,7 @@ impl Dashboard { tasks.push( self.history .close(server, history::Kind::Query(nick)) - .map(|task| { - Task::perform(task, |(server, kind, marker)| { - Message::CloseHistory(server, kind, marker) - }) - }) + .map(|task| Task::perform(task, Message::History)) .unwrap_or_else(Task::none), ); @@ -1237,7 +1234,7 @@ impl Dashboard { pub fn update_read_marker( &mut self, server: Server, - kind: history::Kind, + kind: impl Into + 'static, read_marker: ReadMarker, ) -> Task { if let Some(task) = self.history.update_read_marker(server, kind, read_marker) { @@ -1704,32 +1701,28 @@ impl Dashboard { } } - pub fn exit(&mut self, mut clients: client::Map) -> Task<()> { - let history = self.history.close_all(); - let last_changed = self.last_changed; + pub fn exit(&mut self) -> Task { + let history = self.history.exit(); + let last_changed = self.last_changed.take(); let dashboard = data::Dashboard::from(&*self); - Task::future(async move { - for (server, kind, marker) in history.await { - if let Some((target, marker)) = kind.target().zip(marker) { - clients.send_markread(&server, target, marker); - } - } - - // Give markread messages a chance to send - tokio::time::sleep(Duration::from_millis(500)).await; - - if last_changed.is_some() { - match dashboard.save().await { - Ok(_) => { - log::info!("dashboard saved"); - } - Err(error) => { - log::warn!("error saving dashboard: {error}"); + Task::perform( + async move { + if last_changed.is_some() { + match dashboard.save().await { + Ok(_) => { + log::info!("dashboard saved"); + } + Err(error) => { + log::warn!("error saving dashboard: {error}"); + } } } - } - }) + + history.await + }, + Message::History, + ) } fn open_popout_window(&mut self, main_window: &Window, pane: Pane) -> Task {