refactor: only have one main query for inserting/updating users (#2124)

* refactor: only have one main query for inserting/updating users

* fix: incorrect user mapping for team member

* chore: inline function which is only used once

* fix: updating connection status

* fix: user type not updating for non-team contacts

* fix: typo in partial user update query

* chore: update tests after rebase

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/connection/ConnectionRepository.kt

@@ -39,14 +39,10 @@ import com.wire.kalium.logic.data.user.ConnectionState.NOT_CONNECTED
 import com.wire.kalium.logic.data.user.ConnectionState.PENDING
 import com.wire.kalium.logic.data.user.ConnectionState.SENT
 import com.wire.kalium.logic.data.user.UserId
-import com.wire.kalium.logic.data.user.UserMapper
-import com.wire.kalium.logic.data.user.type.UserEntityTypeMapper
 import com.wire.kalium.logic.di.MapperProvider
 import com.wire.kalium.logic.failure.InvalidMappingFailure
-import com.wire.kalium.logic.feature.SelfTeamIdProvider
 import com.wire.kalium.logic.functional.Either
 import com.wire.kalium.logic.functional.flatMap
-import com.wire.kalium.logic.functional.fold
 import com.wire.kalium.logic.functional.isRight
 import com.wire.kalium.logic.functional.map
 import com.wire.kalium.logic.functional.onFailure
@@ -57,7 +53,6 @@ import com.wire.kalium.logic.wrapStorageRequest
 import com.wire.kalium.network.api.base.authenticated.connection.ConnectionApi
 import com.wire.kalium.network.api.base.authenticated.connection.ConnectionDTO
 import com.wire.kalium.network.api.base.authenticated.connection.ConnectionStateDTO
-import com.wire.kalium.network.api.base.authenticated.userDetails.UserDetailsApi
 import com.wire.kalium.persistence.dao.ConnectionDAO
 import com.wire.kalium.persistence.dao.UserDAO
 import com.wire.kalium.persistence.dao.conversation.ConversationDAO
@@ -88,15 +83,10 @@ internal class ConnectionDataSource(
     private val memberDAO: MemberDAO,
     private val connectionDAO: ConnectionDAO,
     private val connectionApi: ConnectionApi,
-    private val userDetailsApi: UserDetailsApi,
     private val userDAO: UserDAO,
-    private val selfUserId: UserId,
-    private val selfTeamIdProvider: SelfTeamIdProvider,
     private val conversationRepository: ConversationRepository,
     private val connectionStatusMapper: ConnectionStatusMapper = MapperProvider.connectionStatusMapper(),
-    private val connectionMapper: ConnectionMapper = MapperProvider.connectionMapper(),
-    private val userMapper: UserMapper = MapperProvider.userMapper(),
-    private val userTypeEntityTypeMapper: UserEntityTypeMapper = MapperProvider.userTypeEntityMapper()
+    private val connectionMapper: ConnectionMapper = MapperProvider.connectionMapper()
 ) : ConnectionRepository {
 
     override suspend fun fetchSelfUserConnections(): Either<CoreFailure, Unit> {
@@ -148,7 +138,6 @@ internal class ConnectionDataSource(
             val connectionStatus = connectionDTO.copy(status = newConnectionStatus)
             val connectionModel = connectionMapper.fromApiToModel(connectionDTO)
             handleUserConnectionStatusPersistence(connectionMapper.fromApiToModel(connectionStatus))
-            persistConnection(connectionModel)
             connectionModel
         }
     }
@@ -193,7 +182,7 @@ internal class ConnectionDataSource(
     }
 
     override suspend fun insertConnectionFromEvent(event: Event.User.NewConnection): Either<CoreFailure, Unit> =
-        persistConnection(event.connection)
+        handleUserConnectionStatusPersistence(event.connection)
 
     override suspend fun observeConnectionList(): Flow<List<Connection>> {
         return connectionDAO.getConnections().map { connections ->
@@ -203,36 +192,16 @@ internal class ConnectionDataSource(
         }
     }
 
-    // TODO: Vitor : Instead of duplicating, we could pass selfUser.teamId from the UseCases to this function.
-    // This way, the UseCases can tie the different Repos together, calling these functions.
     private suspend fun persistConnection(connection: Connection) =
-        selfTeamIdProvider().flatMap { teamId ->
-            // This can fail, but the connection will be there and get synced in worst case scenario in next SlowSync
-            wrapApiRequest {
-                userDetailsApi.getUserInfo(connection.qualifiedToId.toApi())
-            }.fold({
-                wrapStorageRequest {
-                    connectionDAO.insertConnection(connectionMapper.modelToDao(connection))
-                }
-            }, { userProfileDTO ->
-                wrapStorageRequest {
-                    val userEntity = userMapper.fromUserProfileDtoToUserEntity(
-                        userProfile = userProfileDTO,
-                        connectionState = connectionStatusMapper.toDaoModel(state = connection.status),
-                        userTypeEntity = userTypeEntityTypeMapper.fromTeamAndDomain(
-                            otherUserDomain = userProfileDTO.id.domain,
-                            selfUserTeamId = teamId?.value,
-                            otherUserTeamId = userProfileDTO.teamId,
-                            selfUserDomain = selfUserId.domain,
-                            isService = userProfileDTO.service != null
-                        )
-                    )
-                    insertConversationFromConnection(connection)
-                    // should we insert first user before creating conversation ?
-                    userDAO.insertUser(userEntity)
-                    connectionDAO.insertConnection(connectionMapper.modelToDao(connection))
-                }
-            })
+        wrapStorageRequest {
+            val connectionStatus = connectionStatusMapper.toDaoModel(state = connection.status)
+            userDAO.upsertConnectionStatus(connection.qualifiedToId.toDao(), connectionStatus)
+
+            insertConversationFromConnection(connection)
+
+            if (connection.status != ACCEPTED) {
+                connectionDAO.insertConnection(connectionMapper.modelToDao(connection))
+            }
         }
 
     private suspend fun insertConversationFromConnection(connection: Connection) {
@@ -265,10 +234,9 @@ internal class ConnectionDataSource(
             }
 
             ACCEPTED -> {
-                memberDAO.updateOrInsertOneOnOneMemberWithConnectionStatus(
+                memberDAO.updateOrInsertOneOnOneMember(
                     member = MemberEntity(user = connection.qualifiedToId.toDao(), MemberEntity.Role.Member),
-                    conversationID = connection.qualifiedConversationId.toDao(),
-                    status = connectionStatusMapper.toDaoModel(connection.status)
+                    conversationID = connection.qualifiedConversationId.toDao()
                 )
             }
 
@@ -283,26 +251,13 @@ internal class ConnectionDataSource(
         connectionDAO.deleteConnectionDataAndConversation(conversationId.toDao())
     }
 
-    private suspend fun updateConversationMemberFromConnection(connection: Connection) =
-        wrapStorageRequest {
-            memberDAO.updateOrInsertOneOnOneMemberWithConnectionStatus(
-                // TODO(IMPORTANT!!!!!!): setting a default value for member role is incorrect and can lead to unexpected behaviour
-                member = MemberEntity(user = connection.qualifiedToId.toDao(), MemberEntity.Role.Member),
-                status = connectionStatusMapper.toDaoModel(connection.status),
-                conversationID = connection.qualifiedConversationId.toDao()
-            )
-        }.onFailure {
-            kaliumLogger.e("There was an error when trying to persist the connection: $connection")
-        }
-
     /**
      * This will update the connection status on user table and will insert members only
      * if the [ConnectionDTO.status] is other than [ConnectionStateDTO.PENDING] or [ConnectionStateDTO.SENT]
      */
     private suspend fun handleUserConnectionStatusPersistence(connection: Connection): Either<CoreFailure, Unit> =
         when (connection.status) {
-            MISSING_LEGALHOLD_CONSENT, NOT_CONNECTED, PENDING, SENT, BLOCKED, IGNORED -> persistConnection(connection)
+            ACCEPTED, MISSING_LEGALHOLD_CONSENT, NOT_CONNECTED, PENDING, SENT, BLOCKED, IGNORED -> persistConnection(connection)
             CANCELLED -> deleteConnection(connection.qualifiedConversationId)
-            ACCEPTED -> updateConversationMemberFromConnection(connection)
         }
 }

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt

@@ -99,7 +99,7 @@ internal class ConversationGroupRepositoryImpl(
     private val selfUserId: UserId,
     private val teamIdProvider: SelfTeamIdProvider,
     private val conversationMapper: ConversationMapper = MapperProvider.conversationMapper(selfUserId),
-    private val eventMapper: EventMapper = MapperProvider.eventMapper(),
+    private val eventMapper: EventMapper = MapperProvider.eventMapper(selfUserId),
     private val protocolInfoMapper: ProtocolInfoMapper = MapperProvider.protocolInfoMapper(),
 ) : ConversationGroupRepository {
 

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt

@@ -332,7 +332,7 @@ internal class MLSConversationDataSource(
 
     private suspend fun processCommitBundleEvents(events: List<EventContentDTO>) {
         events.forEach { eventContentDTO ->
-            val event = MapperProvider.eventMapper().fromEventContentDTO("", eventContentDTO, true, false)
+            val event = MapperProvider.eventMapper(selfUserId).fromEventContentDTO("", eventContentDTO, true, false)
             if (event is Event.Conversation) {
                 commitBundleEventReceiver.onEvent(event)
             }

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/event/Event.kt

@@ -632,7 +632,7 @@ sealed class Event(open val id: String, open val transient: Boolean, open val li
             override val id: String,
             override val transient: Boolean,
             override val live: Boolean,
-            val userId: String,
+            val userId: UserId,
             val accentId: Int?,
             val ssoIdDeleted: Boolean?,
             val name: String?,
@@ -645,7 +645,7 @@ sealed class Event(open val id: String, open val transient: Boolean, open val li
             override fun toLogMap(): Map<String, Any?> = mapOf(
                 typeKey to "User.Update",
                 idKey to id.obfuscateId(),
-                userIdKey to userId.obfuscateId()
+                userIdKey to userId.toLogString()
             )
         }
 

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/event/EventMapper.kt

@@ -33,6 +33,7 @@ import com.wire.kalium.logic.data.event.Event.UserProperty.TypingIndicatorModeSe
 import com.wire.kalium.logic.data.featureConfig.FeatureConfigMapper
 import com.wire.kalium.logic.data.id.SubconversationId
 import com.wire.kalium.logic.data.id.toModel
+import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.data.user.toModel
 import com.wire.kalium.logic.di.MapperProvider
 import com.wire.kalium.logic.util.Base64
@@ -50,12 +51,13 @@ import kotlinx.serialization.InternalSerializationApi
 import kotlinx.serialization.SerializationException
 import kotlinx.serialization.serializer
 
-@Suppress("TooManyFunctions")
+@Suppress("TooManyFunctions", "LongParameterList")
 class EventMapper(
     private val memberMapper: MemberMapper,
     private val connectionMapper: ConnectionMapper,
     private val featureConfigMapper: FeatureConfigMapper,
     private val roleMapper: ConversationRoleMapper,
+    private val selfUserId: UserId,
     private val receiptModeMapper: ReceiptModeMapper = MapperProvider.receiptModeMapper(),
     private val clientMapper: ClientMapper = MapperProvider.clientMapper()
 ) {
@@ -650,7 +652,7 @@ class EventMapper(
         live: Boolean
     ) = Event.User.Update(
         id = id,
-        userId = event.userData.nonQualifiedUserId,
+        userId = UserId(event.userData.nonQualifiedUserId, selfUserId.domain),
         accentId = event.userData.accentId,
         ssoIdDeleted = event.userData.ssoIdDeleted,
         name = event.userData.name,

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/event/EventRepository.kt

@@ -22,6 +22,7 @@ import com.wire.kalium.logic.CoreFailure
 import com.wire.kalium.logic.NetworkFailure
 import com.wire.kalium.logic.StorageFailure
 import com.wire.kalium.logic.data.conversation.ClientId
+import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.di.MapperProvider
 import com.wire.kalium.logic.feature.CurrentClientIdProvider
 import com.wire.kalium.logic.functional.Either
@@ -81,7 +82,8 @@ class EventDataSource(
     private val notificationApi: NotificationApi,
     private val metadataDAO: MetadataDAO,
     private val currentClientId: CurrentClientIdProvider,
-    private val eventMapper: EventMapper = MapperProvider.eventMapper()
+    private val selfUserId: UserId,
+    private val eventMapper: EventMapper = MapperProvider.eventMapper(selfUserId)
 ) : EventRepository {
 
     // TODO(edge-case): handle Missing notification response (notify user that some messages are missing)

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/team/TeamRepository.kt

@@ -91,19 +91,16 @@ internal class TeamDataSource(
          */
         if (teamMemberList.hasMore.not()) {
             teamMemberList.members.map { teamMember ->
-                userMapper.fromTeamMemberToDaoModel(
-                    teamId = teamId,
-                    nonQualifiedUserId = teamMember.nonQualifiedUserId,
-                    permissionCode = teamMember.permissions?.own,
-                    userDomain = userDomain,
-                )
+                val userId = QualifiedIDEntity(teamMember.nonQualifiedUserId, userDomain)
+                val userType = userTypeEntityTypeMapper.teamRoleCodeToUserType(teamMember.permissions?.own)
+                userId to userType
             }
         } else {
             listOf()
         }
     }.flatMap { teamMembers ->
         wrapStorageRequest {
-            userDAO.upsertTeamMembersTypes(teamMembers)
+            userDAO.upsertTeamMemberUserTypes(teamMembers.toMap())
         }
     }
 
@@ -123,13 +120,9 @@ internal class TeamDataSource(
 
     override suspend fun updateMemberRole(teamId: String, userId: String, permissionCode: Int?): Either<CoreFailure, Unit> {
         return wrapStorageRequest {
-            val user = userMapper.fromTeamMemberToDaoModel(
-                teamId = TeamId(teamId),
-                nonQualifiedUserId = userId,
-                userDomain = selfUserId.domain,
-                permissionCode = permissionCode
-            )
-            userDAO.upsertTeamMembersTypes(listOf(user))
+            userDAO.upsertTeamMemberUserTypes(mapOf(
+                QualifiedIDEntity(userId, selfUserId.domain) to userTypeEntityTypeMapper.teamRoleCodeToUserType(permissionCode)
+            ))
         }
     }
 
@@ -139,22 +132,16 @@ internal class TeamDataSource(
                 teamId = teamId,
                 userId = userId,
             )
-        }.flatMap { _ ->
+        }.flatMap { member ->
             wrapApiRequest { userDetailsApi.getUserInfo(userId = QualifiedID(userId, selfUserId.domain)) }
                 .flatMap { userProfileDTO ->
                     wrapStorageRequest {
                         val userEntity = userMapper.fromUserProfileDtoToUserEntity(
                             userProfile = userProfileDTO,
                             connectionState = ConnectionEntity.State.ACCEPTED,
-                            userTypeEntity = userTypeEntityTypeMapper.fromTeamAndDomain(
-                                otherUserDomain = userProfileDTO.id.domain,
-                                selfUserTeamId = teamId,
-                                otherUserTeamId = userProfileDTO.teamId,
-                                selfUserDomain = selfUserId.domain,
-                                isService = userProfileDTO.service != null
-                            )
+                            userTypeEntity = userTypeEntityTypeMapper.teamRoleCodeToUserType(member.permissions?.own)
                         )
-                        userDAO.insertUser(userEntity)
+                        userDAO.upsertUser(userEntity)
                     }
                 }
         }