Skip to content

Unify stream with subscription data #395

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

Closed
gnprice opened this issue Nov 20, 2023 · 0 comments · Fixed by #423
Closed

Unify stream with subscription data #395

gnprice opened this issue Nov 20, 2023 · 0 comments · Fixed by #423
Labels
a-model Implementing our data model (PerAccountStore, etc.)

Comments

@gnprice
Copy link
Member

gnprice commented Nov 20, 2023

Currently on PerAccountStore we have the following data structures:

  final Map<int, ZulipStream> streams;
  final Map<String, ZulipStream> streamsByName;
  final Map<int, Subscription> subscriptions;

The two stream maps are different ways of reaching the same objects; but the subscription objects are separate. Subscription objects have all the information that's on the corresponding ZulipStream, so this is duplicative, and raises the possibility of them getting out of sync; keeping them in sync is an invariant we need to maintain.

The duplication reflects what the server API gives us in the initial snapshot — there's both streams and subscriptions, and the items in the latter duplicate all the information that's in the corresponding items of the former. But within our code we should give ourselves just one copy of the stream information.

This came up on a call between me and @sirpengi a couple of weeks ago, and in #382 he took one step toward this, which was to make our Subscription type extend ZulipStream. That will let us have a data structure with a type like Map<int, ZulipStream> where for streams we're subscribed to, the value is a Subscription. It'll take a bit of playing with it to keep the API that's exposed to the rest of the app a convenient one, though. I plan to experiment with that sometime after Beta 1 is out.

@gnprice gnprice added the a-model Implementing our data model (PerAccountStore, etc.) label Nov 20, 2023
@gnprice gnprice added this to the Beta 2 milestone Nov 20, 2023
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Nov 29, 2023
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-model Implementing our data model (PerAccountStore, etc.)
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant