Skip to content

Commit 558d7b5

Browse files
bnjbvrbnjbvr
committed
refactor(event cache): get rid of RoomEventCacheGenericUpdate::Clear
The semantics of this variant are unclear: sometimes a timeline could be cleared, which would result in a `UpdateTimeline` (and if we looked at the vector diffs, it would include a `Clear`), but in this case the `Clear` variant would not be emitted. It was only emitted in a few adhoc spots, but it was missing the whole picture. Also, the current observer was only interested in getting *a* room update with the room id, and didn't react particularly to clears. So, there's apparently little reason in having this variant, and as a result we should get rid of it.
1 parent 1201be4 commit 558d7b5

File tree

4 files changed

+37
-76
lines changed

4 files changed

+37
-76
lines changed

‎crates/matrix-sdk/src/event_cache/mod.rs

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -386,10 +386,8 @@ impl EventCache {
386386
/// [`RoomEventCacheSubscriber`] type triggers side-effects.
387387
///
388388
/// If one wants to get a high-overview, generic, updates for rooms, and
389-
/// without side-effects, this method is recommended. For example, it
390-
/// doesn't provide a list of new events, but rather a
391-
/// [`RoomEventCacheGenericUpdate::UpdateTimeline`] update. Also, dropping
392-
/// the receiver of this channel will not trigger any side-effect.
389+
/// without side-effects, this method is recommended. Also, dropping the
390+
/// receiver of this channel will not trigger any side-effect.
393391
pub fn subscribe_to_room_generic_updates(&self) -> Receiver<RoomEventCacheGenericUpdate> {
394392
self.inner.room_event_cache_generic_update_sender.subscribe()
395393
}
@@ -506,7 +504,7 @@ impl EventCacheInner {
506504
let _ = room
507505
.inner
508506
.generic_update_sender
509-
.send(RoomEventCacheGenericUpdate::Clear { room_id: room.inner.room_id.clone() });
507+
.send(RoomEventCacheGenericUpdate { room_id: room.inner.room_id.clone() });
510508

511509
Ok::<_, EventCacheError>(())
512510
}))
@@ -622,9 +620,9 @@ impl EventCacheInner {
622620
// If at least one event has been loaded, it means there is a timeline. Let's
623621
// emit a generic update.
624622
if timeline_is_not_empty {
625-
let _ = self.room_event_cache_generic_update_sender.send(
626-
RoomEventCacheGenericUpdate::UpdateTimeline { room_id: room_id.to_owned() },
627-
);
623+
let _ = self
624+
.room_event_cache_generic_update_sender
625+
.send(RoomEventCacheGenericUpdate { room_id: room_id.to_owned() });
628626
}
629627

630628
Ok(room_event_cache)
@@ -648,35 +646,15 @@ pub struct BackPaginationOutcome {
648646
pub events: Vec<TimelineEvent>,
649647
}
650648

651-
/// Represents an update of a room. It hides the details of
649+
/// Represents a timeline update of a room. It hides the details of
652650
/// [`RoomEventCacheUpdate`] by being more generic.
653651
///
654652
/// This is used by [`EventCache::subscribe_to_room_generic_updates`]. Please
655653
/// read it to learn more about the motivation behind this type.
656654
#[derive(Clone, Debug)]
657-
pub enum RoomEventCacheGenericUpdate {
658-
/// The timeline has been updated, i.e. an event has been added, redacted,
659-
/// removed, or reloaded.
660-
UpdateTimeline {
661-
/// The room ID owning the timeline.
662-
room_id: OwnedRoomId,
663-
},
664-
665-
/// The room has been cleared, all events have been deleted.
666-
Clear {
667-
/// The ID of the room that has been cleared.
668-
room_id: OwnedRoomId,
669-
},
670-
}
671-
672-
impl RoomEventCacheGenericUpdate {
673-
/// Get the room ID that has triggered this generic update.
674-
pub fn room_id(&self) -> &RoomId {
675-
match self {
676-
Self::UpdateTimeline { room_id } => room_id,
677-
Self::Clear { room_id } => room_id,
678-
}
679-
}
655+
pub struct RoomEventCacheGenericUpdate {
656+
/// The room ID owning the timeline.
657+
pub room_id: OwnedRoomId,
680658
}
681659

682660
/// An update related to events happened in a room.
@@ -942,7 +920,7 @@ mod tests {
942920

943921
assert_matches!(
944922
generic_stream.recv().await,
945-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id }) => {
923+
Ok(RoomEventCacheGenericUpdate { room_id }) => {
946924
assert_eq!(room_id, room_id_0);
947925
}
948926
);
@@ -1030,7 +1008,7 @@ mod tests {
10301008

10311009
assert_matches!(
10321010
generic_stream.recv().await,
1033-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: expected_room_id }) => {
1011+
Ok(RoomEventCacheGenericUpdate { room_id: expected_room_id }) => {
10341012
assert_eq!(room_id, expected_room_id);
10351013
}
10361014
);
@@ -1044,7 +1022,7 @@ mod tests {
10441022
assert!(pagination_outcome.reached_start.not());
10451023
assert_matches!(
10461024
generic_stream.recv().await,
1047-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: expected_room_id }) => {
1025+
Ok(RoomEventCacheGenericUpdate { room_id: expected_room_id }) => {
10481026
assert_eq!(room_id, expected_room_id);
10491027
}
10501028
);

‎crates/matrix-sdk/src/event_cache/pagination.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -154,15 +154,6 @@ impl RoomPagination {
154154
status_observable
155155
.set(RoomPaginationStatus::Idle { hit_timeline_start: outcome.reached_start });
156156

157-
// Send a room event cache generic update.
158-
if !outcome.events.is_empty() {
159-
let _ = self.inner.generic_update_sender.send(
160-
RoomEventCacheGenericUpdate::UpdateTimeline {
161-
room_id: self.inner.room_id.clone(),
162-
},
163-
);
164-
}
165-
166157
Ok(Some(outcome))
167158
}
168159

@@ -239,6 +230,12 @@ impl RoomPagination {
239230
diffs: timeline_event_diffs,
240231
origin: EventsOrigin::Cache,
241232
});
233+
234+
// Send a room event cache generic update.
235+
let _ =
236+
self.inner.generic_update_sender.send(RoomEventCacheGenericUpdate {
237+
room_id: self.inner.room_id.clone(),
238+
});
242239
}
243240

244241
return Ok(Some(BackPaginationOutcome {

‎crates/matrix-sdk/src/event_cache/room/mod.rs

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ impl RoomEventCache {
387387
let _ = self
388388
.inner
389389
.generic_update_sender
390-
.send(RoomEventCacheGenericUpdate::Clear { room_id: self.inner.room_id.clone() });
390+
.send(RoomEventCacheGenericUpdate { room_id: self.inner.room_id.clone() });
391391

392392
Ok(())
393393
}
@@ -551,34 +551,27 @@ impl RoomEventCacheInner {
551551
self.pagination_batch_token_notifier.notify_one();
552552
}
553553

554-
let mut update_has_been_sent = false;
555-
556554
// The order matters here: first send the timeline event diffs, then only the
557555
// related events (read receipts, etc.).
558556
if !timeline_event_diffs.is_empty() {
559557
let _ = self.sender.send(RoomEventCacheUpdate::UpdateTimelineEvents {
560558
diffs: timeline_event_diffs,
561559
origin: EventsOrigin::Sync,
562560
});
563-
update_has_been_sent = true;
561+
562+
let _ = self
563+
.generic_update_sender
564+
.send(RoomEventCacheGenericUpdate { room_id: self.room_id.clone() });
564565
}
565566

566567
if !ephemeral_events.is_empty() {
567568
let _ = self
568569
.sender
569570
.send(RoomEventCacheUpdate::AddEphemeralEvents { events: ephemeral_events });
570-
update_has_been_sent = true;
571571
}
572572

573573
if !ambiguity_changes.is_empty() {
574574
let _ = self.sender.send(RoomEventCacheUpdate::UpdateMembers { ambiguity_changes });
575-
update_has_been_sent = true;
576-
}
577-
578-
if update_has_been_sent {
579-
let _ = self.generic_update_sender.send(RoomEventCacheGenericUpdate::UpdateTimeline {
580-
room_id: self.room_id.clone(),
581-
});
582575
}
583576

584577
Ok(())
@@ -2270,7 +2263,7 @@ mod timed_tests {
22702263
// Just checking the generic update is correct.
22712264
assert_matches!(
22722265
generic_stream.recv().await,
2273-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: expected_room_id }) => {
2266+
Ok(RoomEventCacheGenericUpdate { room_id: expected_room_id }) => {
22742267
assert_eq!(expected_room_id, room_id);
22752268
}
22762269
);
@@ -2349,7 +2342,7 @@ mod timed_tests {
23492342
// Just checking the generic update is correct.
23502343
assert_matches!(
23512344
generic_stream.recv().await,
2352-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: expected_room_id }) => {
2345+
Ok(RoomEventCacheGenericUpdate { room_id: expected_room_id }) => {
23532346
assert_eq!(expected_room_id, room_id);
23542347
}
23552348
);
@@ -2518,13 +2511,7 @@ mod timed_tests {
25182511

25192512
// … same with a generic update.
25202513
assert_let_timeout!(
2521-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: received_room_id }) =
2522-
generic_stream.recv()
2523-
);
2524-
assert_eq!(received_room_id, room_id);
2525-
assert_let_timeout!(
2526-
Ok(RoomEventCacheGenericUpdate::Clear { room_id: received_room_id }) =
2527-
generic_stream.recv()
2514+
Ok(RoomEventCacheGenericUpdate { room_id: received_room_id }) = generic_stream.recv()
25282515
);
25292516
assert_eq!(received_room_id, room_id);
25302517

@@ -2633,7 +2620,7 @@ mod timed_tests {
26332620
// triggered.
26342621
assert_matches!(
26352622
generic_stream.recv().await,
2636-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: expected_room_id }) => {
2623+
Ok(RoomEventCacheGenericUpdate { room_id: expected_room_id }) => {
26372624
assert_eq!(room_id, expected_room_id);
26382625
}
26392626
);
@@ -2666,7 +2653,7 @@ mod timed_tests {
26662653
// A generic update is triggered too.
26672654
assert_matches!(
26682655
generic_stream.recv().await,
2669-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: expected_room_id }) => {
2656+
Ok(RoomEventCacheGenericUpdate { room_id: expected_room_id }) => {
26702657
assert_eq!(expected_room_id, room_id);
26712658
}
26722659
);
@@ -2797,7 +2784,7 @@ mod timed_tests {
27972784
// Just checking the generic update is correct.
27982785
assert_matches!(
27992786
generic_stream.recv().await,
2800-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: expected_room_id }) => {
2787+
Ok(RoomEventCacheGenericUpdate { room_id: expected_room_id }) => {
28012788
assert_eq!(expected_room_id, room_id);
28022789
}
28032790
);
@@ -2858,7 +2845,7 @@ mod timed_tests {
28582845
// Just checking the generic update is correct.
28592846
assert_matches!(
28602847
generic_stream.recv().await,
2861-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: expected_room_id }) => {
2848+
Ok(RoomEventCacheGenericUpdate { room_id: expected_room_id }) => {
28622849
assert_eq!(expected_room_id, room_id);
28632850
}
28642851
);
@@ -2965,8 +2952,7 @@ mod timed_tests {
29652952

29662953
// Same for the generic update.
29672954
assert_let_timeout!(
2968-
Ok(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: received_room_id }) =
2969-
generic_stream.recv()
2955+
Ok(RoomEventCacheGenericUpdate { room_id: received_room_id }) = generic_stream.recv()
29702956
);
29712957
assert_eq!(received_room_id, room_id);
29722958

‎crates/matrix-sdk/src/latest_events/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -592,10 +592,10 @@ async fn listen_to_event_cache_and_send_queue_updates(
592592

593593
room_event_cache_generic_update = event_cache_generic_updates_subscriber.recv().fuse() => {
594594
if let Ok(room_event_cache_generic_update) = room_event_cache_generic_update {
595-
let room_id = room_event_cache_generic_update.room_id();
595+
let room_id = room_event_cache_generic_update.room_id;
596596

597-
if listened_rooms.contains(room_id) {
598-
let _ = latest_event_queue_sender.send(room_id.to_owned());
597+
if listened_rooms.contains(&room_id) {
598+
let _ = latest_event_queue_sender.send(room_id);
599599
}
600600
} else {
601601
error!("`event_cache_generic_updates` channel has been closed");
@@ -929,7 +929,7 @@ mod tests {
929929
// New event cache update, but the `LatestEvents` isn't listening to it.
930930
{
931931
room_event_cache_generic_update_sender
932-
.send(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: room_id.clone() })
932+
.send(RoomEventCacheGenericUpdate { room_id: room_id.clone() })
933933
.unwrap();
934934

935935
// Run the task.
@@ -952,7 +952,7 @@ mod tests {
952952
{
953953
room_registration_sender.send(RoomRegistration::Add(room_id.clone())).await.unwrap();
954954
room_event_cache_generic_update_sender
955-
.send(RoomEventCacheGenericUpdate::UpdateTimeline { room_id: room_id.clone() })
955+
.send(RoomEventCacheGenericUpdate { room_id: room_id.clone() })
956956
.unwrap();
957957

958958
// Run the task to handle the `RoomRegistration` and the

0 commit comments

Comments
 (0)