Skip to content

Commit c33e4e0

Browse files
authored
Merge pull request oxen-io#2335 from Bilb/mark-all-as-read-optimization
optimize markAllAsRead when no expiration timer & fix read receipts
2 parents 6a7df54 + 72348d1 commit c33e4e0

File tree

9 files changed

+106
-22
lines changed

9 files changed

+106
-22
lines changed

stylesheets/_quote.scss

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,5 +280,3 @@
280280
line-height: 18px;
281281
}
282282
}
283-
284-

ts/components/conversation/SessionConversation.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ import { ConversationMessageRequestButtons } from './ConversationRequestButtons'
5454
import { ConversationRequestinfo } from './ConversationRequestInfo';
5555
import { getCurrentRecoveryPhrase } from '../../util/storage';
5656
import loadImage from 'blueimp-load-image';
57+
import { markAllReadByConvoId } from '../../interactions/conversationInteractions';
58+
5759
import { SessionSpinner } from '../basic/SessionSpinner';
5860
import styled from 'styled-components';
5961
// tslint:disable: jsx-curly-spacing
@@ -307,17 +309,18 @@ export class SessionConversation extends React.Component<Props, State> {
307309
}
308310

309311
private async scrollToNow() {
310-
if (!this.props.selectedConversationKey) {
312+
const conversationKey = this.props.selectedConversationKey;
313+
if (!conversationKey) {
311314
return;
312315
}
313-
const mostNowMessage = await Data.getLastMessageInConversation(
314-
this.props.selectedConversationKey
315-
);
316316

317-
if (mostNowMessage) {
317+
await markAllReadByConvoId(conversationKey);
318+
const mostRecentMessage = await Data.getLastMessageInConversation(conversationKey);
319+
320+
if (mostRecentMessage) {
318321
await openConversationToSpecificMessage({
319-
conversationKey: this.props.selectedConversationKey,
320-
messageIdToNavigateTo: mostNowMessage.id,
322+
conversationKey,
323+
messageIdToNavigateTo: mostRecentMessage.id,
321324
shouldHighlightMessage: false,
322325
});
323326
const messageContainer = this.messageContainerRef.current;

ts/components/conversation/message/message-content/MessageAuthorText.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React from 'react';
22
import { useSelector } from 'react-redux';
3+
import styled from 'styled-components';
34
import { MessageRenderingProps } from '../../../../models/messageType';
45
import { PubKey } from '../../../../session/types';
56
import {
@@ -19,6 +20,8 @@ type Props = {
1920
messageId: string;
2021
};
2122

23+
const StyledAuthorContainer = styled(Flex)`color: var(--color-text)`;
24+
2225
export const MessageAuthorText = (props: Props) => {
2326
const selected = useSelector(state => getMessageAuthorProps(state as any, props.messageId));
2427

@@ -38,7 +41,7 @@ export const MessageAuthorText = (props: Props) => {
3841
const displayedPubkey = authorProfileName ? PubKey.shorten(sender) : sender;
3942

4043
return (
41-
<Flex container={true}>
44+
<StyledAuthorContainer container={true}>
4245
<ContactName
4346
pubkey={displayedPubkey}
4447
name={authorName}
@@ -47,6 +50,6 @@ export const MessageAuthorText = (props: Props) => {
4750
boldProfileName={true}
4851
shouldShowPubkey={Boolean(isPublic)}
4952
/>
50-
</Flex>
53+
</StyledAuthorContainer>
5154
);
5255
};

ts/components/conversation/message/message-item/ReadableMessage.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,17 +144,17 @@ export const ReadableMessage = (props: ReadableMessageProps) => {
144144
const found = await Data.getMessageById(messageId);
145145

146146
if (found && Boolean(found.get('unread'))) {
147-
const foundReceivedAt = found.get('received_at');
147+
const foundSentAt = found.get('sent_at');
148148
// mark the message as read.
149149
// this will trigger the expire timer.
150150
await found.markRead(Date.now());
151151

152152
// we should stack those and send them in a single message once every 5secs or something.
153153
// this would be part of an redesign of the sending pipeline
154-
if (foundReceivedAt) {
154+
if (foundSentAt && selectedConversationKey) {
155155
void getConversationController()
156-
.get(found.id)
157-
?.sendReadReceiptsIfNeeded([foundReceivedAt]);
156+
.get(selectedConversationKey)
157+
?.sendReadReceiptsIfNeeded([foundSentAt]);
158158
}
159159
}
160160
}

ts/data/data.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ export const Data = {
148148
getMessageBySenderAndTimestamp,
149149
getUnreadByConversation,
150150
getUnreadCountByConversation,
151+
markAllAsReadByConversationNoExpiration,
151152
getMessageCountByType,
152153
getMessagesByConversation,
153154
getLastMessagesByConversation,
@@ -472,6 +473,7 @@ async function getMessageBySenderAndTimestamp({
472473
source,
473474
timestamp,
474475
});
476+
475477
if (!messages || !messages.length) {
476478
return null;
477479
}
@@ -484,6 +486,13 @@ async function getUnreadByConversation(conversationId: string): Promise<MessageC
484486
return new MessageCollection(messages);
485487
}
486488

489+
async function markAllAsReadByConversationNoExpiration(
490+
conversationId: string
491+
): Promise<Array<number>> {
492+
const messagesIds = await channels.markAllAsReadByConversationNoExpiration(conversationId);
493+
return messagesIds;
494+
}
495+
487496
// might throw
488497
async function getUnreadCountByConversation(conversationId: string): Promise<number> {
489498
return channels.getUnreadCountByConversation(conversationId);

ts/data/dataInit.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const channelsToMake = new Set([
4040
'removeMessage',
4141
'_removeMessages',
4242
'getUnreadByConversation',
43+
'markAllAsReadByConversationNoExpiration',
4344
'getUnreadCountByConversation',
4445
'getMessageCountByType',
4546
'removeAllMessagesInConversation',

ts/interactions/conversationInteractions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,8 @@ export async function markAllReadByConvoId(conversationId: string) {
284284
const conversation = getConversationController().get(conversationId);
285285
perfStart(`markAllReadByConvoId-${conversationId}`);
286286

287-
await conversation.markReadBouncy(Date.now());
287+
await conversation?.markAllAsRead();
288+
288289
perfEnd(`markAllReadByConvoId-${conversationId}`, 'markAllReadByConvoId');
289290
}
290291

ts/models/conversation.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { SignalService } from '../protobuf';
2525
import { MessageModel, sliceQuoteText } from './message';
2626
import { MessageAttributesOptionals, MessageDirection } from './messageType';
2727
import autoBind from 'auto-bind';
28+
2829
import { Data } from '../../ts/data/data';
2930
import { toHex } from '../session/utils/String';
3031
import {
@@ -1223,15 +1224,49 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
12231224
}
12241225
}
12251226

1227+
/**
1228+
* Mark everything as read efficiently if possible.
1229+
*
1230+
* For convos with a expiration timer enable, start the timer as of now.
1231+
* Send read receipt if needed.
1232+
*/
1233+
public async markAllAsRead() {
1234+
if (this.isOpenGroupV2()) {
1235+
// for opengroups, we batch everything as there is no expiration timer to take care (and potentially a lot of messages)
1236+
1237+
await Data.markAllAsReadByConversationNoExpiration(this.id);
1238+
this.set({ mentionedUs: false, unreadCount: 0 });
1239+
1240+
await this.commit();
1241+
return;
1242+
}
1243+
1244+
// if the conversation has no expiration timer, we can also batch everything, but we also need to send read receipts potentially
1245+
// so we grab them from the db
1246+
if (!this.get('expireTimer')) {
1247+
const allReadMessages = await Data.markAllAsReadByConversationNoExpiration(this.id);
1248+
this.set({ mentionedUs: false, unreadCount: 0 });
1249+
await this.commit();
1250+
if (allReadMessages.length) {
1251+
await this.sendReadReceiptsIfNeeded(uniq(allReadMessages));
1252+
}
1253+
return;
1254+
}
1255+
await this.markReadBouncy(Date.now());
1256+
}
1257+
12261258
// tslint:disable-next-line: cyclomatic-complexity
1227-
public async markReadBouncy(newestUnreadDate: number, providedOptions: any = {}) {
1259+
public async markReadBouncy(
1260+
newestUnreadDate: number,
1261+
providedOptions: { sendReadReceipts?: boolean; readAt?: number } = {}
1262+
) {
12281263
const lastReadTimestamp = this.lastReadTimestamp;
12291264
if (newestUnreadDate < lastReadTimestamp) {
12301265
return;
12311266
}
12321267

1233-
const options = providedOptions || {};
1234-
defaults(options, { sendReadReceipts: true });
1268+
const readAt = providedOptions?.readAt || Date.now();
1269+
const sendReadReceipts = providedOptions?.sendReadReceipts || true;
12351270

12361271
const conversationId = this.id;
12371272
Notifications.clearByConversationID(conversationId);
@@ -1245,7 +1280,7 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
12451280

12461281
// Build the list of updated message models so we can mark them all as read on a single sqlite call
12471282
for (const nowRead of oldUnreadNowRead) {
1248-
nowRead.markReadNoCommit(options.readAt);
1283+
nowRead.markReadNoCommit(readAt);
12491284

12501285
const errors = nowRead.get('errors');
12511286
read.push({
@@ -1307,7 +1342,7 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
13071342
// conversation is viewed, another error message shows up for the contact
13081343
read = read.filter(item => !item.hasErrors);
13091344

1310-
if (read.length && options.sendReadReceipts) {
1345+
if (read.length && sendReadReceipts) {
13111346
const timestamps = map(read, 'timestamp').filter(t => !!t) as Array<number>;
13121347
await this.sendReadReceiptsIfNeeded(timestamps);
13131348
}

ts/node/sql.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { app, clipboard, dialog, Notification } from 'electron';
66

77
import {
88
chunk,
9+
compact,
910
difference,
1011
forEach,
1112
fromPairs,
@@ -1112,6 +1113,38 @@ function getUnreadByConversation(conversationId: string) {
11121113
return map(rows, row => jsonToObject(row.json));
11131114
}
11141115

1116+
/**
1117+
* Warning: This does not start expiration timer
1118+
*/
1119+
function markAllAsReadByConversationNoExpiration(
1120+
conversationId: string
1121+
): Array<{ id: string; timestamp: number }> {
1122+
const messagesUnreadBefore = assertGlobalInstance()
1123+
.prepare(
1124+
`SELECT json FROM ${MESSAGES_TABLE} WHERE
1125+
unread = $unread AND
1126+
conversationId = $conversationId;`
1127+
)
1128+
.all({
1129+
unread: 1,
1130+
conversationId,
1131+
});
1132+
1133+
assertGlobalInstance()
1134+
.prepare(
1135+
`UPDATE ${MESSAGES_TABLE} SET
1136+
unread = 0, json = json_set(json, '$.unread', 0)
1137+
WHERE unread = $unread AND
1138+
conversationId = $conversationId;`
1139+
)
1140+
.run({
1141+
unread: 1,
1142+
conversationId,
1143+
});
1144+
1145+
return compact(messagesUnreadBefore.map(row => jsonToObject(row.json).sent_at));
1146+
}
1147+
11151148
function getUnreadCountByConversation(conversationId: string) {
11161149
const row = assertGlobalInstance()
11171150
.prepare(
@@ -1346,7 +1379,7 @@ function getFirstUnreadMessageWithMention(
13461379
function getMessagesBySentAt(sentAt: number) {
13471380
const rows = assertGlobalInstance()
13481381
.prepare(
1349-
`SELECT * FROM ${MESSAGES_TABLE}
1382+
`SELECT json FROM ${MESSAGES_TABLE}
13501383
WHERE sent_at = $sent_at
13511384
ORDER BY received_at DESC;`
13521385
)
@@ -2403,6 +2436,7 @@ export const sqlNode = {
24032436
saveMessages,
24042437
removeMessage,
24052438
getUnreadByConversation,
2439+
markAllAsReadByConversationNoExpiration,
24062440
getUnreadCountByConversation,
24072441
getMessageCountByType,
24082442

0 commit comments

Comments
 (0)