Skip to content

Commit 9429c2e

Browse files
committed
channel: Maintain maps consistently when removing subscriptions
When a subscription is removed, the data in streams and streamsByName are left as instances of `Subscription`. This is inconsistent with the data store's assumption that unsubscribed channels should have plain `ZulipStream` instances. This doesn't seem to have user-facing effects, though. The assertion errors only affect debug builds.
1 parent b506d5e commit 9429c2e

File tree

3 files changed

+44
-1
lines changed

3 files changed

+44
-1
lines changed

lib/api/model/model.dart

+18
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,24 @@ class ZulipStream {
363363
required this.streamWeeklyTraffic,
364364
});
365365

366+
/// Construct a plain [ZulipStream] from [subscription].
367+
factory ZulipStream.fromSubscription(Subscription subscription) {
368+
return ZulipStream(
369+
streamId: subscription.streamId,
370+
name: subscription.name,
371+
description: subscription.description,
372+
renderedDescription: subscription.renderedDescription,
373+
dateCreated: subscription.dateCreated,
374+
firstMessageId: subscription.firstMessageId,
375+
inviteOnly: subscription.inviteOnly,
376+
isWebPublic: subscription.isWebPublic,
377+
historyPublicToSubscribers: subscription.historyPublicToSubscribers,
378+
messageRetentionDays: subscription.messageRetentionDays,
379+
channelPostPolicy: subscription.channelPostPolicy,
380+
streamWeeklyTraffic: subscription.streamWeeklyTraffic,
381+
);
382+
}
383+
366384
factory ZulipStream.fromJson(Map<String, dynamic> json) =>
367385
_$ZulipStreamFromJson(json);
368386

lib/model/channel.dart

+10-1
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,16 @@ class ChannelStoreImpl with ChannelStore {
301301

302302
case SubscriptionRemoveEvent():
303303
for (final streamId in event.streamIds) {
304-
subscriptions.remove(streamId);
304+
assert(streams.containsKey(streamId));
305+
assert(streams[streamId] is Subscription);
306+
assert(streamsByName.containsKey(streams[streamId]!.name));
307+
assert(streamsByName[streams[streamId]!.name] is Subscription);
308+
assert(subscriptions.containsKey(streamId));
309+
final subscription = subscriptions.remove(streamId);
310+
if (subscription == null) continue; // TODO(log)
311+
final stream = ZulipStream.fromSubscription(subscription);
312+
streams[streamId] = stream;
313+
streamsByName[subscription.name] = stream;
305314
}
306315

307316
case SubscriptionUpdateEvent():

test/model/channel_test.dart

+16
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,22 @@ void main() {
6868
));
6969
checkUnified(store);
7070
});
71+
72+
test('removed then added by events', () async {
73+
final stream = eg.stream();
74+
final store = eg.store();
75+
await store.addStream(stream);
76+
await store.addSubscription(eg.subscription(stream));
77+
checkUnified(store);
78+
79+
await store.handleEvent(SubscriptionRemoveEvent(id: 1,
80+
streamIds: [stream.streamId]));
81+
checkUnified(store);
82+
83+
await store.handleEvent(SubscriptionAddEvent(id: 1,
84+
subscriptions: [eg.subscription(stream)]));
85+
checkUnified(store);
86+
});
7187
});
7288

7389
group('SubscriptionEvent', () {

0 commit comments

Comments
 (0)