-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
event cache: introduce the Paginator
API
#3309
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3309 +/- ##
==========================================
- Coverage 83.64% 83.58% -0.06%
==========================================
Files 238 240 +2
Lines 24662 24835 +173
==========================================
+ Hits 20629 20759 +130
- Misses 4033 4076 +43 ☔ View full report in Codecov by Sentry. |
63df8fa
to
001e86a
Compare
f7e053e
to
d90b37a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Lots of questions and suggestions, and an ask for more tests. Thanks!
I've now added isolated tests for getting events from start_from / paginate_backwards / paginate_forwards, and I think it covers all the things you've asked — PTAL! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant, thank you!
async fn paginate( | ||
&self, | ||
dir: Direction, | ||
token_lock: &Mutex<Option<String>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great!
This introduces a new helper object to run arbitrary pagination requests, backwards- or forward-. At the moment they're disconnected from the event cache, although I've put the files there for future convenience, since at some point we'll want to merge the retrieved events with the cache (? maybe).
This little state machine makes it possible to retrieve the initial data, given an initial event id, using the /context endpoint; then allow stateful pagination using a paginator kind of API. Paginating in the timeline indicates whether we've reached the start/end of the timeline.
The test for the state subscription is quite extensive and makes sure the basic functionality works as intended.
Some testing helpers have been (re)introduced in the SDK crate, simplifying the code, and introducing a better
EventFactory
/EventBuilder
pattern than the existing one in thematrix-sdk-test
crate. In particular, this can make use of some types inmatrix-sdk
, notablySyncTimelineEvent
andTimelineEvent
, and I've found the API to be simpler to use as well.Commit history is messy, so we might want to squash when merging.
Part of #3234.