feat: refactor, fixing tests

wireapp · May 17, 2023 · 8b02453 · 8b02453
1 parent 3770919
commit 8b02453
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 25 deletions.
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
@@ -206,16 +206,22 @@ internal class UserDataSource internal constructor(
         wrapApiRequest { userDetailsApi.getUserInfo(userId.toApi()) }
             .flatMap { userProfileDTO -> persistUsers(listOf(userProfileDTO)) }
 
-    override suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>) = 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)
+    override suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>): Either<CoreFailure, Unit> {
+        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)
         }
-        persistUsers(listUserProfileDTO.usersFound)
     }
 
     override suspend fun updateSelfEmail(email: String): Either<NetworkFailure, Boolean> = wrapApiRequest {

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
@@ -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
@@ -98,13 +100,8 @@ 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()
             .withSuccessfulGetUsersByQualifiedIdList(knownUserEntities)
@@ -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<UserId>()
         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