event cache: reset paginator state when receiving a limited timeline

[bnjbvr]

Before this patch, we did reset the paginator state just a bit too late, that is: at the beginning of running the next back-pagination. Resetting the state would trigger an update of the paginator status, which would reflect at the timeline paginator status level too.

But we have a bit of a conundrum here. Imagine the following sequence of events:

• back-pagination => at some point the paginator from the event cache hits the start of the timeline
• it reflects that at the timeline level, setting hit_start_of_timeline to true
• we received a limited/gappy sync via (simplified?) sliding sync, which resets the internal back-pagination token to None in the room event cache (not the associated paginator)
• this clears the timeline

Now, calling the paginator's run_backwards() method (or equivalently, the timeline's live_paginate_backwards method) would reset the pagination token as intended. But an API consumer would not see a status update there, so they're not aware that they can run back-pagination, because the previous status indicated that pagination hit the start of the timeline (and it hasn't changed since then, b/o absence of status update). Kaboom, we're stuck.

The resolution consists in resetting the paginator's state a bit earlier, i.e. just after receiving a limited timeline. In that case, we can even forward the pagination token a bit ahead of time, if it was already available.

Includes a regression test, that failed when checking !hit_start_of_timeline() after the timeline reset, before the patch.

Should fix element-hq/element-x-ios#2024, @pixlwave can you confirm please?

[codecov]

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.28%. Comparing base (08df153) to head (3eef61e).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/matrix-sdk/src/event_cache/paginator.rs	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3974      +/-   ##
==========================================
+ Coverage   84.25%   84.28%   +0.02%     
==========================================
  Files         267      267              
  Lines       28290    28296       +6     
==========================================
+ Hits        23837    23849      +12     
+ Misses       4453     4447       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jmartinesp]

Thanks, the code changes LGTM. Now we wait for @pixlwave 's confirmation that it fixes the issue.

[bnjbvr]

Thanks for the review!

Small API question here: I could've kept the previous code, but! if I did that, there would not be any state notification, because the code below uses self.state.set_if_not_eq(Idle), and the previous value was still Idle. Using Initial is fine because it's also the initial set you get when creating an empty paginator. Another way to fix that would've been to replace set_if_not_eq with set, which forces a notification (and maybe check in the timeline that we're not deduplicating such updates. Any preference over here?

[jmartinesp]

I think using Initial makes sense and as you said prevents the clients from messing up the integration by deduplicating the state updates, which is something we tend to do on other places.

[bnjbvr]

Good point, thanks!

[pixlwave]

Thanks @bnjbvr, I can confirm, this fixes the issue 🎉