From 8f453bc12ea662cdfbeb20ef6c546bec1667b1d7 Mon Sep 17 00:00:00 2001
From: Mikunj <mikunj@live.com.au>
Date: Mon, 20 Apr 2020 09:52:47 +1000
Subject: [PATCH] Fix unnecessary db calls

---
 app/sql.js                         | 35 +++++++++++++++++++-----------
 js/background.js                   | 30 ++++++++++++++-----------
 js/modules/data.js                 | 11 +++-------
 js/modules/loki_app_dot_net_api.js | 24 ++++++++++++++------
 4 files changed, 59 insertions(+), 41 deletions(-)

diff --git a/app/sql.js b/app/sql.js
index d8ecea1c7..82b70e6cc 100644
--- a/app/sql.js
+++ b/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) {
-  const row = await db.get(
-    `SELECT * FROM messages WHERE
-      serverId = $serverId AND
-      conversationId = $conversationId;`,
+async function getMessageIdsFromServerIds(serverIds, conversationId) {
+  if (!Array.isArray(serverIds)) {
+    return [];
+  }
+
+  // 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,
     }
   );
-
-  if (!row) {
-    return null;
-  }
-
-  return jsonToObject(row.json);
+  return rows.map(row => row.id);
 }
 
 async function getMessageById(id) {
diff --git a/js/background.js b/js/background.js
index 6a800039b..6714c5025 100644
--- a/js/background.js
+++ b/js/background.js
@@ -451,24 +451,28 @@
   });
 
   Whisper.events.on(
-    'deleteLocalPublicMessage',
-    async ({ messageServerId, conversationId }) => {
-      const message = await window.Signal.Data.getMessageByServerId(
-        messageServerId,
-        conversationId,
-        {
-          Message: Whisper.Message,
-        }
+    'deleteLocalPublicMessages',
+    async ({ messageServerIds, conversationId }) => {
+      if (!Array.isArray(messageServerIds)) {
+        return;
+      }
+      const messageIds = await window.Signal.Data.getMessageIdsFromServerIds(
+        messageServerIds,
+        conversationId
       );
-      if (message) {
-        const conversation = ConversationController.get(conversationId);
+      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,
         });
-      }
+      });
     }
   );
 
diff --git a/js/modules/data.js b/js/modules/data.js
index 5d97acd9c..79cbf7d66 100644
--- a/js/modules/data.js
+++ b/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 }) {
-  const message = await channels.getMessageByServerId(serverId, conversationId);
-  if (!message) {
-    return null;
-  }
-
-  return new Message(message);
+async function getMessageIdsFromServerIds(serverIds, conversationId) {
+  return channels.getMessageIdsFromServerIds(serverIds, conversationId);
 }
 
 async function getMessageById(id, { Message }) {
diff --git a/js/modules/loki_app_dot_net_api.js b/js/modules/loki_app_dot_net_api.js
index 4d4a1c1b7..a7a17763f 100644
--- a/js/modules/loki_app_dot_net_api.js
+++ b/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 => {
-        // Escalate it up to the subsystem that can check to see if this has
-        // been processed
-        Whisper.events.trigger('deleteLocalPublicMessage', {
-          messageServerId: deleteEntry.message_id,
+      const entries = res.response.data || [];
+      if (entries.length > 0) {
+        Whisper.events.trigger('deleteLocalPublicMessages', {
+          messageServerIds: entries.reverse().map(e => e.message_id),
           conversationId: this.conversationId,
         });
-      });
+      }
 
       // update where we last checked
       this.deleteLastId = res.response.meta.max_id;