From 9251711fa5d2ad7917e4e597004925a97d8479d2 Mon Sep 17 00:00:00 2001
From: Audric Ackermann <audric@loki.network>
Date: Fri, 20 May 2022 13:19:48 +1000
Subject: [PATCH] fix: optmize markAllAsRead when no expiration timer
we basically do a single sql call to mark everything as read for that
conversation, force unreadCount to 0 and mention state to false, and
trigger read syncs if needed.
the optomization cannot work for conversation with expiration timer for
now
---
.../conversation/SessionConversation.tsx | 10 ++--
ts/data/data.ts | 8 ++++
ts/data/dataInit.ts | 1 +
ts/interactions/conversationInteractions.ts | 3 +-
ts/models/conversation.ts | 48 ++++++++++++++++---
ts/node/sql.ts | 36 +++++++++++++-
ts/receiver/userProfileImageUpdates.ts | 2 +-
7 files changed, 96 insertions(+), 12 deletions(-)
diff --git a/ts/components/conversation/SessionConversation.tsx b/ts/components/conversation/SessionConversation.tsx
index 6ddbe06c9..4b594d681 100644
--- a/ts/components/conversation/SessionConversation.tsx
+++ b/ts/components/conversation/SessionConversation.tsx
@@ -54,6 +54,7 @@ import { ConversationMessageRequestButtons } from './ConversationRequestButtons'
import { ConversationRequestinfo } from './ConversationRequestInfo';
import { getCurrentRecoveryPhrase } from '../../util/storage';
import loadImage from 'blueimp-load-image';
+import { markAllReadByConvoId } from '../../interactions/conversationInteractions';
// tslint:disable: jsx-curly-spacing
interface State {
@@ -276,14 +277,17 @@ export class SessionConversation extends React.Component<Props, State> {
}
private async scrollToNow() {
- if (!this.props.selectedConversationKey) {
+ const conversationKey = this.props.selectedConversationKey;
+ if (!conversationKey) {
return;
}
- const mostNowMessage = await getLastMessageInConversation(this.props.selectedConversationKey);
+
+ await markAllReadByConvoId(conversationKey);
+ const mostNowMessage = await getLastMessageInConversation(conversationKey);
if (mostNowMessage) {
await openConversationToSpecificMessage({
- conversationKey: this.props.selectedConversationKey,
+ conversationKey,
messageIdToNavigateTo: mostNowMessage.id,
shouldHighlightMessage: false,
});
diff --git a/ts/data/data.ts b/ts/data/data.ts
index 5cfd0c232..3c426832e 100644
--- a/ts/data/data.ts
+++ b/ts/data/data.ts
@@ -403,6 +403,7 @@ export async function getMessageBySenderAndTimestamp({
source,
timestamp,
});
+
if (!messages || !messages.length) {
return null;
}
@@ -415,6 +416,13 @@ export async function getUnreadByConversation(conversationId: string): Promise<M
return new MessageCollection(messages);
}
+export async function markAllAsReadByConversationNoExpiration(
+ conversationId: string
+): Promise<Array<number>> {
+ const messagesIds = await channels.markAllAsReadByConversationNoExpiration(conversationId);
+ return messagesIds;
+}
+
// might throw
export async function getUnreadCountByConversation(conversationId: string): Promise<number> {
return channels.getUnreadCountByConversation(conversationId);
diff --git a/ts/data/dataInit.ts b/ts/data/dataInit.ts
index 7c9d7380f..1f8071882 100644
--- a/ts/data/dataInit.ts
+++ b/ts/data/dataInit.ts
@@ -43,6 +43,7 @@ const channelsToMake = new Set([
'removeMessage',
'_removeMessages',
'getUnreadByConversation',
+ 'markAllAsReadByConversationNoExpiration',
'getUnreadCountByConversation',
'getMessageCountByType',
'removeAllMessagesInConversation',
diff --git a/ts/interactions/conversationInteractions.ts b/ts/interactions/conversationInteractions.ts
index 7f7de4b9f..e64cba2a1 100644
--- a/ts/interactions/conversationInteractions.ts
+++ b/ts/interactions/conversationInteractions.ts
@@ -292,7 +292,8 @@ export async function markAllReadByConvoId(conversationId: string) {
const conversation = getConversationController().get(conversationId);
perfStart(`markAllReadByConvoId-${conversationId}`);
- await conversation.markReadBouncy(Date.now());
+ await conversation?.markAllAsRead();
+
perfEnd(`markAllReadByConvoId-${conversationId}`, 'markAllReadByConvoId');
}
diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts
index cfcab74c7..d080d57bf 100644
--- a/ts/models/conversation.ts
+++ b/ts/models/conversation.ts
@@ -1,5 +1,5 @@
import Backbone from 'backbone';
-import _ from 'lodash';
+import _, { uniq } from 'lodash';
import { getMessageQueue } from '../session';
import { getConversationController } from '../session/conversations';
import { ClosedGroupVisibleMessage } from '../session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage';
@@ -17,6 +17,7 @@ import {
getMessagesByConversation,
getUnreadByConversation,
getUnreadCountByConversation,
+ markAllAsReadByConversationNoExpiration,
removeMessage as dataRemoveMessage,
saveMessages,
updateConversation,
@@ -1062,15 +1063,50 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
}
}
+ /**
+ * Mark everything as read efficiently if possible.
+ *
+ * For convos with a expiration timer enable, start the timer as of no.
+ * Send read receipt if needed.
+ */
+ public async markAllAsRead() {
+ if (this.isOpenGroupV2()) {
+ // for opengroups, we batch everything as there is no expiration timer to take care (and potentially a lot of messages)
+
+ await markAllAsReadByConversationNoExpiration(this.id);
+ this.set({ mentionedUs: false, unreadCount: 0 });
+
+ await this.commit();
+ return;
+ }
+
+ // if the conversation has no expiration timer, we can also batch everything, but we also need to send read receipts potentially
+ // so we grab them from the db
+ if (!this.get('expireTimer')) {
+ const allReadMessages = await markAllAsReadByConversationNoExpiration(this.id);
+ this.set({ mentionedUs: false, unreadCount: 0 });
+ await this.commit();
+ if (allReadMessages.length) {
+ await this.sendReadReceiptsIfNeeded(uniq(allReadMessages));
+ }
+ return;
+ }
+
+ await this.markReadBouncy(Date.now());
+ }
+
// tslint:disable-next-line: cyclomatic-complexity
- public async markReadBouncy(newestUnreadDate: number, providedOptions: any = {}) {
+ public async markReadBouncy(
+ newestUnreadDate: number,
+ providedOptions: { sendReadReceipts?: boolean; readAt?: number } = {}
+ ) {
const lastReadTimestamp = this.lastReadTimestamp;
if (newestUnreadDate < lastReadTimestamp) {
return;
}
- const options = providedOptions || {};
- _.defaults(options, { sendReadReceipts: true });
+ const defaultedReadAt = providedOptions?.readAt || Date.now();
+ const defaultedSendReadReceipts = providedOptions?.sendReadReceipts || true;
const conversationId = this.id;
Notifications.clearByConversationID(conversationId);
@@ -1084,7 +1120,7 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
// Build the list of updated message models so we can mark them all as read on a single sqlite call
for (const nowRead of oldUnreadNowRead) {
- nowRead.markReadNoCommit(options.readAt);
+ nowRead.markReadNoCommit(defaultedReadAt);
const errors = nowRead.get('errors');
read.push({
@@ -1146,7 +1182,7 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
// conversation is viewed, another error message shows up for the contact
read = read.filter(item => !item.hasErrors);
- if (read.length && options.sendReadReceipts) {
+ if (read.length && defaultedSendReadReceipts) {
const timestamps = _.map(read, 'timestamp').filter(t => !!t) as Array<number>;
await this.sendReadReceiptsIfNeeded(timestamps);
}
diff --git a/ts/node/sql.ts b/ts/node/sql.ts
index 0a0edb60c..3b06bb58b 100644
--- a/ts/node/sql.ts
+++ b/ts/node/sql.ts
@@ -6,6 +6,7 @@ import { app, clipboard, dialog, Notification } from 'electron';
import {
chunk,
+ compact,
difference,
flattenDeep,
forEach,
@@ -2378,6 +2379,38 @@ function getUnreadByConversation(conversationId: string) {
return map(rows, row => jsonToObject(row.json));
}
+/**
+ * Warning: This does not start expiration timer
+ */
+function markAllAsReadByConversationNoExpiration(
+ conversationId: string
+): Array<{ id: string; timestamp: number }> {
+ const messagesUnreadBefore = assertGlobalInstance()
+ .prepare(
+ `SELECT json FROM ${MESSAGES_TABLE} WHERE
+ unread = $unread AND
+ conversationId = $conversationId;`
+ )
+ .all({
+ unread: 1,
+ conversationId,
+ });
+
+ assertGlobalInstance()
+ .prepare(
+ `UPDATE ${MESSAGES_TABLE} SET
+ unread = 0, json = json_set(json, '$.unread', 0)
+ WHERE unread = $unread AND
+ conversationId = $conversationId;`
+ )
+ .run({
+ unread: 1,
+ conversationId,
+ });
+
+ return compact(messagesUnreadBefore.map(row => jsonToObject(row.json).sent_at));
+}
+
function getUnreadCountByConversation(conversationId: string) {
const row = assertGlobalInstance()
.prepare(
@@ -2610,7 +2643,7 @@ function getFirstUnreadMessageWithMention(conversationId: string, ourpubkey: str
function getMessagesBySentAt(sentAt: number) {
const rows = assertGlobalInstance()
.prepare(
- `SELECT * FROM ${MESSAGES_TABLE}
+ `SELECT json FROM ${MESSAGES_TABLE}
WHERE sent_at = $sent_at
ORDER BY received_at DESC;`
)
@@ -3712,6 +3745,7 @@ export const sqlNode = {
saveMessages,
removeMessage,
getUnreadByConversation,
+ markAllAsReadByConversationNoExpiration,
getUnreadCountByConversation,
getMessageCountByType,
diff --git a/ts/receiver/userProfileImageUpdates.ts b/ts/receiver/userProfileImageUpdates.ts
index 9e402e1cd..98041d802 100644
--- a/ts/receiver/userProfileImageUpdates.ts
+++ b/ts/receiver/userProfileImageUpdates.ts
@@ -42,7 +42,7 @@ export async function appendFetchAvatarAndProfileJob(
// );
return;
}
- window.log.info(`[profile-update] queuing fetching avatar for ${conversation.id}`);
+ // window.log.info(`[profile-update] queuing fetching avatar for ${conversation.id}`);
const task = allowOnlyOneAtATime(oneAtaTimeStr, async () => {
return createOrUpdateProfile(conversation, profile, profileKey);
});