feat: performance improvements to quote lookup

getMessagesByConversation optionally returns quotes from messages in view, quoted messages that are deleted are removed from the lookup map. getMessageBySenderAndSentAt supports an array of messages and renamed to getMessagesBySenderAndSentAt

ts/data/data.ts

@@ -18,6 +18,7 @@ import { channels } from './channels';
 import * as dataInit from './dataInit';
 import { StorageItem } from '../node/storage_item';
 import { fromArrayBufferToBase64, fromBase64ToArrayBuffer } from '../session/utils/String';
+import { Quote } from '../receiver/types';
 
 const ERASE_SQL_KEY = 'erase-sql-key';
 const ERASE_ATTACHMENTS_KEY = 'erase-attachments';
@@ -144,7 +145,7 @@ export const Data = {
   getMessageById,
   getMessageByServerId,
   filterAlreadyFetchedOpengroupMessage,
-  getMessageBySenderAndSentAt,
+  getMessagesBySenderAndSentAt,
   getUnreadByConversation,
   getUnreadCountByConversation,
   markAllAsReadByConversationNoExpiration,
@@ -451,26 +452,22 @@ async function filterAlreadyFetchedOpengroupMessage(
 
 /**
  *
- * @param source senders id
- * @param timestamp the timestamp of the message - not to be confused with the serverTimestamp. This is equivalent to sent_at
+ * @param propsList An array of objects containing a source (the sender id) and timestamp of the message - not to be confused with the serverTimestamp. This is equivalent to sent_at
+ * @returns
  */
-async function getMessageBySenderAndSentAt({
-  source,
-  timestamp,
-}: {
-  source: string;
-  timestamp: number;
-}): Promise<MessageModel | null> {
-  const messages = await channels.getMessageBySenderAndSentAt({
-    source,
-    timestamp,
-  });
+async function getMessagesBySenderAndSentAt(
+  propsList: Array<{
+    source: string;
+    timestamp: number;
+  }>
+): Promise<MessageCollection | null> {
+  const messages = await channels.getMessagesBySenderAndSentAt(propsList);
 
   if (!messages || !messages.length) {
     return null;
   }
 
-  return new MessageModel(messages[0]);
+  return new MessageCollection(messages);
 }
 
 async function getUnreadByConversation(conversationId: string): Promise<MessageCollection> {
@@ -512,17 +509,27 @@ async function getMessageCountByType(
 
 async function getMessagesByConversation(
   conversationId: string,
-  { skipTimerInit = false, messageId = null }: { skipTimerInit?: false; messageId: string | null }
-): Promise<MessageCollection> {
-  const messages = await channels.getMessagesByConversation(conversationId, {
+  {
+    skipTimerInit = false,
+    returnQuotes = false,
+    messageId = null,
+  }: { skipTimerInit?: false; returnQuotes?: boolean; messageId: string | null }
+): Promise<{ messages: MessageCollection; quotes: Array<Quote> }> {
+  const { messages, quotes } = await channels.getMessagesByConversation(conversationId, {
     messageId,
+    returnQuotes,
   });
+
   if (skipTimerInit) {
     for (const message of messages) {
       message.skipTimerInit = skipTimerInit;
     }
   }
-  return new MessageCollection(messages);
+
+  return {
+    messages: new MessageCollection(messages),
+    quotes,
+  };
 }
 
 /**

ts/data/dataInit.ts

@@ -46,7 +46,7 @@ const channelsToMake = new Set([
   'removeAllMessagesInConversation',
   'getMessageCount',
   'filterAlreadyFetchedOpengroupMessage',
-  'getMessageBySenderAndSentAt',
+  'getMessagesBySenderAndSentAt',
   'getMessageIdsFromServerIds',
   'getMessageById',
   'getMessagesBySentAt',

ts/models/conversation.ts

@@ -1773,7 +1773,7 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
             );
           }).length === 1;
       const isFirstMessageOfConvo =
-        (await Data.getMessagesByConversation(this.id, { messageId: null })).length === 1;
+        (await Data.getMessagesByConversation(this.id, { messageId: null })).messages.length === 1;
       if (hadNoRequestsPrior && isFirstMessageOfConvo) {
         friendRequestText = window.i18n('youHaveANewFriendRequest');
       } else {

ts/node/sql.ts

@@ -17,6 +17,7 @@ import {
   isString,
   last,
   map,
+  uniq,
 } from 'lodash';
 import { redactAll } from '../util/privacy'; // checked - only node
 import { LocaleMessagesType } from './locale'; // checked - only node
@@ -55,6 +56,7 @@ import {
   updateSchema,
 } from './migration/signalMigrations';
 import { SettingsKey } from '../data/settings-key';
+import { Quote } from '../receiver/types';
 
 // tslint:disable: no-console function-name non-literal-fs-path
 
@@ -1062,19 +1064,32 @@ function getMessagesCountBySender({ source }: { source: string }) {
   return count['count(*)'] || 0;
 }
 
-function getMessageBySenderAndSentAt({ source, timestamp }: { source: string; timestamp: number }) {
-  const rows = assertGlobalInstance()
-    .prepare(
-      `SELECT json FROM ${MESSAGES_TABLE} WHERE
+function getMessagesBySenderAndSentAt(
+  propsList: Array<{
+    source: string;
+    timestamp: number;
+  }>
+) {
+  const db = assertGlobalInstance();
+  const rows = [];
+
+  for (let i = 0; i < propsList.length; i++) {
+    const { source, timestamp } = propsList[i];
+
+    const _rows = db
+      .prepare(
+        `SELECT json FROM ${MESSAGES_TABLE} WHERE
       source = $source AND
       sent_at = $timestamp;`
-    )
-    .all({
-      source,
-      timestamp,
-    });
+      )
+      .all({
+        source,
+        timestamp,
+      });
+    rows.push(..._rows);
+  }
 
-  return map(rows, row => jsonToObject(row.json));
+  return uniq(map(rows, row => jsonToObject(row.json)));
 }
 
 function filterAlreadyFetchedOpengroupMessage(
@@ -1203,7 +1218,10 @@ function getMessageCountByType(conversationId: string, type = '%') {
 const orderByClause = 'ORDER BY COALESCE(serverTimestamp, sent_at, received_at) DESC';
 const orderByClauseASC = 'ORDER BY COALESCE(serverTimestamp, sent_at, received_at) ASC';
 
-function getMessagesByConversation(conversationId: string, { messageId = null } = {}) {
+function getMessagesByConversation(
+  conversationId: string,
+  { messageId = null, returnQuotes = false } = {}
+): { messages: Array<Record<string, any>>; quotes: Array<Quote> } {
   const absLimit = 30;
   // If messageId is given it means we are opening the conversation to that specific messageId,
   // or that we just scrolled to it by a quote click and needs to load around it.
@@ -1213,6 +1231,9 @@ function getMessagesByConversation(conversationId: string, { messageId = null }
   const numberOfMessagesInConvo = getMessagesCountByConversation(conversationId, globalInstance);
   const floorLoadAllMessagesInConvo = 70;
 
+  let messages: Array<Record<string, any>> = [];
+  let quotes = [];
+
   if (messageId || firstUnread) {
     const messageFound = getMessageById(messageId || firstUnread);
 
@@ -1244,33 +1265,39 @@ function getMessagesByConversation(conversationId: string, { messageId = null }
               : absLimit,
         });
 
-      return map(rows, row => jsonToObject(row.json));
+      messages = map(rows, row => jsonToObject(row.json));
     }
     console.info(
       `getMessagesByConversation: Could not find messageId ${messageId} in db with conversationId: ${conversationId}. Just fetching the convo as usual.`
     );
-  }
+  } else {
+    const limit =
+      numberOfMessagesInConvo < floorLoadAllMessagesInConvo
+        ? floorLoadAllMessagesInConvo
+        : absLimit * 2;
 
-  const limit =
-    numberOfMessagesInConvo < floorLoadAllMessagesInConvo
-      ? floorLoadAllMessagesInConvo
-      : absLimit * 2;
-
-  const rows = assertGlobalInstance()
-    .prepare(
-      `
+    const rows = assertGlobalInstance()
+      .prepare(
+        `
     SELECT json FROM ${MESSAGES_TABLE} WHERE
       conversationId = $conversationId
       ${orderByClause}
     LIMIT $limit;
     `
-    )
-    .all({
-      conversationId,
-      limit,
-    });
+      )
+      .all({
+        conversationId,
+        limit,
+      });
 
-  return map(rows, row => jsonToObject(row.json));
+    messages = map(rows, row => jsonToObject(row.json));
+  }
+
+  if (returnQuotes) {
+    quotes = messages.filter(message => message.quote).map(message => message.quote);
+  }
+
+  return { messages, quotes };
 }
 
 function getLastMessagesByConversation(conversationId: string, limit: number) {
@@ -2454,7 +2481,7 @@ export const sqlNode = {
   getMessageCountByType,
 
   filterAlreadyFetchedOpengroupMessage,
-  getMessageBySenderAndSentAt,
+  getMessagesBySenderAndSentAt,
   getMessageIdsFromServerIds,
   getMessageById,
   getMessagesBySentAt,

ts/receiver/contentMessage.ts

@@ -565,10 +565,14 @@ async function handleUnsendMessage(envelope: EnvelopePlus, unsendMessage: Signal
 
     return;
   }
-  const messageToDelete = await Data.getMessageBySenderAndSentAt({
-    source: messageAuthor,
-    timestamp: toNumber(timestamp),
-  });
+  const messageToDelete = (
+    await Data.getMessagesBySenderAndSentAt([
+      {
+        source: messageAuthor,
+        timestamp: toNumber(timestamp),
+      },
+    ])
+  )?.models?.[0];
   const messageHash = messageToDelete?.get('messageHash');
   //#endregion
 
@@ -665,7 +669,7 @@ async function handleMessageRequestResponse(
       )
     );
 
-    const allMessageModels = flatten(allMessagesCollections.map(m => m.models));
+    const allMessageModels = flatten(allMessagesCollections.map(m => m.messages.models));
     allMessageModels.forEach(messageModel => {
       messageModel.set({ conversationId: unblindedConvoId });
 

ts/receiver/dataMessage.ts

@@ -258,10 +258,14 @@ export async function isSwarmMessageDuplicate({
   sentAt: number;
 }) {
   try {
-    const result = await Data.getMessageBySenderAndSentAt({
-      source,
-      timestamp: sentAt,
-    });
+    const result = (
+      await Data.getMessagesBySenderAndSentAt([
+        {
+          source,
+          timestamp: sentAt,
+        },
+      ])
+    )?.models?.length;
 
     return Boolean(result);
   } catch (error) {

ts/state/ducks/conversations.ts

@@ -289,6 +289,7 @@ export type ConversationLookupType = {
 };
 
 export type QuoteLookupType = {
+  // key is message [timestamp]-[author-pubkey]
   [key: string]: MessageModelPropsWithoutConvoProps;
 };
 
@@ -298,7 +299,6 @@ export type ConversationsStateType = {
   // NOTE the messages that are in view
   messages: Array<MessageModelPropsWithoutConvoProps>;
   // NOTE the quotes that are in view
-  // key is message [timestamp]-[author-pubkey]
   quotes: QuoteLookupType;
   firstUnreadMessageId: string | undefined;
   messageDetailProps?: MessagePropsDetails;
@@ -364,48 +364,45 @@ async function getMessages({
     return { messagesProps: [], quotesProps: {} };
   }
 
-  const messageSet = await Data.getMessagesByConversation(conversationKey, {
+  const {
+    messages: messagesCollection,
+    quotes: quotesCollection,
+  } = await Data.getMessagesByConversation(conversationKey, {
     messageId,
+    returnQuotes: true,
   });
 
-  const messagesProps: Array<MessageModelPropsWithoutConvoProps> = messageSet.models.map(m =>
-    m.getMessageModelProps()
+  const messagesProps: Array<MessageModelPropsWithoutConvoProps> = messagesCollection.models.map(
+    m => m.getMessageModelProps()
   );
   const time = Date.now() - beforeTimestamp;
   window?.log?.info(`Loading ${messagesProps.length} messages took ${time}ms to load.`);
 
   const quotesProps: QuoteLookupType = {};
-  const quotes = messagesProps.filter(
-    message => message.propsForMessage?.quote?.messageId && message.propsForMessage.quote?.sender
-  );
 
-  for (let i = 0; i < quotes.length; i++) {
-    const id = quotes[i].propsForMessage?.quote?.messageId;
-    const sender = quotes[i].propsForMessage.quote?.sender;
-
-    if (id && sender) {
-      const timestamp = Number(id);
-      // See if a quoted message is already in memory if not lookup in db
-      let results = messagesProps.filter(
-        message =>
-          message.propsForMessage.timestamp === timestamp &&
-          message.propsForMessage.sender === sender
-      );
-
-      if (!results.length) {
-        const dbResult = (
-          await Data.getMessageBySenderAndSentAt({ source: sender, timestamp })
-        )?.getMessageModelProps();
-        if (dbResult) {
-          results = [dbResult];
+  if (quotesCollection?.length) {
+    const quotePropsList = quotesCollection.map(quote => ({
+      timestamp: Number(quote?.id),
+      source: String(quote?.author),
+    }));
+
+    const quotedMessagesCollection = await Data.getMessagesBySenderAndSentAt(quotePropsList);
+
+    if (quotedMessagesCollection?.length) {
+      for (let i = 0; i < quotedMessagesCollection.length; i++) {
+        const quotedMessage = quotedMessagesCollection.models.at(i)?.getMessageModelProps();
+        if (quotedMessage) {
+          const timestamp = Number(quotedMessage.propsForMessage.timestamp);
+          const sender = quotedMessage.propsForMessage.sender;
+          if (timestamp && sender) {
+            quotesProps[`${timestamp}-${sender}`] = quotedMessage;
+          }
         }
       }
-      quotesProps[`${timestamp}-${sender}`] = results[0];
     }
   }
 
-  // window.log.debug(`WIP: duck quoteProps`, quotesProps);
-
+  // window.log.debug(`WIP: duck quotesProps`, quotesProps);
   return { messagesProps, quotesProps };
 }
 
@@ -563,7 +560,26 @@ function handleMessageExpiredOrDeleted(
     // search if we find this message id.
     // we might have not loaded yet, so this case might not happen
     const messageInStoreIndex = state?.messages.findIndex(m => m.propsForMessage.id === messageId);
+    const editedQuotes = { ...state.quotes };
     if (messageInStoreIndex >= 0) {
+      // Check if the message is quoted somewhere, and if so, remove it from the quotes
+      const msgProps = state.messages[messageInStoreIndex].propsForMessage;
+      // TODO check if message is a group or public group because we will need to use the server timestamp
+      const { timestamp, sender } = msgProps;
+      if (timestamp && sender) {
+        const message2Delete = editedQuotes[`${timestamp}-${sender}`];
+        window.log.debug(
+          `WIP: deleting quote {${timestamp}-${sender}} ${JSON.stringify(message2Delete)}`
+        );
+        window.log.debug(
+          `WIP: editedQuotes count before delete ${Object.keys(editedQuotes).length}`
+        );
+        delete editedQuotes[`${timestamp}-${sender}`];
+        window.log.debug(
+          `WIP: editedQuotes count after delete ${Object.keys(editedQuotes).length}`
+        );
+      }
+
       // we cannot edit the array directly, so slice the first part, and slice the second part,
       // keeping the index removed out
       const editedMessages = [
@@ -578,6 +594,7 @@ function handleMessageExpiredOrDeleted(
       return {
         ...state,
         messages: editedMessages,
+        quotes: editedQuotes,
         firstUnreadMessageId:
           state.firstUnreadMessageId === messageId ? undefined : state.firstUnreadMessageId,
       };