From 8e623c55c42bff00aee5d21cd54b67c0eeadcb41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Mon, 7 Apr 2025 14:32:05 +0200 Subject: [PATCH] notifications: Refactor and clean up Fixes a regression where clicking on notifications would not open the app anymore. --- src/application.rs | 77 ++- src/intent.rs | 76 ++- src/session/model/mod.rs | 2 +- src/session/model/notifications/mod.rs | 72 ++- .../notifications/notifications_settings.rs | 546 +++++++++--------- .../verification/identity_verification.rs | 2 +- src/session/model/verification/mod.rs | 35 +- .../model/verification/verification_list.rs | 6 +- src/window.rs | 1 - 9 files changed, 439 insertions(+), 378 deletions(-) diff --git a/src/application.rs b/src/application.rs index e76c1abc..94a557ab 100644 --- a/src/application.rs +++ b/src/application.rs @@ -167,8 +167,8 @@ mod imp { gio::ActionEntry::builder("quit") .activate(|obj: &super::Application, _, _| { if let Some(window) = obj.active_window() { - // This is needed to trigger the delete event - // and saving the window state + // This is needed to trigger the close request and save the window + // state. window.close(); } @@ -186,12 +186,19 @@ mod imp { gio::ActionEntry::builder(SessionIntentType::ShowMatrixId.action_name()) .parameter_type(Some(&SessionIntentType::static_variant_type())) .activate(|obj: &super::Application, _, variant| { - let Some((session_id, intent)) = SessionIntent::parse_with_session_id( - SessionIntentType::ShowMatrixId, - variant, - ) else { + debug!( + "`app.{}` action activated", + SessionIntentType::ShowMatrixId.action_name() + ); + + let Some((session_id, intent)) = variant.and_then(|variant| { + SessionIntent::from_variant_with_session_id( + SessionIntentType::ShowMatrixId, + variant, + ) + }) else { error!( - "Triggered `{}` action without the proper payload", + "Activated `app.{}` action without the proper payload", SessionIntentType::ShowMatrixId.action_name() ); return; @@ -207,12 +214,19 @@ mod imp { ) .parameter_type(Some(&SessionIntentType::static_variant_type())) .activate(|obj: &super::Application, _, variant| { - let Some((session_id, intent)) = SessionIntent::parse_with_session_id( - SessionIntentType::ShowIdentityVerification, - variant, - ) else { + debug!( + "`app.{}` action activated", + SessionIntentType::ShowIdentityVerification.action_name() + ); + + let Some((session_id, intent)) = variant.and_then(|variant| { + SessionIntent::from_variant_with_session_id( + SessionIntentType::ShowIdentityVerification, + variant, + ) + }) else { error!( - "Triggered `{}` action without the proper payload", + "Activated `app.{}` action without the proper payload", SessionIntentType::ShowIdentityVerification.action_name() ); return; @@ -289,6 +303,7 @@ mod imp { /// Process the given URI. fn process_uri(&self, uri: &str) { + debug!(uri, "Processing URI…"); match MatrixIdUri::parse(uri) { Ok(matrix_id) => { self.select_session_for_intent(SessionIntent::ShowMatrixId(matrix_id)); @@ -299,7 +314,7 @@ mod imp { /// Select a session to handle the given intent as soon as possible. fn select_session_for_intent(&self, intent: SessionIntent) { - debug!("Selecting session for intent {intent:?}"); + debug!(?intent, "Selecting session for intent…"); // We only handle a single intent at time, the latest one. self.intent_handler.disconnect_signals(); @@ -307,26 +322,27 @@ mod imp { if self.session_list.state() == LoadingState::Ready { match self.session_list.n_items() { 0 => { - warn!("Cannot open URI with no logged in session"); + warn!("Cannot process intent with no logged in session"); } 1 => { let session = self .session_list .first() - .expect("There should be one session"); + .expect("there should be one session"); self.process_session_intent(session.session_id(), intent); } _ => { spawn!(clone!( - #[weak(rename_to = obj)] + #[weak(rename_to = imp)] self, async move { - obj.ask_session_for_intent(intent).await; + imp.ask_session_for_intent(intent).await; } )); } } } else { + debug!(?intent, "Session list is not ready, queuing intent…"); // Wait for the list to be ready. let cell = Rc::new(RefCell::new(Some(intent))); let handler = self.session_list.connect_state_notify(clone!( @@ -353,6 +369,7 @@ mod imp { /// /// The session list needs to be ready. async fn ask_session_for_intent(&self, intent: SessionIntent) { + debug!(?intent, "Asking to select a session to process intent…"); let main_window = self.present_main_window(); let Some(session_id) = main_window.ask_session().await else { @@ -366,22 +383,35 @@ mod imp { /// Process the given intent for the given session, as soon as the /// session is ready. fn process_session_intent(&self, session_id: String, intent: SessionIntent) { - debug!(session = session_id, "Processing session intent {intent:?}"); - let Some(session_info) = self.session_list.get(&session_id) else { - warn!("Could not find session to process intent {intent:?}"); + warn!( + session = session_id, + ?intent, + "Could not find session to process intent" + ); toast!(self.present_main_window(), gettext("Session not found")); return; }; + debug!(session = session_id, ?intent, "Processing session intent…"); + if session_info.is::() { // We can't do anything, it should show an error screen. - warn!("Could not process intent {intent:?} for failed session"); + warn!( + session = session_id, + ?intent, + "Could not process intent for failed session" + ); } else if let Some(session) = session_info.downcast_ref::() { if session.state() == SessionState::Ready { self.present_main_window() .process_session_intent(session.session_id(), intent); } else { + debug!( + session = session_id, + ?intent, + "Session is not ready, queuing intent…" + ); // Wait for the session to be ready. let cell = Rc::new(RefCell::new(Some((session_id, intent)))); let handler = session.connect_ready(clone!( @@ -401,6 +431,11 @@ mod imp { self.intent_handler.set(session.upcast_ref(), vec![handler]); } } else { + debug!( + session = session_id, + ?intent, + "Session is still loading, queuing intent…" + ); // Wait for the session to be a `Session`. let cell = Rc::new(RefCell::new(Some((session_id, intent)))); let handler = self.session_list.connect_items_changed(clone!( diff --git a/src/intent.rs b/src/intent.rs index 6352a297..1352bae9 100644 --- a/src/intent.rs +++ b/src/intent.rs @@ -5,7 +5,7 @@ use ruma::OwnedUserId; use crate::{session::model::VerificationKey, utils::matrix::MatrixIdUri}; -/// An intent that can be handled by a session. +/// Intents that can be handled by a session. /// /// It cannot be cloned intentionnally, so it is handled only once. #[derive(Debug)] @@ -17,11 +17,34 @@ pub(crate) enum SessionIntent { } impl SessionIntent { - /// Construct a `SessionIntent` from its type and a payload in a `GVariant`. + /// Get the application action name for this session intent type. + pub(crate) fn app_action_name(&self) -> &'static str { + match self { + SessionIntent::ShowMatrixId(_) => "app.show-matrix-id", + SessionIntent::ShowIdentityVerification(_) => "app.show-identity-verification", + } + } + + /// Convert this intent to a `GVariant` with the given session ID. + pub(crate) fn to_variant_with_session_id(&self, session_id: &str) -> glib::Variant { + let payload = match self { + Self::ShowMatrixId(uri) => uri.to_variant(), + Self::ShowIdentityVerification(key) => key.to_variant(), + }; + (session_id, payload).to_variant() + } + + /// Convert a `GVariant` to a `SessionIntent` and session ID, given the + /// intent type. /// - /// Returns the intent on success. Returns `None` if the payload could not - /// be parsed successfully. - pub(crate) fn parse(intent_type: SessionIntentType, payload: &glib::Variant) -> Option { + /// Returns an `(session_id, intent)` tuple on success. Returns `None` if + /// the payload could not be parsed successfully. + pub(crate) fn from_variant_with_session_id( + intent_type: SessionIntentType, + variant: &glib::Variant, + ) -> Option<(String, Self)> { + let (session_id, payload) = variant.get::<(String, glib::Variant)>()?; + let intent = match intent_type { SessionIntentType::ShowMatrixId => Self::ShowMatrixId(payload.get::()?), SessionIntentType::ShowIdentityVerification => { @@ -31,40 +54,19 @@ impl SessionIntent { } }; - Some(intent) - } - - /// Construct a `SessionIntent` from its type and a payload in a `GVariant` - /// containing a session ID. - /// - /// Returns a `(session_id, intent)` tuple on success. Returns `None` if the - /// payload could not be parsed successfully. - pub(crate) fn parse_with_session_id( - intent_type: SessionIntentType, - payload: Option<&glib::Variant>, - ) -> Option<(String, Self)> { - let (session_id, payload) = payload?.get::<(String, glib::Variant)>()?; - let intent = Self::parse(intent_type, &payload)?; Some((session_id, intent)) } +} - /// Convert this intent to a `GVariant` with the given session ID. - pub(crate) fn to_variant_with_session_id(&self, session_id: &str) -> glib::Variant { - let payload = self.to_variant(); - (session_id, payload).to_variant() +impl From for SessionIntent { + fn from(value: MatrixIdUri) -> Self { + Self::ShowMatrixId(value) } } -impl ToVariant for SessionIntent { - fn to_variant(&self) -> glib::Variant { - match self { - SessionIntent::ShowMatrixId(matrix_uri) => matrix_uri.to_variant(), - SessionIntent::ShowIdentityVerification(verification_key) => ( - verification_key.user_id.as_str(), - verification_key.flow_id.as_str(), - ) - .to_variant(), - } +impl From for SessionIntent { + fn from(value: VerificationKey) -> Self { + Self::ShowIdentityVerification(value) } } @@ -85,14 +87,6 @@ impl SessionIntentType { SessionIntentType::ShowIdentityVerification => "show-identity-verification", } } - - /// Get the application action name for this session intent type. - pub(crate) fn app_action_name(self) -> &'static str { - match self { - SessionIntentType::ShowMatrixId => "app.show-matrix-id", - SessionIntentType::ShowIdentityVerification => "app.show-identity-verification", - } - } } impl StaticVariantType for SessionIntentType { diff --git a/src/session/model/mod.rs b/src/session/model/mod.rs index 08704f1c..3e42e274 100644 --- a/src/session/model/mod.rs +++ b/src/session/model/mod.rs @@ -12,7 +12,7 @@ mod user; mod user_sessions_list; mod verification; -pub use self::{ +pub(crate) use self::{ ignored_users::IgnoredUsers, notifications::{ Notifications, NotificationsGlobalSetting, NotificationsRoomSetting, NotificationsSettings, diff --git a/src/session/model/notifications/mod.rs b/src/session/model/notifications/mod.rs index 9ae2d3ba..7d9ab16d 100644 --- a/src/session/model/notifications/mod.rs +++ b/src/session/model/notifications/mod.rs @@ -6,13 +6,13 @@ use tracing::{debug, warn}; mod notifications_settings; -pub use self::notifications_settings::{ +pub(crate) use self::notifications_settings::{ NotificationsGlobalSetting, NotificationsRoomSetting, NotificationsSettings, }; use super::{IdentityVerification, Session, VerificationKey}; use crate::{ gettext_f, - intent::{SessionIntent, SessionIntentType}, + intent::SessionIntent, prelude::*, spawn_tokio, utils::matrix::{ @@ -35,18 +35,18 @@ mod imp { pub struct Notifications { /// The current session. #[property(get, set = Self::set_session, explicit_notify, nullable)] - pub session: glib::WeakRef, + session: glib::WeakRef, /// The push notifications that were presented. /// /// A map of room ID to list of notification IDs. - pub push: RefCell>>, + pub(super) push: RefCell>>, /// The identity verification notifications that were presented. /// /// A map of verification key to notification ID. - pub identity_verifications: RefCell>, + pub(super) identity_verifications: RefCell>, /// The notifications settings for this session. #[property(get)] - pub settings: NotificationsSettings, + settings: NotificationsSettings, } #[glib::object_subclass] @@ -84,7 +84,7 @@ impl Notifications { } /// Whether notifications are enabled for the current session. - pub fn enabled(&self) -> bool { + pub(crate) fn enabled(&self) -> bool { let settings = self.settings(); settings.account_enabled() && settings.session_enabled() } @@ -94,7 +94,8 @@ impl Notifications { id: &str, title: &str, body: &str, - default_action: (&str, glib::Variant), + session_id: &str, + intent: &SessionIntent, icon: Option<&gdk::Texture>, ) { let notification = gio::Notification::new(title); @@ -102,7 +103,8 @@ impl Notifications { notification.set_priority(gio::NotificationPriority::High); notification.set_body(Some(body)); - let (action, target_value) = default_action; + let action = intent.app_action_name(); + let target_value = intent.to_variant_with_session_id(session_id); notification.set_default_action_and_target_value(action, Some(&target_value)); if let Some(notification_icon) = icon { @@ -116,7 +118,11 @@ impl Notifications { /// /// The notification will not be shown if the application is active and the /// room of the event is displayed. - pub async fn show_push(&self, matrix_notification: Notification, matrix_room: MatrixRoom) { + pub(crate) async fn show_push( + &self, + matrix_notification: Notification, + matrix_room: MatrixRoom, + ) { // Do not show notifications if they are disabled. if !self.enabled() { return; @@ -164,7 +170,7 @@ impl Notifications { let handle = spawn_tokio!(async move { matrix_room.get_member_no_sync(&owned_sender_id).await }); - let sender = match handle.await.unwrap() { + let sender = match handle.await.expect("task was not aborted") { Ok(member) => member, Err(error) => { warn!("Could not get member for notification: {error}"); @@ -212,18 +218,14 @@ impl Notifications { let random_id = glib::uuid_string_random(); format!("{session_id}//{matrix_uri}//{random_id}") }; - let payload = - SessionIntent::ShowMatrixId(matrix_uri).to_variant_with_session_id(session_id); let icon = room.avatar_data().as_notification_icon().await; Self::send_notification( &id, &room.display_name(), &body, - ( - SessionIntentType::ShowMatrixId.app_action_name(), - payload.to_variant(), - ), + session_id, + &SessionIntent::ShowMatrixId(matrix_uri), icon.as_ref(), ); @@ -236,7 +238,10 @@ impl Notifications { } /// Show a notification for the given in-room identity verification. - pub async fn show_in_room_identity_verification(&self, verification: &IdentityVerification) { + pub(crate) async fn show_in_room_identity_verification( + &self, + verification: &IdentityVerification, + ) { // Do not show notifications if they are disabled. if !self.enabled() { return; @@ -265,9 +270,6 @@ impl Notifications { &[("user", &user.display_name())], ); - let payload = SessionIntent::ShowIdentityVerification(verification.key()) - .to_variant_with_session_id(session_id); - let icon = user.avatar_data().as_notification_icon().await; let id = format!("{session_id}//{room_id}//{user_id}//{flow_id}"); @@ -275,10 +277,8 @@ impl Notifications { &id, &title, &body, - ( - SessionIntentType::ShowIdentityVerification.app_action_name(), - payload.to_variant(), - ), + session_id, + &SessionIntent::ShowIdentityVerification(verification.key()), icon.as_ref(), ); @@ -289,7 +289,10 @@ impl Notifications { } /// Show a notification for the given to-device identity verification. - pub async fn show_to_device_identity_verification(&self, verification: &IdentityVerification) { + pub(crate) async fn show_to_device_identity_verification( + &self, + verification: &IdentityVerification, + ) { // Do not show notifications if they are disabled. if !self.enabled() { return; @@ -310,7 +313,7 @@ impl Notifications { let request = get_device::v3::Request::new(other_device_id.clone()); let handle = spawn_tokio!(async move { client.send(request).await }); - let display_name = match handle.await.unwrap() { + let display_name = match handle.await.expect("task was not aborted") { Ok(res) => res.device.display_name, Err(error) => { warn!("Could not get device for notification: {error}"); @@ -329,19 +332,14 @@ impl Notifications { &[("name", display_name)], ); - let payload = SessionIntent::ShowIdentityVerification(verification.key()) - .to_variant_with_session_id(session_id); - let id = format!("{session_id}//{other_device_id}//{flow_id}"); Self::send_notification( &id, &title, &body, - ( - SessionIntentType::ShowIdentityVerification.app_action_name(), - payload.to_variant(), - ), + session_id, + &SessionIntent::ShowIdentityVerification(verification.key()), None, ); @@ -356,7 +354,7 @@ impl Notifications { /// /// Only the notifications that were shown since the application's startup /// are known, older ones might still be present. - pub fn withdraw_all_for_room(&self, room_id: &RoomId) { + pub(crate) fn withdraw_all_for_room(&self, room_id: &RoomId) { if let Some(notifications) = self.imp().push.borrow_mut().remove(room_id) { let app = Application::default(); @@ -368,7 +366,7 @@ impl Notifications { /// Ask the system to remove the known notification for the identity /// verification with the given key. - pub fn withdraw_identity_verification(&self, key: &VerificationKey) { + pub(crate) fn withdraw_identity_verification(&self, key: &VerificationKey) { if let Some(id) = self.imp().identity_verifications.borrow_mut().remove(key) { let app = Application::default(); app.withdraw_notification(&id); @@ -379,7 +377,7 @@ impl Notifications { /// /// Only the notifications that were shown since the application's startup /// are known, older ones might still be present. - pub fn clear(&self) { + pub(crate) fn clear(&self) { let app = Application::default(); for id in self.imp().push.take().values().flatten() { diff --git a/src/session/model/notifications/notifications_settings.rs b/src/session/model/notifications/notifications_settings.rs index d5f66e71..7bddb046 100644 --- a/src/session/model/notifications/notifications_settings.rs +++ b/src/session/model/notifications/notifications_settings.rs @@ -12,8 +12,9 @@ use ruma::{ push::{PredefinedOverrideRuleId, RuleKind}, OwnedRoomId, RoomId, }; -use tokio::sync::broadcast::error::RecvError; -use tracing::{error, warn}; +use tokio::task::AbortHandle; +use tokio_stream::wrappers::BroadcastStream; +use tracing::error; use crate::{ session::model::{Room, Session, SessionState}, @@ -24,36 +25,34 @@ use crate::{ #[derive( Debug, Default, Hash, Eq, PartialEq, Clone, Copy, glib::Enum, strum::Display, strum::EnumString, )] -#[repr(u32)] #[enum_type(name = "NotificationsGlobalSetting")] #[strum(serialize_all = "kebab-case")] pub enum NotificationsGlobalSetting { /// Every message in every room. #[default] - All = 0, + All, /// Every message in 1-to-1 rooms, and mentions and keywords in every room. - DirectAndMentions = 1, + DirectAndMentions, /// Only mentions and keywords in every room. - MentionsOnly = 2, + MentionsOnly, } /// The possible values for a room notifications setting. #[derive( Debug, Default, Hash, Eq, PartialEq, Clone, Copy, glib::Enum, strum::Display, strum::EnumString, )] -#[repr(u32)] #[enum_type(name = "NotificationsRoomSetting")] #[strum(serialize_all = "kebab-case")] pub enum NotificationsRoomSetting { /// Use the global setting. #[default] - Global = 0, + Global, /// All messages. - All = 1, + All, /// Only mentions and keywords. - MentionsOnly = 2, + MentionsOnly, /// No notifications. - Mute = 3, + Mute, } impl NotificationsRoomSetting { @@ -88,25 +87,26 @@ mod imp { pub struct NotificationsSettings { /// The parent `Session`. #[property(get, set = Self::set_session, explicit_notify, nullable)] - pub session: glib::WeakRef, + session: glib::WeakRef, /// The SDK notification settings API. - pub api: RefCell>, + api: RefCell>, /// Whether notifications are enabled for this Matrix account. #[property(get)] - pub account_enabled: Cell, + account_enabled: Cell, /// Whether notifications are enabled for this session. #[property(get, set = Self::set_session_enabled, explicit_notify)] - pub session_enabled: Cell, + session_enabled: Cell, /// The global setting about which messages trigger notifications. #[property(get, builder(NotificationsGlobalSetting::default()))] - pub global_setting: Cell, + global_setting: Cell, /// The list of keywords that trigger notifications. #[property(get)] - pub keywords_list: gtk::StringList, + keywords_list: gtk::StringList, /// The map of room ID to per-room notification setting. /// /// Any room not in this map uses the global setting. - pub per_room_settings: RefCell>, + per_room_settings: RefCell>, + abort_handle: RefCell>, } #[glib::object_subclass] @@ -116,7 +116,13 @@ mod imp { } #[glib::derived_properties] - impl ObjectImpl for NotificationsSettings {} + impl ObjectImpl for NotificationsSettings { + fn dispose(&self) { + if let Some(handle) = self.abort_handle.take() { + handle.abort(); + } + } + } impl NotificationsSettings { /// Set the parent `Session`. @@ -140,10 +146,10 @@ mod imp { obj.notify_session(); spawn!(clone!( - #[weak] - obj, + #[weak(rename_to = imp)] + self, async move { - obj.init_api().await; + imp.init_api().await; } )); } @@ -163,138 +169,256 @@ mod imp { self.session_enabled.set(enabled); self.obj().notify_session_enabled(); } - } -} - -glib::wrapper! { - /// The notifications settings of a `Session`. - pub struct NotificationsSettings(ObjectSubclass); -} -impl NotificationsSettings { - /// Create a new `NotificationsSettings`. - pub fn new() -> Self { - glib::Object::new() - } + /// The SDK notification settings API. + pub(super) fn api(&self) -> Option { + self.api.borrow().clone() + } - /// The SDK notification settings API. - fn api(&self) -> Option { - self.imp().api.borrow().clone() - } + /// Initialize the SDK notification settings API. + async fn init_api(&self) { + let Some(session) = self.session.upgrade() else { + self.api.take(); + return; + }; - /// Initialize the SDK notification settings API. - async fn init_api(&self) { - let Some(session) = self.session() else { - self.imp().api.take(); - return; - }; + // If the session is not ready, there is no client so let's wait to initialize + // the API. + if session.state() != SessionState::Ready { + self.api.take(); + + session.connect_ready(clone!( + #[weak(rename_to = imp)] + self, + move |_| { + spawn!(async move { + imp.init_api().await; + }); + } + )); - // If the session is not ready, there is no client so let's wait to initialize - // the API. - if session.state() != SessionState::Ready { - self.imp().api.take(); + return; + } - session.connect_ready(clone!( - #[weak(rename_to = obj)] - self, - move |_| { - spawn!(async move { - obj.init_api().await; - }); - } - )); + let client = session.client(); + let api = spawn_tokio!(async move { client.notification_settings().await }) + .await + .expect("task was not aborted"); + let stream = BroadcastStream::new(api.subscribe_to_changes()); - return; - } + self.api.replace(Some(api.clone())); - let client = session.client(); - let api = spawn_tokio!(async move { client.notification_settings().await }) - .await - .unwrap(); - let mut api_receiver = api.subscribe_to_changes(); - - self.imp().api.replace(Some(api.clone())); - - let (mut sender, mut receiver) = futures_channel::mpsc::channel(10); - spawn_tokio!(async move { - loop { - match api_receiver.recv().await { - Ok(()) => { - if let Err(error) = sender.try_send(()) { - error!("Error sending notifications settings change: {error}"); - panic!(); - } - } - Err(RecvError::Closed) => { - break; - } - Err(RecvError::Lagged(_)) => { - warn!("Some notifications settings changes were dropped"); + let obj_weak = glib::SendWeakRef::from(self.obj().downgrade()); + let fut = stream.for_each(move |res| { + let obj_weak = obj_weak.clone(); + async move { + if res.is_err() { + return; } + + let ctx = glib::MainContext::default(); + ctx.spawn(async move { + spawn!(async move { + if let Some(obj) = obj_weak.upgrade() { + obj.imp().update().await; + } + }); + }); } - } - }); + }); - spawn!(clone!( - #[weak(rename_to = obj)] - self, - async move { - obj.update().await; - } - )); + self.abort_handle + .replace(Some(spawn_tokio!(fut).abort_handle())); - while let Some(()) = receiver.next().await { spawn!(clone!( - #[weak(rename_to = obj)] + #[weak(rename_to = imp)] self, async move { - obj.update().await; + imp.update().await; } )); } - } - /// Update the notification settings from the SDK API. - async fn update(&self) { - let Some(api) = self.api() else { - return; - }; + /// Update the notification settings from the SDK API. + async fn update(&self) { + let Some(api) = self.api() else { + return; + }; - let api_clone = api.clone(); - let handle = spawn_tokio!(async move { - api_clone - .is_push_rule_enabled(RuleKind::Override, PredefinedOverrideRuleId::Master) + let api_clone = api.clone(); + let handle = spawn_tokio!(async move { + api_clone + .is_push_rule_enabled(RuleKind::Override, PredefinedOverrideRuleId::Master) + .await + }); + + let account_enabled = match handle.await.expect("task was not aborted") { + // The rule disables notifications, so we need to invert the boolean. + Ok(enabled) => !enabled, + Err(error) => { + error!("Could not get account notifications setting: {error}"); + true + } + }; + self.set_account_enabled(account_enabled); + + if default_rooms_notifications_is_all(api.clone(), false).await { + self.set_global_setting(NotificationsGlobalSetting::All); + } else if default_rooms_notifications_is_all(api, true).await { + self.set_global_setting(NotificationsGlobalSetting::DirectAndMentions); + } else { + self.set_global_setting(NotificationsGlobalSetting::MentionsOnly); + } + + self.update_keywords_list().await; + self.update_per_room_settings().await; + } + + /// Set whether notifications are enabled for this session. + pub(super) fn set_account_enabled(&self, enabled: bool) { + if self.account_enabled.get() == enabled { + return; + } + + self.account_enabled.set(enabled); + self.obj().notify_account_enabled(); + } + + /// Set the global setting about which messages trigger notifications. + pub(super) fn set_global_setting(&self, setting: NotificationsGlobalSetting) { + if self.global_setting.get() == setting { + return; + } + + self.global_setting.set(setting); + self.obj().notify_global_setting(); + } + + /// Update the local list of keywords with the remote one. + pub(super) async fn update_keywords_list(&self) { + let Some(api) = self.api() else { + return; + }; + + let keywords = spawn_tokio!(async move { api.enabled_keywords().await }) .await - }); + .expect("task was not aborted"); + + let list = &self.keywords_list; + let mut diverges_at = None; + + let keywords = keywords.iter().map(String::as_str).collect::>(); + let new_len = keywords.len() as u32; + let old_len = list.n_items(); + + // Check if there is any keyword that changed, was moved or was added. + for (pos, keyword) in keywords.iter().enumerate() { + if Some(*keyword) + != list + .item(pos as u32) + .and_downcast::() + .map(|o| o.string()) + .as_deref() + { + diverges_at = Some(pos as u32); + break; + } + } - let account_enabled = match handle.await.unwrap() { - // The rule disables notifications, so we need to invert the boolean. - Ok(enabled) => !enabled, - Err(error) => { - error!("Could not get account notifications setting: {error}"); - true + // Check if keywords were removed. + if diverges_at.is_none() && old_len > new_len { + diverges_at = Some(new_len); } - }; - self.set_account_enabled_inner(account_enabled); - if default_rooms_notifications_is_all(api.clone(), false).await { - self.set_global_setting_inner(NotificationsGlobalSetting::All); - } else if default_rooms_notifications_is_all(api, true).await { - self.set_global_setting_inner(NotificationsGlobalSetting::DirectAndMentions); - } else { - self.set_global_setting_inner(NotificationsGlobalSetting::MentionsOnly); + let Some(pos) = diverges_at else { + // Nothing to do. + return; + }; + + let additions = &keywords[pos as usize..]; + list.splice(pos, old_len.saturating_sub(pos), additions); } - self.update_keywords_list().await; - self.update_per_room_settings().await; + /// Update the local list of per-room settings with the remote one. + pub(super) async fn update_per_room_settings(&self) { + let Some(api) = self.api() else { + return; + }; + + let api_clone = api.clone(); + let room_ids = spawn_tokio!(async move { + api_clone + .get_rooms_with_user_defined_rules(Some(true)) + .await + }) + .await + .expect("task was not aborted"); + + // Update the local map. + let mut per_room_settings = HashMap::with_capacity(room_ids.len()); + for room_id in room_ids { + let Ok(room_id) = RoomId::parse(room_id) else { + continue; + }; + + let room_id_clone = room_id.clone(); + let api_clone = api.clone(); + let handle = spawn_tokio!(async move { + api_clone + .get_user_defined_room_notification_mode(&room_id_clone) + .await + }); + + if let Some(setting) = handle.await.expect("task was not aborted") { + per_room_settings.insert(room_id, setting.into()); + } + } + + self.per_room_settings.replace(per_room_settings.clone()); + + // Update the setting in the rooms. + // Since we don't know when a room was added or removed, we have to update every + // room. + let Some(session) = self.session.upgrade() else { + return; + }; + let room_list = session.room_list(); + + for room in room_list.iter::() { + let Ok(room) = room else { + // Returns an error when the list changed, just stop. + break; + }; + + if let Some(setting) = per_room_settings.get(room.room_id()) { + room.set_notifications_setting(*setting); + } else { + room.set_notifications_setting(NotificationsRoomSetting::Global); + } + } + } + } +} + +glib::wrapper! { + /// The notifications settings of a `Session`. + pub struct NotificationsSettings(ObjectSubclass); +} + +impl NotificationsSettings { + /// Create a new `NotificationsSettings`. + pub fn new() -> Self { + glib::Object::new() } /// Set whether notifications are enabled for this session. - pub async fn set_account_enabled( + pub(crate) async fn set_account_enabled( &self, enabled: bool, ) -> Result<(), NotificationSettingsError> { - let Some(api) = self.api() else { + let imp = self.imp(); + + let Some(api) = imp.api() else { error!("Cannot update notifications settings when API is not initialized"); return Err(NotificationSettingsError::UnableToUpdatePushRule); }; @@ -309,9 +433,9 @@ impl NotificationsSettings { .await }); - match handle.await.unwrap() { + match handle.await.expect("task was not aborted") { Ok(()) => { - self.set_account_enabled_inner(enabled); + imp.set_account_enabled(enabled); Ok(()) } Err(error) => { @@ -321,21 +445,14 @@ impl NotificationsSettings { } } - fn set_account_enabled_inner(&self, enabled: bool) { - if self.account_enabled() == enabled { - return; - } - - self.imp().account_enabled.set(enabled); - self.notify_account_enabled(); - } - /// Set the global setting about which messages trigger notifications. - pub async fn set_global_setting( + pub(crate) async fn set_global_setting( &self, setting: NotificationsGlobalSetting, ) -> Result<(), NotificationSettingsError> { - let Some(api) = self.api() else { + let imp = self.imp(); + + let Some(api) = imp.api() else { error!("Cannot update notifications settings when API is not initialized"); return Err(NotificationSettingsError::UnableToUpdatePushRule); }; @@ -356,68 +473,19 @@ impl NotificationsSettings { return Err(error); } - self.set_global_setting_inner(setting); + imp.set_global_setting(setting); Ok(()) } - fn set_global_setting_inner(&self, setting: NotificationsGlobalSetting) { - if self.global_setting() == setting { - return; - } - - self.imp().global_setting.set(setting); - self.notify_global_setting(); - } - - /// Update the local list of keywords with the remote one. - async fn update_keywords_list(&self) { - let Some(api) = self.api() else { - return; - }; - - let keywords = spawn_tokio!(async move { api.enabled_keywords().await }) - .await - .unwrap(); - - let list = &self.imp().keywords_list; - let mut diverges_at = None; - - let keywords = keywords.iter().map(String::as_str).collect::>(); - let new_len = keywords.len() as u32; - let old_len = list.n_items(); - - // Check if there is any keyword that changed, was moved or was added. - for (pos, keyword) in keywords.iter().enumerate() { - if Some(*keyword) - != list - .item(pos as u32) - .and_downcast::() - .map(|o| o.string()) - .as_deref() - { - diverges_at = Some(pos as u32); - break; - } - } - - // Check if keywords were removed. - if diverges_at.is_none() && old_len > new_len { - diverges_at = Some(new_len); - } - - let Some(pos) = diverges_at else { - // Nothing to do. - return; - }; - - let additions = &keywords[pos as usize..]; - list.splice(pos, old_len.saturating_sub(pos), additions); - } - /// Remove a keyword from the list. - pub async fn remove_keyword(&self, keyword: String) -> Result<(), NotificationSettingsError> { - let Some(api) = self.api() else { + pub(crate) async fn remove_keyword( + &self, + keyword: String, + ) -> Result<(), NotificationSettingsError> { + let imp = self.imp(); + + let Some(api) = imp.api() else { error!("Cannot update notifications settings when API is not initialized"); return Err(NotificationSettingsError::UnableToUpdatePushRule); }; @@ -425,19 +493,24 @@ impl NotificationsSettings { let keyword_clone = keyword.clone(); let handle = spawn_tokio!(async move { api.remove_keyword(&keyword_clone).await }); - if let Err(error) = handle.await.unwrap() { + if let Err(error) = handle.await.expect("task was not aborted") { error!("Could not remove notification keyword `{keyword}`: {error}"); return Err(error); } - self.update_keywords_list().await; + imp.update_keywords_list().await; Ok(()) } /// Add a keyword to the list. - pub async fn add_keyword(&self, keyword: String) -> Result<(), NotificationSettingsError> { - let Some(api) = self.api() else { + pub(crate) async fn add_keyword( + &self, + keyword: String, + ) -> Result<(), NotificationSettingsError> { + let imp = self.imp(); + + let Some(api) = imp.api() else { error!("Cannot update notifications settings when API is not initialized"); return Err(NotificationSettingsError::UnableToUpdatePushRule); }; @@ -445,84 +518,25 @@ impl NotificationsSettings { let keyword_clone = keyword.clone(); let handle = spawn_tokio!(async move { api.add_keyword(keyword_clone).await }); - if let Err(error) = handle.await.unwrap() { + if let Err(error) = handle.await.expect("task was not aborted") { error!("Could not add notification keyword `{keyword}`: {error}"); return Err(error); } - self.update_keywords_list().await; + imp.update_keywords_list().await; Ok(()) } - /// Update the local list of per-room settings with the remote one. - async fn update_per_room_settings(&self) { - let Some(api) = self.api() else { - return; - }; - - let api_clone = api.clone(); - let room_ids = spawn_tokio!(async move { - api_clone - .get_rooms_with_user_defined_rules(Some(true)) - .await - }) - .await - .unwrap(); - - // Update the local map. - let mut per_room_settings = HashMap::with_capacity(room_ids.len()); - for room_id in room_ids { - let Ok(room_id) = RoomId::parse(room_id) else { - continue; - }; - - let room_id_clone = room_id.clone(); - let api_clone = api.clone(); - let handle = spawn_tokio!(async move { - api_clone - .get_user_defined_room_notification_mode(&room_id_clone) - .await - }); - - if let Some(setting) = handle.await.unwrap() { - per_room_settings.insert(room_id, setting.into()); - } - } - - self.imp() - .per_room_settings - .replace(per_room_settings.clone()); - - // Update the setting in the rooms. - // Since we don't know when a room was added or removed, we have to update every - // room. - let Some(session) = self.session() else { - return; - }; - let room_list = session.room_list(); - - for room in room_list.iter::() { - let Ok(room) = room else { - // Returns an error when the list changed, just stop. - break; - }; - - if let Some(setting) = per_room_settings.get(room.room_id()) { - room.set_notifications_setting(*setting); - } else { - room.set_notifications_setting(NotificationsRoomSetting::Global); - } - } - } - /// Set the notification setting for the room with the given ID. - pub async fn set_per_room_setting( + pub(crate) async fn set_per_room_setting( &self, room_id: OwnedRoomId, setting: NotificationsRoomSetting, ) -> Result<(), NotificationSettingsError> { - let Some(api) = self.api() else { + let imp = self.imp(); + + let Some(api) = imp.api() else { error!("Cannot update notifications settings when API is not initialized"); return Err(NotificationSettingsError::UnableToUpdatePushRule); }; @@ -534,12 +548,12 @@ impl NotificationsSettings { spawn_tokio!(async move { api.delete_user_defined_room_rules(&room_id_clone).await }) }; - if let Err(error) = handle.await.unwrap() { + if let Err(error) = handle.await.expect("task was not aborted") { error!("Could not update notifications setting for room `{room_id}`: {error}"); return Err(error); } - self.update_per_room_settings().await; + imp.update_per_room_settings().await; Ok(()) } @@ -560,7 +574,7 @@ async fn default_rooms_notifications_is_all( .await }) .await - .unwrap(); + .expect("task was not aborted"); mode == RoomNotificationMode::AllMessages } @@ -581,5 +595,5 @@ async fn set_default_rooms_notifications_all( .await }) .await - .unwrap() + .expect("task was not aborted") } diff --git a/src/session/model/verification/identity_verification.rs b/src/session/model/verification/identity_verification.rs index 7f1a3c54..71dda6d0 100644 --- a/src/session/model/verification/identity_verification.rs +++ b/src/session/model/verification/identity_verification.rs @@ -422,7 +422,7 @@ impl IdentityVerification { } /// The unique identifying key of this verification. - pub fn key(&self) -> VerificationKey { + pub(crate) fn key(&self) -> VerificationKey { VerificationKey::from_request(self.request()) } diff --git a/src/session/model/verification/mod.rs b/src/session/model/verification/mod.rs index 601ae242..bba7de0d 100644 --- a/src/session/model/verification/mod.rs +++ b/src/session/model/verification/mod.rs @@ -1,10 +1,11 @@ +use gtk::{glib, prelude::*}; use matrix_sdk::encryption::verification::VerificationRequest; -use ruma::{events::key::verification::VerificationMethod, OwnedUserId}; +use ruma::{events::key::verification::VerificationMethod, OwnedUserId, UserId}; mod identity_verification; mod verification_list; -pub use self::{ +pub(crate) use self::{ identity_verification::{ IdentityVerification, VerificationState, VerificationSupportedMethods, }, @@ -14,21 +15,21 @@ use crate::{components::Camera, prelude::*}; /// A unique key to identify an identity verification. #[derive(Debug, Clone, Hash, PartialEq, Eq)] -pub struct VerificationKey { +pub(crate) struct VerificationKey { /// The ID of the user being verified. - pub user_id: OwnedUserId, + pub(crate) user_id: OwnedUserId, /// The ID of the verification. - pub flow_id: String, + pub(crate) flow_id: String, } impl VerificationKey { /// Create a new `VerificationKey` with the given user ID and flow ID. - pub fn new(user_id: OwnedUserId, flow_id: String) -> Self { + pub(crate) fn new(user_id: OwnedUserId, flow_id: String) -> Self { Self { user_id, flow_id } } /// Create a new `VerificationKey` from the given [`VerificationRequest`]. - pub fn from_request(request: &VerificationRequest) -> Self { + pub(crate) fn from_request(request: &VerificationRequest) -> Self { Self::new( request.other_user_id().to_owned(), request.flow_id().to_owned(), @@ -36,6 +37,26 @@ impl VerificationKey { } } +impl StaticVariantType for VerificationKey { + fn static_variant_type() -> std::borrow::Cow<'static, glib::VariantTy> { + <(String, String)>::static_variant_type() + } +} + +impl ToVariant for VerificationKey { + fn to_variant(&self) -> glib::Variant { + (self.user_id.as_str(), self.flow_id.as_str()).to_variant() + } +} + +impl FromVariant for VerificationKey { + fn from_variant(variant: &glib::Variant) -> Option { + let (user_id_str, flow_id) = variant.get::<(String, String)>()?; + let user_id = UserId::parse(user_id_str).ok()?; + Some(Self { user_id, flow_id }) + } +} + /// Load the supported verification methods on this system. async fn load_supported_verification_methods() -> Vec { let mut methods = vec![ diff --git a/src/session/model/verification/verification_list.rs b/src/session/model/verification/verification_list.rs index e1a74078..eb2393f2 100644 --- a/src/session/model/verification/verification_list.rs +++ b/src/session/model/verification/verification_list.rs @@ -29,7 +29,7 @@ mod imp { #[properties(wrapper_type = super::VerificationList)] pub struct VerificationList { /// The ongoing verification requests. - pub list: RefCell>, + pub(super) list: RefCell>, /// The current session. #[property(get, construct_only)] pub session: glib::WeakRef, @@ -282,7 +282,7 @@ impl VerificationList { } /// Remove the verification with the given key. - pub fn remove(&self, key: &VerificationKey) { + pub(crate) fn remove(&self, key: &VerificationKey) { let Some((pos, ..)) = self.imp().list.borrow_mut().shift_remove_full(key) else { return; }; @@ -295,7 +295,7 @@ impl VerificationList { } /// Get the verification with the given key. - pub fn get(&self, key: &VerificationKey) -> Option { + pub(crate) fn get(&self, key: &VerificationKey) -> Option { self.imp().list.borrow().get(key).cloned() } diff --git a/src/window.rs b/src/window.rs index d0d2d821..955a6b6f 100644 --- a/src/window.rs +++ b/src/window.rs @@ -235,7 +235,6 @@ mod imp { } impl WindowImpl for Window { - // save window state on delete event fn close_request(&self) -> glib::Propagation { if let Err(error) = self.save_window_size() { warn!("Could not save window state: {error}");