Merge remote-tracking branch 'origin/release/candidate' into fix/pers…

…ist_proper_team_member_usertype_rc

gradle/libs.versions.toml

@@ -36,7 +36,7 @@ pbandk = "0.14.2"
 turbine = "1.0.0"
 avs = "9.2.22"
 jna = "5.6.0"
-core-crypto = "1.0.0-pre.6+v1-schemafix-003"
+core-crypto = "1.0.0-pre.6+v1-schemafix-007"
 core-crypto-multiplatform = "0.6.0-rc.3-multiplatform-pre1"
 completeKotlin = "1.1.0"
 desugar-jdk = "1.1.5"

logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt

@@ -24,6 +24,7 @@ import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.functional.Either
 import com.wire.kalium.network.exceptions.KaliumException
 import com.wire.kalium.network.exceptions.isFederationDenied
+import com.wire.kalium.network.exceptions.isFederationNotEnabled
 import com.wire.kalium.network.utils.NetworkResponse
 import io.ktor.utils.io.errors.IOException
 import kotlinx.coroutines.flow.Flow
@@ -147,6 +148,7 @@ sealed class NetworkFailure : CoreFailure {
 
         data class General(val label: String) : FederatedBackendFailure()
         data class FederationDenied(val label: String) : FederatedBackendFailure()
+        data class FederationNotEnabled(val label: String) : FederatedBackendFailure()
 
         data class ConflictingBackends(override val domains: List<String>) : FederatedBackendFailure(), RetryableFailure
 
@@ -200,6 +202,8 @@ internal inline fun <T : Any> wrapApiRequest(networkCall: () -> NetworkResponse<
                 exception is KaliumException.FederationError -> {
                     if (exception.isFederationDenied()) {
                         Either.Left(NetworkFailure.FederatedBackendFailure.FederationDenied(exception.errorResponse.label))
+                    } else if (exception.isFederationNotEnabled()) {
+                        Either.Left(NetworkFailure.FederatedBackendFailure.FederationNotEnabled(exception.errorResponse.label))
                     } else {
                         Either.Left(NetworkFailure.FederatedBackendFailure.General(exception.errorResponse.label))
                     }

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt

@@ -18,7 +18,9 @@
 
 package com.wire.kalium.logic.data.user
 
+import com.wire.kalium.logger.obfuscateDomain
 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.MemberMapper
 import com.wire.kalium.logic.data.conversation.Recipient
@@ -37,12 +39,12 @@ import com.wire.kalium.logic.data.team.Team
 import com.wire.kalium.logic.data.team.TeamMapper
 import com.wire.kalium.logic.data.user.type.DomainUserTypeMapper
 import com.wire.kalium.logic.data.user.type.UserEntityTypeMapper
-import com.wire.kalium.logic.data.user.type.isFederated
 import com.wire.kalium.logic.di.MapperProvider
 import com.wire.kalium.logic.failure.SelfUserDeleted
 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.flatMapLeft
 import com.wire.kalium.logic.functional.fold
 import com.wire.kalium.logic.functional.foldToEitherWhileRight
 import com.wire.kalium.logic.functional.getOrNull
@@ -117,6 +119,7 @@ internal interface UserRepository {
      * when backends stops federating.
      */
     suspend fun defederateUser(userId: UserId): Either<CoreFailure, Unit>
+
     // TODO: move to migration repo
     suspend fun insertUsersIfUnknown(users: List<User>): Either<StorageFailure, Unit>
     suspend fun fetchUserInfo(userId: UserId): Either<CoreFailure, Unit>
@@ -155,12 +158,12 @@ internal class UserDataSource internal constructor(
 ) : UserRepository {
 
     /**
-     * In case of federated users, we need to refresh their info every time.
-     * Since the current backend implementation at wire does not emit user events across backends.
+     * Stores the last time a user's details were fetched from remote.
      *
-     * This is an in-memory cache, to help avoid unnecessary requests in a time window.
+     * @see Event.User.Update
+     * @see USER_DETAILS_MAX_AGE
      */
-    private val federatedUsersExpirationCache = ConcurrentMap<UserId, Instant>()
+    private val userDetailsRefreshInstantCache = ConcurrentMap<UserId, Instant>()
 
     override suspend fun fetchSelfUser(): Either<CoreFailure, Unit> = wrapApiRequest { selfApi.getSelfInfo() }
         .flatMap { userDTO ->
@@ -186,20 +189,25 @@ internal class UserDataSource internal constructor(
             .map { userEntity ->
                 userEntity?.let { publicUserMapper.fromUserEntityToOtherUser(userEntity) }
             }.onEach { otherUser ->
-                processFederatedUserRefresh(userId, otherUser)
+                if (otherUser != null) {
+                    refreshUserDetailsIfNeeded(userId)
+                }
             }
 
     /**
-     * Only in case of federated users and if it's expired or not cached, we fetch and refresh the user info.
+     * Only refresh user profiles if it wasn't fetched recently.
+     *
+     * @see userDetailsRefreshInstantCache
+     * @see USER_DETAILS_MAX_AGE
      */
-    private suspend fun processFederatedUserRefresh(userId: UserId, otherUser: OtherUser?) {
-        if (otherUser != null && otherUser.userType.isFederated()
-            && federatedUsersExpirationCache[userId]?.let { DateTimeUtil.currentInstant() > it } != false
-        ) {
+    private suspend fun refreshUserDetailsIfNeeded(userId: UserId) {
+        val now = DateTimeUtil.currentInstant()
+        val wasFetchedRecently = userDetailsRefreshInstantCache[userId]?.let { now < it + USER_DETAILS_MAX_AGE } ?: false
+        if (!wasFetchedRecently) {
             fetchUserInfo(userId).also {
-                kaliumLogger.d("Federated user, refreshing user info from API after $FEDERATED_USER_TTL")
+                kaliumLogger.d("Federated user, refreshing user info from API after $USER_DETAILS_MAX_AGE")
             }
-            federatedUsersExpirationCache[userId] = DateTimeUtil.currentInstant().plus(FEDERATED_USER_TTL)
+            userDetailsRefreshInstantCache[userId] = now
         }
     }
 
@@ -237,6 +245,24 @@ internal class UserDataSource internal constructor(
                         )
                     }
                 }
+                .flatMapLeft { error ->
+                    if (error is NetworkFailure.FederatedBackendFailure.FederationNotEnabled) {
+                        val domains = qualifiedUserIdList
+                            .filterNot { it.domain == selfUserId.domain }
+                            .map { it.domain.obfuscateDomain() }
+                            .toSet()
+                        val domainNames = domains.joinToString(separator = ", ")
+                        kaliumLogger.e("User ids contains different domains when federation is not enabled by backend: $domainNames")
+                        wrapApiRequest {
+                            userDetailsApi.getMultipleUsers(
+                                ListUserRequest.qualifiedIds(qualifiedUserIdList.filter { it.domain == selfUserId.domain }
+                                    .map { userId -> userId.toApi() })
+                            )
+                        }
+                    } else {
+                        Either.Left(error)
+                    }
+                }
                 .flatMap { listUserProfileDTO ->
                     if (listUserProfileDTO.usersFailed.isNotEmpty()) {
                         kaliumLogger.d("Handling ${listUserProfileDTO.usersFailed.size} failed users")
@@ -503,7 +529,16 @@ internal class UserDataSource internal constructor(
 
     companion object {
         internal const val SELF_USER_ID_KEY = "selfUserID"
-        internal val FEDERATED_USER_TTL = 5.minutes
+
+        /**
+         * 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
     }
 }

logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt

@@ -36,6 +36,7 @@ import com.wire.kalium.logic.framework.TestUser.LIST_USERS_DTO
 import com.wire.kalium.logic.functional.Either
 import com.wire.kalium.logic.functional.getOrNull
 import com.wire.kalium.logic.sync.receiver.UserEventReceiverTest
+import com.wire.kalium.logic.test_util.TestNetworkException.federationNotEnabled
 import com.wire.kalium.logic.util.shouldFail
 import com.wire.kalium.logic.util.shouldSucceed
 import com.wire.kalium.network.api.base.authenticated.TeamsApi
@@ -201,6 +202,26 @@ class UserRepositoryTest {
             .wasNotInvoked()
     }
 
+    @Test
+    fun givenAnUserIdListWithDifferentDomain_whenApiReturnsFederationDisabledError_thenShouldTryToFetchOnlyUsersWithSelfDomain() = runTest {
+        // given
+        val requestedUserIds = setOf(TestUser.OTHER_USER_ID, TestUser.OTHER_FEDERATED_USER_ID)
+        val (arrangement, userRepository) = Arrangement()
+            .withGetMultipleUsersApiRequestFederationNotEnabledError()
+            .arrange()
+        // when
+        userRepository.fetchUsersByIds(requestedUserIds).shouldFail()
+        // then
+        verify(arrangement.userDetailsApi)
+            .suspendFunction(arrangement.userDetailsApi::getMultipleUsers)
+            .with(eq(QualifiedUserIdListRequest(requestedUserIds.map { it.toApi() }.toList())))
+            .wasInvoked(exactly = once)
+        verify(arrangement.userDetailsApi)
+            .suspendFunction(arrangement.userDetailsApi::getMultipleUsers)
+            .with(eq(QualifiedUserIdListRequest(listOf(TestUser.OTHER_USER_ID.toApi()))))
+            .wasInvoked(exactly = once)
+    }
+
     @Test
     fun givenAnEmptyUserIdListFromSameDomainAsSelf_whenFetchingUsers_thenShouldNotFetchMultipleUsersAndSucceed() = runTest {
         // given
@@ -356,9 +377,9 @@ class UserRepositoryTest {
     }
 
     @Test
-    fun givenAKnownNOTFederatedUser_whenGettingFromDb_thenShouldNotRefreshItsDataFromAPI() = runTest {
+    fun givenAKnownUser_whenGettingFromDb_thenShouldRefreshItsDataFromAPI() = runTest {
         val (arrangement, userRepository) = Arrangement()
-            .withUserDaoReturning(TestUser.ENTITY.copy(userType = UserTypeEntity.STANDARD))
+            .withUserDaoReturning(TestUser.ENTITY)
             .withSuccessfulGetUsersInfo()
             .arrange()
 
@@ -368,22 +389,22 @@ class UserRepositoryTest {
             verify(arrangement.userDetailsApi)
                 .suspendFunction(arrangement.userDetailsApi::getUserInfo)
                 .with(any())
-                .wasNotInvoked()
+                .wasInvoked(exactly = once)
             verify(arrangement.userDAO)
                 .suspendFunction(arrangement.userDAO::upsertTeamMembers)
                 .with(any())
-                .wasNotInvoked()
+                .wasInvoked(exactly = once)
             verify(arrangement.userDAO)
                 .suspendFunction(arrangement.userDAO::upsertUsers)
                 .with(any())
-                .wasNotInvoked()
+                .wasInvoked(exactly = once)
         }
     }
 
     @Test
-    fun givenAKnownFederatedUser_whenGettingFromDbAndCacheValid_thenShouldNOTRefreshItsDataFromAPI() = runTest {
+    fun givenAKnownUser_whenGettingFromDbAndCacheValid_thenShouldNOTRefreshItsDataFromAPI() = runTest {
         val (arrangement, userRepository) = Arrangement()
-            .withUserDaoReturning(TestUser.ENTITY.copy(userType = UserTypeEntity.FEDERATED))
+            .withUserDaoReturning(TestUser.ENTITY)
             .withSuccessfulGetUsersInfo()
             .arrange()
 
@@ -743,6 +764,13 @@ class UserRepositoryTest {
                 .thenReturn(NetworkResponse.Success(result, mapOf(), HttpStatusCode.OK.value))
         }
 
+        fun withGetMultipleUsersApiRequestFederationNotEnabledError() = apply {
+            given(userDetailsApi)
+                .suspendFunction(userDetailsApi::getMultipleUsers)
+                .whenInvokedWith(any())
+                .thenReturn(NetworkResponse.Error(federationNotEnabled))
+        }
+
         fun withUpdateDisplayNameApiRequestResponse(response: NetworkResponse<Unit>) = apply {
             given(selfApi)
                 .suspendFunction(selfApi::updateSelf)

logic/src/commonTest/kotlin/com/wire/kalium/logic/test_util/TestNetworkException.kt

@@ -130,6 +130,10 @@ object TestNetworkException {
     val guestLinkDisables = KaliumException.InvalidRequestError(
         ErrorResponse(409, "Guest links are disabled", "guest-links-disabled")
     )
+
+    val federationNotEnabled = KaliumException.FederationError(
+        ErrorResponse(400, "no federator configured", "federation-not-enabled")
+    )
 }
 
 object TestNetworkResponseError {

network/src/commonMain/kotlin/com/wire/kalium/network/api/base/model/ErrorResponse.kt

@@ -28,7 +28,7 @@ data class ErrorResponse(
     @SerialName("label") val label: String,
     @SerialName("data") val cause: Cause? = null,
 ) {
-    fun isFederationError() = cause?.type == "federation"
+    fun isFederationError() = cause?.type == "federation" || label.contains("federation")
 }
 
 @Serializable

network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/KaliumException.kt

@@ -32,6 +32,7 @@ import com.wire.kalium.network.exceptions.NetworkErrorLabel.BLACKLISTED_EMAIL
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.DOMAIN_BLOCKED_FOR_REGISTRATION
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.FEDERATION_DENIED
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.FEDERATION_FAILURE
+import com.wire.kalium.network.exceptions.NetworkErrorLabel.FEDERATION_NOT_ENABLED
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.GUEST_LINKS_DISABLED
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.HANDLE_EXISTS
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.INVALID_CODE
@@ -224,3 +225,4 @@ val KaliumException.InvalidRequestError.authenticationCodeFailure: Authenticatio
     }
 
 fun KaliumException.FederationError.isFederationDenied() = errorResponse.label == FEDERATION_DENIED
+fun KaliumException.FederationError.isFederationNotEnabled() = errorResponse.label == FEDERATION_NOT_ENABLED

network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/NetworkErrorLabel.kt

@@ -42,15 +42,18 @@ internal object NetworkErrorLabel {
     const val MLS_KEY_PACKAGE_REF_NOT_FOUND = "mls-key-package-ref-not-found"
     const val MLS_MISSING_GROUP_INFO = "mls-missing-group-info"
     const val UNKNOWN_CLIENT = "unknown-client"
-    const val FEDERATION_FAILURE = "federation-remote-error"
     const val NOT_TEAM_MEMBER = "no-team-member"
     const val NO_CONVERSATION = "no-conversation"
     const val NO_CONVERSATION_CODE = "no-conversation-code"
     const val GUEST_LINKS_DISABLED = "guest-links-disabled"
     const val ACCESS_DENIED = "access-denied"
     const val WRONG_CONVERSATION_PASSWORD = "invalid-conversation-password"
-    const val FEDERATION_UNREACHABLE_DOMAINS = "federation-unreachable-domains-error"
+
+    // Federation
+    const val FEDERATION_FAILURE = "federation-remote-error"
     const val FEDERATION_DENIED = "federation-denied"
+    const val FEDERATION_NOT_ENABLED = "federation-not-enabled"
+    const val FEDERATION_UNREACHABLE_DOMAINS = "federation-unreachable-domains-error"
 
     object KaliumCustom {
         const val MISSING_REFRESH_TOKEN = "missing-refresh_token"