diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index fa8501f66..303e8007b 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -12,7 +12,7 @@ // See the License for that specific language governing permissions and // limitations under the License. -use std::{collections::HashMap, sync::Arc}; +use std::{cmp::Ordering, collections::HashMap, sync::Arc}; use eyeball::{ObservableWriteGuard, SharedObservable, Subscriber}; use eyeball_im::{ObservableVector, VectorSubscriberBatchedStream}; @@ -22,7 +22,7 @@ use itertools::Itertools; use matrix_sdk::{Client, Error, executor::AbortOnDrop, locks::Mutex, paginators::PaginationToken}; use matrix_sdk_common::executor::spawn; use ruma::{ - MilliSecondsSinceUnixEpoch, OwnedRoomId, OwnedServerName, OwnedSpaceChildOrder, RoomId, + OwnedRoomId, api::client::space::get_hierarchy, events::space::child::{HierarchySpaceChildEvent, SpaceChildEventContent}, uint, @@ -328,15 +328,16 @@ impl SpaceRoomList { .map(|room| { let via = children_state .get(&room.summary.room_id) - .and_then(|state| Some(state.content.via.clone())); + .map(|state| state.content.via.clone()); SpaceRoom::new_from_summary( &room.summary, self.client.get_room(&room.summary.room_id), room.children_state.len() as u64, - via.unwrap_or(Default::default()), + via.unwrap_or_default(), ) }) + .sorted_by(|a, b| Self::compare_rooms(a, b, &children_state)) .for_each(|room| rooms.push_back(room)); self.pagination_state.set(SpaceRoomListPaginationState::Idle { @@ -352,10 +353,41 @@ impl SpaceRoomList { } } } + + /// Sorts spare rooms by various criteria as defined in + /// https://spec.matrix.org/latest/client-server-api/#ordering-of-children-within-a-space + fn compare_rooms( + a: &SpaceRoom, + b: &SpaceRoom, + children_state: &HashMap, + ) -> Ordering { + let a_state = children_state.get(&a.room_id); + let b_state = children_state.get(&b.room_id); + + match (a_state, b_state) { + (Some(a_state), Some(b_state)) => { + match (&a_state.content.order, &b_state.content.order) { + (Some(a_order), Some(b_order)) => a_order + .cmp(b_order) + .then(a_state.origin_server_ts.cmp(&b_state.origin_server_ts)) + .then(a.room_id.cmp(&b.room_id)), + (Some(_), None) => Ordering::Greater, + (None, Some(_)) => Ordering::Less, + (None, None) => a_state + .origin_server_ts + .cmp(&b_state.origin_server_ts) + .then(a.room_id.to_string().cmp(&b.room_id.to_string())), + } + } + _ => a.room_id.to_string().cmp(&b.room_id.to_string()), + } + } } #[cfg(test)] mod tests { + use std::{cmp::Ordering, collections::HashMap}; + use assert_matches2::{assert_let, assert_matches}; use eyeball_im::VectorDiff; use futures_util::pin_mut; @@ -364,13 +396,18 @@ mod tests { JoinedRoomBuilder, LeftRoomBuilder, async_test, event_factory::EventFactory, }; use ruma::{ - owned_server_name, + OwnedRoomId, RoomId, + events::space::child::HierarchySpaceChildEvent, + owned_room_id, owned_server_name, room::{JoinRuleSummary, RoomSummary}, room_id, server_name, uint, }; + use serde_json::{from_value, json}; use stream_assert::{assert_next_eq, assert_next_matches, assert_pending, assert_ready}; - use crate::spaces::{SpaceRoom, SpaceService, room_list::SpaceRoomListPaginationState}; + use crate::spaces::{ + SpaceRoom, SpaceRoomList, SpaceService, room_list::SpaceRoomListPaginationState, + }; #[async_test] async fn test_room_list_pagination() { @@ -711,4 +748,190 @@ mod tests { ] ); } + + #[async_test] + async fn test_room_list_sorting() { + let mut children_state = HashMap::::new(); + + // Rooms not present in the `children_state` should be sorted by their room ID + assert_eq!( + SpaceRoomList::compare_rooms( + &make_space_room(owned_room_id!("!Luana:a.b"), None, None, &mut children_state), + &make_space_room(owned_room_id!("!Marțolea:a.b"), None, None, &mut children_state), + &children_state, + ), + Ordering::Less + ); + + assert_eq!( + SpaceRoomList::compare_rooms( + &make_space_room(owned_room_id!("!Marțolea:a.b"), None, None, &mut children_state), + &make_space_room(owned_room_id!("!Luana:a.b"), None, None, &mut children_state), + &children_state, + ), + Ordering::Greater + ); + + // Rooms without an order provided through the `children_state` should be + // sorted by their `m.space.child` `origin_server_ts` + assert_eq!( + SpaceRoomList::compare_rooms( + &make_space_room(owned_room_id!("!Luana:a.b"), None, Some(1), &mut children_state), + &make_space_room( + owned_room_id!("!Marțolea:a.b"), + None, + Some(0), + &mut children_state + ), + &children_state, + ), + Ordering::Greater + ); + + // The `m.space.child` `content.order` field should be used if provided + assert_eq!( + SpaceRoomList::compare_rooms( + &make_space_room( + owned_room_id!("!Joiana:a.b"), + Some("last"), + Some(123), + &mut children_state + ), + &make_space_room( + owned_room_id!("!Mioara:a.b"), + Some("first"), + Some(234), + &mut children_state + ), + &children_state, + ), + Ordering::Greater + ); + + // The timestamp should be used when the `order` is the same + assert_eq!( + SpaceRoomList::compare_rooms( + &make_space_room( + owned_room_id!("!Joiana:a.b"), + Some("Same pasture"), + Some(1), + &mut children_state + ), + &make_space_room( + owned_room_id!("!Mioara:a.b"), + Some("Same pasture"), + Some(0), + &mut children_state + ), + &children_state, + ), + Ordering::Greater + ); + + // And the `room_id` should be used when both the `order` and the + // `timestamp` are equal + assert_eq!( + SpaceRoomList::compare_rooms( + &make_space_room( + owned_room_id!("!Joiana:a.b"), + Some("same_pasture"), + Some(0), + &mut children_state + ), + &make_space_room( + owned_room_id!("!Mioara:a.b"), + Some("same_pasture"), + Some(0), + &mut children_state + ), + &children_state, + ), + Ordering::Less + ); + + // Finally, when one of the rooms is missing `children_state` data the + // other one should take precedence + assert_eq!( + SpaceRoomList::compare_rooms( + &make_space_room(owned_room_id!("!Viola:a.b"), None, None, &mut children_state), + &make_space_room( + owned_room_id!("!Sâmbotina:a.b"), + None, + Some(0), + &mut children_state + ), + &children_state, + ), + Ordering::Greater + ); + + assert_eq!( + SpaceRoomList::compare_rooms( + &make_space_room( + owned_room_id!("!Sâmbotina:a.b"), + None, + Some(1), + &mut children_state + ), + &make_space_room( + owned_room_id!("!Dumana:a.b"), + Some("Some pasture"), + Some(1), + &mut children_state + ), + &children_state, + ), + Ordering::Less + ); + } + + fn make_space_room( + room_id: OwnedRoomId, + order: Option<&str>, + origin_server_ts: Option, + children_state: &mut HashMap, + ) -> SpaceRoom { + if let Some(origin_server_ts) = origin_server_ts { + children_state.insert( + room_id.clone(), + hierarchy_space_child_event(&room_id, order, origin_server_ts), + ); + } + SpaceRoom { + room_id, + canonical_alias: None, + name: None, + topic: None, + avatar_url: None, + room_type: None, + num_joined_members: 0, + join_rule: None, + world_readable: None, + guest_can_join: false, + is_direct: None, + children_count: 0, + state: None, + heroes: None, + via: vec![], + } + } + + fn hierarchy_space_child_event( + room_id: &RoomId, + order: Option<&str>, + origin_server_ts: u32, + ) -> HierarchySpaceChildEvent { + let json = json!({ + "content": { + "order": order.unwrap_or(""), + "via": [] + }, + "origin_server_ts": origin_server_ts, + "sender": "@bob:a.b", + "state_key": room_id.to_string(), + "type": "m.space.child" + }); + + from_value::(json).unwrap() + } }