Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: recover when getting error for updating connection status [WPB-16146] 🍒 #3303

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ import com.wire.kalium.common.error.wrapStorageRequest
import com.wire.kalium.network.api.authenticated.connection.ConnectionDTO
import com.wire.kalium.network.api.authenticated.connection.ConnectionStateDTO
import com.wire.kalium.network.api.base.authenticated.connection.ConnectionApi
import com.wire.kalium.network.exceptions.KaliumException
import com.wire.kalium.network.exceptions.isBadConnectionStatusUpdate
import com.wire.kalium.persistence.dao.ConnectionDAO
import com.wire.kalium.persistence.dao.ConnectionEntity
import com.wire.kalium.persistence.dao.UserDAO
Expand Down Expand Up @@ -129,14 +131,29 @@ internal class ConnectionDataSource(
}.map { }
}

private suspend fun updateRemoteConnectionStatus(userId: UserId, connectionState: ConnectionState): Either<CoreFailure, ConnectionDTO> {
suspend fun updateRemoteConnectionStatus(userId: UserId, connectionState: ConnectionState): Either<CoreFailure, ConnectionDTO> {
val isValidConnectionState = isValidConnectionState(connectionState)
val newConnectionStatus = connectionStatusMapper.toApiModel(connectionState)
if (!isValidConnectionState || newConnectionStatus == null) {
return Either.Left(InvalidMappingFailure)
}

return wrapApiRequest { connectionApi.updateConnection(userId.toApi(), newConnectionStatus) }
return wrapApiRequest {
connectionApi.updateConnection(userId.toApi(), newConnectionStatus)
}.onFailure { networkFailure ->
if (networkFailure is NetworkFailure.ServerMiscommunication &&
networkFailure.kaliumException is KaliumException.InvalidRequestError &&
(networkFailure.kaliumException as KaliumException.InvalidRequestError).isBadConnectionStatusUpdate()
) {
kaliumLogger.e(
"Failed to update connection status for ${userId.toLogString()}" +
" to $connectionState, probably due to internal data error recovering"
)
wrapApiRequest { connectionApi.userConnectionInfo(userId.toApi()) }.flatMap {
persistConnection(connectionMapper.fromApiToModel(it))
}
}
}
}

override suspend fun updateConnectionStatus(userId: UserId, connectionState: ConnectionState): Either<CoreFailure, Connection> =
Expand Down Expand Up @@ -276,7 +293,7 @@ internal class ConnectionDataSource(
}

override suspend fun getConnection(conversationId: ConversationId) = wrapStorageRequest {
connectionDAO.getConnection(conversationId.toDao())?.let { connectionMapper.fromDaoToConversationDetails(it) }
connectionDAO.getConnection(conversationId.toDao())?.let { connectionMapper.fromDaoToConversationDetails(it) }
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.wire.kalium.common.error.StorageFailure
import com.wire.kalium.logic.data.conversation.ConversationRepository
import com.wire.kalium.logic.data.id.SelfTeamIdProvider
import com.wire.kalium.logic.data.id.TeamId
import com.wire.kalium.logic.data.id.toApi
import com.wire.kalium.logic.data.id.toDao
import com.wire.kalium.logic.data.id.toModel
import com.wire.kalium.logic.data.user.ConnectionState
Expand All @@ -41,6 +42,7 @@ import com.wire.kalium.network.api.authenticated.connection.ConnectionResponse
import com.wire.kalium.network.api.authenticated.connection.ConnectionStateDTO
import com.wire.kalium.network.api.base.authenticated.userDetails.UserDetailsApi
import com.wire.kalium.network.api.model.ConversationId
import com.wire.kalium.network.api.model.ErrorResponse
import com.wire.kalium.network.api.model.FederationUnreachableResponse
import com.wire.kalium.network.api.model.LegalHoldStatusDTO
import com.wire.kalium.network.api.model.QualifiedID
Expand Down Expand Up @@ -447,6 +449,41 @@ class ConnectionRepositoryTest {
}.wasInvoked(exactly = 0)
}

@Test
fun givenBadConnectionRequestError_whenUpdatingRemote_thenRecoverByGettingTheCorrectStatus() = runTest {
// given
val userId = UserId("user_id", "domain_id")
val expectedRecoveryResponse = Arrangement.stubConnectionOne.copy(
qualifiedToId = userId.toApi(),
status = ConnectionStateDTO.BLOCKED
)
val (arrangement, connectionRepository) = Arrangement()
.withErrorUpdatingConnectionStatusResponse(
userId = userId.toApi(),
exception = KaliumException.InvalidRequestError(
ErrorResponse(
message = "bad connection update",
code = 403,
label = "bad-conn-update"
)
)
).withUserConnectionInfo(
result = NetworkResponse.Success(
expectedRecoveryResponse,
emptyMap(),
200
),
userId = EqualsMatcher(userId.toApi())
)
.arrange()

connectionRepository.updateRemoteConnectionStatus(userId, ConnectionState.ACCEPTED)

coVerify {
arrangement.connectionDAO.insertConnection(ConnectionMapperImpl().fromApiToDao(expectedRecoveryResponse))
}.wasInvoked(exactly = 1)
}

private class Arrangement :
MemberDAOArrangement by MemberDAOArrangementImpl() {
@Mock
Expand Down Expand Up @@ -622,6 +659,17 @@ class ConnectionRepositoryTest {
}.returns(NetworkResponse.Error(exception))
}

suspend fun withUserConnectionInfo(
result: NetworkResponse<ConnectionDTO>,
userId: Matcher<com.wire.kalium.network.api.model.UserId>
) = apply {
coEvery {
connectionApi.userConnectionInfo(
matches { userId.matches(it) }
)
}.returns(result)
}

suspend fun withDeleteConnectionDataAndConversation(conversationId: QualifiedIDEntity): Arrangement = apply {
coEvery {
connectionDAO.deleteConnectionDataAndConversation(eq(conversationId))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import com.wire.kalium.network.api.model.UserId
import com.wire.kalium.network.utils.NetworkResponse

interface ConnectionApi {

suspend fun fetchSelfUserConnections(pagingState: String?): NetworkResponse<ConnectionResponse>
suspend fun createConnection(userId: UserId): NetworkResponse<ConnectionDTO>
suspend fun updateConnection(userId: UserId, connectionStatus: ConnectionStateDTO): NetworkResponse<ConnectionDTO>
suspend fun userConnectionInfo(userId: UserId): NetworkResponse<ConnectionDTO>
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.wire.kalium.network.api.model.PaginationRequest
import com.wire.kalium.network.api.model.UserId
import com.wire.kalium.network.utils.NetworkResponse
import com.wire.kalium.network.utils.wrapKaliumResponse
import io.ktor.client.request.get
import io.ktor.client.request.post
import io.ktor.client.request.put
import io.ktor.client.request.setBody
Expand Down Expand Up @@ -56,6 +57,10 @@ internal open class ConnectionApiV0 internal constructor(private val authenticat
}
}

override suspend fun userConnectionInfo(userId: UserId): NetworkResponse<ConnectionDTO> = wrapKaliumResponse {
httpClient.get("$PATH_CONNECTIONS_ENDPOINTS/${userId.domain}/${userId.value}")
}

protected companion object {
const val PATH_CONNECTIONS = "list-connections"
const val PATH_CONNECTIONS_ENDPOINTS = "connections"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.wire.kalium.network.api.model.ErrorResponse
import com.wire.kalium.network.api.model.FederationConflictResponse
import com.wire.kalium.network.api.model.FederationUnreachableResponse
import com.wire.kalium.network.exceptions.NetworkErrorLabel.ACCESS_DENIED
import com.wire.kalium.network.exceptions.NetworkErrorLabel.BAD_CONNECTION_UPDATE
import com.wire.kalium.network.exceptions.NetworkErrorLabel.BAD_REQUEST
import com.wire.kalium.network.exceptions.NetworkErrorLabel.BLACKLISTED_EMAIL
import com.wire.kalium.network.exceptions.NetworkErrorLabel.DOMAIN_BLOCKED_FOR_REGISTRATION
Expand Down Expand Up @@ -234,6 +235,10 @@ fun KaliumException.InvalidRequestError.isWrongConversationPassword(): Boolean {
(errorResponse.label == BAD_REQUEST && errorResponse.message.contains("password"))
}

fun KaliumException.InvalidRequestError.isBadConnectionStatusUpdate(): Boolean {
return errorResponse.label == BAD_CONNECTION_UPDATE
}

val KaliumException.InvalidRequestError.authenticationCodeFailure: AuthenticationCodeFailure?
get() = AuthenticationCodeFailure.entries.firstOrNull {
errorResponse.label == it.responseLabel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ internal object NetworkErrorLabel {
const val FEDERATION_NOT_ENABLED = "federation-not-enabled"
const val FEDERATION_UNREACHABLE_DOMAINS = "federation-unreachable-domains-error"

// connection
const val BAD_CONNECTION_UPDATE = "bad-conn-update"
object KaliumCustom {
const val MISSING_REFRESH_TOKEN = "missing-refresh_token"
const val MISSING_NONCE = "missing-nonce"
Expand Down
Loading