Skip to content

Commit 95ea8e8

Browse files
HywanHywan
committed
fix(ui): Consider timeline_limit in sliding sync as non-sticky.
This patch changes the behaviour of `timeline_limit` in sliding sync requests. It previously was sticky, but since it's now mandatory with MSC4186, it's preferable it to be non-sticky, otherwise in some scenarios it might default to 0 (its default value). How? If the server doesn't reply with our `txn_id` (because it doesn't support sticky parameters or because it misses a `txn_id`), the next request will be built with a default `timeline_limit` value, which is zero, and won't get updated to the `timeline_limit` value from `SlidingSyncListStickyParameters`. This is not good. Instead, we must consider `timeline_limit` as non-sticky, and moves it from `SlidingSyncListStickyParameters` to `SlidingSyncListInner`. This is what this patch does.
1 parent 5dca4a9 commit 95ea8e8

File tree

4 files changed

+46
-60
lines changed

4 files changed

+46
-60
lines changed

Diff for: ‎crates/matrix-sdk-ui/tests/integration/room_list_service.rs

+34-34
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ async fn test_sync_all_states() -> Result<(), Error> {
376376
"lists": {
377377
ALL_ROOMS: {
378378
"ranges": [[0, 99]],
379-
"timeline_limit": 0,
379+
"timeline_limit": 1,
380380
},
381381
},
382382
},
@@ -401,7 +401,7 @@ async fn test_sync_all_states() -> Result<(), Error> {
401401
"lists": {
402402
ALL_ROOMS: {
403403
"ranges": [[0, 199]],
404-
"timeline_limit": 0,
404+
"timeline_limit": 1,
405405
},
406406
},
407407
},
@@ -426,7 +426,7 @@ async fn test_sync_all_states() -> Result<(), Error> {
426426
"lists": {
427427
ALL_ROOMS: {
428428
"ranges": [[0, 299]],
429-
"timeline_limit": 0,
429+
"timeline_limit": 1,
430430
},
431431
},
432432
},
@@ -451,7 +451,7 @@ async fn test_sync_all_states() -> Result<(), Error> {
451451
"lists": {
452452
ALL_ROOMS: {
453453
"ranges": [[0, 399]],
454-
"timeline_limit": 0,
454+
"timeline_limit": 1,
455455
},
456456
},
457457
},
@@ -515,7 +515,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> {
515515
"lists": {
516516
ALL_ROOMS: {
517517
"ranges": [[0, 9]],
518-
"timeline_limit": 0,
518+
"timeline_limit": 1,
519519
},
520520
},
521521
},
@@ -543,7 +543,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> {
543543
"lists": {
544544
ALL_ROOMS: {
545545
"ranges": [[0, 9]],
546-
"timeline_limit": 0,
546+
"timeline_limit": 1,
547547
},
548548
},
549549
},
@@ -627,7 +627,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
627627
ALL_ROOMS: {
628628
// The sync-mode has changed to growing, with its initial range.
629629
"ranges": [[0, 99]],
630-
"timeline_limit": 0,
630+
"timeline_limit": 1,
631631
},
632632
},
633633
},
@@ -652,7 +652,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
652652
ALL_ROOMS: {
653653
// Due to previous error, the sync-mode is back to selective, with its initial range.
654654
"ranges": [[0, 19]],
655-
"timeline_limit": 0,
655+
"timeline_limit": 1,
656656
},
657657
},
658658
},
@@ -675,7 +675,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
675675
ALL_ROOMS: {
676676
// Sync-mode is now growing.
677677
"ranges": [[0, 99]],
678-
"timeline_limit": 0,
678+
"timeline_limit": 1,
679679
},
680680
},
681681
},
@@ -699,7 +699,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
699699
ALL_ROOMS: {
700700
// The sync-mode is still growing, and the range has made progress.
701701
"ranges": [[0, 199]],
702-
"timeline_limit": 0,
702+
"timeline_limit": 1,
703703
},
704704
},
705705
},
@@ -724,7 +724,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
724724
ALL_ROOMS: {
725725
// Due to previous error, the sync-mode is back to selective, with its initial range.
726726
"ranges": [[0, 19]],
727-
"timeline_limit": 0,
727+
"timeline_limit": 1,
728728
},
729729
},
730730
},
@@ -747,7 +747,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
747747
ALL_ROOMS: {
748748
// The sync-mode is now growing.
749749
"ranges": [[0, 99]],
750-
"timeline_limit": 0,
750+
"timeline_limit": 1,
751751
},
752752
},
753753
},
@@ -770,7 +770,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
770770
ALL_ROOMS: {
771771
// No error. The range is making progress.
772772
"ranges": [[0, 199]],
773-
"timeline_limit": 0,
773+
"timeline_limit": 1,
774774
},
775775
},
776776
},
@@ -795,7 +795,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
795795
// Range is making progress and is even reaching the maximum
796796
// number of rooms.
797797
"ranges": [[0, 209]],
798-
"timeline_limit": 0,
798+
"timeline_limit": 1,
799799
},
800800
},
801801
},
@@ -820,7 +820,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
820820
ALL_ROOMS: {
821821
// Due to previous error, the sync-mode is back to selective, with its initial range.
822822
"ranges": [[0, 19]],
823-
"timeline_limit": 0,
823+
"timeline_limit": 1,
824824
},
825825
},
826826
},
@@ -843,7 +843,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
843843
ALL_ROOMS: {
844844
// Sync-mode is now growing.
845845
"ranges": [[0, 99]],
846-
"timeline_limit": 0,
846+
"timeline_limit": 1,
847847
},
848848
},
849849
},
@@ -913,7 +913,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
913913
ALL_ROOMS: {
914914
// The sync-mode is still selective, with its initial range.
915915
"ranges": [[0, 19]],
916-
"timeline_limit": 0,
916+
"timeline_limit": 1,
917917
},
918918
},
919919
},
@@ -937,7 +937,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
937937
ALL_ROOMS: {
938938
// The sync-mode is now growing, with its initial range.
939939
"ranges": [[0, 99]],
940-
"timeline_limit": 0,
940+
"timeline_limit": 1,
941941
},
942942
},
943943
},
@@ -969,7 +969,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
969969
ALL_ROOMS: {
970970
// The sync-mode is back to selective.
971971
"ranges": [[0, 19]],
972-
"timeline_limit": 0,
972+
"timeline_limit": 1,
973973
},
974974
},
975975
},
@@ -993,7 +993,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
993993
ALL_ROOMS: {
994994
// Sync-mode is growing, with its initial range.
995995
"ranges": [[0, 99]],
996-
"timeline_limit": 0,
996+
"timeline_limit": 1,
997997
},
998998
},
999999
},
@@ -1017,7 +1017,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
10171017
ALL_ROOMS: {
10181018
// Range is making progress, and has reached its maximum.
10191019
"ranges": [[0, 149]],
1020-
"timeline_limit": 0,
1020+
"timeline_limit": 1,
10211021
},
10221022
},
10231023
},
@@ -1089,7 +1089,7 @@ async fn test_loading_states() -> Result<(), Error> {
10891089
"lists": {
10901090
ALL_ROOMS: {
10911091
"ranges": [[0, 9]],
1092-
"timeline_limit": 0,
1092+
"timeline_limit": 1,
10931093
},
10941094
},
10951095
},
@@ -1120,7 +1120,7 @@ async fn test_loading_states() -> Result<(), Error> {
11201120
"lists": {
11211121
ALL_ROOMS: {
11221122
"ranges": [[0, 11]],
1123-
"timeline_limit": 0,
1123+
"timeline_limit": 1,
11241124
},
11251125
},
11261126
},
@@ -1259,7 +1259,7 @@ async fn test_entries_stream() -> Result<(), Error> {
12591259
"lists": {
12601260
ALL_ROOMS: {
12611261
"ranges": [[0, 9]],
1262-
"timeline_limit": 0,
1262+
"timeline_limit": 1,
12631263
},
12641264
},
12651265
},
@@ -1363,7 +1363,7 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
13631363
"lists": {
13641364
ALL_ROOMS: {
13651365
"ranges": [[0, 9]],
1366-
"timeline_limit": 0,
1366+
"timeline_limit": 1,
13671367
},
13681368
},
13691369
},
@@ -1473,7 +1473,7 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
14731473
"lists": {
14741474
ALL_ROOMS: {
14751475
"ranges": [[0, 9]],
1476-
"timeline_limit": 0,
1476+
"timeline_limit": 1,
14771477
},
14781478
},
14791479
},
@@ -1640,7 +1640,7 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
16401640
"lists": {
16411641
ALL_ROOMS: {
16421642
"ranges": [[0, 9]],
1643-
"timeline_limit": 0,
1643+
"timeline_limit": 1,
16441644
},
16451645
},
16461646
},
@@ -1810,7 +1810,7 @@ async fn test_room_sorting() -> Result<(), Error> {
18101810
"lists": {
18111811
ALL_ROOMS: {
18121812
"ranges": [[0, 4]],
1813-
"timeline_limit": 0,
1813+
"timeline_limit": 1,
18141814
},
18151815
},
18161816
},
@@ -1896,7 +1896,7 @@ async fn test_room_sorting() -> Result<(), Error> {
18961896
"lists": {
18971897
ALL_ROOMS: {
18981898
"ranges": [[0, 4]],
1899-
"timeline_limit": 0,
1899+
"timeline_limit": 1,
19001900
},
19011901
},
19021902
},
@@ -1971,7 +1971,7 @@ async fn test_room_sorting() -> Result<(), Error> {
19711971
"lists": {
19721972
ALL_ROOMS: {
19731973
"ranges": [[0, 5]],
1974-
"timeline_limit": 0,
1974+
"timeline_limit": 1,
19751975
},
19761976
},
19771977
},
@@ -2191,7 +2191,7 @@ async fn test_room_subscription() -> Result<(), Error> {
21912191
"lists": {
21922192
ALL_ROOMS: {
21932193
"ranges": [[0, 2]],
2194-
"timeline_limit": 0,
2194+
"timeline_limit": 1,
21952195
},
21962196
},
21972197
"room_subscriptions": {
@@ -2235,7 +2235,7 @@ async fn test_room_subscription() -> Result<(), Error> {
22352235
"lists": {
22362236
ALL_ROOMS: {
22372237
"ranges": [[0, 2]],
2238-
"timeline_limit": 0,
2238+
"timeline_limit": 1,
22392239
},
22402240
},
22412241
"room_subscriptions": {
@@ -2282,7 +2282,7 @@ async fn test_room_subscription() -> Result<(), Error> {
22822282
"lists": {
22832283
ALL_ROOMS: {
22842284
"ranges": [[0, 2]],
2285-
"timeline_limit": 0,
2285+
"timeline_limit": 1,
22862286
},
22872287
},
22882288
// NO `room_subscriptions`!
@@ -2349,7 +2349,7 @@ async fn test_room_unread_notifications() -> Result<(), Error> {
23492349
"lists": {
23502350
ALL_ROOMS: {
23512351
"ranges": [[0, 0]],
2352-
"timeline_limit": 0,
2352+
"timeline_limit": 1,
23532353
},
23542354
},
23552355
},

Diff for: ‎crates/matrix-sdk/src/sliding_sync/list/builder.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ impl SlidingSyncListBuilder {
177177
self.required_state,
178178
self.include_heroes,
179179
self.filters,
180-
self.timeline_limit,
181180
),
182181
)),
182+
timeline_limit: StdRwLock::new(self.timeline_limit),
183183
name: self.name,
184184
cache_policy: self.cache_policy,
185185

Diff for: ‎crates/matrix-sdk/src/sliding_sync/list/mod.rs

+10-8
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,12 @@ impl SlidingSyncList {
133133

134134
/// Get the timeline limit.
135135
pub fn timeline_limit(&self) -> Bound {
136-
self.inner.sticky.read().unwrap().data().timeline_limit()
136+
*self.inner.timeline_limit.read().unwrap()
137137
}
138138

139139
/// Set timeline limit.
140140
pub fn set_timeline_limit(&self, timeline: Bound) {
141-
self.inner.sticky.write().unwrap().data_mut().set_timeline_limit(timeline);
141+
*self.inner.timeline_limit.write().unwrap() = timeline;
142142
}
143143

144144
/// Get the maximum number of rooms. See [`Self::maximum_number_of_rooms`]
@@ -233,6 +233,9 @@ pub(super) struct SlidingSyncListInner {
233233
/// knows).
234234
sticky: StdRwLock<SlidingSyncStickyManager<SlidingSyncListStickyParameters>>,
235235

236+
/// The maximum number of timeline events to query for.
237+
timeline_limit: StdRwLock<Bound>,
238+
236239
/// The total number of rooms that is possible to interact with for the
237240
/// given list.
238241
///
@@ -307,12 +310,11 @@ impl SlidingSyncListInner {
307310
/// request generator.
308311
#[instrument(skip(self), fields(name = self.name))]
309312
fn request(&self, ranges: Ranges, txn_id: &mut LazyTransactionId) -> http::request::List {
310-
use ruma::UInt;
311-
312-
let ranges =
313-
ranges.into_iter().map(|r| (UInt::from(*r.start()), UInt::from(*r.end()))).collect();
313+
let ranges = ranges.into_iter().map(|r| ((*r.start()).into(), (*r.end()).into())).collect();
314314

315315
let mut request = assign!(http::request::List::default(), { ranges });
316+
request.room_details.timeline_limit = (*self.timeline_limit.read().unwrap()).into();
317+
316318
{
317319
let mut sticky = self.sticky.write().unwrap();
318320
sticky.maybe_apply(&mut request, txn_id);
@@ -594,10 +596,10 @@ mod tests {
594596
.timeline_limit(7)
595597
.build(sender);
596598

597-
assert_eq!(list.inner.sticky.read().unwrap().data().timeline_limit(), 7);
599+
assert_eq!(list.timeline_limit(), 7);
598600

599601
list.set_timeline_limit(42);
600-
assert_eq!(list.inner.sticky.read().unwrap().data().timeline_limit(), 42);
602+
assert_eq!(list.timeline_limit(), 42);
601603
}
602604

603605
macro_rules! assert_ranges {

0 commit comments

Comments
 (0)