Skip to content

Commit

Permalink
refactor: stop updating user info in the background [WPB-14826] (#3249)
Browse files Browse the repository at this point in the history 
* refactor: stop updating user info in the background

* detekt
  • Loading branch information
@MohamadJaara
MohamadJaara authored Jan 29, 2025
1 parent e21b0a8 commit f4afb2a
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 325 deletions.
2 changes: 0 additions & 2 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/data/logout/LogoutRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package com.wire.kalium.logic.data.logout

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.user.UserDataSource
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.wrapApiRequest
import com.wire.kalium.logic.wrapStorageRequest
Expand Down Expand Up @@ -75,7 +74,6 @@ internal class LogoutDataSource(
metadataDAO.clear(
keysToKeep = listOf(
ClientRegistrationStorageImpl.RETAINED_CLIENT_ID_KEY,
UserDataSource.SELF_USER_ID_KEY
)
)
}
Expand Down
134 changes: 9 additions & 125 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

package com.wire.kalium.logic.data.user

import co.touchlab.stately.collections.ConcurrentMutableMap
import com.wire.kalium.logger.obfuscateDomain
import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.NetworkFailure
Expand Down Expand Up @@ -48,7 +47,6 @@ import com.wire.kalium.logic.failure.SelfUserDeleted
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.functional.flatMap
import com.wire.kalium.logic.functional.flatMapLeft
import com.wire.kalium.logic.functional.fold
import com.wire.kalium.logic.functional.foldToEitherWhileRight
import com.wire.kalium.logic.functional.getOrNull
import com.wire.kalium.logic.functional.map
Expand All @@ -74,28 +72,13 @@ import com.wire.kalium.network.api.model.UserProfileDTO
import com.wire.kalium.network.api.model.isTeamMember
import com.wire.kalium.persistence.dao.ConnectionEntity
import com.wire.kalium.persistence.dao.ConversationIDEntity
import com.wire.kalium.persistence.dao.MetadataDAO
import com.wire.kalium.persistence.dao.QualifiedIDEntity
import com.wire.kalium.persistence.dao.UserDAO
import com.wire.kalium.persistence.dao.UserIDEntity
import com.wire.kalium.persistence.dao.UserTypeEntity
import com.wire.kalium.persistence.dao.client.ClientDAO
import com.wire.kalium.util.DateTimeUtil
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.datetime.Instant
import kotlinx.serialization.encodeToString
import kotlinx.serialization.json.Json
import kotlin.time.Duration.Companion.minutes

@Suppress("TooManyFunctions")
interface UserRepository {
Expand Down Expand Up @@ -178,7 +161,6 @@ interface UserRepository {
@Suppress("LongParameterList", "TooManyFunctions")
internal class UserDataSource internal constructor(
private val userDAO: UserDAO,
private val metadataDAO: MetadataDAO,
private val clientDAO: ClientDAO,
private val selfApi: SelfApi,
private val userDetailsApi: UserDetailsApi,
Expand All @@ -196,17 +178,6 @@ internal class UserDataSource internal constructor(
private val memberMapper: MemberMapper = MapperProvider.memberMapper(),
) : UserRepository {

/**
* Stores the last time a user's details were fetched from remote.
*
* @see Event.User.Update
* @see USER_DETAILS_MAX_AGE
*/
private val userDetailsRefreshInstantCache = ConcurrentMutableMap<UserId, Instant>()

private var userRefresh: Deferred<Either<CoreFailure, Unit>>? = null
private val userRefreshMutex: Mutex = Mutex()

override suspend fun fetchSelfUser(): Either<CoreFailure, Unit> = wrapApiRequest { selfApi.getSelfInfo() }
.flatMap { selfUserDTO ->
selfUserDTO.teamId.let { selfUserTeamId ->
Expand All @@ -228,9 +199,6 @@ internal class UserDataSource internal constructor(
}
.flatMap { userEntity ->
wrapStorageRequest { userDAO.upsertUser(userEntity) }
.flatMap {
wrapStorageRequest { metadataDAO.insertValue(Json.encodeToString(userEntity.id), SELF_USER_ID_KEY) }
}
}
}
}
Expand All @@ -242,48 +210,13 @@ internal class UserDataSource internal constructor(
userDAO.observeUserDetailsByQualifiedID(qualifiedID = userId.toDao())
.map { userEntity ->
userEntity?.let { userMapper.fromUserDetailsEntityToOtherUser(userEntity) }
}.onEach { otherUser ->
if (otherUser != null) {
refreshUserDetailsIfNeeded(userId)
}
}

override suspend fun getUsersWithOneOnOneConversation(): List<OtherUser> {
return userDAO.getUsersWithOneOnOneConversation()
.map(userMapper::fromUserEntityToOtherUser)
}

private suspend fun refreshUserDetailsIfNeeded(userId: UserId): Either<CoreFailure, Unit> = coroutineScope {
userRefreshMutex.withLock {
val now = DateTimeUtil.currentInstant()
val wasFetchedRecently = userDetailsRefreshInstantCache[userId]?.let { now < it + USER_DETAILS_MAX_AGE } ?: false

if (wasFetchedRecently) return@coroutineScope Either.Right(Unit)

if (userRefresh?.isActive == true) return@coroutineScope userRefresh?.await() ?: error("")

userRefresh = async { refreshUserDetails(userId) }
}

userRefresh?.await() ?: error("")
}

/**
* Only refresh user profiles if it wasn't fetched recently.
*
* @see userDetailsRefreshInstantCache
* @see USER_DETAILS_MAX_AGE
*/
private suspend fun refreshUserDetails(userId: UserId): Either<CoreFailure, Unit> {
return when (userId) {
selfUserId -> fetchSelfUser()
else -> fetchUserInfo(userId)
}.also {
kaliumLogger.d("Refreshing user info from API after $USER_DETAILS_MAX_AGE")
userDetailsRefreshInstantCache[userId] = DateTimeUtil.currentInstant()
}
}

override suspend fun fetchAllOtherUsers(): Either<CoreFailure, Unit> {
val ids = userDAO.allOtherUsersId().map(UserIDEntity::toModel).toSet()
return fetchUsersByIds(ids).map { }
Expand Down Expand Up @@ -435,52 +368,15 @@ internal class UserDataSource internal constructor(
}

override suspend fun observeSelfUser(): Flow<SelfUser> {

val selfUser = getOrFetchSelfUserId()

return if (selfUser != null) {
userDAO.observeUserDetailsByQualifiedID(selfUser).filterNotNull()
.map(userMapper::fromUserDetailsEntityToSelfUser)
} else {
emptyFlow()
}
return userDAO.observeUserDetailsByQualifiedID(selfUserId.toDao()).filterNotNull()
.map(userMapper::fromUserDetailsEntityToSelfUser)
}

private suspend fun getOrFetchSelfUserId(): QualifiedIDEntity? {

var userId = metadataDAO.valueByKey(SELF_USER_ID_KEY)

if (userId == null) {

val logPrefix = "Observing self user before insertion"
kaliumLogger.w("$logPrefix: Triggering a fetch.")

fetchSelfUser().fold({ failure ->
kaliumLogger.e("""$logPrefix failed: {"failure":"$failure"}""")
}, {
kaliumLogger.i("$logPrefix: Succeeded")
userDetailsRefreshInstantCache[selfUserId] = DateTimeUtil.currentInstant()
})
} else {
refreshUserDetailsIfNeeded(selfUserId)
}

return metadataDAO.valueByKey(SELF_USER_ID_KEY)?.let { Json.decodeFromString(it) }
}

@OptIn(ExperimentalCoroutinesApi::class)
override suspend fun observeSelfUserWithTeam(): Flow<Pair<SelfUser, Team?>> {

val selfUser = getOrFetchSelfUserId()

return if (selfUser != null) {
userDAO.getUserDetailsWithTeamByQualifiedID(selfUser).filterNotNull()
.map { (user, team) ->
userMapper.fromUserDetailsEntityToSelfUser(user) to team?.let { teamMapper.fromDaoModelToTeam(it) }
}
} else {
emptyFlow()
}
return userDAO.getUserDetailsWithTeamByQualifiedID(selfUserId.toDao()).filterNotNull()
.map { (user, team) ->
userMapper.fromUserDetailsEntityToSelfUser(user) to team?.let { teamMapper.fromDaoModelToTeam(it) }
}
}

@Deprecated(
Expand All @@ -500,10 +396,8 @@ internal class UserDataSource internal constructor(
}

override suspend fun getSelfUser(): SelfUser? {
return getOrFetchSelfUserId()?.let {
userDAO.getUserDetailsByQualifiedID(it)?.let {
userMapper.fromUserDetailsEntityToSelfUser(it)
}
return userDAO.getUserDetailsByQualifiedID(selfUserId.toDao())?.let {
userMapper.fromUserDetailsEntityToSelfUser(it)
}
}

Expand Down Expand Up @@ -709,17 +603,7 @@ internal class UserDataSource internal constructor(
}

companion object {
internal const val SELF_USER_ID_KEY = "selfUserID"

/**
* Maximum age for user details.
*
* The USER_DETAILS_MAX_AGE constant represents the maximum age in minutes that user details can be considered valid. After
* this duration, the user details should be refreshed.
*
* This is needed because some users don't get `user.update` events, so we need to refresh their details every so often.
*/
internal val USER_DETAILS_MAX_AGE = 5.minutes

internal const val BATCH_SIZE = 500
}
}
1 change: 0 additions & 1 deletion logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,6 @@ class UserSessionScope internal constructor(

private val userRepository: UserRepository = UserDataSource(
userDAO = userStorage.database.userDAO,
metadataDAO = userStorage.database.metadataDAO,
clientDAO = userStorage.database.clientDAO,
selfApi = authenticatedNetworkContainer.selfApi,
userDetailsApi = authenticatedNetworkContainer.userDetailsApi,
Expand Down
Loading

0 comments on commit f4afb2a

Please sign in to comment.