Skip to content

Commit 1e32458

Browse files
committed
api [nfc]: Pull out DisplayableMessage
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 locally echoed 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<MessageDestination>`. 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 282632f commit 1e32458

File tree

6 files changed

+56
-19
lines changed

6 files changed

+56
-19
lines changed

lib/api/model/model.dart

+34-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'package:json_annotation/json_annotation.dart';
22

3+
import '../route/messages.dart';
34
import 'events.dart';
45
import 'initial_snapshot.dart';
56
import 'reaction.dart';
@@ -531,10 +532,29 @@ String? tryParseEmojiCodeToUnicode(String emojiCode) {
531532
}
532533
}
533534

535+
/// A common class for messages that can appear in a [MessageList].
536+
abstract class DisplayableMessage<T extends MessageDestination> {
537+
/// The Zulip message ID.
538+
///
539+
/// If null, the message doesn't have an ID acknowledged by the server
540+
/// (e.g.: a locally-echoed message).
541+
int? get id;
542+
543+
int get senderId;
544+
int get timestamp;
545+
546+
/// The destination this message was sent to.
547+
// When implementing this, the return type should be either [StreamDestination]
548+
// or [DmDestination]; it should never be [MessageDestination], because we
549+
// expect a concrete subclass of [DisplayableMessage] to represent either
550+
// channel messages or DM messages, not both.
551+
T get destination;
552+
}
553+
534554
/// As in the get-messages response.
535555
///
536556
/// https://zulip.com/api/get-messages#response
537-
sealed class Message {
557+
sealed class Message<T extends MessageDestination> implements DisplayableMessage<T> {
538558
// final String? avatarUrl; // Use [User.avatarUrl] instead; will live-update
539559
final String client;
540560
String content;
@@ -544,6 +564,7 @@ sealed class Message {
544564
@JsonKey(readValue: MessageEditState._readFromMessage, fromJson: Message._messageEditStateFromJson)
545565
MessageEditState editState;
546566

567+
@override
547568
final int id;
548569
bool isMeMessage;
549570
int? lastEditTimestamp;
@@ -554,13 +575,15 @@ sealed class Message {
554575
final int recipientId;
555576
final String senderEmail;
556577
final String senderFullName;
578+
@override
557579
final int senderId;
558580
final String senderRealmStr;
559581

560582
/// Poll data if "submessages" describe a poll, `null` otherwise.
561583
@JsonKey(name: 'submessages', readValue: _readPoll, fromJson: Poll.fromJson, toJson: Poll.toJson)
562584
Poll? poll;
563585

586+
@override
564587
final int timestamp;
565588
String get type;
566589

@@ -619,6 +642,8 @@ sealed class Message {
619642
required this.matchTopic,
620643
});
621644

645+
// TODO(dart): This has to be a static method, because factories/constructors
646+
// do not support type parameters: https://github.com/dart-lang/language/issues/647
622647
static Message fromJson(Map<String, dynamic> json) {
623648
final type = json['type'] as String;
624649
if (type == 'stream') return StreamMessage.fromJson(json);
@@ -715,7 +740,7 @@ extension type const TopicName(String _value) {
715740
}
716741

717742
@JsonSerializable(fieldRename: FieldRename.snake)
718-
class StreamMessage extends Message {
743+
class StreamMessage extends Message<StreamDestination> {
719744
@override
720745
@JsonKey(includeToJson: true)
721746
String get type => 'stream';
@@ -735,6 +760,9 @@ class StreamMessage extends Message {
735760
@JsonKey(name: 'subject')
736761
TopicName topic;
737762

763+
@override
764+
StreamDestination get destination => StreamDestination(streamId, topic);
765+
738766
StreamMessage({
739767
required super.client,
740768
required super.content,
@@ -809,7 +837,7 @@ class DmRecipientListConverter extends JsonConverter<List<DmRecipient>, List<dyn
809837
}
810838

811839
@JsonSerializable(fieldRename: FieldRename.snake)
812-
class DmMessage extends Message {
840+
class DmMessage extends Message<DmDestination> {
813841
@override
814842
@JsonKey(includeToJson: true)
815843
String get type => 'private';
@@ -838,6 +866,9 @@ class DmMessage extends Message {
838866
/// This is a result of [List.map], so it has an efficient `length`.
839867
Iterable<int> get allRecipientIds => displayRecipient.map((e) => e.id);
840868

869+
@override
870+
DmDestination get destination => DmDestination(userIds: allRecipientIds.toList());
871+
841872
DmMessage({
842873
required super.client,
843874
required super.content,

test/api/model/model_checks.dart

+8-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'package:checks/checks.dart';
22
import 'package:zulip/api/model/model.dart';
33
import 'package:zulip/api/model/submessage.dart';
4+
import 'package:zulip/api/route/messages.dart';
45

56
extension UserChecks on Subject<User> {
67
Subject<int> get userId => has((x) => x.userId, 'userId');
@@ -24,6 +25,13 @@ extension UserChecks on Subject<User> {
2425
extension ZulipStreamChecks on Subject<ZulipStream> {
2526
}
2627

28+
extension DisplayableMessageChecks<T extends MessageDestination> on Subject<DisplayableMessage<T>> {
29+
Subject<int?> get id => has((e) => e.id, 'id');
30+
Subject<int> get senderId => has((e) => e.senderId, 'senderId');
31+
Subject<int> get timestamp => has((e) => e.timestamp, 'timestamp');
32+
Subject<T> get destination => has((e) => e.destination, 'destination');
33+
}
34+
2735
extension MessageChecks on Subject<Message> {
2836
Subject<String> get client => has((e) => e.client, 'client');
2937
Subject<String> get content => has((e) => e.content, 'content');
@@ -36,10 +44,8 @@ extension MessageChecks on Subject<Message> {
3644
Subject<int> get recipientId => has((e) => e.recipientId, 'recipientId');
3745
Subject<String> get senderEmail => has((e) => e.senderEmail, 'senderEmail');
3846
Subject<String> get senderFullName => has((e) => e.senderFullName, 'senderFullName');
39-
Subject<int> get senderId => has((e) => e.senderId, 'senderId');
4047
Subject<String> get senderRealmStr => has((e) => e.senderRealmStr, 'senderRealmStr');
4148
Subject<Poll?> get poll => has((e) => e.poll, 'poll');
42-
Subject<int> get timestamp => has((e) => e.timestamp, 'timestamp');
4349
Subject<String> get type => has((e) => e.type, 'type');
4450
Subject<List<MessageFlag>> get flags => has((e) => e.flags, 'flags');
4551
Subject<String?> get matchContent => has((e) => e.matchContent, 'matchContent');

test/model/message_list_test.dart

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

test/model/message_test.dart

+3-3
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ void main() {
8484
final message1 = eg.streamMessage();
8585
final message2 = eg.streamMessage();
8686
final message3 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
87-
final messages = [message1, message2, message3];
87+
final messages = <Message>[message1, message2, message3];
8888
store.reconcileMessages(messages);
8989
check(messages).deepEquals(
9090
[message1, message2, message3]
@@ -99,7 +99,7 @@ void main() {
9999
final message1 = eg.streamMessage();
100100
final message2 = eg.streamMessage();
101101
final message3 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
102-
final messages = [message1, message2, message3];
102+
final messages = <Message>[message1, message2, message3];
103103
await addMessages(messages);
104104
final newMessage = eg.streamMessage();
105105
store.reconcileMessages([newMessage]);
@@ -139,7 +139,7 @@ void main() {
139139

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

test/model/unreads_test.dart

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

251-
final allMessages = [
251+
final allMessages = <Message>[
252252
unreadDmMessage, unreadChannelMessage,
253253
readDmMessage, readChannelMessage,
254254
];
@@ -315,7 +315,7 @@ void main() {
315315
if (isDirectMentioned) MessageFlag.mentioned,
316316
if (isWildcardMentioned) MessageFlag.wildcardMentioned,
317317
];
318-
final message = isStream
318+
final Message message = isStream
319319
? eg.streamMessage(flags: flags)
320320
: eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: flags);
321321
model.handleMessageEvent(MessageEvent(id: 0, message: message));
@@ -402,7 +402,7 @@ void main() {
402402
for (final isKnownToModel in [true, false]) {
403403
for (final isRead in [false, true]) {
404404
final baseFlags = [if (isRead) MessageFlag.read];
405-
for (final (messageDesc, message) in [
405+
for (final (messageDesc, message) in <(String, Message)>[
406406
('stream', eg.streamMessage(flags: baseFlags)),
407407
('1:1 dm', eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: baseFlags)),
408408
]) {
@@ -662,7 +662,7 @@ void main() {
662662
final message13 = eg.streamMessage(id: 13, stream: stream2, topic: 'b', flags: []);
663663
final message14 = eg.streamMessage(id: 14, stream: stream2, topic: 'b', flags: [MessageFlag.mentioned]);
664664

665-
final messages = [
665+
final messages = <Message>[
666666
message1, message2, message3, message4, message5,
667667
message6, message7, message8, message9, message10,
668668
message11, message12, message13, message14,
@@ -849,7 +849,7 @@ void main() {
849849
// That case is indistinguishable from an unread that's unknown to
850850
// the model, so we get coverage for that case too.
851851
test('add flag: ${mentionFlag.name}', () {
852-
final messages = [
852+
final messages = <Message>[
853853
eg.streamMessage(flags: []),
854854
eg.streamMessage(flags: [MessageFlag.read]),
855855
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []),
@@ -886,7 +886,7 @@ void main() {
886886
// That case is indistinguishable from an unread that's unknown to
887887
// the model, so we get coverage for that case too.
888888
test('remove flag: ${mentionFlag.name}', () {
889-
final messages = [
889+
final messages = <Message>[
890890
eg.streamMessage(flags: [mentionFlag]),
891891
eg.streamMessage(flags: [mentionFlag, MessageFlag.read]),
892892
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: [mentionFlag]),
@@ -925,7 +925,7 @@ void main() {
925925
final message2 = eg.streamMessage(id: 2, flags: [MessageFlag.mentioned]);
926926
final message3 = eg.dmMessage(id: 3, from: eg.otherUser, to: [eg.selfUser], flags: []);
927927
final message4 = eg.dmMessage(id: 4, from: eg.otherUser, to: [eg.selfUser], flags: [MessageFlag.wildcardMentioned]);
928-
final messages = [message1, message2, message3, message4];
928+
final messages = <Message>[message1, message2, message3, message4];
929929

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

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

1089-
final messages = [
1089+
final messages = <Message>[
10901090
message1, message2, message3, message4, message5,
10911091
message6, message7, message8, message9, message10,
10921092
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)