From 87f3c2aff0059d6f54de93e367d7d22c35156bc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Wed, 30 Jul 2025 16:33:55 +0200 Subject: [PATCH] room-details: Do no allow to "downgrade" a room --- Cargo.lock | 7 + Cargo.toml | 1 + .../view/content/room_details/general_page.rs | 40 ++- src/session/view/content/room_details/mod.rs | 2 +- .../room_details/upgrade_dialog/mod.rs | 323 +++++++++++++----- .../upgrade_dialog/room_version.rs | 57 +--- 6 files changed, 293 insertions(+), 137 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 73e1068a..561a5db9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1263,6 +1263,7 @@ dependencies = [ "matrix-sdk-ui", "mime", "mime_guess", + "numeric-sort", "oo7", "pulldown-cmark", "qrcode", @@ -3542,6 +3543,12 @@ dependencies = [ "libc", ] +[[package]] +name = "numeric-sort" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2dcb6053ab98da45585315f79932c5c9821fab8efa4301c0d7b637c91630eb7" + [[package]] name = "oauth2" version = "5.0.0" diff --git a/Cargo.toml b/Cargo.toml index 2cc2d40a..7b184bff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ indexmap = "2" linkify = "0.10.0" mime = "0.3" mime_guess = "2" +numeric-sort = "0.1" pulldown-cmark = "0.13" qrcode = { version = "0.14", default-features = false } rand = "0.9" diff --git a/src/session/view/content/room_details/general_page.rs b/src/session/view/content/room_details/general_page.rs index 2e205b13..8f8bf25e 100644 --- a/src/session/view/content/room_details/general_page.rs +++ b/src/session/view/content/room_details/general_page.rs @@ -22,7 +22,7 @@ use ruma::{ }; use tracing::error; -use super::{MemberRow, RoomDetails, UpgradeDialog}; +use super::{MemberRow, RoomDetails, UpgradeDialog, UpgradeInfo}; use crate::{ Window, components::{ @@ -118,6 +118,9 @@ mod imp { /// Whether the room is published in the directory. #[property(get)] is_published: Cell, + capabilities: RefCell, + upgrade_info: RefCell>, + direct_members_list_has_bound_model: Cell, expr_watch: RefCell>, notifications_settings_handlers: RefCell>, membership_handler: RefCell>, @@ -125,8 +128,6 @@ mod imp { canonical_alias_handler: RefCell>, alt_aliases_handler: RefCell>, join_rule_handler: RefCell>, - capabilities: RefCell, - direct_members_list_has_bound_model: Cell, } #[glib::object_subclass] @@ -402,7 +403,7 @@ mod imp { self, async move { let handle = spawn_tokio!(async move { client.get_capabilities().await }); - match handle.await.unwrap() { + match handle.await.expect("task was not aborted") { Ok(capabilities) => { imp.capabilities.replace(capabilities); } @@ -411,6 +412,8 @@ mod imp { imp.capabilities.take(); } } + + imp.update_upgrade_info(); } ) ); @@ -1047,6 +1050,26 @@ mod imp { } } + /// Update the room upgrade info. + fn update_upgrade_info(&self) { + let current_room_version = self + .room + .obj() + .and_then(|room| room.matrix_room().create_content()) + .map(|create_content| create_content.room_version); + + let upgrade_info = current_room_version.map(|current_room_version| { + UpgradeInfo::new( + ¤t_room_version, + &self.capabilities.borrow().room_versions, + ) + }); + + self.upgrade_info.replace(upgrade_info); + + self.update_upgrade_button(); + } + /// Update the room upgrade button. fn update_upgrade_button(&self) { let Some(room) = self.room.obj() else { @@ -1056,7 +1079,8 @@ mod imp { let can_upgrade = !room.is_tombstoned() && room .permissions() - .is_allowed_to(PowerLevelAction::SendState(StateEventType::RoomTombstone)); + .is_allowed_to(PowerLevelAction::SendState(StateEventType::RoomTombstone)) + && self.upgrade_info.borrow().is_some(); self.upgrade_button.set_visible(can_upgrade); } @@ -1083,13 +1107,15 @@ mod imp { let Some(room) = self.room.obj() else { return; }; + let Some(upgrade_info) = self.upgrade_info.borrow().clone() else { + return; + }; let obj = self.obj(); self.upgrade_button.set_is_loading(true); - let room_versions_capability = self.capabilities.borrow().room_versions.clone(); let Some(new_version) = UpgradeDialog::new() - .confirm_upgrade(room_versions_capability, &*obj) + .confirm_upgrade(&upgrade_info, &*obj) .await else { self.upgrade_button.set_is_loading(false); diff --git a/src/session/view/content/room_details/mod.rs b/src/session/view/content/room_details/mod.rs index 1e0092c2..6d359281 100644 --- a/src/session/view/content/room_details/mod.rs +++ b/src/session/view/content/room_details/mod.rs @@ -32,7 +32,7 @@ use self::{ members_page::MembersPage, membership_subpage_item::MembershipSubpageItem, permissions::PermissionsSubpage, - upgrade_dialog::UpgradeDialog, + upgrade_dialog::{UpgradeDialog, UpgradeInfo}, }; use crate::{ components::UserPage, diff --git a/src/session/view/content/room_details/upgrade_dialog/mod.rs b/src/session/view/content/room_details/upgrade_dialog/mod.rs index 8eadac71..79b44a00 100644 --- a/src/session/view/content/room_details/upgrade_dialog/mod.rs +++ b/src/session/view/content/room_details/upgrade_dialog/mod.rs @@ -1,3 +1,5 @@ +use std::cmp::Ordering; + use adw::{prelude::*, subclass::prelude::*}; use futures_channel::oneshot; use gettextrs::gettext; @@ -13,7 +15,7 @@ mod room_version; use self::room_version::RoomVersion; mod imp { - use std::cell::RefCell; + use std::cell::{OnceCell, RefCell}; use glib::subclass::InitializingObject; @@ -26,6 +28,7 @@ mod imp { pub struct UpgradeDialog { #[template_child] version_combo: TemplateChild, + header_factory: OnceCell, /// The sender for the response of the user. sender: RefCell>>>, } @@ -52,46 +55,6 @@ mod imp { self.version_combo .set_expression(Some(RoomVersion::this_expression("id-string"))); - - // Construct the header factory to separate stable from experimental versions. - let header_factory = gtk::SignalListItemFactory::new(); - header_factory.connect_setup(|_, header| { - let Some(header) = header.downcast_ref::() else { - error!("List item factory did not receive a list header: {header:?}"); - return; - }; - - let label = gtk::Label::builder() - .margin_start(12) - .xalign(0.0) - .ellipsize(pango::EllipsizeMode::End) - .css_classes(["heading"]) - .build(); - header.set_child(Some(&label)); - }); - header_factory.connect_bind(|_, header| { - let Some(header) = header.downcast_ref::() else { - error!("List item factory did not receive a list header: {header:?}"); - return; - }; - let Some(label) = header.child().and_downcast::() else { - error!("List header does not have a child GtkLabel"); - return; - }; - let Some(version) = header.item().and_downcast::() else { - error!("List header does not have a RoomVersion item"); - return; - }; - - let text = match version.stability() { - // Translators: As in 'Stable version'. - RoomVersionStability::Stable => gettext("Stable"), - // Translators: As in 'Experimental version'. - _ => gettext("Experimental"), - }; - label.set_label(&text); - }); - self.version_combo.set_header_factory(Some(&header_factory)); } } @@ -111,6 +74,53 @@ mod imp { #[gtk::template_callbacks] impl UpgradeDialog { + /// The header factory to separate stable from experimental versions. + fn header_factory(&self) -> >k::SignalListItemFactory { + self.header_factory.get_or_init(|| { + let header_factory = gtk::SignalListItemFactory::new(); + + header_factory.connect_setup(|_, header| { + let Some(header) = header.downcast_ref::() else { + error!("List item factory did not receive a list header: {header:?}"); + return; + }; + + let label = gtk::Label::builder() + .margin_start(12) + .xalign(0.0) + .ellipsize(pango::EllipsizeMode::End) + .css_classes(["heading"]) + .build(); + header.set_child(Some(&label)); + }); + header_factory.connect_bind(|_, header| { + let Some(header) = header.downcast_ref::() else { + error!("List item factory did not receive a list header: {header:?}"); + return; + }; + let Some(label) = header.child().and_downcast::() else { + error!("List header does not have a child GtkLabel"); + return; + }; + let Some(version) = header.item().and_downcast::() else { + error!("List header does not have a RoomVersion item"); + return; + }; + + let text = if version.is_stable() { + // Translators: As in 'Stable version'. + gettext("Stable") + } else { + // Translators: As in 'Experimental version'. + gettext("Experimental") + }; + label.set_label(&text); + }); + + header_factory + }) + } + /// Ask the user to confirm the room upgrade and select a room version /// among the ones that are supported by the server. /// @@ -118,10 +128,10 @@ mod imp { /// the upgrade. pub(super) async fn confirm_upgrade( &self, - capability: RoomVersionsCapability, + info: &UpgradeInfo, parent: >k::Widget, ) -> Option { - self.update_version_combo(capability); + self.update_version_combo(info); let (sender, receiver) = oneshot::channel(); self.sender.replace(Some(sender)); @@ -132,48 +142,40 @@ mod imp { .expect("sender should not have been dropped prematurely") } - /// Update the room versions combo row with the given capability. - fn update_version_combo(&self, capability: RoomVersionsCapability) { - // Build the lists. - let default = capability.default; - let (mut stable_list, mut experimental_list) = capability - .available - .into_iter() - .map(|(id, stability)| { - let stability = if id == default { - // According to the spec, the default version is always assumed to be - // stable. - RoomVersionStability::Stable - } else { - stability - }; - - RoomVersion::new(id, stability) - }) - .partition::, _>(|version| { - *version.stability() == RoomVersionStability::Stable - }); - - stable_list.sort_unstable_by(RoomVersion::cmp_ids); - experimental_list.sort_unstable_by(RoomVersion::cmp_ids); - - let default_pos = stable_list - .iter() - .position(|v| *v.id() == default) - .unwrap_or_default(); - + /// Update the room versions combo row with the given details. + fn update_version_combo(&self, info: &UpgradeInfo) { // Construct the list models for the combo row. - let stable_model = stable_list.into_iter().collect::(); - let experimental_model = experimental_list.into_iter().collect::(); + let stable_model = (!info.stable_room_versions.is_empty()).then(|| { + info.stable_room_versions + .iter() + .map(|version| RoomVersion::new(version.clone(), true)) + .collect::() + }); + let unstable_model = (!info.unstable_room_versions.is_empty()).then(|| { + info.unstable_room_versions + .iter() + .map(|version| RoomVersion::new(version.clone(), false)) + .collect::() + }); - let model_list = gio::ListStore::new::(); - model_list.append(&stable_model); - model_list.append(&experimental_model); - let flatten_model = gtk::FlattenListModel::new(Some(model_list)); + let use_header_factory = unstable_model.is_some(); + let model = match (stable_model, unstable_model) { + (Some(model), None) | (None, Some(model)) => model.upcast::(), + (Some(stable_model), Some(unstable_model)) => { + let model_list = gio::ListStore::new::(); + model_list.append(&stable_model); + model_list.append(&unstable_model); + gtk::FlattenListModel::new(Some(model_list)).upcast() + } + // We always have at least the current room version. + (None, None) => unreachable!(), + }; - self.version_combo.set_model(Some(&flatten_model)); self.version_combo - .set_selected(default_pos.try_into().unwrap_or(u32::MAX)); + .set_header_factory(use_header_factory.then(|| self.header_factory())); + self.version_combo.set_model(Some(&model)); + self.version_combo + .set_selected(info.selected.try_into().unwrap_or(u32::MAX)); } /// Confirm the upgrade. @@ -223,11 +225,162 @@ impl UpgradeDialog { /// upgrade. pub(crate) async fn confirm_upgrade( &self, - capability: RoomVersionsCapability, + info: &UpgradeInfo, parent: &impl IsA, ) -> Option { - self.imp() - .confirm_upgrade(capability, parent.upcast_ref()) - .await + self.imp().confirm_upgrade(info, parent.upcast_ref()).await + } +} + +/// The information necessary for [`UpgradeDialog`]. +#[derive(Debug, Clone)] +pub(crate) struct UpgradeInfo { + /// The sorted stable room versions available for the upgrade. + pub(crate) stable_room_versions: Vec, + /// The sorted unstable room versions available for the upgrade. + pub(crate) unstable_room_versions: Vec, + /// The position of the room version that should be selected by default, + /// when `stable_room_versions` and `unstable_room_versions` are + /// concatenated. + pub(crate) selected: usize, +} + +impl UpgradeInfo { + /// Construct the `UpgradeInfo` with the given capability and current room + /// version. + /// + /// We do not allow users to: + /// + /// - Downgrade the room, i.e. use a lower room version. + /// - Upgrade to a version lower than the server's default, if the server's + /// default is stable. + /// - Upgrade to an experimental version, unless it is the current version + /// or the server's default. + /// + /// If the server's default is experimental, we also allow to upgrade to the + /// highest stable version. + pub(crate) fn new( + current_room_version: &RoomVersionId, + capability: &RoomVersionsCapability, + ) -> Self { + let current_is_stable = capability.is_stable_version(current_room_version); + let default_is_stable = capability.is_stable_version(&capability.default); + let maximum_stable_version = capability.maximum_stable_version(); + + // The minimum stable version is the highest stable version between the current + // version and the default version. + let minimum_stable_version = match (current_is_stable, default_is_stable) { + (true, false) => Some(current_room_version), + (false, true) => Some(&capability.default), + (true, true) => Some( + match numeric_sort::cmp(current_room_version.as_ref(), capability.default.as_ref()) + { + Ordering::Less => &capability.default, + Ordering::Equal | Ordering::Greater => current_room_version, + }, + ), + (false, false) => None, + }; + let selected_room_version = minimum_stable_version.unwrap_or(&capability.default); + + let mut stable_room_versions = if let Some(minimum) = minimum_stable_version { + // Keep all the stable versions higher than the minimum. + capability + .available + .iter() + .filter_map(|(version, stability)| { + // Discard unstable versions. + if *stability != RoomVersionStability::Stable { + return None; + } + + if numeric_sort::cmp(version.as_ref(), minimum.as_ref()) != Ordering::Less + || maximum_stable_version.is_some_and(|maximum| maximum == version) + { + Some(version) + } else { + None + } + }) + .cloned() + .collect::>() + } else { + // The only allowed stable version will be the maximum. + maximum_stable_version.into_iter().cloned().collect() + }; + + // Add the current and default room versions if they are unstable. + let current_is_default = *current_room_version == capability.default; + let mut unstable_room_versions = Some(current_room_version) + .filter(|_| !current_is_stable) + .cloned() + .into_iter() + .chain( + Some(&capability.default) + .filter(|_| !current_is_default && !default_is_stable) + .cloned(), + ) + .collect::>(); + + // Sort all the versions. + numeric_sort::sort_unstable(&mut stable_room_versions); + numeric_sort::sort_unstable(&mut unstable_room_versions); + + // Find the position of the selected version. + let selected = stable_room_versions + .binary_search_by(|version| { + numeric_sort::cmp(version.as_ref(), selected_room_version.as_ref()) + }) + .or_else(|_| { + unstable_room_versions + .binary_search_by(|version| { + numeric_sort::cmp(version.as_ref(), selected_room_version.as_ref()) + }) + .map(|pos| stable_room_versions.len() + pos) + }) + .unwrap_or_default(); + + Self { + stable_room_versions, + unstable_room_versions, + selected, + } + } +} + +/// Helper trait for [`RoomVersionsCapability`]. +trait RoomVersionsCapabilityExt { + /// Whether the given room version is stable. + fn is_stable_version(&self, version: &RoomVersionId) -> bool; + + /// The maximum stable room version in these capabilities. + fn maximum_stable_version(&self) -> Option<&RoomVersionId>; +} + +impl RoomVersionsCapabilityExt for RoomVersionsCapability { + fn is_stable_version(&self, version: &RoomVersionId) -> bool { + self.available + .get(version) + .is_some_and(|stability| *stability == RoomVersionStability::Stable) + } + + fn maximum_stable_version(&self) -> Option<&RoomVersionId> { + self.available + .iter() + .fold(None, |maximum, (version, stability)| { + // Discard unstable versions. + if *stability != RoomVersionStability::Stable { + return maximum; + } + + // Keep the maximum. + if maximum.is_none_or(|maximum| { + numeric_sort::cmp(version.as_ref(), maximum.as_ref()) == Ordering::Greater + }) { + Some(version) + } else { + maximum + } + }) } } diff --git a/src/session/view/content/room_details/upgrade_dialog/room_version.rs b/src/session/view/content/room_details/upgrade_dialog/room_version.rs index 94be08eb..4b63e24e 100644 --- a/src/session/view/content/room_details/upgrade_dialog/room_version.rs +++ b/src/session/view/content/room_details/upgrade_dialog/room_version.rs @@ -1,10 +1,11 @@ -use std::{cmp::Ordering, str::FromStr}; - use gtk::{glib, prelude::*, subclass::prelude::*}; -use ruma::{RoomVersionId, api::client::discovery::get_capabilities::v3::RoomVersionStability}; +use ruma::RoomVersionId; mod imp { - use std::{cell::OnceCell, marker::PhantomData}; + use std::{ + cell::{Cell, OnceCell}, + marker::PhantomData, + }; use super::*; @@ -16,8 +17,9 @@ mod imp { /// The ID of the version as a string. #[property(get = Self::id_string)] id_string: PhantomData, - /// The stability of the version. - stability: OnceCell, + /// Whether the version is stable. + #[property(get, construct_only)] + is_stable: Cell, } #[glib::object_subclass] @@ -44,18 +46,6 @@ mod imp { fn id_string(&self) -> String { self.id().to_string() } - - /// Set the stability of this version. - pub(super) fn set_stability(&self, stability: RoomVersionStability) { - self.stability - .set(stability) - .expect("stability is uninitialized"); - } - - /// The stability of this version. - pub(super) fn stability(&self) -> &RoomVersionStability { - self.stability.get().expect("stability is initialized") - } } } @@ -66,12 +56,12 @@ glib::wrapper! { impl RoomVersion { /// Constructs a new `RoomVersion`. - pub fn new(id: RoomVersionId, stability: RoomVersionStability) -> Self { - let obj = glib::Object::new::(); + pub fn new(id: RoomVersionId, is_stable: bool) -> Self { + let obj = glib::Object::builder::() + .property("is-stable", is_stable) + .build(); - let imp = obj.imp(); - imp.set_id(id); - imp.set_stability(stability); + obj.imp().set_id(id); obj } @@ -80,25 +70,4 @@ impl RoomVersion { pub(crate) fn id(&self) -> &RoomVersionId { self.imp().id() } - - /// The stability of this version. - pub(crate) fn stability(&self) -> &RoomVersionStability { - self.imp().stability() - } - - /// Compare the IDs of the two given `RoomVersion`s. - /// - /// Correctly sorts numbers: string comparison will sort `1, 10, 2`, we want - /// `1, 2, 10`. - pub(crate) fn cmp_ids(a: &RoomVersion, b: &RoomVersion) -> Ordering { - match ( - i64::from_str(a.id().as_str()), - i64::from_str(b.id().as_str()), - ) { - (Ok(a), Ok(b)) => a.cmp(&b), - (Ok(_), _) => Ordering::Less, - (_, Ok(_)) => Ordering::Greater, - _ => a.id().cmp(b.id()), - } - } }