SES-5135 : Chat with unknown group member - Misplaced control messages (#1844)

* Updated how threadId is fetched in insertUpdateControlMessage, default threadId to 0L for convo

* cleanup

* Added support for null threadId

* Revert conversation loader to take non-null Long

* onCreateLoader param

* Run cursor but return empty if threadId is null

* removed getThreadIdIfExistsFor wrapper, updated some function calls to use storage

Author: jbsession

app/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageProcessor.kt

@@ -153,9 +153,9 @@ class ReceivedMessageProcessor @Inject constructor(
             threadDatabase.getOrCreateThreadIdFor(threadAddress)
                 .also { context.threadIDs[threadAddress] = it }
         } else {
-            threadDatabase.getThreadIdIfExistsFor(threadAddress)
+            storage.getThreadId(threadAddress)
                 .also { id ->
-                    if (id == -1L) {
+                    if (id == null) {
                         log { "Dropping message for non-existing thread ${threadAddress.debugString}" }
                         return@withThreadLock
                     } else {
@@ -200,16 +200,18 @@ class ReceivedMessageProcessor @Inject constructor(
                     context.maxOutgoingMessageTimestamp = message.sentTimestamp!!
                 }
 
-                visibleMessageHandler.get().handleVisibleMessage(
-                    ctx = context,
-                    message = message,
-                    threadId = threadId,
-                    threadAddress = threadAddress,
-                    proto = proto,
-                    runThreadUpdate = false,
-                    runProfileUpdate = true,
-                    pro = pro,
-                )
+                threadId?.let {
+                    visibleMessageHandler.get().handleVisibleMessage(
+                        ctx = context,
+                        message = message,
+                        threadId = it,
+                        threadAddress = threadAddress,
+                        proto = proto,
+                        runThreadUpdate = false,
+                        runProfileUpdate = true,
+                        pro = pro,
+                    )
+                }
             }
 
             is CallMessage -> handleCallMessage(message)

app/src/main/java/org/thoughtcrime/securesms/configs/ConfigToDatabaseSync.kt

@@ -347,9 +347,9 @@ class ConfigToDatabaseSync @Inject constructor(
                 }
             }
 
-            val threadId = threadDatabase.getThreadIdIfExistsFor(address)
+            val threadId = storage.getThreadId(address)
 
-            if (threadId != -1L) {
+            if (threadId != null) {
                 if (conversation.lastRead > storage.getLastSeen(threadId)) {
                     storage.markConversationAsRead(
                         threadId,

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt

@@ -45,11 +45,13 @@ import androidx.core.view.isVisible
 import androidx.core.view.updateLayoutParams
 import androidx.fragment.app.DialogFragment
 import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.LiveData
 import androidx.lifecycle.Observer
 import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.flowWithLifecycle
 import androidx.lifecycle.lifecycleScope
 import androidx.lifecycle.repeatOnLifecycle
+import androidx.lifecycle.viewModelScope
 import androidx.loader.app.LoaderManager
 import androidx.loader.content.Loader
 import androidx.recyclerview.widget.LinearLayoutManager
@@ -76,6 +78,7 @@ import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapNotNull
+import kotlinx.coroutines.flow.onStart
 import kotlinx.coroutines.flow.receiveAsFlow
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
@@ -378,7 +381,8 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     private val adapter by lazy {
         val adapter = ConversationAdapter(
             this,
-            storage.getLastSeen(viewModel.threadId),
+            originalLastSeen = viewModel.threadId
+                ?.let { storage.getLastSeen(it) },
             false,
             onItemPress = { message, position, view, event ->
                 handlePress(message, position, view, event)
@@ -638,8 +642,10 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
                         try {
                             when (it) {
                                 is Long -> {
-                                    if (storage.getLastSeen(viewModel.threadId) < it) {
-                                        storage.markConversationAsRead(viewModel.threadId, it)
+                                    viewModel.threadId?.let { threadId ->
+                                        if (storage.getLastSeen(threadId) < it) {
+                                            storage.markConversationAsRead(threadId, it)
+                                        }
                                     }
                                 }
 
@@ -661,8 +667,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
                 viewModel.conversationReloadNotification
                     .collect {
                         if (!firstLoad.get()) {
-                            LoaderManager.getInstance(this@ConversationActivityV2)
-                                .restartLoader(0, null, this@ConversationActivityV2)
+                            restartConversationLoader()
                         }
                     }
             }
@@ -672,6 +677,10 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         setupUiEventsObserver()
     }
 
+    private fun restartConversationLoader() {
+        LoaderManager.getInstance(this).restartLoader(0, null, this)
+    }
+
     private fun startConversationLoaderWithDelay() {
         conversationLoadAnimationJob = lifecycleScope.launch {
             delay(700)
@@ -782,7 +791,9 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
 
     override fun onResume() {
         super.onResume()
-        messageNotifier.setVisibleThread(viewModel.threadId)
+        viewModel.threadId?.let { threadId ->
+            messageNotifier.setVisibleThread(threadId)
+        }
     }
 
     override fun onPause() {
@@ -844,11 +855,13 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
                                 )
                             }
 
-                            if (isUnread) {
-                                storage.markConversationAsRead(
-                                    viewModel.threadId,
-                                    clock.currentTimeMillis()
-                                )
+                            viewModel.threadId?.let { threadId ->
+                                if (isUnread) {
+                                    storage.markConversationAsRead(
+                                        threadId,
+                                        clock.currentTimeMillis()
+                                    )
+                                }
                             }
                         }
                     }
@@ -882,7 +895,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         val layoutManager = LinearLayoutManager(this, LinearLayoutManager.VERTICAL, false)
         binding.conversationRecyclerView.layoutManager = layoutManager
         // Workaround for the fact that CursorRecyclerViewAdapter doesn't auto-update automatically (even though it says it will)
-        LoaderManager.getInstance(this).restartLoader(0, null, this)
+        restartConversationLoader()
         binding.conversationRecyclerView.addOnScrollListener(object : RecyclerView.OnScrollListener() {
 
             override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) {
@@ -1026,19 +1039,45 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     }
 
     // called from onCreate
+    private var typistsLiveData: LiveData<TypingStatusRepository.TypingState>? = null
     private fun setUpTypingObserver() {
-        typingStatusRepository.getTypists(viewModel.threadId).observe(this) { state ->
-            val recipients = if (state != null) state.typists else listOf()
-            // FIXME: Also checking isScrolledToBottom is a quick fix for an issue where the
-            //        typing indicator overlays the recycler view when scrolled up
-            val viewContainer = binding.typingIndicatorViewContainer
-            viewContainer.isVisible = recipients.isNotEmpty() && isScrolledToBottom
-            viewContainer.setTypists(recipients)
+        // Observe typists only when we have a real threadId,
+        // and swap if it changes, example: message request was created then accepted
+        lifecycleScope.launch {
+            repeatOnLifecycle(Lifecycle.State.STARTED) {
+                viewModel.threadIdFlow
+                    .collect { threadId ->
+                        // Detach any previous observer
+                        typistsLiveData?.removeObservers(this@ConversationActivityV2)
+                        typistsLiveData = null
+
+                        if (threadId == null) {
+                            binding.typingIndicatorViewContainer.isVisible = false
+                            return@collect
+                        }
+
+                        // Attach observer for the real threadId
+                        typistsLiveData =
+                            typingStatusRepository.getTypists(threadId).also { liveData ->
+                                liveData.observe(this@ConversationActivityV2) { state ->
+                                    val recipients = state?.typists ?: emptyList()
+
+                                    // Quick-fix behavior kept as-is
+                                    val viewContainer = binding.typingIndicatorViewContainer
+                                    viewContainer.isVisible =
+                                        recipients.isNotEmpty() && isScrolledToBottom
+                                    viewContainer.setTypists(recipients)
+                                }
+                            }
+                    }
+            }
         }
         if (textSecurePreferences.isTypingIndicatorsEnabled()) {
             binding.inputBar.addTextChangedListener {
-                if(it.isNotEmpty()) {
-                    typingStatusSender.onTypingStarted(viewModel.threadId)
+                if (it.isNotEmpty()) {
+                    viewModel.threadId?.let { threadId ->
+                        typingStatusSender.onTypingStarted(threadId)
+                    }
                 }
             }
         }
@@ -1241,7 +1280,10 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
             return
         }
 
-        val lastSeenTimestamp = threadDb.getLastSeenAndHasSent(viewModel.threadId).first()
+        val threadId = viewModel.threadId
+        if (threadId == null) return // Maybe don't scroll
+
+        val lastSeenTimestamp = threadDb.getLastSeenAndHasSent(threadId).first()
         val lastSeenItemPosition = adapter.findLastSeenItemPosition(lastSeenTimestamp) ?: return
 
         binding.conversationRecyclerView.runWhenLaidOut {
@@ -1250,7 +1292,6 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
                 ((layoutManager?.height ?: 0) / 2)
             )
         }
-
     }
 
     private fun highlightViewAtPosition(position: Int) {
@@ -1847,7 +1888,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
                 messageSender.send(reactionMessage, recipient.address)
             }
 
-            LoaderManager.getInstance(this).restartLoader(0, null, this)
+            restartConversationLoader()
         }
     }
 
@@ -1903,7 +1944,8 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
             } else {
                 messageSender.send(message, recipient.address)
             }
-            LoaderManager.getInstance(this).restartLoader(0, null, this)
+
+            restartConversationLoader()
         }
     }
 
@@ -2197,8 +2239,8 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
             waitForApprovalJobToBeSubmitted()
             messageSender.send(message, recipient.address)
         }
-        // Send a typing stopped message
-        typingStatusSender.onTypingStopped(viewModel.threadId)
+
+        stopTyping()
         return Pair(recipient.address, sentTimestamp)
     }
 
@@ -2294,11 +2336,17 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
             }
         }
 
-        // Send a typing stopped message
-        typingStatusSender.onTypingStopped(viewModel.threadId)
+        stopTyping()
         return Pair(recipient.address, sentTimestamp)
     }
 
+    private fun stopTyping(){
+        // Send a typing stopped message
+        viewModel.threadId?.let { threadId ->
+            typingStatusSender.onTypingStopped(threadId)
+        }
+    }
+
     private fun showGIFPicker() {
         val hasSeenGIFMetaDataWarning: Boolean = textSecurePreferences.hasSeenGIFMetaDataWarning()
         if (!hasSeenGIFMetaDataWarning) {
@@ -2849,9 +2897,11 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     }
 
     fun onSearchQueryUpdated(query: String) {
-        binding.searchBottomBar.showLoading()
-        searchViewModel.onQueryUpdated(query, viewModel.threadId)
-        adapter.onSearchQueryUpdated(query.takeUnless { it.length < 2 })
+        viewModel.threadId?.let {  threadId ->
+            binding.searchBottomBar.showLoading()
+            searchViewModel.onQueryUpdated(query, threadId)
+            adapter.onSearchQueryUpdated(query.takeUnless { it.length < 2 })
+        }
     }
 
     override fun onSearchMoveUpPressed() {
@@ -2863,9 +2913,11 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     }
 
     private fun jumpToMessage(author: Address, timestamp: Long, highlight: Boolean, onMessageNotFound: Runnable?) {
-        SimpleTask.run(lifecycle, {
-            mmsSmsDb.getMessagePositionInConversation(viewModel.threadId, timestamp, author, false)
-        }) { p: Int -> moveToMessagePosition(p, highlight, onMessageNotFound) }
+        viewModel.threadId?.let { threadId ->
+            SimpleTask.run(lifecycle, {
+                mmsSmsDb.getMessagePositionInConversation(threadId, timestamp, author, false)
+            }) { p: Int -> moveToMessagePosition(p, highlight, onMessageNotFound) }
+        }
     }
 
     private fun moveToMessagePosition(position: Int, highlight: Boolean, onMessageNotFound: Runnable?) {

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapter.kt

@@ -19,11 +19,12 @@ import org.thoughtcrime.securesms.database.MmsSmsDatabase
 import org.thoughtcrime.securesms.database.model.MessageId
 import org.thoughtcrime.securesms.database.model.MessageRecord
 import java.util.concurrent.atomic.AtomicLong
+import java.util.concurrent.atomic.AtomicReference
 import kotlin.math.min
 
 class ConversationAdapter(
     context: Context,
-    originalLastSeen: Long,
+    originalLastSeen: Long?,
     private val isReversed: Boolean,
     private val onItemPress: (MessageRecord, Int, VisibleMessageView, MotionEvent) -> Unit,
     private val onItemSwipeToReply: (MessageRecord, Int) -> Unit,
@@ -42,7 +43,7 @@ class ConversationAdapter(
     private var searchQuery: String? = null
     var visibleMessageViewDelegate: VisibleMessageViewDelegate? = null
 
-    private val lastSeen = AtomicLong(originalLastSeen)
+    private val lastSeen :  AtomicReference<Long?> = AtomicReference(originalLastSeen)
 
     var lastSentMessageId: MessageId? = null
         set(value) {

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationLoader.kt

@@ -3,6 +3,7 @@ package org.thoughtcrime.securesms.conversation.v2
 import android.app.Application
 import android.database.ContentObserver
 import android.database.Cursor
+import android.database.MatrixCursor
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
@@ -11,16 +12,22 @@ import org.thoughtcrime.securesms.database.Storage
 import org.thoughtcrime.securesms.util.AbstractCursorLoader
 
 class ConversationLoader @AssistedInject constructor(
-    @Assisted private val threadID: Long,
+    @Assisted private val threadID: Long?,
     @Assisted private val reverse: Boolean,
     application: Application,
     private val mmsSmsDatabase: MmsSmsDatabase,
 ) : AbstractCursorLoader<ConversationLoader.Data>(application) {
 
     override fun getData(): Data {
+        // Return an empty cursor
+        val id = threadID ?: return Data(
+            messageCursor = MatrixCursor(emptyArray<String>()),
+            threadUnreadCount = 0
+        )
+
         return Data(
-            messageCursor = mmsSmsDatabase.getConversation(threadID, reverse),
-            threadUnreadCount = mmsSmsDatabase.getUnreadCount(threadID),
+            messageCursor = mmsSmsDatabase.getConversation(id, reverse),
+            threadUnreadCount = mmsSmsDatabase.getUnreadCount(id),
         )
     }
 
@@ -37,6 +44,6 @@ class ConversationLoader @AssistedInject constructor(
 
     @AssistedFactory
     interface Factory {
-        fun create(threadID: Long, reverse: Boolean): ConversationLoader
+        fun create(threadID: Long?, reverse: Boolean): ConversationLoader
     }
 }