-
Notifications
You must be signed in to change notification settings - Fork 38
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
Feature/get space hierarchy cache #1885
base: main
Are you sure you want to change the base?
Feature/get space hierarchy cache #1885
Conversation
This includes changes from authors, that haven't signed the CLA. Could you ask them to sign the CLA or remove their contribution? |
lib/src/client.dart
Outdated
@@ -2315,6 +2316,10 @@ class Client extends MatrixApi { | |||
} | |||
onEvent.add(update); | |||
|
|||
if (event.type == EventTypes.SpaceChild) { |
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.
You also need to trigger this, when a parent is changed, since you don't need both relations.
@@ -3323,6 +3328,55 @@ class Client extends MatrixApi { | |||
waitUntilLoadCompletedLoaded: false, | |||
); | |||
} | |||
|
|||
@override | |||
Future<GetSpaceHierarchyResponse> getSpaceHierarchy(String roomId, |
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.
You seem to just be caching the whole response. This can introduce some weirdness if you mix cached and uncached responses. Additionally such responses shouldn't be cached indefinitely, I would say.
You also aren't indexing your cache entries by the other parameters for the request, which means for spaces with more than "limit" entries in the response, you will be returning the wrong response. You will also return the wrong response if any of the parameters (apart from the roomid) changes. Additionally you aren't invalidating the cache if a subspace member changes, which means for any depth > 1 this will yield wrong results, if the subspace changes.
I think this needs a lot more tests and quite a bit of work on the cache invalidation logic. While it would certainly be useful to have more efficient access to the space hierarchy, we don't want to return wrong results.
27e24e5
to
d02c60f
Compare
d02c60f
to
2962e68
Compare
No description provided.