timeline: make SyncTimelineEvent fields private

... and add accessors instead.

I'm going to change the inner structure of `SyncTimelineEvent`, meaning that
access will have to be via an accessor in future. Let's start by making the
fields private, and use accessors where we previously used direct access.
This commit is contained in:
Richard van der Hoff
2024-10-09 14:24:25 +01:00
committed by Richard van der Hoff
parent ce231e6c2b
commit 4d472f6aed
18 changed files with 119 additions and 63 deletions

View File

@@ -358,7 +358,7 @@ impl BaseClient {
let event: SyncTimelineEvent =
olm.decrypt_room_event(event.cast_ref(), room_id, &decryption_settings).await?.into();
if let Ok(AnySyncTimelineEvent::MessageLike(e)) = event.event.deserialize() {
if let Ok(AnySyncTimelineEvent::MessageLike(e)) = event.raw().deserialize() {
match &e {
AnySyncMessageLikeEvent::RoomMessage(SyncMessageLikeEvent::Original(
original_event,
@@ -398,7 +398,7 @@ impl BaseClient {
for event in events {
let mut event: SyncTimelineEvent = event.into();
match event.event.deserialize() {
match event.raw().deserialize() {
Ok(e) => {
#[allow(clippy::single_match)]
match &e {
@@ -432,7 +432,7 @@ impl BaseClient {
}
}
let raw_event: Raw<AnySyncStateEvent> = event.event.clone().cast();
let raw_event: Raw<AnySyncStateEvent> = event.raw().clone().cast();
changes.add_state_event(room.room_id(), s.clone(), raw_event);
}
@@ -443,8 +443,8 @@ impl BaseClient {
room_info.room_version().unwrap_or(&RoomVersionId::V1);
if let Some(redacts) = r.redacts(room_version) {
room_info.handle_redaction(r, event.event.cast_ref());
let raw_event = event.event.clone().cast();
room_info.handle_redaction(r, event.raw().cast_ref());
let raw_event = event.raw().clone().cast();
changes.add_redaction(room.room_id(), redacts, raw_event);
}
@@ -456,7 +456,7 @@ impl BaseClient {
SyncMessageLikeEvent::Original(_),
) => {
if let Ok(Some(e)) = Box::pin(
self.decrypt_sync_room_event(&event.event, room.room_id()),
self.decrypt_sync_room_event(event.raw(), room.room_id()),
)
.await
{
@@ -494,14 +494,14 @@ impl BaseClient {
}
if let Some(context) = &push_context {
let actions = push_rules.get_actions(&event.event, context);
let actions = push_rules.get_actions(event.raw(), context);
if actions.iter().any(Action::should_notify) {
notifications.entry(room.room_id().to_owned()).or_default().push(
Notification {
actions: actions.to_owned(),
event: RawAnySyncOrStrippedTimelineEvent::Sync(
event.event.clone(),
event.raw().clone(),
),
},
);
@@ -773,7 +773,7 @@ impl BaseClient {
if let Ok(Some(decrypted)) = decrypt_sync_room_event.await {
// We found an event we can decrypt
if let Ok(any_sync_event) = decrypted.event.deserialize() {
if let Ok(any_sync_event) = decrypted.raw().deserialize() {
// We can deserialize it to find its type
match is_suitable_for_latest_event(&any_sync_event) {
PossibleLatestEvent::YesRoomMessage(_)

View File

@@ -203,7 +203,7 @@ impl RoomReadReceipts {
/// Returns whether a new event triggered a new unread/notification/mention.
#[inline(always)]
fn process_event(&mut self, event: &SyncTimelineEvent, user_id: &UserId) {
if marks_as_unread(&event.event, user_id) {
if marks_as_unread(event.raw(), user_id) {
self.num_unread += 1;
}
@@ -408,7 +408,7 @@ impl ReceiptSelector {
fn try_match_implicit(&mut self, user_id: &UserId, new_events: &[SyncTimelineEvent]) {
for ev in new_events {
// Get the `sender` field, if any, or skip this event.
let Ok(Some(sender)) = ev.event.get_field::<OwnedUserId>("sender") else { continue };
let Ok(Some(sender)) = ev.raw().get_field::<OwnedUserId>("sender") else { continue };
if sender == user_id {
// Get the event id, if any, or skip this event.
let Some(event_id) = ev.event_id() else { continue };

View File

@@ -1283,9 +1283,9 @@ impl RoomInfo {
if let Some(latest_event) = &mut self.latest_event {
tracing::trace!("Checking if redaction applies to latest event");
if latest_event.event_id().as_deref() == Some(redacts) {
match apply_redaction(&latest_event.event().event, _raw, room_version) {
match apply_redaction(latest_event.event().raw(), _raw, room_version) {
Some(redacted) => {
latest_event.event_mut().event = redacted;
latest_event.event_mut().set_raw(redacted);
debug!("Redacted latest event");
}
None => {

View File

@@ -680,7 +680,7 @@ async fn cache_latest_events(
Vec::with_capacity(room.latest_encrypted_events.read().unwrap().capacity());
for event in events.iter().rev() {
if let Ok(timeline_event) = event.event.deserialize() {
if let Ok(timeline_event) = event.raw().deserialize() {
match is_suitable_for_latest_event(&timeline_event) {
PossibleLatestEvent::YesRoomMessage(_)
| PossibleLatestEvent::YesPoll(_)
@@ -760,7 +760,7 @@ async fn cache_latest_events(
// Check how many encrypted events we have seen. Only store another if we
// haven't already stored the maximum number.
if encrypted_events.len() < encrypted_events.capacity() {
encrypted_events.push(event.event.clone());
encrypted_events.push(event.raw().clone());
}
}
_ => {
@@ -1686,7 +1686,7 @@ mod tests {
// But it's now redacted
assert_matches!(
latest_event.event().event.deserialize().unwrap(),
latest_event.event().raw().deserialize().unwrap(),
AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomMessage(
SyncRoomMessageEvent::Redacted(_)
))

View File

@@ -305,10 +305,14 @@ pub struct EncryptionInfo {
#[derive(Clone, Deserialize, Serialize)]
pub struct SyncTimelineEvent {
/// The actual event.
pub event: Raw<AnySyncTimelineEvent>,
#[serde(rename = "event")]
inner_event: Raw<AnySyncTimelineEvent>,
/// The encryption info about the event. Will be `None` if the event was not
/// encrypted.
pub encryption_info: Option<EncryptionInfo>,
#[serde(rename = "encryption_info")]
inner_encryption_info: Option<EncryptionInfo>,
/// The push actions associated with this event.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub push_actions: Vec<Action>,
@@ -325,7 +329,12 @@ impl SyncTimelineEvent {
/// This is a convenience constructor for when you don't need to set
/// `encryption_info` or `push_action`, for example inside a test.
pub fn new(event: Raw<AnySyncTimelineEvent>) -> Self {
Self { event, encryption_info: None, push_actions: vec![], unsigned_encryption_info: None }
Self {
inner_event: event,
inner_encryption_info: None,
push_actions: vec![],
unsigned_encryption_info: None,
}
}
/// Create a new `SyncTimelineEvent` from the given raw event and push
@@ -337,24 +346,56 @@ impl SyncTimelineEvent {
event: Raw<AnySyncTimelineEvent>,
push_actions: Vec<Action>,
) -> Self {
Self { event, encryption_info: None, push_actions, unsigned_encryption_info: None }
Self {
inner_event: event,
inner_encryption_info: None,
push_actions,
unsigned_encryption_info: None,
}
}
/// Get the event id of this `SyncTimelineEvent` if the event has any valid
/// id.
pub fn event_id(&self) -> Option<OwnedEventId> {
self.event.get_field::<OwnedEventId>("event_id").ok().flatten()
self.inner_event.get_field::<OwnedEventId>("event_id").ok().flatten()
}
/// Returns a reference to the (potentially decrypted) Matrix event inside
/// this `TimelineEvent`.
pub fn raw(&self) -> &Raw<AnySyncTimelineEvent> {
&self.inner_event
}
/// If the event was a decrypted event that was successfully decrypted, get
/// its encryption info. Otherwise, `None`.
pub fn encryption_info(&self) -> Option<&EncryptionInfo> {
self.inner_encryption_info.as_ref()
}
/// Takes ownership of this `TimelineEvent`, returning the (potentially
/// decrypted) Matrix event within.
pub fn into_raw(self) -> Raw<AnySyncTimelineEvent> {
self.inner_event
}
/// Replace the Matrix event within this event. Used to handle redaction.
pub fn set_raw(&mut self, event: Raw<AnySyncTimelineEvent>) {
self.inner_event = event;
}
}
#[cfg(not(tarpaulin_include))]
impl fmt::Debug for SyncTimelineEvent {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let SyncTimelineEvent { event, encryption_info, push_actions, unsigned_encryption_info } =
self;
let SyncTimelineEvent {
inner_event,
inner_encryption_info,
push_actions,
unsigned_encryption_info,
} = self;
let mut s = f.debug_struct("SyncTimelineEvent");
s.field("event", &DebugRawEvent(event));
s.maybe_field("encryption_info", encryption_info);
s.field("event", &DebugRawEvent(inner_event));
s.maybe_field("encryption_info", inner_encryption_info);
if !push_actions.is_empty() {
s.field("push_actions", push_actions);
}
@@ -376,8 +417,8 @@ impl From<TimelineEvent> for SyncTimelineEvent {
// this way, we simply cause the `room_id` field in the json to be
// ignored by a subsequent deserialization.
Self {
event: o.event.cast(),
encryption_info: o.encryption_info,
inner_event: o.event.cast(),
inner_encryption_info: o.encryption_info,
push_actions: o.push_actions.unwrap_or_default(),
unsigned_encryption_info: o.unsigned_encryption_info,
}
@@ -579,7 +620,7 @@ mod tests {
let converted_room_event: SyncTimelineEvent = room_event.into();
let converted_event: AnySyncTimelineEvent =
converted_room_event.event.deserialize().unwrap();
converted_room_event.raw().deserialize().unwrap();
assert_eq!(converted_event.event_id(), "$xxxxx:example.org");
assert_eq!(converted_event.sender(), "@carl:example.com");
@@ -628,8 +669,8 @@ mod tests {
#[test]
fn sync_timeline_event_serialisation() {
let room_event = SyncTimelineEvent {
event: Raw::new(&example_event()).unwrap().cast(),
encryption_info: Some(EncryptionInfo {
inner_event: Raw::new(&example_event()).unwrap().cast(),
inner_encryption_info: Some(EncryptionInfo {
sender: user_id!("@sender:example.com").to_owned(),
sender_device: None,
algorithm_info: AlgorithmInfo::MegolmV1AesSha2 {
@@ -674,7 +715,7 @@ mod tests {
let event: SyncTimelineEvent = serde_json::from_value(serialized).unwrap();
assert_eq!(event.event_id(), Some(event_id!("$xxxxx:example.org").to_owned()));
assert_matches!(
event.encryption_info.unwrap().algorithm_info,
event.encryption_info().unwrap().algorithm_info,
AlgorithmInfo::MegolmV1AesSha2 { .. }
)
}

View File

@@ -421,7 +421,7 @@ impl TimelineStateTransaction<'_> {
settings: &TimelineSettings,
day_divider_adjuster: &mut DayDividerAdjuster,
) -> HandleEventResult {
let raw = event.event;
let raw = event.raw();
let (event_id, sender, timestamp, txn_id, event_kind, should_add) = match raw.deserialize()
{
Ok(event) => {
@@ -580,8 +580,8 @@ impl TimelineStateTransaction<'_> {
is_highlighted: event.push_actions.iter().any(Action::is_highlight),
flow: Flow::Remote {
event_id: event_id.clone(),
raw_event: raw,
encryption_info: event.encryption_info,
raw_event: raw.clone(),
encryption_info: event.encryption_info().cloned(),
txn_id,
position,
},

View File

@@ -25,7 +25,7 @@ use matrix_sdk::{
Client, Error,
};
use matrix_sdk_base::{
deserialized_responses::{ShieldStateCode, SyncTimelineEvent, SENT_IN_CLEAR},
deserialized_responses::{ShieldStateCode, SENT_IN_CLEAR},
latest_event::LatestEvent,
};
use once_cell::sync::Lazy;
@@ -161,8 +161,8 @@ impl EventTimelineItem {
// potential footgun which could one day turn into a security issue.
use super::traits::RoomDataProvider;
let SyncTimelineEvent { event: raw_sync_event, encryption_info, .. } =
latest_event.event().clone();
let raw_sync_event = latest_event.event().raw().clone();
let encryption_info = latest_event.event().encryption_info().cloned();
let Ok(event) = raw_sync_event.deserialize_as::<AnySyncTimelineEvent>() else {
warn!("Unable to deserialize latest_event as an AnySyncTimelineEvent!");

View File

@@ -109,7 +109,7 @@ impl PinnedEventsLoader {
// Sort using chronological ordering (oldest -> newest)
loaded_events.sort_by_key(|item| {
item.event
item.raw()
.deserialize()
.map(|e| e.origin_server_ts())
.unwrap_or_else(|_| MilliSecondsSinceUnixEpoch::now())

View File

@@ -19,6 +19,7 @@ use eyeball_im::VectorDiff;
use matrix_sdk::deserialized_responses::{
AlgorithmInfo, EncryptionInfo, VerificationLevel, VerificationState,
};
use matrix_sdk_base::deserialized_responses::{DecryptedRoomEvent, SyncTimelineEvent};
use matrix_sdk_test::{async_test, ALICE};
use ruma::{
event_id,
@@ -26,6 +27,7 @@ use ruma::{
room::message::{MessageType, RedactedRoomMessageEventContent},
BundledMessageLikeRelations,
},
room_id,
};
use stream_assert::{assert_next_matches, assert_pending};
@@ -158,13 +160,15 @@ async fn test_edit_updates_encryption_info() {
let timeline = TestTimeline::new();
let event_factory = &timeline.factory;
let room_id = room_id!("!room:id");
let original_event_id = event_id!("$original_event");
let mut original_event = event_factory
let original_event = event_factory
.text_msg("**original** message")
.sender(*ALICE)
.event_id(original_event_id)
.into_sync();
.room(room_id)
.into_raw_timeline();
let mut encryption_info = EncryptionInfo {
sender: (*ALICE).into(),
@@ -176,7 +180,12 @@ async fn test_edit_updates_encryption_info() {
verification_state: VerificationState::Verified,
};
original_event.encryption_info = Some(encryption_info.clone());
let original_event: SyncTimelineEvent = DecryptedRoomEvent {
event: original_event.cast(),
encryption_info: encryption_info.clone(),
unsigned_encryption_info: None,
}
.into();
timeline.handle_live_event(original_event).await;
@@ -192,14 +201,20 @@ async fn test_edit_updates_encryption_info() {
assert_let!(MessageType::Text(text) = message.msgtype());
assert_eq!(text.body, "**original** message");
let mut edit_event = event_factory
let edit_event = event_factory
.text_msg(" * !!edited!! **better** message")
.sender(*ALICE)
.room(room_id)
.edit(original_event_id, MessageType::text_plain("!!edited!! **better** message").into())
.into_sync();
.into_raw_timeline();
encryption_info.verification_state =
VerificationState::Unverified(VerificationLevel::UnverifiedIdentity);
edit_event.encryption_info = Some(encryption_info);
let edit_event: SyncTimelineEvent = DecryptedRoomEvent {
event: edit_event.cast(),
encryption_info: encryption_info.clone(),
unsigned_encryption_info: None,
}
.into();
timeline.handle_live_event(edit_event).await;

View File

@@ -2111,7 +2111,7 @@ impl Client {
/// while let Some(Ok(response)) = sync_stream.next().await {
/// for room in response.rooms.join.values() {
/// for e in &room.timeline.events {
/// if let Ok(event) = e.event.deserialize() {
/// if let Ok(event) = e.raw().deserialize() {
/// println!("Received event {:?}", event);
/// }
/// }

View File

@@ -820,7 +820,7 @@ impl RoomEventCacheInner {
event: &SyncTimelineEvent,
) {
// Handle and cache events and relations.
if let Ok(AnySyncTimelineEvent::MessageLike(ev)) = event.event.deserialize() {
if let Ok(AnySyncTimelineEvent::MessageLike(ev)) = event.raw().deserialize() {
// Handle redactions separately, as their logic is slightly different.
if let AnySyncMessageLikeEvent::RoomRedaction(SyncRoomRedactionEvent::Original(ev)) =
&ev

View File

@@ -388,7 +388,7 @@ impl Client {
for item in timeline_events {
let TimelineEventDetails { event_type, state_key, unsigned } =
item.event.deserialize_as()?;
item.raw().deserialize_as()?;
let redacted = unsigned.and_then(|u| u.redacted_because).is_some();
let (handler_kind_g, handler_kind_r) = match state_key {
@@ -396,8 +396,8 @@ impl Client {
None => (HandlerKind::MessageLike, HandlerKind::message_like_redacted(redacted)),
};
let raw_event = item.event.json();
let encryption_info = item.encryption_info.as_ref();
let raw_event = item.raw().json();
let encryption_info = item.encryption_info();
let push_actions = &item.push_actions;
// Event handlers for possibly-redacted timeline events

View File

@@ -142,7 +142,7 @@ async fn make_edit_event<S: EventSource>(
) -> Result<AnyMessageLikeEventContent, EditError> {
let target = source.get_event(event_id).await?;
let event = target.event.deserialize().map_err(EditError::Deserialize)?;
let event = target.raw().deserialize().map_err(EditError::Deserialize)?;
// The event must be message-like.
let AnySyncTimelineEvent::MessageLike(message_like_event) = event else {
@@ -186,7 +186,7 @@ async fn make_edit_event<S: EventSource>(
let replied_to_original_room_msg = replied_to_sync_timeline_event
.and_then(|sync_timeline_event| {
sync_timeline_event
.event
.raw()
.deserialize()
.map_err(|err| warn!("unable to deserialize replied-to event: {err}"))
.ok()

View File

@@ -2303,7 +2303,7 @@ mod tests {
let no_local_events = room_id!("!crepe:example.org");
let already_limited = room_id!("!paris:example.org");
let response_timeline = vec![event_c.event.clone(), event_d.event.clone()];
let response_timeline = vec![event_c.raw().clone(), event_d.raw().clone()];
let local_rooms = BTreeMap::from_iter([
(
@@ -2386,21 +2386,21 @@ mod tests {
no_overlap.to_owned(),
assign!(http::response::Room::default(), {
initial: Some(true),
timeline: vec![event_c.event.clone(), event_d.event.clone()],
timeline: vec![event_c.raw().clone(), event_d.raw().clone()],
}),
),
(
partial_overlap.to_owned(),
assign!(http::response::Room::default(), {
initial: Some(true),
timeline: vec![event_c.event.clone(), event_d.event.clone()],
timeline: vec![event_c.raw().clone(), event_d.raw().clone()],
}),
),
(
complete_overlap.to_owned(),
assign!(http::response::Room::default(), {
initial: Some(true),
timeline: vec![event_c.event.clone(), event_d.event.clone()],
timeline: vec![event_c.raw().clone(), event_d.raw().clone()],
}),
),
(
@@ -2414,7 +2414,7 @@ mod tests {
no_local_events.to_owned(),
assign!(http::response::Room::default(), {
initial: Some(true),
timeline: vec![event_c.event.clone(), event_d.event.clone()],
timeline: vec![event_c.raw().clone(), event_d.raw().clone()],
}),
),
(
@@ -2422,7 +2422,7 @@ mod tests {
assign!(http::response::Room::default(), {
initial: Some(true),
limited: true,
timeline: vec![event_c.event, event_d.event],
timeline: vec![event_c.into_raw(), event_d.into_raw()],
}),
),
]);

View File

@@ -363,7 +363,7 @@ mod tests {
let timeline = & $( $timeline_queue ).*;
$(
assert_eq!(timeline[ $nth ].event.deserialize().unwrap().event_id(), $event_id);
assert_eq!(timeline[ $nth ].raw().deserialize().unwrap().event_id(), $event_id);
)*
};
}
@@ -714,7 +714,7 @@ mod tests {
// Check that the last event is the last event of the timeline, i.e. we only
// keep the _latest_ events, not the _first_ events.
assert_eq!(
frozen_room.timeline_queue.last().unwrap().event.deserialize().unwrap().event_id(),
frozen_room.timeline_queue.last().unwrap().raw().deserialize().unwrap().event_id(),
&format!("$x{max}:baz.org")
);
}
@@ -755,7 +755,7 @@ mod tests {
// Check that the last event is the last event of the timeline, i.e. we only
// keep the _latest_ events, not the _first_ events.
assert_eq!(
frozen_room.timeline_queue.last().unwrap().event.deserialize().unwrap().event_id(),
frozen_room.timeline_queue.last().unwrap().raw().deserialize().unwrap().event_id(),
&format!("$x{max}:baz.org")
);
}

View File

@@ -24,7 +24,7 @@ use crate::{
#[track_caller]
pub fn assert_event_matches_msg<E: Clone + Into<SyncTimelineEvent>>(event: &E, expected: &str) {
let event: SyncTimelineEvent = event.clone().into();
let event = event.event.deserialize().unwrap();
let event = event.raw().deserialize().unwrap();
assert_let!(
AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomMessage(message)) = event
);

View File

@@ -729,7 +729,7 @@ impl App {
let rendered_events = events
.into_iter()
.map(|sync_timeline_item| sync_timeline_item.event.json().to_string())
.map(|sync_timeline_item| sync_timeline_item.raw().json().to_string())
.collect::<Vec<_>>()
.join("\n\n");

View File

@@ -154,7 +154,7 @@ async fn test_notification() -> Result<()> {
.events
.iter()
.find_map(|event| {
let event = event.event.deserialize().ok()?;
let event = event.raw().deserialize().ok()?;
(event.event_type() == TimelineEventType::RoomMessage)
.then(|| event.event_id().to_owned())
})