fix: slow conversation list queries [WPB-11808] 🍒 (#3083)

* Commit with unresolved merge conflicts * trigger build * add missing ConversationFilter to non-paginated query * Revert "add missing ConversationFilter to non-paginated query" This reverts commit 38ed62f. --------- Co-authored-by: Michał Saleniuk <[email protected]> Co-authored-by: Michał Saleniuk <[email protected]>
wireapp · Nov 12, 2024 · f69841c · f69841c
1 parent f417fd4
commit f69841c
Show file tree

Hide file tree

Showing 5 changed files with 170 additions and 21 deletions.
diff --git a/...tence/src/commonMain/db_user/com/wire/kalium/persistence/ConversationDetailsWithEvents.sq b/...tence/src/commonMain/db_user/com/wire/kalium/persistence/ConversationDetailsWithEvents.sq
@@ -31,7 +31,7 @@ SELECT
     -- last message
     LastMessage.id AS lastMessageId,
     LastMessage.content_type AS lastMessageContentType,
-    LastMessage.creation_date AS lastMessageDate,
+    MAX(LastMessage.creation_date) AS lastMessageDate,
     LastMessage.visibility AS lastMessageVisibility,
     LastMessage.sender_user_id AS lastMessageSenderUserId,
     (LastMessage.expire_after_millis IS NOT NULL) AS lastMessageIsEphemeral,
@@ -52,13 +52,7 @@ LEFT JOIN UnreadEvent
 LEFT JOIN MessageDraft
     ON ConversationDetails.qualifiedId = MessageDraft.conversation_id AND ConversationDetails.archived = 0 -- only return message draft for non-archived conversations
 LEFT JOIN Message AS LastMessage
-    ON LastMessage.id = (
-            SELECT Message.id
-            FROM Message
-            WHERE ConversationDetails.qualifiedId = Message.conversation_id
-            ORDER BY Message.creation_date DESC
-            LIMIT 1
-        ) AND ConversationDetails.archived = 0 -- only return last message for non-archived conversations
+    ON LastMessage.conversation_id = ConversationDetails.qualifiedId AND ConversationDetails.archived = 0 -- only return last message for non-archived conversations
 LEFT JOIN User
     ON LastMessage.sender_user_id = User.qualified_id
 LEFT JOIN MessageMemberChangeContent AS MemberChangeContent
@@ -143,7 +137,16 @@ OFFSET :offset;
 countConversationDetailsWithEvents:
 SELECT COUNT(*) FROM ConversationDetails
 WHERE
-    archived = :fromArchive
+    ConversationDetails.type IS NOT 'SELF'
+    AND (
+        ConversationDetails.type IS 'GROUP'
+        OR (ConversationDetails.type IS 'ONE_ON_ONE' AND (ConversationDetails.name IS NOT NULL AND ConversationDetails.otherUserId IS NOT NULL)) -- show 1:1 convos if they have user metadata
+        OR (ConversationDetails.type IS 'ONE_ON_ONE' AND ConversationDetails.userDeleted = 1) -- show deleted 1:1 convos to maintain prev, logic
+        OR (ConversationDetails.type IS 'CONNECTION_PENDING' AND ConversationDetails.otherUserId IS NOT NULL) -- show connection requests even without metadata
+    )
+    AND (ConversationDetails.protocol IS 'PROTEUS' OR ConversationDetails.protocol IS 'MIXED' OR (ConversationDetails.protocol IS 'MLS' AND ConversationDetails.mls_group_state IS 'ESTABLISHED'))
+    AND ConversationDetails.isActive
+    AND archived = :fromArchive
     AND CASE
         -- When filter is ALL, do not apply additional filters on conversation type
         WHEN :conversationFilter = 'ALL' THEN 1 = 1
@@ -159,7 +162,16 @@ WHERE
 countConversationDetailsWithEventsFromSearch:
 SELECT COUNT(*) FROM ConversationDetails
 WHERE
-    archived = :fromArchive
+    ConversationDetails.type IS NOT 'SELF'
+    AND (
+        ConversationDetails.type IS 'GROUP'
+        OR (ConversationDetails.type IS 'ONE_ON_ONE' AND (ConversationDetails.name IS NOT NULL AND ConversationDetails.otherUserId IS NOT NULL)) -- show 1:1 convos if they have user metadata
+        OR (ConversationDetails.type IS 'ONE_ON_ONE' AND ConversationDetails.userDeleted = 1) -- show deleted 1:1 convos to maintain prev, logic
+        OR (ConversationDetails.type IS 'CONNECTION_PENDING' AND ConversationDetails.otherUserId IS NOT NULL) -- show connection requests even without metadata
+    )
+    AND (ConversationDetails.protocol IS 'PROTEUS' OR ConversationDetails.protocol IS 'MIXED' OR (ConversationDetails.protocol IS 'MLS' AND ConversationDetails.mls_group_state IS 'ESTABLISHED'))
+    AND ConversationDetails.isActive
+    AND archived = :fromArchive
     AND CASE
         -- When filter is ALL, do not apply additional filters on conversation type
         WHEN :conversationFilter = 'ALL' THEN 1 = 1

diff --git a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/MessageDrafts.sq b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/MessageDrafts.sq
@@ -22,13 +22,21 @@ upsertDraft:
 INSERT INTO MessageDraft(conversation_id, text, edit_message_id, quoted_message_id, mention_list)
 VALUES( ?, ?, ?, ?, ?)
 ON CONFLICT(conversation_id) DO UPDATE SET
-text = excluded.text,
-edit_message_id = excluded.edit_message_id,
-quoted_message_id = excluded.quoted_message_id,
-mention_list = excluded.mention_list;
+    text = excluded.text,
+    edit_message_id = excluded.edit_message_id,
+    quoted_message_id = excluded.quoted_message_id,
+    mention_list = excluded.mention_list
+WHERE -- execute the update only if any of the fields changed
+    MessageDraft.text != excluded.text
+    OR MessageDraft.edit_message_id != excluded.edit_message_id
+    OR MessageDraft.quoted_message_id != excluded.quoted_message_id
+    OR MessageDraft.mention_list != excluded.mention_list;
 
 getDraft:
 SELECT * FROM MessageDraft WHERE conversation_id = ?;
 
 getDrafts:
 SELECT * FROM MessageDraft;
+
+selectChanges:
+SELECT changes();
diff --git a/persistence/src/commonMain/db_user/migrations/89.sqm b/persistence/src/commonMain/db_user/migrations/89.sqm
@@ -0,0 +1,80 @@
+DROP VIEW IF EXISTS ConversationDetailsWithEvents;
+
+CREATE VIEW IF NOT EXISTS ConversationDetailsWithEvents AS
+SELECT
+    ConversationDetails.*,
+    -- unread events
+    SUM(CASE WHEN UnreadEvent.type = 'KNOCK' THEN 1 ELSE 0 END) AS unreadKnocksCount,
+    SUM(CASE WHEN UnreadEvent.type = 'MISSED_CALL' THEN 1 ELSE 0 END) AS unreadMissedCallsCount,
+    SUM(CASE WHEN UnreadEvent.type = 'MENTION' THEN 1 ELSE 0 END) AS unreadMentionsCount,
+    SUM(CASE WHEN UnreadEvent.type = 'REPLY' THEN 1 ELSE 0 END) AS unreadRepliesCount,
+    SUM(CASE WHEN UnreadEvent.type = 'MESSAGE' THEN 1 ELSE 0 END) AS unreadMessagesCount,
+    CASE
+        WHEN ConversationDetails.callStatus = 'STILL_ONGOING' AND ConversationDetails.type = 'GROUP' THEN 1 -- if ongoing call in a group, move it to the top
+        WHEN ConversationDetails.mutedStatus = 'ALL_ALLOWED' THEN
+           CASE
+                WHEN COUNT(UnreadEvent.id) > 0 THEN 1 -- if any unread events, move it to the top
+                WHEN ConversationDetails.type = 'CONNECTION_PENDING' AND ConversationDetails.connectionStatus = 'PENDING' THEN 1 -- if received connection request, move it to the top
+                ELSE 0
+            END
+        WHEN ConversationDetails.mutedStatus = 'ONLY_MENTIONS_AND_REPLIES_ALLOWED' THEN
+            CASE
+                WHEN SUM(CASE WHEN UnreadEvent.type IN ('MENTION', 'REPLY') THEN 1 ELSE 0 END) > 0 THEN 1 -- only if unread mentions or replies, move it to the top
+                WHEN ConversationDetails.type = 'CONNECTION_PENDING' AND ConversationDetails.connectionStatus = 'PENDING' THEN 1 -- if received connection request, move it to the top
+                ELSE 0
+            END
+        ELSE 0
+    END AS hasNewActivitiesToShow,
+    -- draft message
+    MessageDraft.text AS messageDraftText,
+    MessageDraft.edit_message_id AS messageDraftEditMessageId,
+    MessageDraft.quoted_message_id AS messageDraftQuotedMessageId,
+    MessageDraft.mention_list AS messageDraftMentionList,
+    -- last message
+    LastMessage.id AS lastMessageId,
+    LastMessage.content_type AS lastMessageContentType,
+    MAX(LastMessage.creation_date) AS lastMessageDate,
+    LastMessage.visibility AS lastMessageVisibility,
+    LastMessage.sender_user_id AS lastMessageSenderUserId,
+    (LastMessage.expire_after_millis IS NOT NULL) AS lastMessageIsEphemeral,
+    User.name AS lastMessageSenderName,
+    User.connection_status AS lastMessageSenderConnectionStatus,
+    User.deleted AS lastMessageSenderIsDeleted,
+    (LastMessage.sender_user_id IS NOT NULL AND LastMessage.sender_user_id == ConversationDetails.selfUserId) AS lastMessageIsSelfMessage,
+    MemberChangeContent.member_change_list AS lastMessageMemberChangeList,
+    MemberChangeContent.member_change_type AS lastMessageMemberChangeType,
+    ConversationNameChangedContent.conversation_name AS lastMessageUpdateConversationName,
+    (Mention.user_id IS NOT NULL) AS lastMessageIsMentioningSelfUser,
+    TextContent.is_quoting_self AS lastMessageIsQuotingSelfUser,
+    TextContent.text_body AS lastMessageText,
+    AssetContent.asset_mime_type AS lastMessageAssetMimeType
+FROM ConversationDetails
+LEFT JOIN UnreadEvent
+    ON UnreadEvent.conversation_id = ConversationDetails.qualifiedId
+LEFT JOIN MessageDraft
+    ON ConversationDetails.qualifiedId = MessageDraft.conversation_id AND ConversationDetails.archived = 0 -- only return message draft for non-archived conversations
+LEFT JOIN Message AS LastMessage
+    ON LastMessage.conversation_id = ConversationDetails.qualifiedId AND ConversationDetails.archived = 0 -- only return last message for non-archived conversations
+LEFT JOIN User
+    ON LastMessage.sender_user_id = User.qualified_id
+LEFT JOIN MessageMemberChangeContent AS MemberChangeContent
+    ON LastMessage.id = MemberChangeContent.message_id AND LastMessage.conversation_id = MemberChangeContent.conversation_id
+LEFT JOIN MessageMention AS Mention
+    ON LastMessage.id == Mention.message_id AND ConversationDetails.selfUserId == Mention.user_id
+LEFT JOIN MessageConversationChangedContent AS ConversationNameChangedContent
+    ON LastMessage.id = ConversationNameChangedContent.message_id AND LastMessage.conversation_id = ConversationNameChangedContent.conversation_id
+LEFT JOIN MessageAssetContent AS AssetContent
+    ON LastMessage.id = AssetContent.message_id AND LastMessage.conversation_id = AssetContent.conversation_id
+LEFT JOIN MessageTextContent AS TextContent
+    ON LastMessage.id = TextContent.message_id AND LastMessage.conversation_id = TextContent.conversation_id
+WHERE
+    ConversationDetails.type IS NOT 'SELF'
+    AND (
+        ConversationDetails.type IS 'GROUP'
+        OR (ConversationDetails.type IS 'ONE_ON_ONE' AND (ConversationDetails.name IS NOT NULL AND ConversationDetails.otherUserId IS NOT NULL)) -- show 1:1 convos if they have user metadata
+        OR (ConversationDetails.type IS 'ONE_ON_ONE' AND ConversationDetails.userDeleted = 1) -- show deleted 1:1 convos to maintain prev, logic
+        OR (ConversationDetails.type IS 'CONNECTION_PENDING' AND ConversationDetails.otherUserId IS NOT NULL) -- show connection requests even without metadata
+    )
+    AND (ConversationDetails.protocol IS 'PROTEUS' OR ConversationDetails.protocol IS 'MIXED' OR (ConversationDetails.protocol IS 'MLS' AND ConversationDetails.mls_group_state IS 'ESTABLISHED'))
+    AND ConversationDetails.isActive
+GROUP BY ConversationDetails.qualifiedId;
diff --git a/...rc/commonMain/kotlin/com/wire/kalium/persistence/dao/message/draft/MessageDraftDAOImpl.kt b/...rc/commonMain/kotlin/com/wire/kalium/persistence/dao/message/draft/MessageDraftDAOImpl.kt
@@ -61,13 +61,20 @@ class MessageDraftDAOImpl internal constructor(
                 }
             }
 
-            queries.upsertDraft(
-                conversation_id = messageDraft.conversationId,
-                text = messageDraft.text,
-                edit_message_id = messageDraft.editMessageId,
-                quoted_message_id = messageDraft.quotedMessageId,
-                mention_list = messageDraft.selectedMentionList
-            )
+            queries.transaction {
+                queries.upsertDraft(
+                    conversation_id = messageDraft.conversationId,
+                    text = messageDraft.text,
+                    edit_message_id = messageDraft.editMessageId,
+                    quoted_message_id = messageDraft.quotedMessageId,
+                    mention_list = messageDraft.selectedMentionList
+                )
+                val changes = queries.selectChanges().executeAsOne()
+                if (changes == 0L) {
+                    // rollback the transaction if no changes were made so that it doesn't notify other queries about changes if not needed
+                    this.rollback()
+                }
+            }
         }
 
     override suspend fun getMessageDraft(conversationIDEntity: ConversationIDEntity): MessageDraftEntity? =

diff --git a/...rc/commonTest/kotlin/com/wire/kalium/persistence/dao/message/draft/MessageDraftDAOTest.kt b/...rc/commonTest/kotlin/com/wire/kalium/persistence/dao/message/draft/MessageDraftDAOTest.kt
@@ -18,6 +18,7 @@
 
 package com.wire.kalium.persistence.dao.message.draft
 
+import app.cash.turbine.test
 import com.wire.kalium.persistence.BaseDatabaseTest
 import com.wire.kalium.persistence.dao.QualifiedIDEntity
 import com.wire.kalium.persistence.dao.UserDAO
@@ -155,6 +156,47 @@ class MessageDraftDAOTest : BaseDatabaseTest() {
         assertEquals(null, result)
     }
 
+    @Test
+    fun givenSavedDraft_whenUpsertingTheSameExactDraft_thenItShouldIgnoreAndNotNotifyOtherQueries() = runTest {
+        // Given
+        insertInitialData()
+        val draft = MESSAGE_DRAFT
+        messageDraftDAO.upsertMessageDraft(draft)
+
+        messageDraftDAO.observeMessageDrafts().test {
+            val initialValue = awaitItem()
+            assertEquals(listOf(draft), initialValue)
+
+            // When
+            messageDraftDAO.upsertMessageDraft(draft) // the same exact draft is being saved again
+
+            // Then
+            expectNoEvents() // other query should not be notified
+        }
+    }
+
+    @Test
+    fun givenSavedDraft_whenUpsertingUpdatedDraft_thenItShouldBeSavedAndOtherQueriesShouldBeUpdated() = runTest {
+        // Given
+        insertInitialData()
+        val draft = MESSAGE_DRAFT
+        val updatedDraft = MESSAGE_DRAFT.copy(text = MESSAGE_DRAFT.text + " :)")
+        messageDraftDAO.upsertMessageDraft(draft)
+
+        messageDraftDAO.observeMessageDrafts().test {
+            val initialValue = awaitItem()
+            assertEquals(listOf(draft), initialValue)
+
+            // When
+            messageDraftDAO.upsertMessageDraft(updatedDraft) // the same exact draft is being saved again
+
+            // Then
+            val updatedValue = awaitItem() // other query should be notified
+            assertEquals(listOf(updatedDraft), updatedValue)
+        }
+
+    }
+
     private suspend fun insertInitialData() {
         userDAO.upsertUsers(listOf(userEntity1))
         conversationDAO.insertConversation(conversationEntity1)