From 790b885dcfa3456462e0a9caf8098e2a74054988 Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Tue, 3 Dec 2024 13:09:15 +0100 Subject: [PATCH] fix: update federation flag when fetching server config [WPB-14728] (#3143) * fix: update federation flag when updating api version * fix test * remove not needed comment * fix jvm tests --- .../server/CustomServerConfigRepository.kt | 6 ++- .../server/ServerConfigRepository.kt | 24 ++++++++--- .../server/UpdateApiVersionsUseCase.kt | 2 +- .../CustomServerConfigRepositoryTest.kt | 6 +-- .../ServerConfigRepositoryTest.kt | 4 +- .../server/UpdateApiVersionUseCaseTest.kt | 26 ++++++------ .../kalium/persistence/GlobalDBBaseTest.kt | 1 + .../kalium/persistence/ServerConfiguration.sq | 4 +- .../daokaliumdb/ServerConfigurationDAO.kt | 8 ++-- .../daokaliumdb/ServerConfigurationDAOTest.kt | 41 +++++++++++++++---- .../persistence/globalDB/AccountsDAOTest.kt | 7 ++++ 11 files changed, 91 insertions(+), 38 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/server/CustomServerConfigRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/server/CustomServerConfigRepository.kt index 519f7943258..8025edc4393 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/server/CustomServerConfigRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/server/CustomServerConfigRepository.kt @@ -94,7 +94,11 @@ internal class CustomServerConfigDataSource internal constructor( val storedConfigId = serverConfigurationDAO.configByLinks(serverConfigMapper.toEntity(links))?.id if (storedConfigId != null) { // if already exists then just update it - serverConfigurationDAO.updateApiVersion(storedConfigId, metadata.commonApiVersion.version) + serverConfigurationDAO.updateServerMetaData( + id = storedConfigId, + federation = metadata.federation, + commonApiVersion = metadata.commonApiVersion.version + ) if (metadata.federation) serverConfigurationDAO.setFederationToTrue(storedConfigId) storedConfigId } else { diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/server/ServerConfigRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/server/ServerConfigRepository.kt index 54f10b8df38..625008145e6 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/server/ServerConfigRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/server/ServerConfigRepository.kt @@ -54,9 +54,9 @@ internal interface ServerConfigRepository { suspend fun fetchApiVersionAndStore(links: ServerConfig.Links): Either /** - * update the api version of a locally stored config + * update the api version and federation status of a locally stored config */ - suspend fun updateConfigApiVersion(serverConfig: ServerConfig): Either + suspend fun updateConfigMetaData(serverConfig: ServerConfig): Either /** * Return the server links and metadata for the given userId @@ -86,7 +86,11 @@ internal class ServerConfigDataSource( val storedConfigId = dao.configByLinks(serverConfigMapper.toEntity(links))?.id if (storedConfigId != null) { // if already exists then just update it - dao.updateApiVersion(storedConfigId, metadata.commonApiVersion.version) + dao.updateServerMetaData( + id = storedConfigId, + federation = metadata.federation, + commonApiVersion = metadata.commonApiVersion.version + ) if (metadata.federation) dao.setFederationToTrue(storedConfigId) storedConfigId } else { @@ -126,9 +130,17 @@ internal class ServerConfigDataSource( storeConfig(links, metaData) } - override suspend fun updateConfigApiVersion(serverConfig: ServerConfig): Either = - fetchMetadata(serverConfig.links) - .flatMap { wrapStorageRequest { dao.updateApiVersion(serverConfig.id, it.commonApiVersion.version) } } + override suspend fun updateConfigMetaData(serverConfig: ServerConfig): Either = + fetchMetadata(serverConfig.links) + .flatMap { newMetaData -> + wrapStorageRequest { + dao.updateServerMetaData( + id = serverConfig.id, + federation = newMetaData.federation, + commonApiVersion = newMetaData.commonApiVersion.version + ) + } + } override suspend fun configForUser(userId: UserId): Either = wrapStorageRequest { dao.configForUser(userId.toDao()) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/server/UpdateApiVersionsUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/server/UpdateApiVersionsUseCase.kt index 0d6eb9dea57..db4d2746aa0 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/server/UpdateApiVersionsUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/server/UpdateApiVersionsUseCase.kt @@ -80,6 +80,6 @@ class UpdateApiVersionsUseCaseImpl internal constructor( } else { null } - serverConfigRepoProvider(serverConfig, proxyCredentials).updateConfigApiVersion(serverConfig) + serverConfigRepoProvider(serverConfig, proxyCredentials).updateConfigMetaData(serverConfig) } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/configuration/CustomServerConfigRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/configuration/CustomServerConfigRepositoryTest.kt index 401b0fbc908..3737f9b8900 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/configuration/CustomServerConfigRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/configuration/CustomServerConfigRepositoryTest.kt @@ -78,7 +78,7 @@ class CustomServerConfigRepositoryTest { arrangement.serverConfigurationDAO.insert(any()) }.wasNotInvoked() coVerify { - arrangement.serverConfigurationDAO.updateApiVersion(any(), any()) + arrangement.serverConfigurationDAO.updateServerMetaData(any(), any(), any()) }.wasInvoked(exactly = once) coVerify { arrangement.serverConfigurationDAO.setFederationToTrue(any()) @@ -106,7 +106,7 @@ class CustomServerConfigRepositoryTest { arrangement.serverConfigurationDAO.insert(any()) }.wasInvoked(exactly = once) coVerify { - arrangement.serverConfigurationDAO.updateApiVersion(any(), any()) + arrangement.serverConfigurationDAO.updateServerMetaData(any(), any(), any()) }.wasNotInvoked() coVerify { arrangement.serverConfigurationDAO.setFederationToTrue(any()) @@ -145,7 +145,7 @@ class CustomServerConfigRepositoryTest { arrangement.serverConfigurationDAO.insert(any()) }.wasInvoked(exactly = once) coVerify { - arrangement.serverConfigurationDAO.updateApiVersion(any(), any()) + arrangement.serverConfigurationDAO.updateServerMetaData(any(), any(), any()) }.wasNotInvoked() coVerify { arrangement.serverConfigurationDAO.setFederationToTrue(any()) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/configuration/ServerConfigRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/configuration/ServerConfigRepositoryTest.kt index 79c65a3dd2a..8f2e1416856 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/configuration/ServerConfigRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/configuration/ServerConfigRepositoryTest.kt @@ -131,7 +131,7 @@ class ServerConfigRepositoryTest { arrangement.serverConfigDAO.insert(any()) }.wasNotInvoked() coVerify { - arrangement.serverConfigDAO.updateApiVersion(any(), any()) + arrangement.serverConfigDAO.updateServerMetaData(any(), any(), any()) }.wasInvoked(exactly = once) coVerify { arrangement.serverConfigDAO.setFederationToTrue(any()) @@ -159,7 +159,7 @@ class ServerConfigRepositoryTest { arrangement.serverConfigDAO.insert(any()) }.wasInvoked(exactly = once) coVerify { - arrangement.serverConfigDAO.updateApiVersion(any(), any()) + arrangement.serverConfigDAO.updateServerMetaData(any(), any(), any()) }.wasNotInvoked() coVerify { arrangement.serverConfigDAO.setFederationToTrue(any()) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/server/UpdateApiVersionUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/server/UpdateApiVersionUseCaseTest.kt index d883b34e4fe..3401f58963a 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/server/UpdateApiVersionUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/server/UpdateApiVersionUseCaseTest.kt @@ -77,7 +77,7 @@ class UpdateApiVersionUseCaseTest { ) ) ) - withUpdateConfigApiVersion(serverConfig1, Either.Right(Unit)) + withUpdateConfigMetaData(serverConfig1, Either.Right(Unit)) } updateApiVersionsUseCase() @@ -88,7 +88,7 @@ class UpdateApiVersionUseCaseTest { }.wasNotInvoked() coVerify { - arrangement.serverConfigRepository1.updateConfigApiVersion(eq(serverConfig1)) + arrangement.serverConfigRepository1.updateConfigMetaData(eq(serverConfig1)) }.wasInvoked(exactly = once) } @@ -111,7 +111,7 @@ class UpdateApiVersionUseCaseTest { ) ) ) - withUpdateConfigApiVersion(serverConfig1, Either.Right(Unit)) + withUpdateConfigMetaData(serverConfig1, Either.Right(Unit)) } updateApiVersionsUseCase() @@ -122,7 +122,7 @@ class UpdateApiVersionUseCaseTest { }.wasNotInvoked() coVerify { - arrangement.serverConfigRepository1.updateConfigApiVersion(any()) + arrangement.serverConfigRepository1.updateConfigMetaData(any()) }.wasInvoked(exactly = once) } @@ -145,7 +145,7 @@ class UpdateApiVersionUseCaseTest { ) ) ) - withUpdateConfigApiVersion(serverConfig1, Either.Right(Unit)) + withUpdateConfigMetaData(serverConfig1, Either.Right(Unit)) withProxyCredForUser(userId1.toDao(), ProxyCredentialsEntity("user", "pass")) } @@ -158,7 +158,7 @@ class UpdateApiVersionUseCaseTest { }.wasInvoked(exactly = once) coVerify { - arrangement.serverConfigRepository1.updateConfigApiVersion(any()) + arrangement.serverConfigRepository1.updateConfigMetaData(any()) }.wasInvoked(exactly = once) } @@ -190,8 +190,8 @@ class UpdateApiVersionUseCaseTest { ) ) ) - withUpdateConfigApiVersion(serverConfig1, Either.Right(Unit)) - withUpdateConfigApiVersion(serverConfig2, Either.Right(Unit)) + withUpdateConfigMetaData(serverConfig1, Either.Right(Unit)) + withUpdateConfigMetaData(serverConfig2, Either.Right(Unit)) withProxyCredForUser(userId2.toDao(), ProxyCredentialsEntity("user", "pass")) } @@ -208,11 +208,11 @@ class UpdateApiVersionUseCaseTest { }.wasNotInvoked() coVerify { - arrangement.serverConfigRepository1.updateConfigApiVersion(any()) + arrangement.serverConfigRepository1.updateConfigMetaData(any()) }.wasInvoked(exactly = once) coVerify { - arrangement.serverConfigRepository2.updateConfigApiVersion(any()) + arrangement.serverConfigRepository2.updateConfigMetaData(any()) }.wasInvoked(exactly = once) } @@ -249,16 +249,16 @@ class UpdateApiVersionUseCaseTest { }.returns(result) } - suspend fun withUpdateConfigApiVersion( + suspend fun withUpdateConfigMetaData( serverConfig: ServerConfig, result: Either ) { when (serverConfig.id) { serverConfig1.id -> - coEvery { serverConfigRepository1.updateConfigApiVersion(any()) } + coEvery { serverConfigRepository1.updateConfigMetaData(any()) } .returns(result) - serverConfig2.id -> coEvery { serverConfigRepository2.updateConfigApiVersion(any()) } + serverConfig2.id -> coEvery { serverConfigRepository2.updateConfigMetaData(any()) } .returns(result) else -> throw IllegalArgumentException("Unexpected server config: $serverConfig") diff --git a/persistence/src/androidUnitTest/kotlin/com/wire/kalium/persistence/GlobalDBBaseTest.kt b/persistence/src/androidUnitTest/kotlin/com/wire/kalium/persistence/GlobalDBBaseTest.kt index edb29062eb1..df3c50b7c8d 100644 --- a/persistence/src/androidUnitTest/kotlin/com/wire/kalium/persistence/GlobalDBBaseTest.kt +++ b/persistence/src/androidUnitTest/kotlin/com/wire/kalium/persistence/GlobalDBBaseTest.kt @@ -19,6 +19,7 @@ package com.wire.kalium.persistence import com.wire.kalium.persistence.db.GlobalDatabaseBuilder +import kotlinx.coroutines.test.TestDispatcher actual abstract class GlobalDBBaseTest { actual fun deleteDatabase() { diff --git a/persistence/src/commonMain/db_global/com/wire/kalium/persistence/ServerConfiguration.sq b/persistence/src/commonMain/db_global/com/wire/kalium/persistence/ServerConfiguration.sq index b09efc6e29a..90c251dd8df 100644 --- a/persistence/src/commonMain/db_global/com/wire/kalium/persistence/ServerConfiguration.sq +++ b/persistence/src/commonMain/db_global/com/wire/kalium/persistence/ServerConfiguration.sq @@ -28,8 +28,8 @@ insert: INSERT OR FAIL INTO ServerConfiguration(id, apiBaseUrl, accountBaseUrl, webSocketBaseUrl, blackListUrl, teamsUrl, websiteUrl, title, isOnPremises, federation, domain, commonApiVersion, apiProxyHost, apiProxyNeedsAuthentication, apiProxyPort) VALUES( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,?,?,?); -updateApiVersion: -UPDATE ServerConfiguration SET commonApiVersion = ? WHERE id = ?; +updateServerMetaData: +UPDATE ServerConfiguration SET federation = ?, commonApiVersion = ? WHERE id = ?; /** this function will be used when a config get updated from v0 where domain can be null */ updateApiVersionAndDomain: diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/daokaliumdb/ServerConfigurationDAO.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/daokaliumdb/ServerConfigurationDAO.kt index 707e857eb1c..c41bc1ea1ab 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/daokaliumdb/ServerConfigurationDAO.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/daokaliumdb/ServerConfigurationDAO.kt @@ -125,7 +125,7 @@ interface ServerConfigurationDAO { suspend fun allConfig(): List fun configById(id: String): ServerConfigEntity? suspend fun configByLinks(links: ServerConfigEntity.Links): ServerConfigEntity? - suspend fun updateApiVersion(id: String, commonApiVersion: Int) + suspend fun updateServerMetaData(id: String, federation: Boolean, commonApiVersion: Int) suspend fun updateApiVersionAndDomain(id: String, domain: String, commonApiVersion: Int) suspend fun configForUser(userId: UserIDEntity): ServerConfigEntity? suspend fun setFederationToTrue(id: String) @@ -209,8 +209,10 @@ internal class ServerConfigurationDAOImpl internal constructor( }.executeAsOneOrNull() } - override suspend fun updateApiVersion(id: String, commonApiVersion: Int) = withContext(queriesContext) { - queries.updateApiVersion(commonApiVersion, id) + override suspend fun updateServerMetaData(id: String, federation: Boolean, commonApiVersion: Int) { + withContext(queriesContext) { + queries.updateServerMetaData(federation, commonApiVersion, id) + } } override suspend fun updateApiVersionAndDomain(id: String, domain: String, commonApiVersion: Int) = diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/daokaliumdb/ServerConfigurationDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/daokaliumdb/ServerConfigurationDAOTest.kt index 5ab0fe61e87..befaae85ff9 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/daokaliumdb/ServerConfigurationDAOTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/daokaliumdb/ServerConfigurationDAOTest.kt @@ -24,17 +24,26 @@ import com.wire.kalium.persistence.GlobalDBBaseTest import com.wire.kalium.persistence.db.GlobalDatabaseBuilder import com.wire.kalium.persistence.model.ServerConfigEntity import com.wire.kalium.persistence.utils.stubs.newServerConfig +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withContext import kotlin.test.AfterTest import kotlin.test.BeforeTest +import kotlin.test.Ignore import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFails import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull -@OptIn(ExperimentalCoroutinesApi::class) class ServerConfigurationDAOTest : GlobalDBBaseTest() { private val config1 = newServerConfig(id = 1) @@ -142,13 +151,31 @@ class ServerConfigurationDAOTest : GlobalDBBaseTest() { @Test fun givenNewApiVersion_thenItCanBeUpdated() = runTest { - insertConfig(config1) - val newVersion = config1.metaData.copy(apiVersion = 2) - val expected = config1.copy(metaData = newVersion) + val oldConfig = config1.copy( + metaData = config1.metaData.copy( + apiVersion = 1, + federation = false + ), + ) - globalDatabaseBuilder.serverConfigurationDAO.updateApiVersion(config1.id, newVersion.apiVersion) - val actual = globalDatabaseBuilder.serverConfigurationDAO.configById(config1.id) - assertEquals(expected, actual) + val newVersion = config1.metaData.copy( + apiVersion = 2, + federation = true + ) + + val expected = oldConfig.copy(metaData = newVersion) + + insertConfig(oldConfig) + globalDatabaseBuilder.serverConfigurationDAO.updateServerMetaData( + id = oldConfig.id, + federation = true, + commonApiVersion = 2 + ) + globalDatabaseBuilder.serverConfigurationDAO.configById(oldConfig.id) + .also { actual -> + assertEquals(expected.metaData.federation, actual!!.metaData.federation) + assertEquals(expected.metaData.apiVersion, actual!!.metaData.apiVersion) + } } @Test diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/globalDB/AccountsDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/globalDB/AccountsDAOTest.kt index 31ba21d5bb5..d5d55507e5e 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/globalDB/AccountsDAOTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/globalDB/AccountsDAOTest.kt @@ -29,8 +29,15 @@ import com.wire.kalium.persistence.db.GlobalDatabaseBuilder import com.wire.kalium.persistence.model.LogoutReason import com.wire.kalium.persistence.model.ServerConfigEntity import com.wire.kalium.persistence.model.SsoIdEntity +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.TestDispatcher +import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals