Merge pull request #1729 from Infomaniak/optimize-threadState

Use a class to store Thread state

Author: KevinBoulongne

app/src/main/java/com/infomaniak/mail/ui/main/thread/PrintMailFragment.kt

@@ -65,15 +65,16 @@ class PrintMailFragment : Fragment() {
         threadViewModel.reassignMessagesLiveWithoutSuperCollapsedBlock(navigationArgs.messageUid)
     }
 
-    private fun setupAdapter() {
+    private fun setupAdapter() = with(threadViewModel) {
         binding.messagesList.adapter = ThreadAdapter(
             shouldLoadDistantResources = true,
             isForPrinting = true,
             threadAdapterState = object : ThreadAdapterState {
-                override var isExpandedMap by threadViewModel::isExpandedMap
-                override var isThemeTheSameMap by threadViewModel::isThemeTheSameMap
-                override var hasSuperCollapsedBlockBeenClicked by threadViewModel::hasSuperCollapsedBlockBeenClicked
-                override var verticalScroll by threadViewModel::verticalScroll
+                override val isExpandedMap by threadState::isExpandedMap
+                override val isThemeTheSameMap by threadState::isThemeTheSameMap
+                override val hasSuperCollapsedBlockBeenClicked by threadState::hasSuperCollapsedBlockBeenClicked
+                override val verticalScroll by threadState::verticalScroll
+                override val isCalendarEventExpandedMap by threadState::isCalendarEventExpandedMap
             },
             threadAdapterCallbacks = ThreadAdapterCallbacks(
                 onBodyWebViewFinishedLoading = { startPrintingView() },

app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadAdapter.kt

@@ -70,7 +70,6 @@ import com.google.android.material.R as RMaterial
 class ThreadAdapter(
     private val shouldLoadDistantResources: Boolean,
     private val isForPrinting: Boolean = false,
-    private val isCalendarEventExpandedMap: MutableMap<String, Boolean> = mutableMapOf(),
     private val threadAdapterState: ThreadAdapterState,
     private var threadAdapterCallbacks: ThreadAdapterCallbacks? = null,
 ) : ListAdapter<Any, ThreadAdapterViewHolder>(MessageDiffCallback()) {
@@ -220,7 +219,7 @@ class ThreadAdapter(
                     shouldDisplayReplyOptions = calendarEventResponse.isReplyAuthorized(),
                     attachment = calendarAttachment,
                     hasAssociatedInfomaniakCalendarEvent = calendarEventResponse.hasAssociatedInfomaniakCalendarEvent(),
-                    shouldStartExpanded = isCalendarEventExpandedMap[message.uid] ?: false,
+                    shouldStartExpanded = threadAdapterState.isCalendarEventExpandedMap[message.uid] ?: false,
                 )
             }
 
@@ -234,7 +233,9 @@ class ThreadAdapter(
                 replyToCalendarEvent = { attendanceState ->
                     threadAdapterCallbacks?.replyToCalendarEvent?.invoke(attendanceState, message)
                 },
-                onAttendeesButtonClicked = { isExpanded -> isCalendarEventExpandedMap[message.uid] = isExpanded },
+                onAttendeesButtonClicked = { isExpanded ->
+                    threadAdapterState.isCalendarEventExpandedMap[message.uid] = isExpanded
+                },
             )
         }
     }

app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadAdapterState.kt

@@ -153,7 +153,7 @@ class ThreadFragment : Fragment() {
     }
 
     override fun onStop() {
-        threadViewModel.verticalScroll = binding.messagesListNestedScrollView.scrollY
+        threadViewModel.threadState.verticalScroll = binding.messagesListNestedScrollView.scrollY
         super.onStop()
     }
 
@@ -163,11 +163,8 @@ class ThreadFragment : Fragment() {
         _binding = null
     }
 
-    fun resetThreadState() = with(threadViewModel) {
-        isExpandedMap = mutableMapOf()
-        isThemeTheSameMap = mutableMapOf()
-        hasSuperCollapsedBlockBeenClicked = false
-        verticalScroll = null
+    fun resetThreadState() {
+        threadViewModel.threadState.reset()
     }
 
     private fun setupUi() = with(binding) {
@@ -212,12 +209,12 @@ class ThreadFragment : Fragment() {
 
         binding.messagesList.adapter = ThreadAdapter(
             shouldLoadDistantResources = shouldLoadDistantResources(),
-            isCalendarEventExpandedMap = isCalendarEventExpandedMap,
             threadAdapterState = object : ThreadAdapterState {
-                override var isExpandedMap by threadViewModel::isExpandedMap
-                override var isThemeTheSameMap by threadViewModel::isThemeTheSameMap
-                override var hasSuperCollapsedBlockBeenClicked by threadViewModel::hasSuperCollapsedBlockBeenClicked
-                override var verticalScroll by threadViewModel::verticalScroll
+                override val isExpandedMap by threadState::isExpandedMap
+                override val isThemeTheSameMap by threadState::isThemeTheSameMap
+                override val hasSuperCollapsedBlockBeenClicked by threadState::hasSuperCollapsedBlockBeenClicked
+                override val verticalScroll by threadState::verticalScroll
+                override val isCalendarEventExpandedMap by threadState::isCalendarEventExpandedMap
             },
             threadAdapterCallbacks = ThreadAdapterCallbacks(
                 onContactClicked = {
@@ -332,7 +329,6 @@ class ThreadFragment : Fragment() {
 
         twoPaneViewModel.currentThreadUid.distinctUntilChanged().observeNotNull(viewLifecycleOwner) { threadUid ->
 
-            resetMessagesRelatedCache()
             displayThreadView()
 
             openThread(threadUid).observe(viewLifecycleOwner) { thread ->
@@ -382,8 +378,8 @@ class ThreadFragment : Fragment() {
         messagesLive.observe(viewLifecycleOwner) { (items, messagesToFetch) ->
             SentryLog.i("UI", "Received ${items.count()} messages")
 
-            if (shouldMarkThreadAsSeen) {
-                shouldMarkThreadAsSeen = false
+            if (threadState.shouldMarkThreadAsSeen) {
+                threadState.shouldMarkThreadAsSeen = false
                 markThreadAsSeen()
             }
 
@@ -565,7 +561,7 @@ class ThreadFragment : Fragment() {
             args = MessageActionsBottomSheetDialogArgs(
                 messageUid = uid,
                 threadUid = twoPaneViewModel.currentThreadUid.value ?: return,
-                isThemeTheSame = threadViewModel.isThemeTheSameMap[uid] ?: return,
+                isThemeTheSame = threadViewModel.threadState.isThemeTheSameMap[uid] ?: return,
                 shouldLoadDistantResources = shouldLoadDistantResources(uid),
             ).toBundle(),
         )
@@ -575,9 +571,9 @@ class ThreadFragment : Fragment() {
 
         fun getBottomY(): Int = binding.messagesListNestedScrollView.maxScrollAmount
 
-        val scrollY = verticalScroll ?: run {
+        val scrollY = threadState.verticalScroll ?: run {
 
-            val indexToScroll = threadAdapter.items.indexOfFirst { it is Message && isExpandedMap[it.uid] == true }
+            val indexToScroll = threadAdapter.items.indexOfFirst { it is Message && threadState.isExpandedMap[it.uid] == true }
 
             // If no Message is expanded (e.g. the last Message of the Thread is a Draft),
             // we want to automatically scroll to the very bottom.
@@ -590,7 +586,7 @@ class ThreadFragment : Fragment() {
                         scope.level = SentryLevel.ERROR
                         scope.setExtra("indexToScroll", indexToScroll.toString())
                         scope.setExtra("messageCount", threadAdapter.items.count().toString())
-                        scope.setExtra("isExpandedMap", isExpandedMap.toString())
+                        scope.setExtra("isExpandedMap", threadState.isExpandedMap.toString())
                         scope.setExtra("isLastMessageDraft", (threadAdapter.items.lastOrNull() as Message?)?.isDraft.toString())
                         Sentry.captureMessage("Target child for scroll in ThreadFragment is null. Fallback to scrolling to bottom")
                     }
@@ -605,7 +601,7 @@ class ThreadFragment : Fragment() {
     }
 
     private fun expandSuperCollapsedBlock() = with(threadViewModel) {
-        hasSuperCollapsedBlockBeenClicked = true
+        threadState.hasSuperCollapsedBlockBeenClicked = true
         reassignMessagesLive(twoPaneViewModel.currentThreadUid.value!!)
     }
 

app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadFragment.kt

@@ -0,0 +1,54 @@
+/*
+ * Infomaniak Mail - Android
+ * Copyright (C) 2024 Infomaniak Network SA
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package com.infomaniak.mail.ui.main.thread
+
+import com.infomaniak.mail.ui.main.thread.ThreadAdapter.SuperCollapsedBlock
+import com.infomaniak.mail.utils.MessageBodyUtils.SplitBody
+
+interface ThreadAdapterState {
+    val isExpandedMap: MutableMap<String, Boolean>
+    val isThemeTheSameMap: MutableMap<String, Boolean>
+    val hasSuperCollapsedBlockBeenClicked: Boolean
+    val verticalScroll: Int?
+    val isCalendarEventExpandedMap: MutableMap<String, Boolean>
+}
+
+class ThreadState {
+
+    val isExpandedMap: MutableMap<String, Boolean> = mutableMapOf()
+    val isThemeTheSameMap: MutableMap<String, Boolean> = mutableMapOf()
+    var hasSuperCollapsedBlockBeenClicked: Boolean = false
+    var verticalScroll: Int? = null
+    val isCalendarEventExpandedMap: MutableMap<String, Boolean> = mutableMapOf()
+    val treatedMessagesForCalendarEvent: MutableSet<String> = mutableSetOf()
+    val cachedSplitBodies: MutableMap<String, SplitBody> = mutableMapOf()
+    var shouldMarkThreadAsSeen: Boolean = false
+    var superCollapsedBlock: SuperCollapsedBlock? = null
+
+    fun reset() {
+        isExpandedMap.clear()
+        isThemeTheSameMap.clear()
+        hasSuperCollapsedBlockBeenClicked = false
+        verticalScroll = null
+        isCalendarEventExpandedMap.clear()
+        treatedMessagesForCalendarEvent.clear()
+        cachedSplitBodies.clear()
+        shouldMarkThreadAsSeen = false
+        superCollapsedBlock = null
+    }
+}

app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadState.kt

@@ -36,7 +36,6 @@ import com.infomaniak.mail.data.models.thread.Thread
 import com.infomaniak.mail.di.IoDispatcher
 import com.infomaniak.mail.ui.main.thread.ThreadAdapter.SuperCollapsedBlock
 import com.infomaniak.mail.utils.*
-import com.infomaniak.mail.utils.MessageBodyUtils.SplitBody
 import com.infomaniak.mail.utils.extensions.MergedContactDictionary
 import com.infomaniak.mail.utils.extensions.context
 import com.infomaniak.mail.utils.extensions.getUids
@@ -74,31 +73,15 @@ class ThreadViewModel @Inject constructor(
     private var fetchMessagesJob: Job? = null
     private var fetchCalendarEventJob: Job? = null
 
-    val quickActionBarClicks = SingleLiveEvent<QuickActionBarResult>()
-
-    //region Calendar Events
-    private val treatedMessagesForCalendarEvent = mutableSetOf<String>()
-    val isCalendarEventExpandedMap = mutableMapOf<String, Boolean>()
-    //endregion
-
-    var deletedMessagesUids = mutableSetOf<String>()
-    val failedMessagesUids = SingleLiveEvent<List<String>>()
-
     val threadLive = MutableLiveData<Thread?>()
     val messagesLive = MutableLiveData<Pair<ThreadAdapterItems, MessagesWithoutHeavyData>>()
 
-    private val cachedSplitBodies = mutableMapOf<String, SplitBody>()
-
-    var shouldMarkThreadAsSeen: Boolean = false
+    val quickActionBarClicks = SingleLiveEvent<QuickActionBarResult>()
 
-    private var superCollapsedBlock: SuperCollapsedBlock? = null
+    val failedMessagesUids = SingleLiveEvent<List<String>>()
+    var deletedMessagesUids = mutableSetOf<String>()
 
-    //region Restore Thread state after going to MoveFragment or somewhere else, and then coming back to ThreadFragment.
-    var isExpandedMap: MutableMap<String, Boolean> = mutableMapOf()
-    var isThemeTheSameMap: MutableMap<String, Boolean> = mutableMapOf()
-    var hasSuperCollapsedBlockBeenClicked: Boolean = false
-    var verticalScroll: Int? = null
-    //endregion
+    val threadState = ThreadState()
 
     private val mailbox by lazy { mailboxController.getMailbox(AccountUtils.currentUserId, AccountUtils.currentMailboxId)!! }
 
@@ -107,14 +90,6 @@ class ThreadViewModel @Inject constructor(
         AccountUtils.currentMailboxId,
     ).map { it.obj }.asLiveData(ioCoroutineContext)
 
-    fun resetMessagesRelatedCache() {
-        treatedMessagesForCalendarEvent.clear()
-        isCalendarEventExpandedMap.clear()
-        cachedSplitBodies.clear()
-        shouldMarkThreadAsSeen = false
-        superCollapsedBlock = null
-    }
-
     fun reassignThreadLive(threadUid: String) {
         threadLiveJob?.cancel()
         threadLiveJob = viewModelScope.launch(ioCoroutineContext) {
@@ -144,7 +119,7 @@ class ThreadViewModel @Inject constructor(
     private suspend fun mapRealmMessagesResult(
         messages: RealmResults<Message>,
         threadUid: String,
-    ): Pair<ThreadAdapterItems, MessagesWithoutHeavyData> {
+    ): Pair<ThreadAdapterItems, MessagesWithoutHeavyData> = with(threadState) {
 
         superCollapsedBlock = superCollapsedBlock ?: SuperCollapsedBlock()
 
@@ -212,7 +187,7 @@ class ThreadViewModel @Inject constructor(
      * - If there's any unread Message in between, it will be displayed (hence, all following Messages will be displayed too).
      * After all these Messages are displayed, if there's at least 2 remaining Messages, they're gonna be collapsed in the Block.
      */
-    private fun shouldBlockBeDisplayed(messagesCount: Int, firstIndexAfterBlock: Int): Boolean {
+    private fun shouldBlockBeDisplayed(messagesCount: Int, firstIndexAfterBlock: Int): Boolean = with(threadState) {
         return superCollapsedBlock?.shouldBeDisplayed == true && // If the Block was hidden for any reason, we mustn't ever display it again
                 !hasSuperCollapsedBlockBeenClicked && // Block hasn't been expanded by the user
                 messagesCount >= SUPER_COLLAPSED_BLOCK_MINIMUM_MESSAGES_LIMIT && // At least 5 Messages in the Thread
@@ -223,7 +198,7 @@ class ThreadViewModel @Inject constructor(
     private suspend fun formatLists(
         messages: List<Message>,
         computeBehavior: (Int, String) -> MessageBehavior,
-    ): Pair<MutableList<Any>, MutableList<Message>> {
+    ): Pair<MutableList<Any>, MutableList<Message>> = with(threadState) {
 
         val items = mutableListOf<Any>()
         val messagesToFetch = mutableListOf<Message>()
@@ -237,12 +212,8 @@ class ThreadViewModel @Inject constructor(
 
         messages.forEachIndexed { index, message ->
             when (computeBehavior(index, message.uid)) {
-                MessageBehavior.DISPLAYED -> {
-                    addMessage(message)
-                }
-                MessageBehavior.COLLAPSED -> {
-                    superCollapsedBlock!!.messagesUids.add(message.uid)
-                }
+                MessageBehavior.DISPLAYED -> addMessage(message)
+                MessageBehavior.COLLAPSED -> superCollapsedBlock!!.messagesUids.add(message.uid)
                 MessageBehavior.FIRST_AFTER_BLOCK -> {
                     items += superCollapsedBlock!!
                     addMessage(message.apply { shouldHideDivider = true })
@@ -258,9 +229,9 @@ class ThreadViewModel @Inject constructor(
 
         message.apply {
             body?.let {
-                val isNotAlreadySplit = !cachedSplitBodies.contains(message.uid)
-                if (isNotAlreadySplit) cachedSplitBodies[message.uid] = MessageBodyUtils.splitContentAndQuote(it)
-                splitBody = cachedSplitBodies[message.uid]
+                val isNotAlreadySplit = !threadState.cachedSplitBodies.contains(message.uid)
+                if (isNotAlreadySplit) threadState.cachedSplitBodies[message.uid] = MessageBodyUtils.splitContentAndQuote(it)
+                splitBody = threadState.cachedSplitBodies[message.uid]
             }
         }
 
@@ -277,14 +248,14 @@ class ThreadViewModel @Inject constructor(
         sendMatomoAndSentryAboutThreadMessagesCount(thread)
 
         // These 2 will always be empty or not all together at the same time.
-        if (isExpandedMap.isEmpty() || isThemeTheSameMap.isEmpty()) {
+        if (threadState.isExpandedMap.isEmpty() || threadState.isThemeTheSameMap.isEmpty()) {
             thread.messages.forEachIndexed { index, message ->
-                isExpandedMap[message.uid] = message.shouldBeExpanded(index, thread.messages.lastIndex)
-                isThemeTheSameMap[message.uid] = true
+                threadState.isExpandedMap[message.uid] = message.shouldBeExpanded(index, thread.messages.lastIndex)
+                threadState.isThemeTheSameMap[message.uid] = true
             }
         }
 
-        shouldMarkThreadAsSeen = thread.unseenMessagesCount > 0
+        threadState.shouldMarkThreadAsSeen = thread.unseenMessagesCount > 0
 
         emit(thread)
     }
@@ -382,7 +353,7 @@ class ThreadViewModel @Inject constructor(
         if (!message.isFullyDownloaded()) return // Only treat message that have their Attachments downloaded
 
         if (!forceFetch) {
-            val alreadyTreated = !treatedMessagesForCalendarEvent.add(message.uid)
+            val alreadyTreated = !threadState.treatedMessagesForCalendarEvent.add(message.uid)
             if (alreadyTreated) return
         }