refactor: remove the fetch of self user when only the ID is used

wireapp · Dec 23, 2024 · f647aef · f647aef
1 parent 3c7855a
commit f647aef
Show file tree

Hide file tree

Showing 15 changed files with 65 additions and 55 deletions.
diff --git a/logic/src/appleMain/kotlin/com/wire/kalium/logic/feature/call/GlobalCallManager.kt b/logic/src/appleMain/kotlin/com/wire/kalium/logic/feature/call/GlobalCallManager.kt
@@ -44,7 +44,6 @@ actual class GlobalCallManager {
     internal actual fun getCallManagerForClient(
         userId: QualifiedID,
         callRepository: CallRepository,
-        userRepository: UserRepository,
         currentClientIdProvider: CurrentClientIdProvider,
         selfConversationIdProvider: SelfConversationIdProvider,
         conversationRepository: ConversationRepository,

diff --git a/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt b/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt
@@ -104,7 +104,6 @@ import java.util.Collections
 class CallManagerImpl internal constructor(
     private val calling: Calling,
     private val callRepository: CallRepository,
-    private val userRepository: UserRepository,
     private val currentClientIdProvider: CurrentClientIdProvider,
     selfConversationIdProvider: SelfConversationIdProvider,
     private val conversationRepository: ConversationRepository,
@@ -121,6 +120,7 @@ class CallManagerImpl internal constructor(
     private val mediaManagerService: MediaManagerService,
     private val flowManagerService: FlowManagerService,
     private val createAndPersistRecentlyEndedCallMetadata: CreateAndPersistRecentlyEndedCallMetadataUseCase,
+    private val selfUserId: UserId,
     private val json: Json = Json { ignoreUnknownKeys = true },
     private val shouldRemoteMuteChecker: ShouldRemoteMuteChecker = ShouldRemoteMuteCheckerImpl(),
     private val serverTimeHandler: ServerTimeHandler = ServerTimeHandlerImpl(),
@@ -153,11 +153,6 @@ class CallManagerImpl internal constructor(
             it
         })
     }
-    private val userId: Deferred<UserId> = scope.async(start = CoroutineStart.LAZY) {
-        userRepository.observeSelfUser().first().id.also {
-            callingLogger.d("$TAG - userId ${it.toLogString()}")
-        }
-    }
 
     @Suppress("UNUSED_ANONYMOUS_PARAMETER")
     private val metricsHandler = MetricsHandler { conversationId: String, metricsJson: String, arg: Pointer? ->
@@ -186,7 +181,7 @@ class CallManagerImpl internal constructor(
             }
             joinAll(flowManagerStartJob, mediaManagerStartJob)
             callingLogger.i("$TAG: Creating Handle")
-            val selfUserId = federatedIdMapper.parseToFederatedId(userId.await())
+            val selfUserId = federatedIdMapper.parseToFederatedId(selfUserId)
             val selfClientId = clientId.await().value
 
             val waitInitializationJob = Job()
@@ -254,7 +249,7 @@ class CallManagerImpl internal constructor(
         val conversationMembers = conversationRepository.observeConversationMembers(message.conversationId).first()
         val shouldRemoteMute = shouldRemoteMuteChecker.check(
             senderUserId = message.senderUserId,
-            selfUserId = userId.await(),
+            selfUserId = selfUserId,
             selfClientId = clientId.await().value,
             targets = callingValue.targets,
             conversationMembers = conversationMembers
@@ -306,7 +301,7 @@ class CallManagerImpl internal constructor(
             isMuted = false,
             isCameraOn = isCameraOn,
             isCbrEnabled = isAudioCbr,
-            callerId = userId.await()
+            callerId = selfUserId
         )
 
         withCalling {
@@ -401,7 +396,7 @@ class CallManagerImpl internal constructor(
         }
         callingLogger.d("$TAG -> set test video to $logString")
 
-        val selfUserId = federatedIdMapper.parseToFederatedId(userId.await())
+        val selfUserId = federatedIdMapper.parseToFederatedId(selfUserId)
         val selfClientId = clientId.await().value
         val handle = deferredHandle.await()
 

diff --git a/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/GlobalCallManager.kt b/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/GlobalCallManager.kt
@@ -82,7 +82,6 @@ actual class GlobalCallManager(
     internal actual fun getCallManagerForClient(
         userId: QualifiedID,
         callRepository: CallRepository,
-        userRepository: UserRepository,
         currentClientIdProvider: CurrentClientIdProvider,
         selfConversationIdProvider: SelfConversationIdProvider,
         conversationRepository: ConversationRepository,
@@ -103,7 +102,7 @@ actual class GlobalCallManager(
                 CallManagerImpl(
                     calling = calling,
                     callRepository = callRepository,
-                    userRepository = userRepository,
+                    selfUserId = userId,
                     currentClientIdProvider = currentClientIdProvider,
                     selfConversationIdProvider = selfConversationIdProvider,
                     callMapper = callMapper,

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallingParticipantsOrder.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallingParticipantsOrder.kt
@@ -18,27 +18,26 @@
 
 package com.wire.kalium.logic.data.call
 
-import com.wire.kalium.logic.data.user.UserRepository
 import com.wire.kalium.logic.data.id.CurrentClientIdProvider
+import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.functional.fold
 
 internal interface CallingParticipantsOrder {
     suspend fun reorderItems(participants: List<Participant>): List<Participant>
 }
 
 internal class CallingParticipantsOrderImpl(
-    private val userRepository: UserRepository,
     private val currentClientIdProvider: CurrentClientIdProvider,
     private val participantsFilter: ParticipantsFilter,
     private val participantsOrderByName: ParticipantsOrderByName,
+    private val selfUserId: UserId
 ) : CallingParticipantsOrder {
 
     override suspend fun reorderItems(participants: List<Participant>): List<Participant> {
         return if (participants.isNotEmpty()) {
             currentClientIdProvider().fold({
                 participants
             }, { selfClientId ->
-                val selfUserId = userRepository.getSelfUser()?.id!!
 
                 val selfParticipant = participantsFilter.selfParticipant(participants, selfUserId, selfClientId.value)
                 val otherParticipants = participantsFilter.otherParticipants(participants, selfClientId.value)

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt
@@ -235,6 +235,7 @@ internal class UserDataSource internal constructor(
                 userEntity?.let { userMapper.fromUserDetailsEntityToOtherUser(userEntity) }
             }.onEach { otherUser ->
                 if (otherUser != null) {
+//                     TODO: reuse TCP connection aka update in parallel
                     refreshUserDetailsIfNeeded(userId)
                 }
             }
@@ -414,6 +415,7 @@ internal class UserDataSource internal constructor(
         else fetchUsersByIds(missingIds.map { it.toModel() }.toSet()).map { }
     }
 
+    // TODO: this can cause many issues since it will
     @OptIn(ExperimentalCoroutinesApi::class)
     override suspend fun observeSelfUser(): Flow<SelfUser> {
         return metadataDAO.valueByKeyFlow(SELF_USER_ID_KEY).onEach {
@@ -437,6 +439,28 @@ internal class UserDataSource internal constructor(
                 .map(userMapper::fromUserDetailsEntityToSelfUser)
         }
     }
+//         return metadataDAO.valueByKeyFlow(SELF_USER_ID_KEY).onEach {
+//             // If the self user is not in the database, proactively fetch it.
+//             if (it == null) {
+//                 val logPrefix = "Observing self user before insertion"
+//                 kaliumLogger.w("cccc: $logPrefix: Triggering a fetch.")
+//                 fetchSelfUser().fold({ failure ->
+//                     kaliumLogger.e("""$logPrefix failed: {"failure":"$failure"}""")
+//                 }, {
+//                     kaliumLogger.i("$logPrefix: Succeeded")
+//                     userDetailsRefreshInstantCache[selfUserId] = DateTimeUtil.currentInstant()
+//                 })
+//             } else {
+//                 kaliumLogger.d("cccc: Self user found in metadata")
+//                 refreshUserDetailsIfNeeded(selfUserId)
+//             }
+//         }.filterNotNull().flatMapMerge { encodedValue ->
+//             val selfUserID: QualifiedIDEntity = Json.decodeFromString(encodedValue)
+//             userDAO.observeUserDetailsByQualifiedID(selfUserID)
+//                 .filterNotNull()
+//                 .map(userMapper::fromUserDetailsEntityToSelfUser)
+//         }
+//     }
 
     @OptIn(ExperimentalCoroutinesApi::class)
     override suspend fun observeSelfUserWithTeam(): Flow<Pair<SelfUser, Team?>> {
@@ -467,8 +491,10 @@ internal class UserDataSource internal constructor(
     }
 
     // TODO: replace the flow with selfUser and cache it
-    override suspend fun getSelfUser(): SelfUser? =
-        observeSelfUser().firstOrNull()
+    override suspend fun getSelfUser(): SelfUser? {
+        kaliumLogger.d("cccc: Getting self user")
+        return observeSelfUser().firstOrNull()
+    }
 
     override suspend fun observeAllKnownUsers(): Flow<Either<StorageFailure, List<OtherUser>>> {
         val selfUserId = selfUserId.toDao()
@@ -656,7 +682,7 @@ internal class UserDataSource internal constructor(
             CreateUserTeam(dto.teamId, dto.teamName)
         }
             .onSuccess {
-                kaliumLogger.d("Migrated user to team")
+                kaliumLogger.d("cccc: Migrated user to team")
                 fetchSelfUser()
             }
             .onFailure { failure ->

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
@@ -1264,7 +1264,6 @@ class UserSessionScope internal constructor(
         globalCallManager.getCallManagerForClient(
             userId = userId,
             callRepository = callRepository,
-            userRepository = userRepository,
             currentClientIdProvider = clientIdProvider,
             conversationRepository = conversationRepository,
             userConfigRepository = userConfigRepository,
@@ -2060,7 +2059,6 @@ class UserSessionScope internal constructor(
             callManager = callManager,
             callRepository = callRepository,
             conversationRepository = conversationRepository,
-            userRepository = userRepository,
             flowManagerService = flowManagerService,
             mediaManagerService = mediaManagerService,
             syncManager = syncManager,
@@ -2071,6 +2069,8 @@ class UserSessionScope internal constructor(
             conversationClientsInCallUpdater = conversationClientsInCallUpdater,
             kaliumConfigs = kaliumConfigs,
             inCallReactionsRepository = inCallReactionsRepository,
+            selfUserId = userId,
+            userRepository = userRepository
         )
 
     val connection: ConnectionScope

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/CallsScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/CallsScope.kt
@@ -28,6 +28,7 @@ import com.wire.kalium.logic.data.call.ParticipantsOrderByNameImpl
 import com.wire.kalium.logic.data.conversation.ConversationRepository
 import com.wire.kalium.logic.data.id.CurrentClientIdProvider
 import com.wire.kalium.logic.data.id.QualifiedIdMapper
+import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.data.user.UserRepository
 import com.wire.kalium.logic.feature.call.usecase.AnswerCallUseCase
 import com.wire.kalium.logic.feature.call.usecase.AnswerCallUseCaseImpl
@@ -107,6 +108,7 @@ class CallsScope internal constructor(
     private val getCallConversationType: GetCallConversationTypeProvider,
     private val kaliumConfigs: KaliumConfigs,
     private val inCallReactionsRepository: InCallReactionsRepository,
+    private val selfUserId: UserId,
     internal val dispatcher: KaliumDispatcher = KaliumDispatcherImpl
 ) {
 
@@ -122,7 +124,7 @@ class CallsScope internal constructor(
         get() = GetIncomingCallsUseCaseImpl(
             callRepository = callRepository,
             conversationRepository = conversationRepository,
-            userRepository = userRepository
+            userRepository = userRepository,
         )
     val observeOutgoingCall: ObserveOutgoingCallUseCase
         get() = ObserveOutgoingCallUseCaseImpl(
@@ -211,10 +213,10 @@ class CallsScope internal constructor(
 
     private val callingParticipantsOrder: CallingParticipantsOrder
         get() = CallingParticipantsOrderImpl(
-            userRepository = userRepository,
             currentClientIdProvider = currentClientIdProvider,
             participantsFilter = ParticipantsFilterImpl(qualifiedIdMapper),
-            participantsOrderByName = ParticipantsOrderByNameImpl()
+            participantsOrderByName = ParticipantsOrderByNameImpl(),
+            selfUserId = selfUserId
         )
 
     val isLastCallClosed: IsLastCallClosedUseCase get() = IsLastCallClosedUseCaseImpl(callRepository)

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/GlobalCallManager.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/GlobalCallManager.kt
@@ -45,7 +45,6 @@ expect class GlobalCallManager {
     internal fun getCallManagerForClient(
         userId: QualifiedID,
         callRepository: CallRepository,
-        userRepository: UserRepository,
         currentClientIdProvider: CurrentClientIdProvider,
         selfConversationIdProvider: SelfConversationIdProvider,
         conversationRepository: ConversationRepository,

diff --git a/...c/commonMain/kotlin/com/wire/kalium/logic/feature/call/usecase/GetIncomingCallsUseCase.kt b/...c/commonMain/kotlin/com/wire/kalium/logic/feature/call/usecase/GetIncomingCallsUseCase.kt
@@ -70,9 +70,9 @@ internal class GetIncomingCallsUseCaseImpl internal constructor(
 
     @OptIn(ExperimentalCoroutinesApi::class)
     private suspend fun observeIncomingCallsIfUserStatusAllows(): Flow<List<Call>> =
+        // TODO: do not refresh self form remote in this case and just get status from local
         userRepository.observeSelfUser()
             .flatMapLatest {
-
                 // if user is AWAY we don't show any IncomingCalls
                 if (it.availabilityStatus == UserAvailabilityStatus.AWAY) {
                     logger.d("$TAG; Ignoring possible calls based user's status")

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/ConversationScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/ConversationScope.kt
@@ -137,7 +137,7 @@ class ConversationScope internal constructor(
         get() = ObserveConversationMembersUseCaseImpl(conversationRepository, userRepository)
 
     val getMembersToMention: MembersToMentionUseCase
-        get() = MembersToMentionUseCase(observeConversationMembers, userRepository)
+        get() = MembersToMentionUseCase(observeConversationMembers = observeConversationMembers, selfUserId = selfUserId)
 
     val observeUserListById: ObserveUserListByIdUseCase
         get() = ObserveUserListByIdUseCase(userRepository)

diff --git a/...c/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/MembersToMentionUseCase.kt b/...c/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/MembersToMentionUseCase.kt
@@ -20,7 +20,7 @@ package com.wire.kalium.logic.feature.conversation
 
 import com.wire.kalium.logic.data.conversation.MemberDetails
 import com.wire.kalium.logic.data.id.ConversationId
-import com.wire.kalium.logic.data.user.UserRepository
+import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.util.KaliumDispatcher
 import com.wire.kalium.util.KaliumDispatcherImpl
 import kotlinx.coroutines.flow.first
@@ -42,7 +42,7 @@ import kotlinx.coroutines.withContext
 @Suppress("ReturnCount")
 class MembersToMentionUseCase internal constructor(
     private val observeConversationMembers: ObserveConversationMembersUseCase,
-    private val userRepository: UserRepository,
+    private val selfUserId: UserId,
     private val dispatcher: KaliumDispatcher = KaliumDispatcherImpl
 ) {
     /**
@@ -57,7 +57,7 @@ class MembersToMentionUseCase internal constructor(
         // TODO apply normalization techniques that are used for other searches to the name (e.g. ö -> oe)
 
         val usersToSearch = conversationMembers.filter {
-            it.user.id != userRepository.getSelfUser()?.id
+            it.user.id != selfUserId
         }
         if (searchQuery.isEmpty())
             return@withContext usersToSearch

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/debug/DebugScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/debug/DebugScope.kt
@@ -116,10 +116,10 @@ class DebugScope internal constructor(
 
     val sendConfirmation: SendConfirmationUseCase
         get() = SendConfirmationUseCase(
-            userRepository,
-            currentClientIdProvider,
-            slowSyncRepository,
-            messageSender
+            currentClientIdProvider = currentClientIdProvider,
+            slowSyncRepository = slowSyncRepository,
+            messageSender = messageSender,
+            selfUserId = userId,
         )
 
     val disableEventProcessing: DisableEventProcessingUseCase

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/debug/SendConfirmationUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/debug/SendConfirmationUseCase.kt
@@ -26,8 +26,8 @@ import com.wire.kalium.logic.data.message.MessageContent
 import com.wire.kalium.logic.data.message.receipt.ReceiptType
 import com.wire.kalium.logic.data.sync.SlowSyncRepository
 import com.wire.kalium.logic.data.sync.SlowSyncStatus
-import com.wire.kalium.logic.data.user.UserRepository
 import com.wire.kalium.logic.data.id.CurrentClientIdProvider
+import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.feature.message.MessageSender
 import com.wire.kalium.logic.functional.Either
 import com.wire.kalium.logic.functional.flatMap
@@ -41,10 +41,10 @@ import kotlinx.datetime.Clock
  * client behaviour. It should not be used by clients itself.
  */
 class SendConfirmationUseCase internal constructor(
-    private val userRepository: UserRepository,
     private val currentClientIdProvider: CurrentClientIdProvider,
     private val slowSyncRepository: SlowSyncRepository,
-    private val messageSender: MessageSender
+    private val messageSender: MessageSender,
+    private val selfUserId: UserId
 ) {
 
     suspend operator fun invoke(
@@ -57,8 +57,6 @@ class SendConfirmationUseCase internal constructor(
             it is SlowSyncStatus.Complete
         }
 
-        val selfUser = userRepository.observeSelfUser().first()
-
         val generatedMessageUuid = uuid4().toString()
 
         return currentClientIdProvider().flatMap { currentClientId ->
@@ -67,7 +65,7 @@ class SendConfirmationUseCase internal constructor(
                 content = MessageContent.Receipt(type, listOf(firstMessageId) + moreMessageIds),
                 conversationId = conversationId,
                 date = Clock.System.now(),
-                senderUserId = selfUser.id,
+                senderUserId = selfUserId,
                 senderClientId = currentClientId,
                 status = Message.Status.Pending,
                 isSelfMessage = true,

diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/CallingParticipantsOrderTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/CallingParticipantsOrderTest.kt
@@ -45,10 +45,6 @@ import kotlin.test.Test
 import kotlin.test.assertEquals
 
 class CallingParticipantsOrderTest {
-
-    @Mock
-    private val userRepository = mock(UserRepository::class)
-
     @Mock
     private val participantsFilter = mock(ParticipantsFilter::class)
 
@@ -63,7 +59,12 @@ class CallingParticipantsOrderTest {
     @BeforeTest
     fun setup() {
         callingParticipantsOrder =
-            CallingParticipantsOrderImpl(userRepository, currentClientIdProvider, participantsFilter, participantsOrderByName)
+            CallingParticipantsOrderImpl(
+                currentClientIdProvider,
+                participantsFilter,
+                selfUserId = selfUserId,
+                participantsOrderByName = participantsOrderByName
+            )
     }
 
     @Test
@@ -93,10 +94,6 @@ class CallingParticipantsOrderTest {
             currentClientIdProvider.invoke()
         }.returns(Either.Right(ClientId(selfClientId)))
 
-        coEvery {
-            userRepository.getSelfUser()
-        }.returns(selfUser)
-
         every {
             participantsFilter.otherParticipants(participants, selfClientId)
         }.returns(otherParticipants)