From 0c26988cf579daabdb51e1bb1cc52a5b952e0b4e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 16 Oct 2024 17:27:42 +0100 Subject: [PATCH] refactor(base): Remove `impl From` for `SyncTimelineEvent` I feel like the ability to convert straight from a `Raw>` into a `SyncTimelineEvent` is somewhat over-simplified: the two are only occasionally equivalent, and it's better to be explicit. Changelog: `SyncTimelineEvent` no longer implements `From>`. --- crates/matrix-sdk-base/src/client.rs | 6 +++-- crates/matrix-sdk-base/src/rooms/normal.rs | 6 ++--- .../src/deserialized_responses.rs | 6 ----- .../src/timeline/event_item/mod.rs | 5 ++-- .../matrix-sdk-ui/src/timeline/tests/basic.rs | 7 ++--- .../src/timeline/tests/encryption.rs | 24 ++++++++--------- .../src/timeline/tests/event_filter.rs | 9 ++++--- .../src/timeline/tests/invalid.rs | 13 ++++----- .../matrix-sdk-ui/src/timeline/tests/mod.rs | 10 +++---- .../matrix-sdk-ui/src/timeline/tests/polls.rs | 3 ++- .../src/timeline/tests/shields.rs | 6 ++--- .../matrix-sdk/src/event_cache/pagination.rs | 27 ++++++++++--------- crates/matrix-sdk/src/sliding_sync/mod.rs | 4 +-- 13 files changed, 63 insertions(+), 63 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 2e3835a8c..031beab66 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -396,8 +396,10 @@ impl BaseClient { let mut timeline = Timeline::new(limited, prev_batch); let mut push_context = self.get_push_room_context(room, room_info, changes).await?; - for event in events { - let mut event: SyncTimelineEvent = event.into(); + for raw_event in events { + // Start by assuming we have a plaintext event. We'll replace it with a + // decrypted or UTD event below if necessary. + let mut event = SyncTimelineEvent::new(raw_event); match event.raw().deserialize() { Ok(e) => { diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 9f790ba33..3d426261f 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -1876,9 +1876,9 @@ mod tests { last_prev_batch: Some("pb".to_owned()), sync_info: SyncInfo::FullySynced, encryption_state_synced: true, - latest_event: Some(Box::new(LatestEvent::new( - Raw::from_json_string(json!({"sender": "@u:i.uk"}).to_string()).unwrap().into(), - ))), + latest_event: Some(Box::new(LatestEvent::new(SyncTimelineEvent::new( + Raw::from_json_string(json!({"sender": "@u:i.uk"}).to_string()).unwrap(), + )))), base_info: Box::new( assign!(BaseRoomInfo::new(), { pinned_events: Some(RoomPinnedEventsEventContent::new(vec![owned_event_id!("$a")])) }), ), diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index f19d523ee..672e057b6 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -359,12 +359,6 @@ impl SyncTimelineEvent { } } -impl From> for SyncTimelineEvent { - fn from(inner: Raw) -> Self { - Self::new(inner) - } -} - impl From for SyncTimelineEvent { fn from(o: TimelineEvent) -> Self { Self { kind: o.kind, push_actions: o.push_actions.unwrap_or_default() } diff --git a/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs b/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs index 67ee12990..9c28bad5b 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_item/mod.rs @@ -1023,7 +1023,7 @@ mod tests { formatted_body: &str, ts: u64, ) -> SyncTimelineEvent { - sync_timeline_event!({ + SyncTimelineEvent::new(sync_timeline_event!({ "event_id": "$eventid6", "sender": user_id, "origin_server_ts": ts, @@ -1035,7 +1035,6 @@ mod tests { "formatted_body": formatted_body, "msgtype": "m.text" }, - }) - .into() + })) } } diff --git a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs index bf613932f..3d86d2c43 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs @@ -16,6 +16,7 @@ use assert_matches::assert_matches; use assert_matches2::assert_let; use eyeball_im::VectorDiff; use futures_util::StreamExt; +use matrix_sdk::deserialized_responses::SyncTimelineEvent; use matrix_sdk_test::{async_test, sync_timeline_event, ALICE, BOB, CAROL}; use ruma::{ events::{ @@ -112,7 +113,7 @@ async fn test_sticker() { let mut stream = timeline.subscribe_events().await; timeline - .handle_live_event(sync_timeline_event!({ + .handle_live_event(SyncTimelineEvent::new(sync_timeline_event!({ "content": { "body": "Happy sticker", "info": { @@ -127,7 +128,7 @@ async fn test_sticker() { "origin_server_ts": 143273582, "sender": "@alice:server.name", "type": "m.sticker", - })) + }))) .await; let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); @@ -278,7 +279,7 @@ async fn test_dedup_pagination() { let event = timeline .event_builder .make_sync_message_event(*ALICE, RoomMessageEventContent::text_plain("o/")); - timeline.handle_live_event(event.clone()).await; + timeline.handle_live_event(SyncTimelineEvent::new(event.clone())).await; // This cast is not actually correct, sync events aren't valid // back-paginated events, as they are missing `room_id`. However, the // timeline doesn't care about that `room_id` and casts back to diff --git a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs index 8e0952d2e..ce038e615 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs @@ -27,15 +27,13 @@ use matrix_sdk::{ crypto::{decrypt_room_key_export, types::events::UtdCause, OlmMachine}, test_utils::test_client_builder, }; +use matrix_sdk_base::deserialized_responses::SyncTimelineEvent; use matrix_sdk_test::{async_test, BOB}; use ruma::{ assign, - events::{ - room::encrypted::{ - EncryptedEventScheme, MegolmV1AesSha2ContentInit, Relation, Replacement, - RoomEncryptedEventContent, - }, - AnySyncTimelineEvent, + events::room::encrypted::{ + EncryptedEventScheme, MegolmV1AesSha2ContentInit, Relation, Replacement, + RoomEncryptedEventContent, }, room_id, serde::Raw, @@ -475,7 +473,7 @@ async fn test_utd_cause_for_nonmember_event_is_found() { let mut stream = timeline.subscribe().await; // When we add an event with "membership: leave" - timeline.handle_live_event(raw_event_with_unsigned(json!({ "membership": "leave" }))).await; + timeline.handle_live_event(utd_event_with_unsigned(json!({ "membership": "leave" }))).await; // Then its UTD cause is membership let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); @@ -495,7 +493,7 @@ async fn test_utd_cause_for_nonmember_event_is_found_unstable_prefix() { // When we add an event with "io.element.msc4115.membership: leave" timeline - .handle_live_event(raw_event_with_unsigned( + .handle_live_event(utd_event_with_unsigned( json!({ "io.element.msc4115.membership": "leave" }), )) .await; @@ -517,7 +515,7 @@ async fn test_utd_cause_for_member_event_is_unknown() { let mut stream = timeline.subscribe().await; // When we add an event with "membership: join" - timeline.handle_live_event(raw_event_with_unsigned(json!({ "membership": "join" }))).await; + timeline.handle_live_event(utd_event_with_unsigned(json!({ "membership": "join" }))).await; // Then its UTD cause is membership let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); @@ -536,7 +534,7 @@ async fn test_utd_cause_for_missing_membership_is_unknown() { let mut stream = timeline.subscribe().await; // When we add an event with no membership in unsigned - timeline.handle_live_event(raw_event_with_unsigned(json!({}))).await; + timeline.handle_live_event(utd_event_with_unsigned(json!({}))).await; // Then its UTD cause is membership let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); @@ -548,8 +546,8 @@ async fn test_utd_cause_for_missing_membership_is_unknown() { assert_eq!(*cause, UtdCause::Unknown); } -fn raw_event_with_unsigned(unsigned: serde_json::Value) -> Raw { - Raw::from_json( +fn utd_event_with_unsigned(unsigned: serde_json::Value) -> SyncTimelineEvent { + SyncTimelineEvent::new(Raw::from_json( to_raw_value(&json!({ "event_id": "$myevent", "sender": "@u:s", @@ -566,5 +564,5 @@ fn raw_event_with_unsigned(unsigned: serde_json::Value) -> Raw value); @@ -78,14 +79,14 @@ async fn test_invalid_event_content() { // Similar to above, the m.room.member state event must also not have an // empty content object. timeline - .handle_live_event(sync_timeline_event!({ + .handle_live_event(SyncTimelineEvent::new(sync_timeline_event!({ "content": {}, "event_id": "$d5G0HA0FAZ37wP8kXlNkxx3I", "origin_server_ts": 2179, "sender": "@alice:example.org", "type": "m.room.member", "state_key": "@alice:example.org", - })) + }))) .await; let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); @@ -106,7 +107,7 @@ async fn test_invalid_event() { // This event is missing the sender field which the homeserver must add to // all timeline events. Because the event is malformed, it will be ignored. timeline - .handle_live_event(sync_timeline_event!({ + .handle_live_event(SyncTimelineEvent::new(sync_timeline_event!({ "content": { "body": "hello world", "msgtype": "m.text" @@ -114,7 +115,7 @@ async fn test_invalid_event() { "event_id": "$eeG0HA0FAZ37wP8kXlNkxx3I", "origin_server_ts": 10, "type": "m.room.message", - })) + }))) .await; assert_eq!(timeline.controller.items().await.len(), 0); } diff --git a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs index 46ae26f58..26cadd3b6 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs @@ -183,7 +183,7 @@ impl TestTimeline { C: RedactedMessageLikeEventContent, { let ev = self.event_builder.make_sync_redacted_message_event(sender, content); - self.handle_live_event(Raw::new(&ev).unwrap().cast()).await; + self.handle_live_event(SyncTimelineEvent::new(ev)).await; } async fn handle_live_state_event(&self, sender: &UserId, content: C, prev_content: Option) @@ -191,7 +191,7 @@ impl TestTimeline { C: StaticStateEventContent, { let ev = self.event_builder.make_sync_state_event(sender, "", content, prev_content); - self.handle_live_event(ev).await; + self.handle_live_event(SyncTimelineEvent::new(ev)).await; } async fn handle_live_state_event_with_state_key( @@ -209,7 +209,7 @@ impl TestTimeline { content, prev_content, ); - self.handle_live_event(Raw::new(&ev).unwrap().cast()).await; + self.handle_live_event(SyncTimelineEvent::new(ev)).await; } async fn handle_live_redacted_state_event(&self, sender: &UserId, content: C) @@ -217,7 +217,7 @@ impl TestTimeline { C: RedactedStateEventContent, { let ev = self.event_builder.make_sync_redacted_state_event(sender, "", content); - self.handle_live_event(Raw::new(&ev).unwrap().cast()).await; + self.handle_live_event(SyncTimelineEvent::new(ev)).await; } async fn handle_live_redacted_state_event_with_state_key( @@ -230,7 +230,7 @@ impl TestTimeline { { let ev = self.event_builder.make_sync_redacted_state_event(sender, state_key.as_ref(), content); - self.handle_live_event(Raw::new(&ev).unwrap().cast()).await; + self.handle_live_event(SyncTimelineEvent::new(ev)).await; } async fn handle_live_event(&self, event: impl Into) { diff --git a/crates/matrix-sdk-ui/src/timeline/tests/polls.rs b/crates/matrix-sdk-ui/src/timeline/tests/polls.rs index b26f2a385..74d678a99 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/polls.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/polls.rs @@ -1,3 +1,4 @@ +use matrix_sdk_base::deserialized_responses::SyncTimelineEvent; use matrix_sdk_test::{async_test, ALICE, BOB}; use ruma::{ events::{ @@ -216,7 +217,7 @@ impl TestTimeline { ); let event = self.event_builder.make_sync_message_event_with_id(sender, event_id, event_content); - self.handle_live_event(event).await; + self.handle_live_event(SyncTimelineEvent::new(event)).await; } async fn send_poll_response(&self, sender: &UserId, answers: Vec<&str>, poll_id: &EventId) { diff --git a/crates/matrix-sdk-ui/src/timeline/tests/shields.rs b/crates/matrix-sdk-ui/src/timeline/tests/shields.rs index ad86b21c9..5cc973c40 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/shields.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/shields.rs @@ -1,6 +1,6 @@ use assert_matches::assert_matches; use eyeball_im::VectorDiff; -use matrix_sdk_base::deserialized_responses::{ShieldState, ShieldStateCode}; +use matrix_sdk_base::deserialized_responses::{ShieldState, ShieldStateCode, SyncTimelineEvent}; use matrix_sdk_test::{async_test, sync_timeline_event, ALICE}; use ruma::{ event_id, @@ -97,7 +97,7 @@ async fn test_local_sent_in_clear_shield() { // When the remote echo comes in. timeline - .handle_live_event(sync_timeline_event!({ + .handle_live_event(SyncTimelineEvent::new(sync_timeline_event!({ "content": { "body": "Local message", "msgtype": "m.text", @@ -106,7 +106,7 @@ async fn test_local_sent_in_clear_shield() { "event_id": event_id, "origin_server_ts": timestamp, "type": "m.room.message", - })) + }))) .await; let item = assert_next_matches!(stream, VectorDiff::Set { index: 1, value } => value); let event_item = item.as_event().unwrap(); diff --git a/crates/matrix-sdk/src/event_cache/pagination.rs b/crates/matrix-sdk/src/event_cache/pagination.rs index 035093711..74e943aae 100644 --- a/crates/matrix-sdk/src/event_cache/pagination.rs +++ b/crates/matrix-sdk/src/event_cache/pagination.rs @@ -320,7 +320,10 @@ mod tests { use ruma::room_id; use tokio::{spawn, time::sleep}; - use crate::{event_cache::store::Gap, test_utils::logged_in_client}; + use crate::{ + deserialized_responses::SyncTimelineEvent, event_cache::store::Gap, + test_utils::logged_in_client, + }; #[async_test] async fn test_wait_no_pagination_token() { @@ -335,14 +338,15 @@ mod tests { let (room_event_cache, _drop_handlers) = event_cache.for_room(room_id).await.unwrap(); // When I only have events in a room, - room_event_cache.inner.state.write().await.events.push_events([sync_timeline_event!({ - "sender": "b@z.h", - "type": "m.room.message", - "event_id": "$ida", - "origin_server_ts": 12344446, - "content": { "body":"yolo", "msgtype": "m.text" }, - }) - .into()]); + room_event_cache.inner.state.write().await.events.push_events([ + SyncTimelineEvent::new(sync_timeline_event!({ + "sender": "b@z.h", + "type": "m.room.message", + "event_id": "$ida", + "origin_server_ts": 12344446, + "content": { "body":"yolo", "msgtype": "m.text" }, + })), + ]); let pagination = room_event_cache.pagination(); @@ -395,14 +399,13 @@ mod tests { { let room_events = &mut room_event_cache.inner.state.write().await.events; room_events.push_gap(Gap { prev_token: expected_token.clone() }); - room_events.push_events([sync_timeline_event!({ + room_events.push_events([SyncTimelineEvent::new(sync_timeline_event!({ "sender": "b@z.h", "type": "m.room.message", "event_id": "$ida", "origin_server_ts": 12344446, "content": { "body":"yolo", "msgtype": "m.text" }, - }) - .into()]); + }))]); } let pagination = room_event_cache.pagination(); diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 9de9a21af..43258f4f6 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -36,7 +36,7 @@ use async_stream::stream; pub use client::{Version, VersionBuilder}; use futures_core::stream::Stream; pub use matrix_sdk_base::sliding_sync::http; -use matrix_sdk_common::timer; +use matrix_sdk_common::{deserialized_responses::SyncTimelineEvent, timer}; use ruma::{ api::{client::error::ErrorKind, OutgoingRequest}, assign, OwnedEventId, OwnedRoomId, RoomId, @@ -345,7 +345,7 @@ impl SlidingSync { if let Some(joined_room) = sync_response.rooms.join.remove(&room_id) { joined_room.timeline.events } else { - room_data.timeline.drain(..).map(Into::into).collect() + room_data.timeline.drain(..).map(SyncTimelineEvent::new).collect() }; match rooms_map.get_mut(&room_id) {