mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-05-05 14:35:20 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(pinned_events): get pinned event ids from the HS if the sync doesn't contain it

Browse Source 
This should take care of a bug that caused pinned events to be incorrectly removed when the new pinned event ids list was based on an empty one if the required state of the room didn't contain any pinned events info
This commit is contained in:
Jorge Martín
2024-11-04 13:26:25 +01:00
committed by Jorge Martin Espinosa
parent 494532d579
commit ee252437d1
10 changed files with 90 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
benchmarks/benches/room_bench.rs
View File

@@ -171,8 +171,9 @@ pub fn load_pinned_events_benchmark(c: &mut Criterion) {
);
let room = client.get_room(&room_id).expect("Room not found");
assert!(!room.pinned_event_ids().is_empty());
assert_eq!(room.pinned_event_ids().len(), PINNED_EVENTS_COUNT);
let pinned_event_ids = room.pinned_event_ids().unwrap_or_default();
assert!(!pinned_event_ids.is_empty());
assert_eq!(pinned_event_ids.len(), PINNED_EVENTS_COUNT);
let count = PINNED_EVENTS_COUNT;
let name = format!("{count} pinned events");
@@ -191,8 +192,9 @@ pub fn load_pinned_events_benchmark(c: &mut Criterion) {
group.bench_function(BenchmarkId::new("load_pinned_events", name), |b| {
b.to_async(&runtime).iter(|| async {
assert!(!room.pinned_event_ids().is_empty());
assert_eq!(room.pinned_event_ids().len(), PINNED_EVENTS_COUNT);
let pinned_event_ids = room.pinned_event_ids().unwrap_or_default();
assert!(!pinned_event_ids.is_empty());
assert_eq!(pinned_event_ids.len(), PINNED_EVENTS_COUNT);
// Reset cache so it always loads the events from the mocked endpoint
client.event_cache().empty_immutable_cache().await;

3
bindings/matrix-sdk-ffi/src/room_info.rs
View File

@@ -67,7 +67,8 @@ impl RoomInfo {
for (id, level) in power_levels_map.iter() {
user_power_levels.insert(id.to_string(), *level);
}
let pinned_event_ids = room.pinned_event_ids().iter().map(|id| id.to_string()).collect();
let pinned_event_ids =
room.pinned_event_ids().unwrap_or_default().iter().map(|id| id.to_string()).collect();
Ok(Self {
id: room.room_id().to_string(),

6
crates/matrix-sdk-base/src/rooms/normal.rs
View File

@@ -1019,7 +1019,7 @@ impl Room {
}
/// Returns the current pinned event ids for this room.
pub fn pinned_event_ids(&self) -> Vec<OwnedEventId> {
pub fn pinned_event_ids(&self) -> Option<Vec<OwnedEventId>> {
self.inner.read().pinned_event_ids()
}
}
@@ -1596,8 +1596,8 @@ impl RoomInfo {
}
/// Returns the current pinned event ids for this room.
pub fn pinned_event_ids(&self) -> Vec<OwnedEventId> {
self.base_info.pinned_events.clone().map(|c| c.pinned).unwrap_or_default()
pub fn pinned_event_ids(&self) -> Option<Vec<OwnedEventId>> {
self.base_info.pinned_events.clone().map(|c| c.pinned)
}
/// Checks if an `EventId` is currently pinned.

6
crates/matrix-sdk-base/src/sliding_sync/mod.rs
View File

@@ -2509,7 +2509,7 @@ mod tests {
// The newly created room has no pinned event ids
let room = client.get_room(room_id).unwrap();
let pinned_event_ids = room.pinned_event_ids();
assert!(pinned_event_ids.is_empty());
assert_matches!(pinned_event_ids, None);
// Load new pinned event id
let mut room_response = http::response::Room::new();
@@ -2522,7 +2522,7 @@ mod tests {
let response = response_with_room(room_id, room_response);
client.process_sliding_sync(&response, &(), true).await.expect("Failed to process sync");
let pinned_event_ids = room.pinned_event_ids();
let pinned_event_ids = room.pinned_event_ids().unwrap_or_default();
assert_eq!(pinned_event_ids.len(), 1);
assert_eq!(pinned_event_ids[0], pinned_event_id);
@@ -2536,7 +2536,7 @@ mod tests {
));
let response = response_with_room(room_id, room_response);
client.process_sliding_sync(&response, &(), true).await.expect("Failed to process sync");
let pinned_event_ids = room.pinned_event_ids();
let pinned_event_ids = room.pinned_event_ids().unwrap();
assert!(pinned_event_ids.is_empty());
}

28
crates/matrix-sdk-ui/src/timeline/mod.rs
View File

@@ -729,10 +729,18 @@ impl Timeline {
/// Adds a new pinned event by sending an updated `m.room.pinned_events`
/// event containing the new event id.
///
/// Returns `true` if we sent the request, `false` if the event was already
/// This method will first try to get the pinned events from the current
/// room's state and if it fails to do so it'll try to load them from the
/// homeserver.
///
/// Returns `true` if we pinned the event, `false` if the event was already
/// pinned.
pub async fn pin_event(&self, event_id: &EventId) -> Result<bool> {
let mut pinned_event_ids = self.room().pinned_event_ids();
let mut pinned_event_ids = if let Some(event_ids) = self.room().pinned_event_ids() {
event_ids
} else {
self.room().load_pinned_events().await?.unwrap_or_default()
};
let event_id = event_id.to_owned();
if pinned_event_ids.contains(&event_id) {
Ok(false)
@@ -744,13 +752,21 @@ impl Timeline {
}
}
/// Adds a new pinned event by sending an updated `m.room.pinned_events`
/// Removes a pinned event by sending an updated `m.room.pinned_events`
/// event without the event id we want to remove.
///
/// Returns `true` if we sent the request, `false` if the event wasn't
/// pinned.
/// This method will first try to get the pinned events from the current
/// room's state and if it fails to do so it'll try to load them from the
/// homeserver.
///
/// Returns `true` if we unpinned the event, `false` if the event wasn't
/// pinned before.
pub async fn unpin_event(&self, event_id: &EventId) -> Result<bool> {
let mut pinned_event_ids = self.room().pinned_event_ids();
let mut pinned_event_ids = if let Some(event_ids) = self.room().pinned_event_ids() {
event_ids
} else {
self.room().load_pinned_events().await?.unwrap_or_default()
};
let event_id = event_id.to_owned();
if let Some(idx) = pinned_event_ids.iter().position(|e| *e == *event_id) {
pinned_event_ids.remove(idx);

5
crates/matrix-sdk-ui/src/timeline/pinned_events_loader.rs
View File

@@ -61,6 +61,7 @@ impl PinnedEventsLoader {
let pinned_event_ids: Vec<OwnedEventId> = self
.room
.pinned_event_ids()
.unwrap_or_default()
.into_iter()
.rev()
.take(self.max_events_to_load)
@@ -134,7 +135,7 @@ pub trait PinnedEventsRoom: SendOutsideWasm + SyncOutsideWasm {
) -> BoxFuture<'a, Result<(SyncTimelineEvent, Vec<SyncTimelineEvent>), PaginatorError>>;
/// Get the pinned event ids for a room.
fn pinned_event_ids(&self) -> Vec<OwnedEventId>;
fn pinned_event_ids(&self) -> Option<Vec<OwnedEventId>>;
/// Checks whether an event id is pinned in this room.
///
@@ -168,7 +169,7 @@ impl PinnedEventsRoom for Room {
.boxed()
}
fn pinned_event_ids(&self) -> Vec<OwnedEventId> {
fn pinned_event_ids(&self) -> Option<Vec<OwnedEventId>> {
self.clone_info().pinned_event_ids()
}

2
crates/matrix-sdk-ui/src/timeline/tests/mod.rs
View File

@@ -351,7 +351,7 @@ impl PinnedEventsRoom for TestRoomDataProvider {
unimplemented!();
}
fn pinned_event_ids(&self) -> Vec<OwnedEventId> {
fn pinned_event_ids(&self) -> Option<Vec<OwnedEventId>> {
unimplemented!();
}

27
crates/matrix-sdk-ui/tests/integration/timeline/mod.rs
View File

@@ -628,7 +628,7 @@ async fn test_pin_event_is_sent_successfully() {
// Pinning a remote event succeeds.
setup
.mock_response(ResponseTemplate::new(200).set_body_json(json!({
.mock_pin_unpin_response(ResponseTemplate::new(200).set_body_json(json!({
"event_id": "$42"
})))
.await;
@@ -662,7 +662,7 @@ async fn test_pin_event_is_returning_an_error() {
assert!(!timeline.items().await.is_empty());
// Pinning a remote event fails.
setup.mock_response(ResponseTemplate::new(400)).await;
setup.mock_pin_unpin_response(ResponseTemplate::new(400)).await;
let event_id = setup.event_id();
assert!(timeline.pin_event(event_id).await.is_err());
@@ -680,7 +680,7 @@ async fn test_unpin_event_is_sent_successfully() {
// Unpinning a remote event succeeds.
setup
.mock_response(ResponseTemplate::new(200).set_body_json(json!({
.mock_pin_unpin_response(ResponseTemplate::new(200).set_body_json(json!({
"event_id": "$42"
})))
.await;
@@ -714,7 +714,7 @@ async fn test_unpin_event_is_returning_an_error() {
assert!(!timeline.items().await.is_empty());
// Unpinning a remote event fails.
setup.mock_response(ResponseTemplate::new(400)).await;
setup.mock_pin_unpin_response(ResponseTemplate::new(400)).await;
let event_id = setup.event_id();
assert!(timeline.unpin_event(event_id).await.is_err());
@@ -834,7 +834,13 @@ impl PinningTestSetup<'_> {
let _response = client.sync_once(sync_settings.clone()).await.unwrap();
server.reset().await;
Self { event_id, room_id, client, server, sync_settings, sync_builder }
let setup = Self { event_id, room_id, client, server, sync_settings, sync_builder };
// This is necessary to get an empty list of pinned events when there are no
// pinned events state event in the required state
setup.mock_get_empty_pinned_events_state_response().await;
setup
}
async fn timeline(&self) -> Timeline {
@@ -847,7 +853,7 @@ impl PinningTestSetup<'_> {
self.server.reset().await;
}
async fn mock_response(&self, response: ResponseTemplate) {
async fn mock_pin_unpin_response(&self, response: ResponseTemplate) {
Mock::given(method("PUT"))
.and(path_regex(r"^/_matrix/client/r0/rooms/.*/state/m.room.pinned_events/.*?"))
.and(header("authorization", "Bearer 1234"))
@@ -856,6 +862,15 @@ impl PinningTestSetup<'_> {
.await;
}
async fn mock_get_empty_pinned_events_state_response(&self) {
Mock::given(method("GET"))
.and(path_regex(r"^/_matrix/client/r0/rooms/.*/state/m.room.pinned_events/.*"))
.and(header("authorization", "Bearer 1234"))
.respond_with(ResponseTemplate::new(404).set_body_json(json!({})))
.mount(&self.server)
.await;
}
async fn mock_sync(&mut self, is_using_pinned_state_event: bool) {
let f = EventFactory::new().sender(user_id!("@a:b.c"));
let mut joined_room_builder = JoinedRoomBuilder::new(self.room_id)

28
crates/matrix-sdk/src/room/mod.rs
View File

@@ -30,6 +30,7 @@ use futures_util::{
future::{try_join, try_join_all},
stream::FuturesUnordered,
};
use http::StatusCode;
#[cfg(all(feature = "e2e-encryption", not(target_arch = "wasm32")))]
pub use identity_status_changes::IdentityStatusChanges;
#[cfg(feature = "e2e-encryption")]
@@ -91,6 +92,7 @@ use ruma::{
VideoMessageEventContent,
},
name::RoomNameEventContent,
pinned_events::RoomPinnedEventsEventContent,
power_levels::{RoomPowerLevels, RoomPowerLevelsEventContent},
server_acl::RoomServerAclEventContent,
topic::RoomTopicEventContent,
@@ -3130,6 +3132,32 @@ impl Room {
.await?;
Ok(())
}
/// Load pinned state events for a room from the `/state` endpoint in the
/// home server.
pub async fn load_pinned_events(&self) -> Result<Option<Vec<OwnedEventId>>> {
let response = self
.client
.send(
get_state_events_for_key::v3::Request::new(
self.room_id().to_owned(),
StateEventType::RoomPinnedEvents,
"".to_owned(),
),
None,
)
.await;
match response {
Ok(response) => {
Ok(Some(response.content.deserialize_as::<RoomPinnedEventsEventContent>()?.pinned))
}
Err(http_error) => match http_error.as_client_api_error() {
Some(error) if error.status_code == StatusCode::NOT_FOUND => Ok(None),
_ => Err(http_error.into()),
},
}
}
}
#[cfg(all(feature = "e2e-encryption", not(target_arch = "wasm32")))]

2
crates/matrix-sdk/tests/integration/client.rs
View File

@@ -1388,5 +1388,5 @@ async fn test_restore_room() {
let room = client.get_room(room_id).unwrap();
assert!(room.is_favourite());
assert!(!room.pinned_event_ids().is_empty());
assert!(!room.pinned_event_ids().unwrap().is_empty());
}