chore(spaces): Refactor SpaceRoom sorting and move the core logic outside of the space room list

Rooms will soon need to be sorted outside of /hierarchy responses and as such the sorting algorithm will need to be shared between multiple users.
This commit is contained in:
Stefan Ceriu
2026-01-14 16:29:36 +02:00
committed by Stefan Ceriu
parent d278f20dd4
commit c5bdeb26ed
2 changed files with 209 additions and 20 deletions

View File

@@ -12,10 +12,16 @@
// See the License for that specific language governing permissions and
// limitations under the License.
use std::cmp::Ordering;
use matrix_sdk::{Room, RoomHero, RoomState};
use ruma::{
OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, OwnedServerName,
events::room::{guest_access::GuestAccess, history_visibility::HistoryVisibility},
MilliSecondsSinceUnixEpoch, OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, OwnedServerName,
OwnedSpaceChildOrder,
events::{
room::{guest_access::GuestAccess, history_visibility::HistoryVisibility},
space::child::HierarchySpaceChildEvent,
},
room::{JoinRuleSummary, RoomSummary, RoomType},
};
@@ -132,4 +138,203 @@ impl SpaceRoom {
via: vec![],
}
}
/// Sorts space rooms by various criteria as defined in
/// https://spec.matrix.org/latest/client-server-api/#ordering-of-children-within-a-space
pub(crate) fn compare_rooms(
a: &SpaceRoom,
b: &SpaceRoom,
a_state: Option<SpaceRoomChildState>,
b_state: Option<SpaceRoomChildState>,
) -> Ordering {
match (a_state, b_state) {
(Some(a_state), Some(b_state)) => match (&a_state.order, &b_state.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::Less,
(None, Some(_)) => Ordering::Greater,
(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()),
}
}
}
#[derive(Clone, Debug)]
pub(crate) struct SpaceRoomChildState {
pub(crate) order: Option<OwnedSpaceChildOrder>,
pub(crate) origin_server_ts: MilliSecondsSinceUnixEpoch,
}
impl From<&HierarchySpaceChildEvent> for SpaceRoomChildState {
fn from(event: &HierarchySpaceChildEvent) -> Self {
SpaceRoomChildState {
order: event.content.order.clone(),
origin_server_ts: event.origin_server_ts,
}
}
}
#[cfg(test)]
mod tests {
use std::cmp::Ordering;
use matrix_sdk_test::async_test;
use ruma::{MilliSecondsSinceUnixEpoch, OwnedRoomId, SpaceChildOrder, owned_room_id, uint};
use crate::spaces::{SpaceRoom, room::SpaceRoomChildState};
#[async_test]
async fn test_room_list_sorting() {
// Rooms without a `m.space.child` state event should be sorted by their
// `room_id`
assert_eq!(
SpaceRoom::compare_rooms(
&make_space_room(owned_room_id!("!A:a.b")),
&make_space_room(owned_room_id!("!B:a.b")),
None,
None
),
Ordering::Less
);
assert_eq!(
SpaceRoom::compare_rooms(
&make_space_room(owned_room_id!("!Marțolea:a.b")),
&make_space_room(owned_room_id!("!Luana:a.b")),
None,
None,
),
Ordering::Greater
);
// Rooms without an order provided through the `children_state` should be
// sorted by their `m.space.child` `origin_server_ts`
assert_eq!(
SpaceRoom::compare_rooms(
&make_space_room(owned_room_id!("!Luana:a.b")),
&make_space_room(owned_room_id!("!Marțolea:a.b")),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(1)),
order: None
}),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)),
order: None
})
),
Ordering::Greater
);
// The `m.space.child` `content.order` field should be used if provided
assert_eq!(
SpaceRoom::compare_rooms(
&make_space_room(owned_room_id!("!Joiana:a.b"),),
&make_space_room(owned_room_id!("!Mioara:a.b"),),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(123)),
order: Some(SpaceChildOrder::parse("second").unwrap())
}),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(234)),
order: Some(SpaceChildOrder::parse("first").unwrap())
}),
),
Ordering::Greater
);
// The timestamp should be used when the `order` is the same
assert_eq!(
SpaceRoom::compare_rooms(
&make_space_room(owned_room_id!("!Joiana:a.b")),
&make_space_room(owned_room_id!("!Mioara:a.b")),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(1)),
order: Some(SpaceChildOrder::parse("Same pasture").unwrap())
}),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)),
order: Some(SpaceChildOrder::parse("Same pasture").unwrap())
}),
),
Ordering::Greater
);
// And the `room_id` should be used when both the `order` and the
// `timestamp` are equal
assert_eq!(
SpaceRoom::compare_rooms(
&make_space_room(owned_room_id!("!Joiana:a.b")),
&make_space_room(owned_room_id!("!Mioara:a.b")),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)),
order: Some(SpaceChildOrder::parse("Same pasture").unwrap())
}),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)),
order: Some(SpaceChildOrder::parse("Same pasture").unwrap())
}),
),
Ordering::Less
);
// When one of the rooms is missing `children_state` data the other one
// should take precedence
assert_eq!(
SpaceRoom::compare_rooms(
&make_space_room(owned_room_id!("!Viola:a.b")),
&make_space_room(owned_room_id!("!Sâmbotina:a.b")),
None,
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(0)),
order: None
}),
),
Ordering::Greater
);
// If the `order` is missing from one of the rooms but `children_state`
// is present then the other one should come first
assert_eq!(
SpaceRoom::compare_rooms(
&make_space_room(owned_room_id!("!Sâmbotina:a.b")),
&make_space_room(owned_room_id!("!Dumana:a.b")),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(1)),
order: None
}),
Some(SpaceRoomChildState {
origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(1)),
order: Some(SpaceChildOrder::parse("Some pasture").unwrap())
}),
),
Ordering::Greater
);
}
fn make_space_room(room_id: OwnedRoomId) -> SpaceRoom {
SpaceRoom {
room_id,
canonical_alias: None,
name: Some("New room name".to_owned()),
display_name: "Empty room".to_owned(),
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![],
}
}
}

View File

@@ -372,7 +372,7 @@ impl SpaceRoomList {
self.pagination_state.set(SpaceRoomListPaginationState::Idle { end_reached: false });
}
/// Sorts spare rooms by various criteria as defined in
/// Sorts space 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,
@@ -382,23 +382,7 @@ impl SpaceRoomList {
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::Less,
(None, Some(_)) => Ordering::Greater,
(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()),
}
SpaceRoom::compare_rooms(a, b, a_state.map(Into::into), b_state.map(Into::into))
}
}