Caching added similar to Swift - #1318
Conversation
Changeset ✓This PR includes a changeset covering all affected packages:
|
4adf9c6 to
c99ad83
Compare
c99ad83 to
a8baf86
Compare
a8baf86 to
c94b983
Compare
c94b983 to
8774ee0
Compare
8774ee0 to
d281120
Compare
d281120 to
1b15456
Compare
1b15456 to
1fdbc89
Compare
xianshijing-lk
left a comment
There was a problem hiding this comment.
some nits and questions, lgtm in general
| /// Removes the stored credentials, forcing the next fetch to hit the | ||
| /// underlying source. | ||
| pub async fn invalidate(&self) { | ||
| self.store.clear().await; |
There was a problem hiding this comment.
what will happen if a fetch is ongoing here ?
There was a problem hiding this comment.
Added documentation:
/// A fetch already in flight is unaffected: it still resolves and stores
/// its response afterwards, repopulating the cache (last writer wins).
| } | ||
| } | ||
|
|
||
| let response = self.source.fetch(options).await?; |
There was a problem hiding this comment.
curiously, can multiple fetch() be called at the same time ? I wonder if we will need to protect such corner case.
There was a problem hiding this comment.
Yes, and we don't protect against this, same as in Swift btw. This would be a bigger design. I think as other SDKs are not that advanced either yet, maybe we keep it in mind as a follow up.
|
|
||
| #[async_trait] | ||
| impl TokenSourceConfigurable for CountingSource { | ||
| async fn fetch( |
There was a problem hiding this comment.
nit, should this fetch() has a .await to mimic the real use case ?
There was a problem hiding this comment.
In these tests it would not change the behaviour, it would still run in sequence.
I added tests though for a token source that can mimic parallel in flight requests to test the existing contract of last writer wins.
fdb0638 to
98e948c
Compare
Background
Stacked cached token source in Rust, similar to swift: https://github.com/livekit/client-sdk-swift/blob/main/Sources/LiveKit/Token/CachingTokenSource.swift