Skip to content

Commit 79e0ca0

Browse files
chrisbobbegnprice
authored andcommitted
model: Restore matchContent/matchTopic but remove them in reconcileMessages
This reverts commit ac7be16 while still keeping these per-keyword-search fields out of MessageStore (by removing them in reconcileMessages). Related: #1663
1 parent a81e43a commit 79e0ca0

File tree

9 files changed

+81
-4
lines changed

9 files changed

+81
-4
lines changed

lib/api/model/events.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,11 @@ class MessageEvent extends Event {
763763
// In the server API, the `flags` field appears directly on the event rather
764764
// than on the message object. To avoid proliferating message types, we
765765
// normalize that away in deserialization.
766+
//
767+
// The other difference in the server API between message objects in these
768+
// events and in the get-messages results is that `matchContent` and
769+
// `matchTopic` are absent here. Already [Message.matchContent] and
770+
// [Message.matchTopic] are optional, so no action is needed on that.
766771
@JsonKey(readValue: _readMessageValue, fromJson: Message.fromJson, includeToJson: false)
767772
final Message message;
768773

lib/api/model/model.dart

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -865,8 +865,9 @@ sealed class Message<T extends Conversation> extends MessageBase<T> {
865865
// final string type; // handled by runtime type of object
866866
@JsonKey(fromJson: _flagsFromJson)
867867
List<MessageFlag> flags; // Unrecognized flags won't roundtrip through {to,from}Json.
868-
// TODO(#1663) Add matchContent and matchTopic back again;
869-
// revert the commit that removed these and related test/comment changes.
868+
String? matchContent;
869+
@JsonKey(name: 'match_subject')
870+
String? matchTopic;
870871

871872
static MessageEditState _messageEditStateFromJson(Object? json) {
872873
// This is a no-op so that [MessageEditState._readFromMessage]
@@ -911,6 +912,8 @@ sealed class Message<T extends Conversation> extends MessageBase<T> {
911912
required this.senderRealmStr,
912913
required super.timestamp,
913914
required this.flags,
915+
required this.matchContent,
916+
required this.matchTopic,
914917
});
915918

916919
// TODO(dart): This has to be a static method, because factories/constructors
@@ -994,6 +997,8 @@ class StreamMessage extends Message<StreamConversation> {
994997
required super.senderRealmStr,
995998
required super.timestamp,
996999
required super.flags,
1000+
required super.matchContent,
1001+
required super.matchTopic,
9971002
required this.conversation,
9981003
});
9991004

@@ -1054,6 +1059,8 @@ class DmMessage extends Message<DmConversation> {
10541059
required super.senderRealmStr,
10551060
required super.timestamp,
10561061
required super.flags,
1062+
required super.matchContent,
1063+
required super.matchTopic,
10571064
required this.conversation,
10581065
});
10591066

lib/api/model/model.g.dart

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/model/message.dart

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,14 @@ mixin MessageStore {
5252
/// to the store.
5353
///
5454
/// The list's length will not change, but some entries may be replaced
55-
/// by a different [Message] object with the same [Message.id].
55+
/// by a different [Message] object with the same [Message.id],
56+
/// or mutated to remove [Message.matchContent] and [Message.matchTopic]
57+
/// (since these are appropriate for search views but not the central store).
5658
/// All [Message] objects in the resulting list will be present in
5759
/// [this.messages].
60+
///
61+
/// [Message.matchTopic] and [Message.matchContent] should be captured,
62+
/// as needed for search, before this is called.
5863
void reconcileMessages(List<Message> messages);
5964

6065
/// Whether the current edit request for the given message, if any, has failed.
@@ -280,7 +285,11 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
280285
// those events' changes. So we always stick with the version we have.
281286
for (int i = 0; i < messages.length; i++) {
282287
final message = messages[i];
283-
messages[i] = this.messages.putIfAbsent(message.id, () => message);
288+
messages[i] = this.messages.putIfAbsent(message.id, () {
289+
message.matchContent = null;
290+
message.matchTopic = null;
291+
return message;
292+
});
284293
}
285294
}
286295

lib/model/store.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -942,6 +942,10 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
942942

943943
case MessageEvent():
944944
assert(debugLog("server event: message ${jsonEncode(event.message.toJson())}"));
945+
// Assert against malformed events that might be created in test code.
946+
assert(event.message.matchContent == null);
947+
assert(event.message.matchTopic == null);
948+
945949
_messages.handleMessageEvent(event);
946950
unreads.handleMessageEvent(event);
947951
recentDmConversationsView.handleMessageEvent(event);

test/api/model/model_checks.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ extension MessageChecks on Subject<Message> {
6969
Subject<Poll?> get poll => has((e) => e.poll, 'poll');
7070
Subject<String> get type => has((e) => e.type, 'type');
7171
Subject<List<MessageFlag>> get flags => has((e) => e.flags, 'flags');
72+
Subject<String?> get matchContent => has((e) => e.matchContent, 'matchContent');
73+
Subject<String?> get matchTopic => has((e) => e.matchTopic, 'matchTopic');
7274
}
7375

7476
extension StreamMessageChecks on Subject<StreamMessage> {

test/api/model/model_test.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,13 @@ void main() {
9393
.topic.equals(const TopicName('hello'));
9494
});
9595

96+
test('match_subject -> matchTopic', () {
97+
check(baseStreamJson()).not((it) => it.containsKey('match_topic'));
98+
check(Message.fromJson(baseStreamJson()
99+
..['match_subject'] = 'yo'
100+
)).matchTopic.equals('yo');
101+
});
102+
96103
test('no crash on unrecognized flag', () {
97104
final m1 = Message.fromJson(
98105
(deepToJson(eg.streamMessage()) as Map<String, dynamic>)

test/example_data.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,8 @@ StreamMessage streamMessage({
522522
List<Reaction>? reactions,
523523
int? timestamp,
524524
List<MessageFlag>? flags,
525+
String? matchContent,
526+
String? matchTopic,
525527
List<Submessage>? submessages,
526528
}) {
527529
_checkPositive(id, 'message ID');
@@ -545,6 +547,8 @@ StreamMessage streamMessage({
545547
'submessages': submessages ?? [],
546548
'timestamp': timestamp ?? 1678139636,
547549
'type': 'stream',
550+
'match_content': matchContent,
551+
'match_subject': matchTopic,
548552
}) as Map<String, dynamic>);
549553
}
550554

test/model/message_test.dart

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,37 @@ void main() {
518518
check(messages).single.identicalTo(message);
519519
check(store.messages).deepEquals({1: message});
520520
});
521+
522+
test('matchContent and matchTopic are removed', () async {
523+
await prepare();
524+
final message1 = eg.streamMessage(id: 1, content: '<p>foo</p>');
525+
await addMessages([message1]);
526+
check(store.messages).deepEquals({1: message1});
527+
final otherMessage1 = eg.streamMessage(id: 1, content: '<p>foo</p>',
528+
matchContent: 'some highlighted content',
529+
matchTopic: 'some highlighted topic');
530+
final message2 = eg.streamMessage(id: 2, content: '<p>bar</p>',
531+
matchContent: 'some highlighted content',
532+
matchTopic: 'some highlighted topic');
533+
final messages = [otherMessage1, message2];
534+
store.reconcileMessages(messages);
535+
536+
Condition<Object?> conditionIdenticalAndNullMatchFields(Message message) {
537+
return (it) => it.isA<Message>()
538+
..identicalTo(message)
539+
..matchContent.isNull()..matchTopic.isNull();
540+
}
541+
542+
check(messages).deepEquals([
543+
conditionIdenticalAndNullMatchFields(message1),
544+
conditionIdenticalAndNullMatchFields(message2),
545+
]);
546+
547+
check(store.messages).deepEquals({
548+
1: conditionIdenticalAndNullMatchFields(message1),
549+
2: conditionIdenticalAndNullMatchFields(message2),
550+
});
551+
});
521552
});
522553

523554
group('edit-message methods', () {

0 commit comments

Comments
 (0)