From 74748a08b313824bfffbfa07918cd5e242df4988 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Mon, 12 Aug 2024 00:09:39 +0430 Subject: [PATCH 1/5] api: Mark as non-final the ZulipStream fields that can change These are the fields that gets updated via `ChannelUpdateEvent` in the next commit. --- lib/api/model/model.dart | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index ef5297ac61..6694c6f1d8 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -313,28 +313,28 @@ enum UserRole{ @JsonSerializable(fieldRename: FieldRename.snake) class ZulipStream { final int streamId; - final String name; - final String description; - final String renderedDescription; + String name; + String description; + String renderedDescription; final int dateCreated; - final int? firstMessageId; + int? firstMessageId; - final bool inviteOnly; - final bool isWebPublic; // present since 2.1, according to /api/changelog - final bool historyPublicToSubscribers; - final int? messageRetentionDays; + bool inviteOnly; + bool isWebPublic; // present since 2.1, according to /api/changelog + bool historyPublicToSubscribers; + int? messageRetentionDays; @JsonKey(name: 'stream_post_policy') - final ChannelPostPolicy channelPostPolicy; + ChannelPostPolicy channelPostPolicy; // final bool isAnnouncementOnly; // deprecated for `channelPostPolicy`; ignore // TODO(server-6): `canRemoveSubscribersGroupId` added in FL 142 // TODO(server-8): in FL 197 renamed to `canRemoveSubscribersGroup` @JsonKey(readValue: _readCanRemoveSubscribersGroup) - final int? canRemoveSubscribersGroup; + int? canRemoveSubscribersGroup; // TODO(server-8): added in FL 199, was previously only on [Subscription] objects - final int? streamWeeklyTraffic; + int? streamWeeklyTraffic; static int? _readCanRemoveSubscribersGroup(Map json, String key) { return (json[key] as int?) From 2f0aac5565035e925774d19fc85f1fffc0f3bf94 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Mon, 12 Aug 2024 00:29:23 +0430 Subject: [PATCH 2/5] api: Add stream/update event Fixes: #182 --- lib/api/model/events.dart | 71 ++++++++++++++++++++++++++++++++++-- lib/api/model/events.g.dart | 41 +++++++++++++++++++++ lib/api/model/model.dart | 45 +++++++++++++++++++++++ lib/api/model/model.g.dart | 13 +++++++ lib/model/channel.dart | 44 ++++++++++++++++++++++ test/example_data.dart | 31 ++++++++++++++++ test/model/channel_test.dart | 13 ++++++- 7 files changed, 254 insertions(+), 4 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 8dc395a8da..3bd5ac72fc 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -40,7 +40,7 @@ sealed class Event { switch (json['op'] as String) { case 'create': return ChannelCreateEvent.fromJson(json); case 'delete': return ChannelDeleteEvent.fromJson(json); - // TODO(#182): case 'update': … + case 'update': return ChannelUpdateEvent.fromJson(json); default: return UnexpectedEvent.fromJson(json); } case 'subscription': @@ -379,8 +379,73 @@ class ChannelDeleteEvent extends ChannelEvent { Map toJson() => _$ChannelDeleteEventToJson(this); } -// TODO(#182) ChannelUpdateEvent, for a [ChannelEvent] with op `update`: -// https://zulip.com/api/get-events#stream-update +/// A [ChannelEvent] with op `update`: https://zulip.com/api/get-events#stream-update +@JsonSerializable(fieldRename: FieldRename.snake) +class ChannelUpdateEvent extends ChannelEvent { + @override + String get op => 'update'; + + final int streamId; + final String name; + + /// The name of the channel property, or null if we don't recognize it. + @JsonKey(unknownEnumValue: JsonKey.nullForUndefinedEnumValue) + final ChannelPropertyName? property; + + /// The new value, or null if we don't recognize the property. + /// + /// This will have the type appropriate for [property]; for example, + /// if the property is boolean, then `value is bool` will always be true. + /// This invariant is enforced by [ChannelUpdateEvent.fromJson]. + @JsonKey(readValue: _readValue) + final Object? value; + + final String? renderedDescription; + final bool? historyPublicToSubscribers; + final bool? isWebPublic; + + ChannelUpdateEvent({ + required super.id, + required this.streamId, + required this.name, + required this.property, + required this.value, + this.renderedDescription, + this.historyPublicToSubscribers, + this.isWebPublic, + }); + + /// [value], with a check that its type corresponds to [property] + /// (e.g., `value as bool`). + static Object? _readValue(Map json, String key) { + final value = json['value']; + switch (ChannelPropertyName.fromRawString(json['property'] as String)) { + case ChannelPropertyName.name: + case ChannelPropertyName.description: + return value as String; + case ChannelPropertyName.firstMessageId: + return value as int?; + case ChannelPropertyName.inviteOnly: + return value as bool; + case ChannelPropertyName.messageRetentionDays: + return value as int?; + case ChannelPropertyName.channelPostPolicy: + return ChannelPostPolicy.fromApiValue(value as int); + case ChannelPropertyName.canRemoveSubscribersGroup: + case ChannelPropertyName.canRemoveSubscribersGroupId: + case ChannelPropertyName.streamWeeklyTraffic: + return value as int?; + case null: + return null; + } + } + + factory ChannelUpdateEvent.fromJson(Map json) => + _$ChannelUpdateEventFromJson(json); + + @override + Map toJson() => _$ChannelUpdateEventToJson(this); +} /// A Zulip event of type `subscription`. /// diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index dc988b45cb..9226278d26 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -228,6 +228,47 @@ Map _$ChannelDeleteEventToJson(ChannelDeleteEvent instance) => 'streams': instance.streams, }; +ChannelUpdateEvent _$ChannelUpdateEventFromJson(Map json) => + ChannelUpdateEvent( + id: (json['id'] as num).toInt(), + streamId: (json['stream_id'] as num).toInt(), + name: json['name'] as String, + property: $enumDecodeNullable( + _$ChannelPropertyNameEnumMap, json['property'], + unknownValue: JsonKey.nullForUndefinedEnumValue), + value: ChannelUpdateEvent._readValue(json, 'value'), + renderedDescription: json['rendered_description'] as String?, + historyPublicToSubscribers: + json['history_public_to_subscribers'] as bool?, + isWebPublic: json['is_web_public'] as bool?, + ); + +Map _$ChannelUpdateEventToJson(ChannelUpdateEvent instance) => + { + 'id': instance.id, + 'type': instance.type, + 'stream_id': instance.streamId, + 'name': instance.name, + 'property': _$ChannelPropertyNameEnumMap[instance.property], + 'value': instance.value, + 'rendered_description': instance.renderedDescription, + 'history_public_to_subscribers': instance.historyPublicToSubscribers, + 'is_web_public': instance.isWebPublic, + }; + +const _$ChannelPropertyNameEnumMap = { + ChannelPropertyName.name: 'name', + ChannelPropertyName.description: 'description', + ChannelPropertyName.firstMessageId: 'first_message_id', + ChannelPropertyName.inviteOnly: 'invite_only', + ChannelPropertyName.messageRetentionDays: 'message_retention_days', + ChannelPropertyName.channelPostPolicy: 'stream_post_policy', + ChannelPropertyName.canRemoveSubscribersGroup: 'can_remove_subscribers_group', + ChannelPropertyName.canRemoveSubscribersGroupId: + 'can_remove_subscribers_group_id', + ChannelPropertyName.streamWeeklyTraffic: 'stream_weekly_traffic', +}; + SubscriptionAddEvent _$SubscriptionAddEventFromJson( Map json) => SubscriptionAddEvent( diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 6694c6f1d8..729278f6db 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -382,6 +382,51 @@ enum ChannelPostPolicy { final int? apiValue; int? toJson() => apiValue; + + static ChannelPostPolicy fromApiValue(int value) => _byApiValue[value]!; + + static final _byApiValue = _$ChannelPostPolicyEnumMap + .map((key, value) => MapEntry(value, key)); +} + +/// The name of a property of [ZulipStream] that gets updated +/// through [ChannelUpdateEvent.property]. +/// +/// In Zulip event-handling code (for [ChannelUpdateEvent]), +/// we switch exhaustively on a value of this type +/// to ensure that every property in [ZulipStream] responds to the event. +/// +/// Fields on [ZulipStream] not present here: +/// streamId, dateCreated +/// Each of those is immutable on any given channel, and there is no +/// [ChannelUpdateEvent] that updates them. +/// +/// Other fields on [ZulipStream] not present here: +/// renderedDescription, historyPublicToSubscribers, isWebPublic +/// Each of those are updated through separate fields of [ChannelUpdateEvent] +/// with the same names. +@JsonEnum(fieldRename: FieldRename.snake, alwaysCreate: true) +enum ChannelPropertyName { + name, + description, + firstMessageId, + inviteOnly, + messageRetentionDays, + @JsonValue('stream_post_policy') + channelPostPolicy, + canRemoveSubscribersGroup, + canRemoveSubscribersGroupId, // TODO(server-8): remove, replaced by canRemoveSubscribersGroup + streamWeeklyTraffic; + + /// Get a [ChannelPropertyName] from a raw, snake-case string we recognize, else null. + /// + /// Example: + /// 'invite_only' -> ChannelPropertyName.inviteOnly + static ChannelPropertyName? fromRawString(String raw) => _byRawString[raw]; + + // _$…EnumMap is thanks to `alwaysCreate: true` and `fieldRename: FieldRename.snake` + static final _byRawString = _$ChannelPropertyNameEnumMap + .map((key, value) => MapEntry(value, key)); } /// As in `subscriptions` in the initial snapshot. diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 0171278e4d..2c9ac0163c 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -399,6 +399,19 @@ const _$EmojisetEnumMap = { Emojiset.text: 'text', }; +const _$ChannelPropertyNameEnumMap = { + ChannelPropertyName.name: 'name', + ChannelPropertyName.description: 'description', + ChannelPropertyName.firstMessageId: 'first_message_id', + ChannelPropertyName.inviteOnly: 'invite_only', + ChannelPropertyName.messageRetentionDays: 'message_retention_days', + ChannelPropertyName.channelPostPolicy: 'stream_post_policy', + ChannelPropertyName.canRemoveSubscribersGroup: 'can_remove_subscribers_group', + ChannelPropertyName.canRemoveSubscribersGroupId: + 'can_remove_subscribers_group_id', + ChannelPropertyName.streamWeeklyTraffic: 'stream_weekly_traffic', +}; + const _$MessageFlagEnumMap = { MessageFlag.read: 'read', MessageFlag.starred: 'starred', diff --git a/lib/model/channel.dart b/lib/model/channel.dart index 81dc4123cb..a4820340c4 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -199,6 +199,50 @@ class ChannelStoreImpl with ChannelStore { streamsByName.remove(stream.name); subscriptions.remove(stream.streamId); } + + case ChannelUpdateEvent(): + final stream = streams[event.streamId]; + if (stream == null) return; // TODO(log) + assert(stream.streamId == event.streamId); + + if (event.renderedDescription != null) { + stream.renderedDescription = event.renderedDescription!; + } + if (event.historyPublicToSubscribers != null) { + stream.historyPublicToSubscribers = event.historyPublicToSubscribers!; + } + if (event.isWebPublic != null) { + stream.isWebPublic = event.isWebPublic!; + } + + if (event.property == null) { + // unrecognized property; do nothing + return; + } + switch (event.property!) { + case ChannelPropertyName.name: + final streamName = stream.name; + assert(streamName == event.name); + assert(identical(streams[stream.streamId], streamsByName[streamName])); + stream.name = event.value as String; + streamsByName.remove(streamName); + streamsByName[stream.name] = stream; + case ChannelPropertyName.description: + stream.description = event.value as String; + case ChannelPropertyName.firstMessageId: + stream.firstMessageId = event.value as int?; + case ChannelPropertyName.inviteOnly: + stream.inviteOnly = event.value as bool; + case ChannelPropertyName.messageRetentionDays: + stream.messageRetentionDays = event.value as int?; + case ChannelPropertyName.channelPostPolicy: + stream.channelPostPolicy = event.value as ChannelPostPolicy; + case ChannelPropertyName.canRemoveSubscribersGroup: + case ChannelPropertyName.canRemoveSubscribersGroupId: + stream.canRemoveSubscribersGroup = event.value as int?; + case ChannelPropertyName.streamWeeklyTraffic: + stream.streamWeeklyTraffic = event.value as int?; + } } } diff --git a/test/example_data.dart b/test/example_data.dart index e14b7c437a..1716869f35 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -655,6 +655,37 @@ ReactionEvent reactionEvent(Reaction reaction, ReactionOp op, int messageId) { ); } +ChannelUpdateEvent channelUpdateEvent( + ZulipStream stream, { + required ChannelPropertyName property, + required Object? value, +}) { + switch (property) { + case ChannelPropertyName.name: + case ChannelPropertyName.description: + assert(value is String); + case ChannelPropertyName.firstMessageId: + assert(value is int?); + case ChannelPropertyName.inviteOnly: + assert(value is bool); + case ChannelPropertyName.messageRetentionDays: + assert(value is int?); + case ChannelPropertyName.channelPostPolicy: + assert(value is ChannelPostPolicy); + case ChannelPropertyName.canRemoveSubscribersGroup: + case ChannelPropertyName.canRemoveSubscribersGroupId: + case ChannelPropertyName.streamWeeklyTraffic: + assert(value is int?); + } + return ChannelUpdateEvent( + id: 1, + streamId: stream.streamId, + name: stream.name, + property: property, + value: value, + ); +} + //////////////////////////////////////////////////////////////// // The entire per-account or global state. // diff --git a/test/model/channel_test.dart b/test/model/channel_test.dart index 219529e2da..2a9909e4c1 100644 --- a/test/model/channel_test.dart +++ b/test/model/channel_test.dart @@ -42,7 +42,7 @@ void main() { ))); }); - test('added by events', () async { + test('added/updated by events', () async { final stream1 = eg.stream(); final stream2 = eg.stream(); final store = eg.store(); @@ -56,6 +56,17 @@ void main() { await store.addSubscription(eg.subscription(stream1)); checkUnified(store); + + await store.handleEvent(eg.channelUpdateEvent(store.streams[stream1.streamId]!, + property: ChannelPropertyName.name, value: 'new stream', + )); + checkUnified(store); + + await store.handleEvent(eg.channelUpdateEvent(store.streams[stream1.streamId]!, + property: ChannelPropertyName.channelPostPolicy, + value: ChannelPostPolicy.administrators, + )); + checkUnified(store); }); }); From 1ee4bb68bf3046a016c1b85225c1b972e45dfa15 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 21 Aug 2024 14:10:58 -0700 Subject: [PATCH 3/5] api [nfc]: Put ChannelPropertyName right after ZulipStream This helps make it easy to compare the class's fields to the enum's values and look for any discrepancies. --- lib/api/model/model.dart | 52 ++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 729278f6db..57bdc2e351 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -363,32 +363,6 @@ class ZulipStream { Map toJson() => _$ZulipStreamToJson(this); } -/// Policy for which users can post to the stream. -/// -/// For docs, search for "stream_post_policy" -/// in -@JsonEnum(valueField: 'apiValue') -enum ChannelPostPolicy { - any(apiValue: 1), - administrators(apiValue: 2), - fullMembers(apiValue: 3), - moderators(apiValue: 4), - unknown(apiValue: null); - - const ChannelPostPolicy({ - required this.apiValue, - }); - - final int? apiValue; - - int? toJson() => apiValue; - - static ChannelPostPolicy fromApiValue(int value) => _byApiValue[value]!; - - static final _byApiValue = _$ChannelPostPolicyEnumMap - .map((key, value) => MapEntry(value, key)); -} - /// The name of a property of [ZulipStream] that gets updated /// through [ChannelUpdateEvent.property]. /// @@ -429,6 +403,32 @@ enum ChannelPropertyName { .map((key, value) => MapEntry(value, key)); } +/// Policy for which users can post to the stream. +/// +/// For docs, search for "stream_post_policy" +/// in +@JsonEnum(valueField: 'apiValue') +enum ChannelPostPolicy { + any(apiValue: 1), + administrators(apiValue: 2), + fullMembers(apiValue: 3), + moderators(apiValue: 4), + unknown(apiValue: null); + + const ChannelPostPolicy({ + required this.apiValue, + }); + + final int? apiValue; + + int? toJson() => apiValue; + + static ChannelPostPolicy fromApiValue(int value) => _byApiValue[value]!; + + static final _byApiValue = _$ChannelPostPolicyEnumMap + .map((key, value) => MapEntry(value, key)); +} + /// As in `subscriptions` in the initial snapshot. /// /// For docs, search for "subscriptions:" From 03f385a5642a7ef1c1937dfdbd2b253499480e67 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 21 Aug 2024 14:04:09 -0700 Subject: [PATCH 4/5] api [nfc]: Explain each missing ChannelPropertyName inline --- lib/api/model/model.dart | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 57bdc2e351..75edd26b70 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -312,6 +312,14 @@ enum UserRole{ /// in . @JsonSerializable(fieldRename: FieldRename.snake) class ZulipStream { + // When adding a field to this class: + // * Add it to [ChannelPropertyName] too, or add a comment there explaining + // why there isn't a corresponding value in that enum. + // * If the field can never change for a given Zulip stream, mark it final. + // Otherwise, make sure it gets updated on [ChannelUpdateEvent]. + // * (If it can change but [ChannelUpdateEvent] doesn't cover that, + // then that's a bug in the API; raise it in `#api design`.) + final int streamId; String name; String description; @@ -369,22 +377,17 @@ class ZulipStream { /// In Zulip event-handling code (for [ChannelUpdateEvent]), /// we switch exhaustively on a value of this type /// to ensure that every property in [ZulipStream] responds to the event. -/// -/// Fields on [ZulipStream] not present here: -/// streamId, dateCreated -/// Each of those is immutable on any given channel, and there is no -/// [ChannelUpdateEvent] that updates them. -/// -/// Other fields on [ZulipStream] not present here: -/// renderedDescription, historyPublicToSubscribers, isWebPublic -/// Each of those are updated through separate fields of [ChannelUpdateEvent] -/// with the same names. @JsonEnum(fieldRename: FieldRename.snake, alwaysCreate: true) enum ChannelPropertyName { + // streamId is immutable name, description, + // renderedDescription is updated via its own [ChannelUpdateEvent] field + // dateCreated is immutable firstMessageId, inviteOnly, + // isWebPublic is updated via its own [ChannelUpdateEvent] field + // historyPublicToSubscribers is updated via its own [ChannelUpdateEvent] field messageRetentionDays, @JsonValue('stream_post_policy') channelPostPolicy, From fc3db42c8e3eced8e404e9ffac8540a477854c30 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 21 Aug 2024 14:27:36 -0700 Subject: [PATCH 5/5] api: Tighten value type on ChannelUpdateEvent for canRemoveSubscribersGroup This property on streams/channels is documented as type "integer": https://zulip.com/api/get-streams#response (And the same goes for its former name of canRemoveSubscribersGroupId.) So when the server supports this property at all, and therefore might send us an event for it, the value it supplies will be non-null. The corresponding field on ZulipStream is nullable only because of servers that don't yet support the property. --- lib/api/model/events.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 3bd5ac72fc..6d5fde6895 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -433,6 +433,7 @@ class ChannelUpdateEvent extends ChannelEvent { return ChannelPostPolicy.fromApiValue(value as int); case ChannelPropertyName.canRemoveSubscribersGroup: case ChannelPropertyName.canRemoveSubscribersGroupId: + return value as int; case ChannelPropertyName.streamWeeklyTraffic: return value as int?; case null: