Merge pull request #1086 from Mikunj/cpu-fix

Fix unnecessary db calls

app/sql.js

@@ -137,7 +137,7 @@ module.exports = {
   removeMessage,
   getUnreadByConversation,
   getMessageBySender,
-  getMessageByServerId,
+  getMessageIdsFromServerIds,
   getMessageById,
   getAllMessages,
   getAllMessageIds,
@@ -2345,22 +2345,31 @@ async function removeMessage(id) {
   );
 }
 
-async function getMessageByServerId(serverId, conversationId) {
+async function getMessageIdsFromServerIds(serverIds, conversationId) {
-  const row = await db.get(
+  if (!Array.isArray(serverIds)) {
-    `SELECT * FROM messages WHERE
+    return [];
-      serverId = $serverId AND
+  }
+
+  // Sanitize the input as we're going to use it directly in the query
+  const validIds = serverIds
+    .map(id => Number(id))
+    .filter(n => !Number.isNaN(n));
+
+  /*
+    Sqlite3 doesn't have a good way to have `IN` query with another query.
+    See: https://github.com/mapbox/node-sqlite3/issues/762.
+
+    So we have to use templating to insert the values.
+  */
+  const rows = await db.all(
+    `SELECT id FROM messages WHERE
+    serverId IN (${validIds.join(',')}) AND
     conversationId = $conversationId;`,
     {
-      $serverId: serverId,
       $conversationId: conversationId,
     }
   );
-
+  return rows.map(row => row.id);
-  if (!row) {
-    return null;
-  }
-
-  return jsonToObject(row.json);
 }
 
 async function getMessageById(id) {

js/background.js

@@ -451,24 +451,28 @@
   });
 
   Whisper.events.on(
-    'deleteLocalPublicMessage',
+    'deleteLocalPublicMessages',
-    async ({ messageServerId, conversationId }) => {
+    async ({ messageServerIds, conversationId }) => {
-      const message = await window.Signal.Data.getMessageByServerId(
+      if (!Array.isArray(messageServerIds)) {
-        messageServerId,
+        return;
-        conversationId,
-        {
-          Message: Whisper.Message,
       }
+      const messageIds = await window.Signal.Data.getMessageIdsFromServerIds(
+        messageServerIds,
+        conversationId
       );
-      if (message) {
+      if (messageIds.length === 0) {
+        return;
+      }
+
       const conversation = ConversationController.get(conversationId);
+      messageIds.forEach(id => {
         if (conversation) {
-          conversation.removeMessage(message.id);
+          conversation.removeMessage(id);
         }
-        await window.Signal.Data.removeMessage(message.id, {
+        window.Signal.Data.removeMessage(id, {
           Message: Whisper.Message,
         });
-      }
+      });
     }
   );
 

js/modules/data.js

@@ -164,7 +164,7 @@ module.exports = {
   removeAllMessagesInConversation,
 
   getMessageBySender,
-  getMessageByServerId,
+  getMessageIdsFromServerIds,
   getMessageById,
   getAllMessages,
   getAllUnsentMessages,
@@ -1007,13 +1007,8 @@ async function _removeMessages(ids) {
   await channels.removeMessage(ids);
 }
 
-async function getMessageByServerId(serverId, conversationId, { Message }) {
+async function getMessageIdsFromServerIds(serverIds, conversationId) {
-  const message = await channels.getMessageByServerId(serverId, conversationId);
+  return channels.getMessageIdsFromServerIds(serverIds, conversationId);
-  if (!message) {
-    return null;
-  }
-
-  return new Message(message);
 }
 
 async function getMessageById(id, { Message }) {

js/modules/loki_app_dot_net_api.js

@@ -1423,22 +1423,32 @@ class LokiPublicChannelAPI {
       );
 
       // if any problems, abort out
-      if (res.err || !res.response) {
+      if (
+        res.err ||
+        !res.response ||
+        !res.response.data ||
+        !res.response.meta
+      ) {
         if (res.err) {
           log.error(`pollOnceForDeletions Error ${res.err}`);
+        } else {
+          log.error(
+            `pollOnceForDeletions Error: Received incorrect response ${
+              res.response
+            }`
+          );
         }
         break;
       }
 
       // Process results
-      res.response.data.reverse().forEach(deleteEntry => {
+      const entries = res.response.data || [];
-        // Escalate it up to the subsystem that can check to see if this has
+      if (entries.length > 0) {
-        // been processed
+        Whisper.events.trigger('deleteLocalPublicMessages', {
-        Whisper.events.trigger('deleteLocalPublicMessage', {
+          messageServerIds: entries.reverse().map(e => e.message_id),
-          messageServerId: deleteEntry.message_id,
           conversationId: this.conversationId,
         });
-      });
+      }
 
       // update where we last checked
       this.deleteLastId = res.response.meta.max_id;