feat(spaces): sort space room list rooms as defined in the spec
The ordering criteria is defined at https://spec.matrix.org/latest/client-server-api/#ordering-of-children-within-a-space. The gist is that `order` comes first, then `timestamp` and finally the `room_id` This is not available for top level spaces, but there is an MSC that addresses it at https://github.com/matrix-org/matrix-spec-proposals/pull/3230 and Ruma support has been added in https://github.com/ruma/ruma/pull/2231. The SDK side implementation for that will come in a later PR.
This commit is contained in:
committed by
Damir Jelić
parent
b43237536d
commit
d85b45ed64
@@ -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<OwnedRoomId, HierarchySpaceChildEvent>,
|
||||
) -> 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::<OwnedRoomId, HierarchySpaceChildEvent>::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<u32>,
|
||||
children_state: &mut HashMap<OwnedRoomId, HierarchySpaceChildEvent>,
|
||||
) -> 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::<HierarchySpaceChildEvent>(json).unwrap()
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user