mirror of
https://github.com/matrix-org/matrix-rust-sdk.git
synced 2026-05-06 23:15:08 -04:00
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.
This commit is contained in:
@@ -376,7 +376,7 @@ async fn test_sync_all_states() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 99]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -401,7 +401,7 @@ async fn test_sync_all_states() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 199]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -426,7 +426,7 @@ async fn test_sync_all_states() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 299]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -451,7 +451,7 @@ async fn test_sync_all_states() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 399]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -515,7 +515,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 9]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -543,7 +543,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 9]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -627,7 +627,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// The sync-mode has changed to growing, with its initial range.
|
||||
"ranges": [[0, 99]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -652,7 +652,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// Due to previous error, the sync-mode is back to selective, with its initial range.
|
||||
"ranges": [[0, 19]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -675,7 +675,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// Sync-mode is now growing.
|
||||
"ranges": [[0, 99]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -699,7 +699,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// The sync-mode is still growing, and the range has made progress.
|
||||
"ranges": [[0, 199]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -724,7 +724,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// Due to previous error, the sync-mode is back to selective, with its initial range.
|
||||
"ranges": [[0, 19]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -747,7 +747,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// The sync-mode is now growing.
|
||||
"ranges": [[0, 99]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -770,7 +770,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// No error. The range is making progress.
|
||||
"ranges": [[0, 199]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -795,7 +795,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
// Range is making progress and is even reaching the maximum
|
||||
// number of rooms.
|
||||
"ranges": [[0, 209]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -820,7 +820,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// Due to previous error, the sync-mode is back to selective, with its initial range.
|
||||
"ranges": [[0, 19]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -843,7 +843,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// Sync-mode is now growing.
|
||||
"ranges": [[0, 99]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -913,7 +913,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// The sync-mode is still selective, with its initial range.
|
||||
"ranges": [[0, 19]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -937,7 +937,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// The sync-mode is now growing, with its initial range.
|
||||
"ranges": [[0, 99]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -969,7 +969,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// The sync-mode is back to selective.
|
||||
"ranges": [[0, 19]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -993,7 +993,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// Sync-mode is growing, with its initial range.
|
||||
"ranges": [[0, 99]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1017,7 +1017,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
|
||||
ALL_ROOMS: {
|
||||
// Range is making progress, and has reached its maximum.
|
||||
"ranges": [[0, 149]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1089,7 +1089,7 @@ async fn test_loading_states() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 9]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1120,7 +1120,7 @@ async fn test_loading_states() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 11]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1271,7 +1271,7 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 9]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1381,7 +1381,7 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 9]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1548,7 +1548,7 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 9]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1718,7 +1718,7 @@ async fn test_room_sorting() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 4]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1804,7 +1804,7 @@ async fn test_room_sorting() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 4]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1879,7 +1879,7 @@ async fn test_room_sorting() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 5]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -2099,7 +2099,7 @@ async fn test_room_subscription() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 2]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
"room_subscriptions": {
|
||||
@@ -2143,7 +2143,7 @@ async fn test_room_subscription() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 2]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
"room_subscriptions": {
|
||||
@@ -2190,7 +2190,7 @@ async fn test_room_subscription() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 2]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
// NO `room_subscriptions`!
|
||||
@@ -2257,7 +2257,7 @@ async fn test_room_unread_notifications() -> Result<(), Error> {
|
||||
"lists": {
|
||||
ALL_ROOMS: {
|
||||
"ranges": [[0, 0]],
|
||||
"timeline_limit": 0,
|
||||
"timeline_limit": 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -177,9 +177,9 @@ impl SlidingSyncListBuilder {
|
||||
self.required_state,
|
||||
self.include_heroes,
|
||||
self.filters,
|
||||
self.timeline_limit,
|
||||
),
|
||||
)),
|
||||
timeline_limit: StdRwLock::new(self.timeline_limit),
|
||||
name: self.name,
|
||||
cache_policy: self.cache_policy,
|
||||
|
||||
|
||||
@@ -133,12 +133,12 @@ impl SlidingSyncList {
|
||||
|
||||
/// Get the timeline limit.
|
||||
pub fn timeline_limit(&self) -> Bound {
|
||||
self.inner.sticky.read().unwrap().data().timeline_limit()
|
||||
*self.inner.timeline_limit.read().unwrap()
|
||||
}
|
||||
|
||||
/// Set timeline limit.
|
||||
pub fn set_timeline_limit(&self, timeline: Bound) {
|
||||
self.inner.sticky.write().unwrap().data_mut().set_timeline_limit(timeline);
|
||||
*self.inner.timeline_limit.write().unwrap() = timeline;
|
||||
}
|
||||
|
||||
/// Get the maximum number of rooms. See [`Self::maximum_number_of_rooms`]
|
||||
@@ -233,6 +233,9 @@ pub(super) struct SlidingSyncListInner {
|
||||
/// knows).
|
||||
sticky: StdRwLock<SlidingSyncStickyManager<SlidingSyncListStickyParameters>>,
|
||||
|
||||
/// The maximum number of timeline events to query for.
|
||||
timeline_limit: StdRwLock<Bound>,
|
||||
|
||||
/// The total number of rooms that is possible to interact with for the
|
||||
/// given list.
|
||||
///
|
||||
@@ -307,12 +310,11 @@ impl SlidingSyncListInner {
|
||||
/// request generator.
|
||||
#[instrument(skip(self), fields(name = self.name))]
|
||||
fn request(&self, ranges: Ranges, txn_id: &mut LazyTransactionId) -> http::request::List {
|
||||
use ruma::UInt;
|
||||
|
||||
let ranges =
|
||||
ranges.into_iter().map(|r| (UInt::from(*r.start()), UInt::from(*r.end()))).collect();
|
||||
let ranges = ranges.into_iter().map(|r| ((*r.start()).into(), (*r.end()).into())).collect();
|
||||
|
||||
let mut request = assign!(http::request::List::default(), { ranges });
|
||||
request.room_details.timeline_limit = (*self.timeline_limit.read().unwrap()).into();
|
||||
|
||||
{
|
||||
let mut sticky = self.sticky.write().unwrap();
|
||||
sticky.maybe_apply(&mut request, txn_id);
|
||||
@@ -594,10 +596,10 @@ mod tests {
|
||||
.timeline_limit(7)
|
||||
.build(sender);
|
||||
|
||||
assert_eq!(list.inner.sticky.read().unwrap().data().timeline_limit(), 7);
|
||||
assert_eq!(list.timeline_limit(), 7);
|
||||
|
||||
list.set_timeline_limit(42);
|
||||
assert_eq!(list.inner.sticky.read().unwrap().data().timeline_limit(), 42);
|
||||
assert_eq!(list.timeline_limit(), 42);
|
||||
}
|
||||
|
||||
macro_rules! assert_ranges {
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
use matrix_sdk_base::sliding_sync::http;
|
||||
use ruma::events::StateEventType;
|
||||
|
||||
use super::Bound;
|
||||
use crate::sliding_sync::sticky_parameters::StickyData;
|
||||
|
||||
/// The set of `SlidingSyncList` request parameters that are *sticky*, as
|
||||
@@ -17,9 +16,6 @@ pub(super) struct SlidingSyncListStickyParameters {
|
||||
|
||||
/// Any filters to apply to the query.
|
||||
filters: Option<http::request::ListFilters>,
|
||||
|
||||
/// The maximum number of timeline events to query for.
|
||||
timeline_limit: Bound,
|
||||
}
|
||||
|
||||
impl SlidingSyncListStickyParameters {
|
||||
@@ -27,21 +23,10 @@ impl SlidingSyncListStickyParameters {
|
||||
required_state: Vec<(StateEventType, String)>,
|
||||
include_heroes: Option<bool>,
|
||||
filters: Option<http::request::ListFilters>,
|
||||
timeline_limit: Bound,
|
||||
) -> Self {
|
||||
// Consider that each list will have at least one parameter set, so invalidate
|
||||
// it by default.
|
||||
Self { required_state, include_heroes, filters, timeline_limit }
|
||||
}
|
||||
}
|
||||
|
||||
impl SlidingSyncListStickyParameters {
|
||||
pub(super) fn timeline_limit(&self) -> Bound {
|
||||
self.timeline_limit
|
||||
}
|
||||
|
||||
pub(super) fn set_timeline_limit(&mut self, timeline: Bound) {
|
||||
self.timeline_limit = timeline;
|
||||
Self { required_state, include_heroes, filters }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -50,7 +35,6 @@ impl StickyData for SlidingSyncListStickyParameters {
|
||||
|
||||
fn apply(&self, request: &mut Self::Request) {
|
||||
request.room_details.required_state = self.required_state.to_vec();
|
||||
request.room_details.timeline_limit = self.timeline_limit.into();
|
||||
request.include_heroes = self.include_heroes;
|
||||
request.filters = self.filters.clone();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user