Skip to content

Commit 4e8ceaa

Browse files
authored
refactor(sdk-ui): remove expect() with better borrow (#6513)
<!-- description of the changes in this PR --> Follow up on review on this PR #6494 (comment) Remove the usage of if/expect that was introduced because of a borrowing problem. Instead we use `ok_or_else`, but we clone the debug_string early (to avoid needing to borrow event again in the `ok_or_else`), it is not ideal either but better that the expect/panic I also removed the `is_rtc_notification` and `is_live_location` that was created I believe just to avoid the borrowing problem? They were only used in test and it just a shortcut for `matches!()` - [ ] I've documented the public API Changes in the appropriate `CHANGELOG.md` files. - [ ] This PR was made with the help of AI. <!-- Sign-off, if not part of the commits --> <!-- See CONTRIBUTING.md if you don't know what this is --> Signed-off-by:
1 parent 8a0dcdd commit 4e8ceaa

4 files changed

Lines changed: 34 additions & 59 deletions

File tree

crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
4040
use std::{borrow::Cow, collections::HashMap, sync::Arc};
4141

42-
use as_variant::as_variant;
4342
use matrix_sdk::{check_validity_of_replacement_events, deserialized_responses::EncryptionInfo};
4443
use ruma::{
4544
MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId,
@@ -183,18 +182,16 @@ pub(crate) struct Aggregation {
183182
fn poll_state_from_item<'a>(
184183
event: &'a mut Cow<'_, EventTimelineItem>,
185184
) -> Result<&'a mut PollState, AggregationError> {
186-
if event.content().is_poll() {
187-
// It was a poll! Now return the state as mutable.
188-
let state = as_variant!(
189-
event.to_mut().content_mut(),
190-
TimelineItemContent::MsgLike(MsgLikeContent { kind: MsgLikeKind::Poll(s), ..}) => s
191-
)
192-
.expect("it was a poll just above");
185+
let content = event.to_mut().content_mut();
186+
187+
if let TimelineItemContent::MsgLike(MsgLikeContent { kind: MsgLikeKind::Poll(state), .. }) =
188+
content
189+
{
193190
Ok(state)
194191
} else {
195192
Err(AggregationError::InvalidType {
196193
expected: "a poll".to_owned(),
197-
actual: event.content().debug_string().to_owned(),
194+
actual: content.debug_string().to_owned(),
198195
})
199196
}
200197
}
@@ -203,18 +200,18 @@ fn poll_state_from_item<'a>(
203200
fn live_location_state_from_item<'a>(
204201
event: &'a mut Cow<'_, EventTimelineItem>,
205202
) -> Result<&'a mut LiveLocationState, AggregationError> {
206-
if event.content().is_live_location() {
207-
// It was a live location! Now return the state as mutable.
208-
let state = event
209-
.to_mut()
210-
.content_mut()
211-
.as_live_location_state_mut()
212-
.expect("it was a live location just above");
203+
let content = event.to_mut().content_mut();
204+
205+
if let TimelineItemContent::MsgLike(MsgLikeContent {
206+
kind: MsgLikeKind::LiveLocation(state),
207+
..
208+
}) = content
209+
{
213210
Ok(state)
214211
} else {
215212
Err(AggregationError::InvalidType {
216213
expected: "a live location".to_owned(),
217-
actual: event.content().debug_string().to_owned(),
214+
actual: content.debug_string().to_owned(),
218215
})
219216
}
220217
}
@@ -223,13 +220,16 @@ fn live_location_state_from_item<'a>(
223220
fn rtc_notification_declinations_from_item<'a>(
224221
event: &'a mut Cow<'_, EventTimelineItem>,
225222
) -> Result<&'a mut Vec<OwnedUserId>, AggregationError> {
226-
let debug_string = event.content().debug_string().to_owned();
227-
event.to_mut().content_mut().as_rtc_notification_mut().ok_or_else(|| {
228-
AggregationError::InvalidType {
223+
let content = event.to_mut().content_mut();
224+
225+
if let TimelineItemContent::RtcNotification { declined_by, .. } = content {
226+
Ok(declined_by)
227+
} else {
228+
Err(AggregationError::InvalidType {
229229
expected: "an rtc notification".to_owned(),
230-
actual: debug_string,
231-
}
232-
})
230+
actual: content.debug_string().to_owned(),
231+
})
232+
}
233233
}
234234

235235
impl Aggregation {

crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -223,38 +223,6 @@ impl TimelineItemContent {
223223
}) => state)
224224
}
225225

226-
/// If `self` is of the [`MsgLike`][Self::MsgLike] variant with a
227-
/// [`LiveLocation`][MsgLikeKind::LiveLocation] kind, return the inner
228-
/// [`LiveLocationState`] mutably.
229-
pub(in crate::timeline) fn as_live_location_state_mut(
230-
&mut self,
231-
) -> Option<&mut LiveLocationState> {
232-
as_variant!(self, Self::MsgLike(MsgLikeContent {
233-
kind: MsgLikeKind::LiveLocation(state),
234-
..
235-
}) => state)
236-
}
237-
238-
pub(in crate::timeline) fn as_rtc_notification_mut(&mut self) -> Option<&mut Vec<OwnedUserId>> {
239-
if let TimelineItemContent::RtcNotification { declined_by, .. } = self {
240-
Some(declined_by)
241-
} else {
242-
None
243-
}
244-
}
245-
246-
/// Check whether this item's content is a
247-
/// [`LiveLocation`][MsgLikeKind::LiveLocation].
248-
pub fn is_live_location(&self) -> bool {
249-
matches!(self, Self::MsgLike(MsgLikeContent { kind: MsgLikeKind::LiveLocation(_), .. }))
250-
}
251-
252-
/// Check whether this item's content is an
253-
/// [`RtcNotification`][Self::RtcNotification].
254-
pub fn is_rtc_notification(&self) -> bool {
255-
matches!(self, Self::RtcNotification { .. })
256-
}
257-
258226
/// If `self` is of the [`MsgLike`][Self::MsgLike] variant, return the
259227
/// inner [`Message`].
260228
pub fn as_message(&self) -> Option<&Message> {

crates/matrix-sdk-ui/src/timeline/tests/shields.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ use ruma::{
2222
use stream_assert::{assert_next_matches, assert_pending};
2323

2424
use crate::timeline::{
25-
EventSendState, TimelineEventShieldState, TimelineEventShieldStateCode,
25+
EventSendState, MsgLikeContent, MsgLikeKind, TimelineEventShieldState,
26+
TimelineEventShieldStateCode, TimelineItemContent,
2627
tests::{TestTimeline, TestTimelineBuilder},
2728
};
2829

@@ -156,7 +157,13 @@ async fn test_live_location_no_sent_in_clear_shield() {
156157

157158
// No beacons yet → shield should be None.
158159
let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value);
159-
assert!(item.content().is_live_location(), "timeline item should be a live location");
160+
assert!(
161+
matches!(
162+
item.content(),
163+
TimelineItemContent::MsgLike(MsgLikeContent { kind: MsgLikeKind::LiveLocation(_), .. })
164+
),
165+
"timeline item should be a live location"
166+
);
160167

161168
let shield = item.get_shield(false);
162169
assert_eq!(shield, TimelineEventShieldState::None);

crates/matrix-sdk-ui/tests/integration/timeline/rtc.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ async fn test_decline_call() {
5252

5353
assert_let!(VectorDiff::PushBack { value: message } = &timeline_updates[0]);
5454
let event_item = message.as_event().unwrap();
55-
assert!(event_item.content().is_rtc_notification());
55+
assert!(matches!(event_item.content(), TimelineItemContent::RtcNotification { .. }));
5656
assert_let!(
5757
TimelineItemContent::RtcNotification { call_intent: _, declined_by } = event_item.content()
5858
);
@@ -116,7 +116,7 @@ async fn test_multiple_decline_call() {
116116

117117
assert_let!(VectorDiff::PushBack { value: message } = &timeline_updates[0]);
118118
let event_item = message.as_event().unwrap();
119-
assert!(event_item.content().is_rtc_notification());
119+
assert!(matches!(event_item.content(), TimelineItemContent::RtcNotification { .. }));
120120
assert_let!(
121121
TimelineItemContent::RtcNotification { call_intent: _, declined_by } = event_item.content()
122122
);

0 commit comments

Comments
 (0)