From 53114bd675924a86825aa4ef32e6c550902c3101 Mon Sep 17 00:00:00 2001 From: Yamil Medina Date: Fri, 19 May 2023 11:42:59 +0200 Subject: [PATCH] feat(offline-backends): users and conversations without metadata refetch pt1. (AR-3123) (#1736) * feat: adjust query to hide 1:1 convos without metadata * feat: adjustment query to consider deleted users logic as it is now * feat: tests for query conversations details * feat: persistence for getting users out of sync * feat: persistence for getting users out of sync * feat: pr comments single quotes * feat: invok operator * feat: persist failed convos * feat: cleanup * feat: tests cov * feat: tests cov * feat: tests cov * feat: tests cov * feat: refactor, persist users withoutmetadata with dedicated field * feat: refactor, relay on missing metadata field for refetch usres * feat: refactor, relay on missing metadata field for refetch conversations * feat: refactor, relay on missing metadata field for refetch conversations * feat: refactor, relay on missing metadata field for refetch conversations * feat: refactor, fixing tests * chore: add migration tests --- .../data/conversation/ConversationMapper.kt | 28 +++++- .../conversation/ConversationRepository.kt | 17 +++- .../wire/kalium/logic/data/user/UserMapper.kt | 30 ++++++- .../kalium/logic/data/user/UserRepository.kt | 63 +++++++------ .../RefreshUsersWithoutMetadataUseCase.kt | 49 +++++++++++ .../kalium/logic/feature/user/UserScope.kt | 3 + .../ConversationRepositoryTest.kt | 10 +++ .../logic/data/user/UserRepositoryTest.kt | 88 +++++++++++++++---- .../RefreshUsersWithoutMetadataUseCaseTest.kt | 64 ++++++++++++++ .../wire/kalium/persistence/Conversations.sq | 11 ++- .../com/wire/kalium/persistence/Users.sq | 20 +++-- .../src/commonMain/db_user/migrations/38.sqm | 4 + .../persistence/dao/ConnectionDAOImpl.kt | 4 +- .../kalium/persistence/dao/ConversationDAO.kt | 3 +- .../persistence/dao/ConversationDAOImpl.kt | 3 +- .../wire/kalium/persistence/dao/UserDAO.kt | 4 +- .../kalium/persistence/dao/UserDAOImpl.kt | 28 ++++-- .../kalium/persistence/dao/UserDAOTest.kt | 13 +++ 18 files changed, 373 insertions(+), 69 deletions(-) create mode 100644 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/publicuser/RefreshUsersWithoutMetadataUseCase.kt create mode 100644 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/user/RefreshUsersWithoutMetadataUseCaseTest.kt create mode 100644 persistence/src/commonMain/db_user/migrations/38.sqm diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapper.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapper.kt index 99597b4f2c8..687197fa739 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapper.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapper.kt @@ -20,6 +20,7 @@ package com.wire.kalium.logic.data.conversation import com.wire.kalium.logic.data.connection.ConnectionStatusMapper import com.wire.kalium.logic.data.id.IdMapper +import com.wire.kalium.logic.data.id.NetworkQualifiedId import com.wire.kalium.logic.data.id.TeamId import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.id.toDao @@ -75,6 +76,7 @@ interface ConversationMapper { fun toApiModel(name: String?, members: List, teamId: String?, options: ConversationOptions): CreateConversationRequest fun fromMigrationModel(conversation: Conversation): ConversationEntity + fun fromFailedGroupConversationToEntity(conversationId: NetworkQualifiedId): ConversationEntity } @Suppress("TooManyFunctions", "LongParameterList") @@ -108,7 +110,8 @@ internal class ConversationMapperImpl( lastModifiedDate = apiModel.lastEventTime.toInstant(), access = apiModel.access.map { it.toDAO() }, accessRole = apiModel.accessRole.map { it.toDAO() }, - receiptMode = receiptModeMapper.fromApiToDaoModel(apiModel.receiptMode) + receiptMode = receiptModeMapper.fromApiToDaoModel(apiModel.receiptMode), + hasIncompleteMetadata = false ) override fun fromApiModelToDaoModel(apiModel: ConvProtocol): Protocol = when (apiModel) { @@ -347,6 +350,29 @@ internal class ConversationMapperImpl( ) } + /** + * Default values and marked as [ConversationEntity.hasIncompleteMetadata] = true. + * So later we can re-fetch them. + */ + override fun fromFailedGroupConversationToEntity(conversationId: NetworkQualifiedId): ConversationEntity = ConversationEntity( + id = conversationId.toDao(), + name = null, + type = ConversationEntity.Type.GROUP, + teamId = null, + protocolInfo = ProtocolInfo.Proteus, + mutedStatus = ConversationEntity.MutedStatus.ALL_ALLOWED, + mutedTime = 0, + removedBy = null, + creatorId = "", + lastNotificationDate = "1970-01-01T00:00:00.000Z".toInstant(), + lastModifiedDate = "1970-01-01T00:00:00.000Z".toInstant(), + lastReadDate = "1970-01-01T00:00:00.000Z".toInstant(), + access = emptyList(), + accessRole = emptyList(), + receiptMode = ConversationEntity.ReceiptMode.DISABLED, + hasIncompleteMetadata = true + ) + private fun ConversationResponse.getProtocolInfo(mlsGroupState: GroupState?): ProtocolInfo { return when (protocol) { ConvProtocol.MLS -> ProtocolInfo.MLS( diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepository.kt index 441ae06e626..dfca5996bf4 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepository.kt @@ -26,6 +26,7 @@ import com.wire.kalium.logic.data.client.MLSClientProvider import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.GroupID import com.wire.kalium.logic.data.id.IdMapper +import com.wire.kalium.logic.data.id.NetworkQualifiedId import com.wire.kalium.logic.data.id.QualifiedID import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.id.toCrypto @@ -245,7 +246,8 @@ internal class ConversationDataSource internal constructor( }.onSuccess { conversations -> if (conversations.conversationsFailed.isNotEmpty()) { kaliumLogger.withFeatureId(CONVERSATIONS) - .d("Skipping ${conversations.conversationsFailed.size} conversations failed") + .d("Handling ${conversations.conversationsFailed.size} conversations failed") + handleFailedConversations(conversations.conversationsFailed) } if (conversations.conversationsNotFound.isNotEmpty()) { kaliumLogger.withFeatureId(CONVERSATIONS) @@ -599,6 +601,7 @@ internal class ConversationDataSource internal constructor( conversationDAO.deleteConversationByQualifiedID(conversationId.toDao()) } } + is Conversation.ProtocolInfo.Proteus -> wrapStorageRequest { conversationDAO.deleteConversationByQualifiedID(conversationId.toDao()) } @@ -686,6 +689,18 @@ internal class ConversationDataSource internal constructor( } } + private suspend fun handleFailedConversations( + conversationsFailed: List + ): Either { + return wrapStorageRequest { + if (conversationsFailed.isNotEmpty()) { + conversationDAO.insertConversations(conversationsFailed.map { conversationId -> + conversationMapper.fromFailedGroupConversationToEntity(conversationId) + }) + } + } + } + companion object { const val DEFAULT_MEMBER_ROLE = "wire_member" } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserMapper.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserMapper.kt index 64f08812257..1b919e35004 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserMapper.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserMapper.kt @@ -22,6 +22,7 @@ import com.wire.kalium.logic.data.client.ClientMapper import com.wire.kalium.logic.data.client.OtherUserClient import com.wire.kalium.logic.data.event.Event import com.wire.kalium.logic.data.id.IdMapper +import com.wire.kalium.logic.data.id.NetworkQualifiedId import com.wire.kalium.logic.data.id.TeamId import com.wire.kalium.logic.data.id.toDao import com.wire.kalium.logic.data.id.toModel @@ -88,6 +89,8 @@ interface UserMapper { fun apiToEntity(user: UserProfileDTO, member: TeamsApi.TeamMemberDTO?, teamId: String?, selfUser: QualifiedID): UserEntity fun toUpdateDaoFromEvent(event: Event.User.Update, userEntity: UserEntity): UserEntity + + fun fromFailedUserToEntity(userId: NetworkQualifiedId): UserEntity } internal class UserMapperImpl( @@ -131,7 +134,8 @@ internal class UserMapperImpl( availabilityStatus = UserAvailabilityStatusEntity.NONE, userType = userTypeEntity ?: UserTypeEntity.STANDARD, botService = userProfileDTO.service?.let { BotEntity(it.id, it.provider) }, - deleted = userProfileDTO.deleted ?: false + deleted = userProfileDTO.deleted ?: false, + hasIncompleteMetadata = false ) } @@ -304,4 +308,28 @@ internal class UserMapperImpl( ) } } + + /** + * Default values and marked as [UserEntity.hasIncompleteMetadata] = true. + * So later we can re-fetch them. + */ + override fun fromFailedUserToEntity(userId: NetworkQualifiedId): UserEntity { + return UserEntity( + id = userId.toDao(), + name = null, + handle = null, + email = null, + phone = null, + accentId = 1, + team = null, + connectionStatus = ConnectionEntity.State.ACCEPTED, + previewAssetId = null, + completeAssetId = null, + availabilityStatus = UserAvailabilityStatusEntity.NONE, + userType = UserTypeEntity.STANDARD, + botService = null, + deleted = false, + hasIncompleteMetadata = true + ) + } } 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 index 8f3bd7c998b..5227e428518 100644 --- 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 @@ -26,6 +26,7 @@ import com.wire.kalium.logic.data.conversation.Recipient import com.wire.kalium.logic.data.event.Event import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.IdMapper +import com.wire.kalium.logic.data.id.NetworkQualifiedId import com.wire.kalium.logic.data.id.QualifiedIdMapper import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.id.toDao @@ -117,6 +118,11 @@ internal interface UserRepository { * otherwise [Either.Left] with [NetworkFailure] */ suspend fun updateSelfEmail(email: String): Either + + /** + * Updates users without metadata from the server. + */ + suspend fun syncUsersWithoutMetadata(): Either } @Suppress("LongParameterList", "TooManyFunctions") @@ -196,41 +202,36 @@ internal class UserDataSource internal constructor( return fetchUsersByIds(ids.toSet()) } - override suspend fun fetchUsersByIds(qualifiedUserIdList: Set): Either { - val selfUserDomain = selfUserId.domain - qualifiedUserIdList.groupBy { it.domain } - .filter { it.value.isNotEmpty() } - .map { (domain: String, usersOnDomain: List) -> - when (selfUserDomain == domain) { - true -> fetchMultipleUsers(usersOnDomain) - false -> { - usersOnDomain.forEach { userId -> - fetchUserInfo(userId).fold({ - kaliumLogger.w("Ignoring external users details") - }) { kaliumLogger.d("External users details saved") } - } - Either.Right(Unit) - } - } - } - - return Either.Right(Unit) - } - - private suspend fun fetchMultipleUsers(qualifiedUsersOnSameDomainList: List) = wrapApiRequest { - userDetailsApi.getMultipleUsers( - ListUserRequest.qualifiedIds(qualifiedUsersOnSameDomainList.map { userId -> userId.toApi() }) - ) - }.flatMap { listUserProfileDTO -> persistUsers(listUserProfileDTO.usersFound) } - override suspend fun fetchUserInfo(userId: UserId) = wrapApiRequest { userDetailsApi.getUserInfo(userId.toApi()) } .flatMap { userProfileDTO -> persistUsers(listOf(userProfileDTO)) } + override suspend fun fetchUsersByIds(qualifiedUserIdList: Set): Either { + if (qualifiedUserIdList.isEmpty()) { + return Either.Right(Unit) + } + + return wrapApiRequest { + userDetailsApi.getMultipleUsers( + ListUserRequest.qualifiedIds(qualifiedUserIdList.map { userId -> userId.toApi() }) + ) + }.flatMap { listUserProfileDTO -> + if (listUserProfileDTO.usersFailed.isNotEmpty()) { + kaliumLogger.d("Handling ${listUserProfileDTO.usersFailed.size} failed users") + persistIncompleteUsers(listUserProfileDTO.usersFailed) + } + persistUsers(listUserProfileDTO.usersFound) + } + } + override suspend fun updateSelfEmail(email: String): Either = wrapApiRequest { selfApi.updateEmailAddress(email) } + private suspend fun persistIncompleteUsers(usersFailed: List) = wrapStorageRequest { + userDAO.insertOrIgnoreUsers(usersFailed.map { userMapper.fromFailedUserToEntity(it) }) + } + private suspend fun persistUsers(listUserProfileDTO: List) = wrapStorageRequest { val selfUserDomain = selfUserId.domain val selfUserTeamId = selfTeamIdProvider().getOrNull()?.value @@ -463,6 +464,14 @@ internal class UserDataSource internal constructor( ) } + override suspend fun syncUsersWithoutMetadata(): Either = wrapStorageRequest { + userDAO.getUsersWithoutMetadata() + }.flatMap { usersWithoutMetadata -> + kaliumLogger.d("Numbers of users refreshed: ${usersWithoutMetadata.size}") + val userIds = usersWithoutMetadata.map { it.id.toModel() }.toSet() + fetchUsersByIds(userIds) + } + companion object { internal const val SELF_USER_ID_KEY = "selfUserID" internal val FEDERATED_USER_TTL = 5.minutes diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/publicuser/RefreshUsersWithoutMetadataUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/publicuser/RefreshUsersWithoutMetadataUseCase.kt new file mode 100644 index 00000000000..075b9974b9b --- /dev/null +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/publicuser/RefreshUsersWithoutMetadataUseCase.kt @@ -0,0 +1,49 @@ +/* + * Wire + * Copyright (C) 2023 Wire Swiss GmbH + * + * 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.wire.kalium.logic.feature.publicuser + +import com.wire.kalium.logic.data.user.UserRepository +import com.wire.kalium.logic.functional.fold +import com.wire.kalium.logic.kaliumLogger +import com.wire.kalium.util.KaliumDispatcher +import com.wire.kalium.util.KaliumDispatcherImpl +import kotlinx.coroutines.withContext + +/** + * Refresh users without metadata, only if necessary. + */ +interface RefreshUsersWithoutMetadataUseCase { + suspend operator fun invoke() +} + +internal class RefreshUsersWithoutMetadataUseCaseImpl( + private val userRepository: UserRepository, + private val dispatchers: KaliumDispatcher = KaliumDispatcherImpl +) : RefreshUsersWithoutMetadataUseCase { + + override suspend fun invoke() = withContext(dispatchers.io) { + kaliumLogger.d("Started syncing users without metadata") + userRepository.syncUsersWithoutMetadata() + .fold({ + kaliumLogger.w("Error while syncing users without metadata $it") + }) { + kaliumLogger.d("Finished syncing users without metadata") + } + } + +} diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UserScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UserScope.kt index 537664e7a9d..d67cba5d426 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UserScope.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UserScope.kt @@ -45,6 +45,8 @@ import com.wire.kalium.logic.feature.publicuser.GetAllContactsUseCase import com.wire.kalium.logic.feature.publicuser.GetAllContactsUseCaseImpl import com.wire.kalium.logic.feature.publicuser.GetKnownUserUseCase import com.wire.kalium.logic.feature.publicuser.GetKnownUserUseCaseImpl +import com.wire.kalium.logic.feature.publicuser.RefreshUsersWithoutMetadataUseCase +import com.wire.kalium.logic.feature.publicuser.RefreshUsersWithoutMetadataUseCaseImpl import com.wire.kalium.logic.feature.publicuser.search.SearchKnownUsersUseCase import com.wire.kalium.logic.feature.publicuser.search.SearchKnownUsersUseCaseImpl import com.wire.kalium.logic.feature.publicuser.search.SearchPublicUsersUseCase @@ -98,6 +100,7 @@ class UserScope internal constructor( val getAllKnownUsers: GetAllContactsUseCase get() = GetAllContactsUseCaseImpl(userRepository) val getKnownUser: GetKnownUserUseCase get() = GetKnownUserUseCaseImpl(userRepository) val getUserInfo: GetUserInfoUseCase get() = GetUserInfoUseCaseImpl(userRepository, teamRepository) + val refreshUsersWithoutMetadata: RefreshUsersWithoutMetadataUseCase get() = RefreshUsersWithoutMetadataUseCaseImpl(userRepository) val updateSelfAvailabilityStatus: UpdateSelfAvailabilityStatusUseCase get() = UpdateSelfAvailabilityStatusUseCase(userRepository, messageSender, clientIdProvider, selfUserId) val getAllContactsNotInConversation: GetAllContactsNotInConversationUseCase diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepositoryTest.kt index 6d6f0c4347c..b7c920bc4a9 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepositoryTest.kt @@ -199,6 +199,16 @@ class ConversationRepositoryTest { conversationRepository.fetchConversations() // then + verify(arrangement.conversationDAO) + .suspendFunction(arrangement.conversationDAO::insertConversations) + .with( + matching { conversations -> + conversations.any { entity -> + entity.id.value == CONVERSATION_RESPONSE_DTO.conversationsFailed.first().value + } + } + ).wasInvoked(exactly = once) + verify(arrangement.conversationDAO) .suspendFunction(arrangement.conversationDAO::insertConversations) .with( diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt index afdfc72e18b..120501e2071 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt @@ -19,6 +19,7 @@ package com.wire.kalium.logic.data.user import com.wire.kalium.logic.data.id.QualifiedIdMapper +import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.session.SessionRepository import com.wire.kalium.logic.data.user.UserDataSource.Companion.SELF_USER_ID_KEY import com.wire.kalium.logic.failure.SelfUserDeleted @@ -32,9 +33,10 @@ import com.wire.kalium.logic.test_util.TestNetworkResponseError import com.wire.kalium.logic.util.shouldFail import com.wire.kalium.logic.util.shouldSucceed import com.wire.kalium.network.api.base.authenticated.self.SelfApi +import com.wire.kalium.network.api.base.authenticated.userDetails.ListUserRequest import com.wire.kalium.network.api.base.authenticated.userDetails.ListUsersDTO +import com.wire.kalium.network.api.base.authenticated.userDetails.QualifiedUserIdListRequest import com.wire.kalium.network.api.base.authenticated.userDetails.UserDetailsApi -import com.wire.kalium.network.api.base.model.QualifiedID import com.wire.kalium.network.exceptions.KaliumException import com.wire.kalium.network.utils.NetworkResponse import com.wire.kalium.persistence.dao.MetadataDAO @@ -50,6 +52,7 @@ import io.mockative.classOf import io.mockative.configure import io.mockative.eq import io.mockative.given +import io.mockative.matching import io.mockative.mock import io.mockative.once import io.mockative.verify @@ -97,16 +100,10 @@ class UserRepositoryTest { @Test fun givenAUserIsNotKnown_whenFetchingUsersIfUnknown_thenShouldFetchFromAPIAndSucceed() = runTest { val missingUserId = UserId(value = "id2", domain = "domain2") - val requestedUserIds = setOf( - UserId(value = "id1", domain = "domain1"), - missingUserId - ) - val knownUserEntities = listOf( - TestUser.ENTITY.copy(id = UserIDEntity(value = "id1", domain = "domain1")) - ) + val requestedUserIds = setOf(UserId(value = "id1", domain = "domain1"), missingUserId) + val knownUserEntities = listOf(TestUser.ENTITY.copy(id = UserIDEntity(value = "id1", domain = "domain1"))) val (arrangement, userRepository) = Arrangement() .withGetSelfUserId() - .withSuccessfulGetUsersInfo() .withSuccessfulGetUsersByQualifiedIdList(knownUserEntities) .withSuccessfulGetMultipleUsersApiRequest(ListUsersDTO(usersFailed = emptyList(), listOf(TestUser.USER_PROFILE_DTO))) .arrange() @@ -114,8 +111,10 @@ class UserRepositoryTest { userRepository.fetchUsersIfUnknownByIds(requestedUserIds).shouldSucceed() verify(arrangement.userDetailsApi) - .suspendFunction(arrangement.userDetailsApi::getUserInfo) - .with(eq(QualifiedID("id2", "domain2"))) + .suspendFunction(arrangement.userDetailsApi::getMultipleUsers) + .with(matching { request: ListUserRequest -> + (request as QualifiedUserIdListRequest).qualifiedIds.first() == missingUserId.toApi() + }) .wasInvoked(exactly = once) } @@ -177,6 +176,12 @@ class UserRepositoryTest { // given val requestedUserIds = emptySet() val (arrangement, userRepository) = Arrangement() + .withSuccessfulGetMultipleUsersApiRequest( + ListUsersDTO( + usersFailed = emptyList(), + usersFound = listOf(TestUser.USER_PROFILE_DTO) + ) + ) .arrange() // when userRepository.fetchUsersByIds(requestedUserIds).shouldSucceed() @@ -185,10 +190,6 @@ class UserRepositoryTest { .suspendFunction(arrangement.userDetailsApi::getMultipleUsers) .with(any()) .wasNotInvoked() - verify(arrangement.userDetailsApi) - .suspendFunction(arrangement.userDetailsApi::getUserInfo) - .with(any()) - .wasNotInvoked() } @Test @@ -199,7 +200,7 @@ class UserRepositoryTest { UserId(value = "id2", domain = "domain2") ) val (arrangement, userRepository) = Arrangement() - .withSuccessfulGetUsersInfo() + .withSuccessfulGetMultipleUsersApiRequest(ListUsersDTO(usersFailed = emptyList(), listOf(TestUser.USER_PROFILE_DTO))) .arrange() assertTrue { requestedUserIds.none { it.domain == arrangement.selfUserId.domain } } // when @@ -208,7 +209,7 @@ class UserRepositoryTest { verify(arrangement.userDetailsApi) .suspendFunction(arrangement.userDetailsApi::getMultipleUsers) .with(any()) - .wasNotInvoked() + .wasInvoked(exactly = once) } @Test @@ -415,6 +416,53 @@ class UserRepositoryTest { } } + @Test + fun givenThereAreUsersWithoutMetadata_whenSyncingUsers_thenShouldUpdateThem() = runTest { + // given + val (arrangement, userRepository) = Arrangement() + .withDaoReturningNoMetadataUsers(listOf(TestUser.ENTITY.copy(name = null))) + .withSuccessfulGetMultipleUsersApiRequest(ListUsersDTO(emptyList(), listOf(TestUser.USER_PROFILE_DTO))) + .arrange() + + // when + userRepository.syncUsersWithoutMetadata() + .shouldSucceed() + + // then + verify(arrangement.userDetailsApi) + .suspendFunction(arrangement.userDetailsApi::getMultipleUsers) + .with(any()) + .wasInvoked(exactly = once) + verify(arrangement.userDAO) + .suspendFunction(arrangement.userDAO::upsertUsers) + .with(matching { + it.first().name != null + }) + .wasInvoked(exactly = once) + } + + @Test + fun givenThereAreNOUsersWithoutMetadata_whenSyncingUsers_thenShouldNOTUpdateThem() = runTest { + // given + val (arrangement, userRepository) = Arrangement() + .withDaoReturningNoMetadataUsers(listOf()) + .arrange() + + // when + userRepository.syncUsersWithoutMetadata() + .shouldSucceed() + + // then + verify(arrangement.userDetailsApi) + .suspendFunction(arrangement.userDetailsApi::getMultipleUsers) + .with(any()) + .wasNotInvoked() + verify(arrangement.userDAO) + .suspendFunction(arrangement.userDAO::upsertUsers) + .with(any()) + .wasNotInvoked() + } + // TODO other UserRepository tests private class Arrangement { @@ -504,6 +552,12 @@ class UserRepositoryTest { .then { flowOf(userEntity) } } + fun withDaoReturningNoMetadataUsers(userEntity: List = emptyList()) = apply { + given(userDAO).suspendFunction(userDAO::getUsersWithoutMetadata) + .whenInvoked() + .then { userEntity } + } + fun withGetSelfUserId() = apply { given(metadataDAO) .suspendFunction(metadataDAO::valueByKey) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/user/RefreshUsersWithoutMetadataUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/user/RefreshUsersWithoutMetadataUseCaseTest.kt new file mode 100644 index 00000000000..778365dbaa0 --- /dev/null +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/user/RefreshUsersWithoutMetadataUseCaseTest.kt @@ -0,0 +1,64 @@ +/* + * Wire + * Copyright (C) 2023 Wire Swiss GmbH + * + * 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.wire.kalium.logic.feature.user + +import com.wire.kalium.logic.CoreFailure +import com.wire.kalium.logic.data.user.UserRepository +import com.wire.kalium.logic.feature.publicuser.RefreshUsersWithoutMetadataUseCaseImpl +import com.wire.kalium.logic.functional.Either +import io.mockative.Mock +import io.mockative.classOf +import io.mockative.given +import io.mockative.mock +import io.mockative.once +import io.mockative.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.runTest +import kotlin.test.Test + +@ExperimentalCoroutinesApi +class RefreshUsersWithoutMetadataUseCaseTest { + + @Test + fun givenUsersWithoutMetadata_whenRefreshing_thenShouldRefreshThoseUsersInformation() = runTest { + val (arrangement, refreshUsersWithoutMetadata) = Arrangement() + .withResponse() + .arrange() + + refreshUsersWithoutMetadata() + + verify(arrangement.userRepository) + .suspendFunction(arrangement.userRepository::syncUsersWithoutMetadata) + .wasInvoked(once) + } + + private class Arrangement { + @Mock + val userRepository = mock(classOf()) + + fun withResponse(result: Either = Either.Right(Unit)) = apply { + given(userRepository) + .suspendFunction(userRepository::syncUsersWithoutMetadata) + .whenInvoked() + .thenReturn(result) + } + + fun arrange() = this to RefreshUsersWithoutMetadataUseCaseImpl(userRepository) + } +} diff --git a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Conversations.sq b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Conversations.sq index 5a3917da047..ca438e1b13a 100644 --- a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Conversations.sq +++ b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Conversations.sq @@ -2,6 +2,7 @@ import com.wire.kalium.persistence.dao.ConversationEntity; import com.wire.kalium.persistence.dao.QualifiedIDEntity; import kotlin.collections.List; import kotlinx.datetime.Instant; +import kotlin.Boolean; CREATE TABLE Conversation ( qualified_id TEXT AS QualifiedIDEntity NOT NULL PRIMARY KEY, @@ -31,7 +32,8 @@ CREATE TABLE Conversation ( mls_last_keying_material_update_date INTEGER AS Instant DEFAULT 0 NOT NULL, mls_cipher_suite TEXT AS ConversationEntity.CipherSuite NOT NULL, receipt_mode TEXT AS ConversationEntity.ReceiptMode DEFAULT "DISABLED" NOT NULL, - guest_room_link TEXT + guest_room_link TEXT, + incomplete_metadata INTEGER AS Boolean NOT NULL DEFAULT 0 ); -- Optimise comparisons and sorting by dates: @@ -47,8 +49,8 @@ deleteConversation: DELETE FROM Conversation WHERE qualified_id = ?; insertConversation: -INSERT INTO Conversation(qualified_id, name, type, team_id, mls_group_id, mls_group_state, mls_epoch, protocol, muted_status, muted_time, creator_id, last_modified_date, last_notified_date, access_list, access_role_list, last_read_date, mls_last_keying_material_update_date, mls_cipher_suite, receipt_mode) -VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +INSERT INTO Conversation(qualified_id, name, type, team_id, mls_group_id, mls_group_state, mls_epoch, protocol, muted_status, muted_time, creator_id, last_modified_date, last_notified_date, access_list, access_role_list, last_read_date, mls_last_keying_material_update_date, mls_cipher_suite, receipt_mode, incomplete_metadata) +VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(qualified_id) DO UPDATE SET name = excluded.name, type = excluded.type, @@ -66,7 +68,8 @@ last_notified_date = last_notified_date, last_read_date = last_read_date, mls_last_keying_material_update_date = excluded.mls_last_keying_material_update_date, mls_cipher_suite = excluded.mls_cipher_suite, -receipt_mode = excluded.receipt_mode; +receipt_mode = excluded.receipt_mode, +incomplete_metadata = excluded.incomplete_metadata; updateConversation: UPDATE Conversation diff --git a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq index b3b0b54f21b..5c9a72bb310 100644 --- a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq +++ b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq @@ -3,8 +3,8 @@ import com.wire.kalium.persistence.dao.ConnectionEntity; import com.wire.kalium.persistence.dao.QualifiedIDEntity; import com.wire.kalium.persistence.dao.UserAvailabilityStatusEntity; import com.wire.kalium.persistence.dao.UserTypeEntity; -import kotlin.Int; import kotlin.Boolean; +import kotlin.Int; CREATE TABLE User ( qualified_id TEXT AS QualifiedIDEntity NOT NULL PRIMARY KEY, @@ -20,7 +20,8 @@ CREATE TABLE User ( user_availability_status TEXT AS UserAvailabilityStatusEntity NOT NULL DEFAULT 'NONE', user_type TEXT AS UserTypeEntity NOT NULL DEFAULT 'STANDARD', bot_service TEXT AS BotEntity, - deleted INTEGER AS Boolean NOT NULL DEFAULT 0 + deleted INTEGER AS Boolean NOT NULL DEFAULT 0, + incomplete_metadata INTEGER AS Boolean NOT NULL DEFAULT 0 ); CREATE INDEX user_team_index ON User(team); @@ -31,8 +32,8 @@ deleteUser: DELETE FROM User WHERE qualified_id = ?; insertUser: -INSERT INTO User(qualified_id, name, handle, email, phone, accent_id, team, connection_status, preview_asset_id, complete_asset_id, user_type, bot_service, deleted) -VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +INSERT INTO User(qualified_id, name, handle, email, phone, accent_id, team, connection_status, preview_asset_id, complete_asset_id, user_type, bot_service, deleted, incomplete_metadata) +VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(qualified_id) DO UPDATE SET name = excluded.name, handle = excluded.handle, @@ -45,11 +46,12 @@ preview_asset_id = excluded.preview_asset_id, complete_asset_id = excluded.complete_asset_id, user_type = excluded.user_type, bot_service = excluded.bot_service, -deleted = excluded.deleted; +deleted = excluded.deleted, +incomplete_metadata = excluded.incomplete_metadata; insertOrIgnoreUser: -INSERT OR IGNORE INTO User(qualified_id, name, handle, email, phone, accent_id, team, connection_status, preview_asset_id, complete_asset_id, user_type, bot_service, deleted) -VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?); +INSERT OR IGNORE INTO User(qualified_id, name, handle, email, phone, accent_id, team, connection_status, preview_asset_id, complete_asset_id, user_type, bot_service, deleted, incomplete_metadata) +VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?); updateUser: UPDATE User @@ -147,3 +149,7 @@ SELECT * FROM User AS user updateUserDisplayName: UPDATE User SET name = ? WHERE qualified_id = ?; + +selectUsersWithoutMetadata: +SELECT * FROM User AS user +WHERE deleted = 0 AND incomplete_metadata = 1; diff --git a/persistence/src/commonMain/db_user/migrations/38.sqm b/persistence/src/commonMain/db_user/migrations/38.sqm new file mode 100644 index 00000000000..440373388ae --- /dev/null +++ b/persistence/src/commonMain/db_user/migrations/38.sqm @@ -0,0 +1,4 @@ +import kotlin.Boolean; + +ALTER TABLE Conversation ADD COLUMN incomplete_metadata INTEGER AS Boolean NOT NULL DEFAULT 0; +ALTER TABLE User ADD COLUMN incomplete_metadata INTEGER AS Boolean NOT NULL DEFAULT 0; diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConnectionDAOImpl.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConnectionDAOImpl.kt index f57f0189fa7..b4136e94379 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConnectionDAOImpl.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConnectionDAOImpl.kt @@ -67,7 +67,8 @@ private class ConnectionMapper { user_availability_status: UserAvailabilityStatusEntity?, user_type: UserTypeEntity?, bot_service: BotEntity?, - deleted: Boolean? + deleted: Boolean?, + incomplete_metadata: Boolean? ): ConnectionEntity = ConnectionEntity( conversationId = conversation_id, from = from_id, @@ -92,6 +93,7 @@ private class ConnectionMapper { userType = user_type.requireField("user_type"), botService = bot_service, deleted = deleted.requireField("deleted"), + hasIncompleteMetadata = incomplete_metadata.requireField("incomplete_metadata") ) else null ) diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConversationDAO.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConversationDAO.kt index c0183aba0f7..ccde1bd4d3d 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConversationDAO.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConversationDAO.kt @@ -40,7 +40,8 @@ data class ConversationEntity( val access: List, val accessRole: List, val receiptMode: ReceiptMode, - val guestRoomLink: String? = null + val guestRoomLink: String? = null, + val hasIncompleteMetadata: Boolean = false, ) { enum class AccessRole { TEAM_MEMBER, NON_TEAM_MEMBER, GUEST, SERVICE, EXTERNAL; } diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConversationDAOImpl.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConversationDAOImpl.kt index 2cec25b7c50..8f20dd5466c 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConversationDAOImpl.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/ConversationDAOImpl.kt @@ -264,7 +264,8 @@ class ConversationDAOImpl( else Instant.fromEpochMilliseconds(MLS_DEFAULT_LAST_KEY_MATERIAL_UPDATE_MILLI), if (protocolInfo is ConversationEntity.ProtocolInfo.MLS) protocolInfo.cipherSuite else MLS_DEFAULT_CIPHER_SUITE, - receiptMode + receiptMode, + hasIncompleteMetadata ) } } diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt index 4cdceac1a41..3d4dc46b75d 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt @@ -52,7 +52,8 @@ data class UserEntity( val availabilityStatus: UserAvailabilityStatusEntity, val userType: UserTypeEntity, val botService: BotEntity?, - val deleted: Boolean + val deleted: Boolean, + val hasIncompleteMetadata: Boolean = false ) data class UserEntityMinimized( @@ -192,4 +193,5 @@ interface UserDAO { suspend fun getUsersNotInConversationByHandle(conversationId: QualifiedIDEntity, handle: String): Flow> suspend fun getAllUsersByTeam(teamId: String): List suspend fun updateUserDisplayName(selfUserId: QualifiedIDEntity, displayName: String) + suspend fun getUsersWithoutMetadata(): List } diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt index 262cf150bec..e7d7ff5b854 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt @@ -49,7 +49,8 @@ class UserMapper { availabilityStatus = user.user_availability_status, userType = user.user_type, botService = user.bot_service, - deleted = user.deleted + deleted = user.deleted, + hasIncompleteMetadata = user.incomplete_metadata ) } @@ -69,6 +70,7 @@ class UserMapper { userType: UserTypeEntity, botService: BotEntity?, deleted: Boolean, + hasIncompleteMetadata: Boolean, id: String?, teamName: String?, teamIcon: String?, @@ -87,7 +89,8 @@ class UserMapper { availabilityStatus = userAvailabilityStatus, userType = userType, botService = botService, - deleted = deleted + deleted = deleted, + hasIncompleteMetadata = hasIncompleteMetadata ) val teamEntity = if (team != null && teamName != null && teamIcon != null) { @@ -134,7 +137,8 @@ class UserDAOImpl internal constructor( user.completeAssetId, user.userType, user.botService, - user.deleted + user.deleted, + user.hasIncompleteMetadata ) } @@ -154,7 +158,8 @@ class UserDAOImpl internal constructor( user.completeAssetId, user.userType, user.botService, - user.deleted + user.deleted, + user.hasIncompleteMetadata ) } } @@ -190,7 +195,8 @@ class UserDAOImpl internal constructor( user.completeAssetId, user.userType, user.botService, - user.deleted + user.deleted, + user.hasIncompleteMetadata ) } } @@ -228,7 +234,8 @@ class UserDAOImpl internal constructor( user.completeAssetId, user.userType, user.botService, - user.deleted + user.deleted, + user.hasIncompleteMetadata ) } } @@ -254,7 +261,8 @@ class UserDAOImpl internal constructor( user.completeAssetId, user.userType, user.botService, - user.deleted + user.deleted, + user.hasIncompleteMetadata ) } } @@ -385,4 +393,10 @@ class UserDAOImpl internal constructor( override suspend fun updateUserDisplayName(selfUserId: QualifiedIDEntity, displayName: String) = withContext(queriesContext) { userQueries.updateUserDisplayName(displayName, selfUserId) } + + override suspend fun getUsersWithoutMetadata() = withContext(queriesContext) { + userQueries.selectUsersWithoutMetadata() + .executeAsList() + .map(mapper::toModel) + } } diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt index f6739ad33a9..24caed6e7cf 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt @@ -610,6 +610,19 @@ class UserDAOTest : BaseDatabaseTest() { assertEquals(expectedNewDisplayName, persistedUser?.name) } + @Test + fun givenExistingUserWithoutMetadata_whenQueryingThem_thenShouldReturnUsersWithoutMetadata() = runTest(dispatcher) { + // given + db.userDAO.insertUser(user1.copy(name = null, handle = null, hasIncompleteMetadata = true)) + + // when + val usersWithoutMetadata = db.userDAO.getUsersWithoutMetadata() + + // then + assertEquals(1, usersWithoutMetadata.size) + assertEquals(user1.id, usersWithoutMetadata.first().id) + } + private companion object { val USER_ENTITY_1 = newUserEntity(QualifiedIDEntity("1", "wire.com")) val USER_ENTITY_2 = newUserEntity(QualifiedIDEntity("2", "wire.com"))