Skip to content

Commit ef08c2b

Browse files
committed
api [nfc]: Pull out MessageBase, make Message generic
See also CZO discussion on the design of this, and the drawbacks of the alternatives: https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/A.20new.20variant.20of.20Zulip.20messages.20-.20inheritance.20structure/with/2141288 This will help support outbox messages in MessageListView later. This requiring specifying the element type of `List`s in some tests (or in some cases, the type of a variable declared). This is a side effect of making `StreamMessage` and `DmMessage` extend `Message<T>` with different `T`'s. When both appear in the same `List`, the upper bound is `Object`, instead of the more specific `Message<Conversation>`. See "least-upper-bound" tagged issues for reference: https://github.com/dart-lang/language/issues?q=state%3Aopen%20label%3A%22least-upper-bound%22
1 parent 2ae3c2f commit ef08c2b

File tree

7 files changed

+64
-29
lines changed

7 files changed

+64
-29
lines changed

lib/api/model/model.dart

+39-8
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ extension type const TopicName(String _value) {
591591
String toJson() => apiName;
592592
}
593593

594-
/// As in [StreamMessage.conversation] and [DmMessage.conversation].
594+
/// As in [MessageBase.conversation].
595595
///
596596
/// Different from [MessageDestination], this information comes from
597597
/// [getMessages] or [getEvents], identifying the conversation that contains a
@@ -637,10 +637,38 @@ class DmConversation extends Conversation {
637637
final List<int> allRecipientIds;
638638
}
639639

640+
/// A message or message-like object, for showing in a message list.
641+
///
642+
/// Other than [Message], we use this for "outbox messages",
643+
/// representing outstanding [sendMessage] requests.
644+
abstract class MessageBase<T extends Conversation> {
645+
const MessageBase({
646+
required this.senderId,
647+
required this.timestamp,
648+
});
649+
650+
/// The Zulip message ID.
651+
///
652+
/// If null, the message doesn't have an ID acknowledged by the server
653+
/// (e.g.: a locally-echoed message).
654+
int? get id;
655+
656+
final int senderId;
657+
final int timestamp;
658+
659+
/// The conversation that contains this message.
660+
///
661+
/// When implementing this, the return type should be either
662+
/// [StreamConversation] or [DmConversation]; it should never be
663+
/// [Conversation], because we expect a concrete subclass of [MessageBase]
664+
/// to represent either a channel message or a DM message, not both.
665+
T get conversation;
666+
}
667+
640668
/// As in the get-messages response.
641669
///
642670
/// https://zulip.com/api/get-messages#response
643-
sealed class Message {
671+
sealed class Message<T extends Conversation> extends MessageBase<T> {
644672
// final String? avatarUrl; // Use [User.avatarUrl] instead; will live-update
645673
final String client;
646674
String content;
@@ -650,6 +678,7 @@ sealed class Message {
650678
@JsonKey(readValue: MessageEditState._readFromMessage, fromJson: Message._messageEditStateFromJson)
651679
MessageEditState editState;
652680

681+
@override
653682
final int id;
654683
bool isMeMessage;
655684
int? lastEditTimestamp;
@@ -660,14 +689,12 @@ sealed class Message {
660689
final int recipientId;
661690
final String senderEmail;
662691
final String senderFullName;
663-
final int senderId;
664692
final String senderRealmStr;
665693

666694
/// Poll data if "submessages" describe a poll, `null` otherwise.
667695
@JsonKey(name: 'submessages', readValue: _readPoll, fromJson: Poll.fromJson, toJson: Poll.toJson)
668696
Poll? poll;
669697

670-
final int timestamp;
671698
String get type;
672699

673700
// final List<TopicLink> topicLinks; // TODO handle
@@ -717,14 +744,16 @@ sealed class Message {
717744
required this.recipientId,
718745
required this.senderEmail,
719746
required this.senderFullName,
720-
required this.senderId,
747+
required super.senderId,
721748
required this.senderRealmStr,
722-
required this.timestamp,
749+
required super.timestamp,
723750
required this.flags,
724751
required this.matchContent,
725752
required this.matchTopic,
726753
});
727754

755+
// TODO(dart): This has to be a static method, because factories/constructors
756+
// do not support type parameters: https://github.com/dart-lang/language/issues/647
728757
static Message fromJson(Map<String, dynamic> json) {
729758
final type = json['type'] as String;
730759
if (type == 'stream') return StreamMessage.fromJson(json);
@@ -762,7 +791,7 @@ enum MessageFlag {
762791
}
763792

764793
@JsonSerializable(fieldRename: FieldRename.snake)
765-
class StreamMessage extends Message {
794+
class StreamMessage extends Message<StreamConversation> {
766795
@override
767796
@JsonKey(includeToJson: true)
768797
String get type => 'stream';
@@ -780,6 +809,7 @@ class StreamMessage extends Message {
780809
@JsonKey(name: 'subject', includeToJson: true)
781810
TopicName get topic => conversation.topic;
782811

812+
@override
783813
@JsonKey(readValue: _readConversation, includeToJson: false)
784814
StreamConversation conversation;
785815

@@ -816,7 +846,7 @@ class StreamMessage extends Message {
816846
}
817847

818848
@JsonSerializable(fieldRename: FieldRename.snake)
819-
class DmMessage extends Message {
849+
class DmMessage extends Message<DmConversation> {
820850
@override
821851
@JsonKey(includeToJson: true)
822852
String get type => 'private';
@@ -834,6 +864,7 @@ class DmMessage extends Message {
834864
@JsonKey(name: 'display_recipient', toJson: _allRecipientIdsToJson, includeToJson: true)
835865
List<int> get allRecipientIds => conversation.allRecipientIds;
836866

867+
@override
837868
@JsonKey(name: 'display_recipient', fromJson: _conversationFromJson, includeToJson: false)
838869
final DmConversation conversation;
839870

lib/api/model/model.g.dart

+4-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/api/model/model_checks.dart

+7-3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ extension StreamConversationChecks on Subject<StreamConversation> {
3333
Subject<String?> get displayRecipient => has((x) => x.displayRecipient, 'displayRecipient');
3434
}
3535

36+
extension MessageBaseChecks<T extends Conversation> on Subject<MessageBase<T>> {
37+
Subject<int?> get id => has((e) => e.id, 'id');
38+
Subject<int> get senderId => has((e) => e.senderId, 'senderId');
39+
Subject<int> get timestamp => has((e) => e.timestamp, 'timestamp');
40+
Subject<T> get conversation => has((e) => e.conversation, 'conversation');
41+
}
42+
3643
extension MessageChecks on Subject<Message> {
3744
Subject<String> get client => has((e) => e.client, 'client');
3845
Subject<String> get content => has((e) => e.content, 'content');
@@ -45,10 +52,8 @@ extension MessageChecks on Subject<Message> {
4552
Subject<int> get recipientId => has((e) => e.recipientId, 'recipientId');
4653
Subject<String> get senderEmail => has((e) => e.senderEmail, 'senderEmail');
4754
Subject<String> get senderFullName => has((e) => e.senderFullName, 'senderFullName');
48-
Subject<int> get senderId => has((e) => e.senderId, 'senderId');
4955
Subject<String> get senderRealmStr => has((e) => e.senderRealmStr, 'senderRealmStr');
5056
Subject<Poll?> get poll => has((e) => e.poll, 'poll');
51-
Subject<int> get timestamp => has((e) => e.timestamp, 'timestamp');
5257
Subject<String> get type => has((e) => e.type, 'type');
5358
Subject<List<MessageFlag>> get flags => has((e) => e.flags, 'flags');
5459
Subject<String?> get matchContent => has((e) => e.matchContent, 'matchContent');
@@ -58,7 +63,6 @@ extension MessageChecks on Subject<Message> {
5863
extension StreamMessageChecks on Subject<StreamMessage> {
5964
Subject<int> get streamId => has((e) => e.streamId, 'streamId');
6065
Subject<TopicName> get topic => has((e) => e.topic, 'topic');
61-
Subject<StreamConversation> get conversation => has((e) => e.conversation, 'conversation');
6266
}
6367

6468
extension ReactionsChecks on Subject<Reactions> {

test/model/message_list_test.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ void main() {
526526
test('unmute a topic -> refetch from scratch', () => awaitFakeAsync((async) async {
527527
await prepare(narrow: const CombinedFeedNarrow());
528528
await prepareMutes(true);
529-
final messages = [
529+
final messages = <Message>[
530530
eg.dmMessage(id: 1, from: eg.otherUser, to: [eg.selfUser]),
531531
eg.streamMessage(id: 2, stream: stream, topic: topic),
532532
];

test/model/message_test.dart

+3-3
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ void main() {
8282
final message1 = eg.streamMessage();
8383
final message2 = eg.streamMessage();
8484
final message3 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
85-
final messages = [message1, message2, message3];
85+
final messages = <Message>[message1, message2, message3];
8686
store.reconcileMessages(messages);
8787
check(messages).deepEquals(
8888
[message1, message2, message3]
@@ -97,7 +97,7 @@ void main() {
9797
final message1 = eg.streamMessage();
9898
final message2 = eg.streamMessage();
9999
final message3 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
100-
final messages = [message1, message2, message3];
100+
final messages = <Message>[message1, message2, message3];
101101
await addMessages(messages);
102102
final newMessage = eg.streamMessage();
103103
store.reconcileMessages([newMessage]);
@@ -137,7 +137,7 @@ void main() {
137137

138138
test('from not-empty', () async {
139139
await prepare();
140-
final messages = [
140+
final messages = <Message>[
141141
eg.streamMessage(),
142142
eg.streamMessage(),
143143
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]),

test/model/unreads_test.dart

+9-9
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ void main() {
247247
final unreadChannelMessage = eg.streamMessage(flags: []);
248248
final readChannelMessage = eg.streamMessage(flags: [MessageFlag.read]);
249249

250-
final allMessages = [
250+
final allMessages = <Message>[
251251
unreadDmMessage, unreadChannelMessage,
252252
readDmMessage, readChannelMessage,
253253
];
@@ -314,7 +314,7 @@ void main() {
314314
if (isDirectMentioned) MessageFlag.mentioned,
315315
if (isWildcardMentioned) MessageFlag.wildcardMentioned,
316316
];
317-
final message = isStream
317+
final Message message = isStream
318318
? eg.streamMessage(flags: flags)
319319
: eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: flags);
320320
model.handleMessageEvent(eg.messageEvent(message));
@@ -401,7 +401,7 @@ void main() {
401401
for (final isKnownToModel in [true, false]) {
402402
for (final isRead in [false, true]) {
403403
final baseFlags = [if (isRead) MessageFlag.read];
404-
for (final (messageDesc, message) in [
404+
for (final (messageDesc, message) in <(String, Message)>[
405405
('stream', eg.streamMessage(flags: baseFlags)),
406406
('1:1 dm', eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: baseFlags)),
407407
]) {
@@ -661,7 +661,7 @@ void main() {
661661
final message13 = eg.streamMessage(id: 13, stream: stream2, topic: 'b', flags: []);
662662
final message14 = eg.streamMessage(id: 14, stream: stream2, topic: 'b', flags: [MessageFlag.mentioned]);
663663

664-
final messages = [
664+
final messages = <Message>[
665665
message1, message2, message3, message4, message5,
666666
message6, message7, message8, message9, message10,
667667
message11, message12, message13, message14,
@@ -848,7 +848,7 @@ void main() {
848848
// That case is indistinguishable from an unread that's unknown to
849849
// the model, so we get coverage for that case too.
850850
test('add flag: ${mentionFlag.name}', () {
851-
final messages = [
851+
final messages = <Message>[
852852
eg.streamMessage(flags: []),
853853
eg.streamMessage(flags: [MessageFlag.read]),
854854
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []),
@@ -885,7 +885,7 @@ void main() {
885885
// That case is indistinguishable from an unread that's unknown to
886886
// the model, so we get coverage for that case too.
887887
test('remove flag: ${mentionFlag.name}', () {
888-
final messages = [
888+
final messages = <Message>[
889889
eg.streamMessage(flags: [mentionFlag]),
890890
eg.streamMessage(flags: [mentionFlag, MessageFlag.read]),
891891
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: [mentionFlag]),
@@ -924,7 +924,7 @@ void main() {
924924
final message2 = eg.streamMessage(id: 2, flags: [MessageFlag.mentioned]);
925925
final message3 = eg.dmMessage(id: 3, from: eg.otherUser, to: [eg.selfUser], flags: []);
926926
final message4 = eg.dmMessage(id: 4, from: eg.otherUser, to: [eg.selfUser], flags: [MessageFlag.wildcardMentioned]);
927-
final messages = [message1, message2, message3, message4];
927+
final messages = <Message>[message1, message2, message3, message4];
928928

929929
prepare();
930930
fillWithMessages([message1, message2, message3, message4]);
@@ -973,7 +973,7 @@ void main() {
973973
final message13 = eg.streamMessage(id: 13, stream: stream2, topic: 'b', flags: []);
974974
final message14 = eg.streamMessage(id: 14, stream: stream2, topic: 'b', flags: [MessageFlag.mentioned]);
975975

976-
final messages = [
976+
final messages = <Message>[
977977
message1, message2, message3, message4, message5,
978978
message6, message7, message8, message9, message10,
979979
message11, message12, message13, message14,
@@ -1085,7 +1085,7 @@ void main() {
10851085
final message13 = eg.streamMessage(id: 13, stream: stream2, topic: 'b', flags: [MessageFlag.read]);
10861086
final message14 = eg.streamMessage(id: 14, stream: stream2, topic: 'b', flags: [MessageFlag.mentioned, MessageFlag.read]);
10871087

1088-
final messages = [
1088+
final messages = <Message>[
10891089
message1, message2, message3, message4, message5,
10901090
message6, message7, message8, message9, message10,
10911091
message11, message12, message13, message14,

test/widgets/message_list_test.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ void main() {
538538
final streamMessage = eg.streamMessage();
539539
final topicNarrow = TopicNarrow.ofMessage(streamMessage);
540540

541-
for (final (description, message, narrow) in [
541+
for (final (description, message, narrow) in <(String, Message, SendableNarrow)>[
542542
('typing in dm', dmMessage, dmNarrow),
543543
('typing in topic', streamMessage, topicNarrow),
544544
]) {

0 commit comments

Comments
 (0)