From 281fc75a2710a02adb7bea6b63c6b52b750a838d 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 (cherry picked from commit 790b885dcfa3456462e0a9caf8098e2a74054988) --- .../server/CustomServerConfigRepository.kt | 6 ++- .../server/ServerConfigRepository.kt | 24 ++++++++--- .../server/UpdateApiVersionsUseCase.kt | 2 +- .../CustomServerConfigRepositoryTest.kt | 25 +++++------ .../ServerConfigRepositoryTest.kt | 8 ++-- .../server/UpdateApiVersionUseCaseTest.kt | 26 ++++++------ .../kalium/persistence/ServerConfiguration.sq | 4 +- .../daokaliumdb/ServerConfigurationDAO.kt | 8 ++-- .../daokaliumdb/ServerConfigurationDAOTest.kt | 41 +++++++++++++++---- .../persistence/globalDB/AccountsDAOTest.kt | 7 ++++ 10 files changed, 102 insertions(+), 49 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 6302326a6af..aa0f1c7ab05 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 62ee0d48ff7..03f66658396 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 3bffc343166..dcaab58a6d6 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 @@ -43,6 +43,7 @@ import kotlinx.coroutines.test.runTest import kotlin.properties.Delegates import kotlin.test.Test import kotlin.test.assertEquals + class CustomServerConfigRepositoryTest { @Test @@ -78,8 +79,8 @@ class CustomServerConfigRepositoryTest { .with(any()) .wasNotInvoked() verify(arrangement.serverConfigurationDAO) - .suspendFunction(arrangement.serverConfigurationDAO::updateApiVersion) - .with(any(), any()) + .suspendFunction(arrangement.serverConfigurationDAO::updateServerMetaData) + .with(any(), any(), any()) .wasInvoked(exactly = once) verify(arrangement.serverConfigurationDAO) .suspendFunction(arrangement.serverConfigurationDAO::setFederationToTrue) @@ -111,8 +112,8 @@ class CustomServerConfigRepositoryTest { .with(any()) .wasInvoked(exactly = once) verify(arrangement.serverConfigurationDAO) - .suspendFunction(arrangement.serverConfigurationDAO::updateApiVersion) - .with(any(), any()) + .suspendFunction(arrangement.serverConfigurationDAO::updateServerMetaData) + .with(any(), any(), any()) .wasNotInvoked() verify(arrangement.serverConfigurationDAO) .suspendFunction(arrangement.serverConfigurationDAO::setFederationToTrue) @@ -156,8 +157,8 @@ class CustomServerConfigRepositoryTest { .with(any()) .wasInvoked(exactly = once) verify(arrangement.serverConfigurationDAO) - .suspendFunction(arrangement.serverConfigurationDAO::updateApiVersion) - .with(any(), any()) + .suspendFunction(arrangement.serverConfigurationDAO::updateServerMetaData) + .with(any(), any(), any()) .wasNotInvoked() verify(arrangement.serverConfigurationDAO) .suspendFunction(arrangement.serverConfigurationDAO::setFederationToTrue) @@ -176,18 +177,19 @@ class CustomServerConfigRepositoryTest { @Mock val serverConfigApi = mock(classOf()) - + var developmentApiEnabled by Delegates.notNull() @Mock val serverConfigurationDAO = mock(classOf()) + init { developmentApiEnabled = false } - + @Mock val backendMetaDataUtil = mock(classOf()) - + private var customServerConfigRepository: CustomServerConfigRepository = CustomServerConfigDataSource(serverConfigApi, developmentApiEnabled, serverConfigurationDAO, backendMetaDataUtil) @@ -199,8 +201,7 @@ class CustomServerConfigRepositoryTest { domain = serverConfigEntity.metaData.domain ) ) - - + suspend fun withConfigForNewRequest(serverConfigEntity: ServerConfigEntity): Arrangement { given(serverConfigurationDAO) @@ -247,7 +248,7 @@ class CustomServerConfigRepositoryTest { .then { flowOf(listOf(newServerConfigEntity(1), newServerConfigEntity(2), newServerConfigEntity(3))) } return this } - + suspend fun withUpdatedServerConfig(): Arrangement { val newServerConfigEntity = serverConfigEntity.copy( metaData = serverConfigEntity.metaData.copy( 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 aae4d34c2bd..dea557adc2b 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 @@ -138,8 +138,8 @@ class ServerConfigRepositoryTest { .with(any()) .wasNotInvoked() verify(arrangement.serverConfigDAO) - .suspendFunction(arrangement.serverConfigDAO::updateApiVersion) - .with(any(), any()) + .suspendFunction(arrangement.serverConfigDAO::updateServerMetaData) + .with(any(), any(), any()) .wasInvoked(exactly = once) verify(arrangement.serverConfigDAO) .suspendFunction(arrangement.serverConfigDAO::setFederationToTrue) @@ -171,8 +171,8 @@ class ServerConfigRepositoryTest { .with(any()) .wasInvoked(exactly = once) verify(arrangement.serverConfigDAO) - .suspendFunction(arrangement.serverConfigDAO::updateApiVersion) - .with(any(), any()) + .suspendFunction(arrangement.serverConfigDAO::updateServerMetaData) + .with(any(), any(), any()) .wasNotInvoked() verify(arrangement.serverConfigDAO) .suspendFunction(arrangement.serverConfigDAO::setFederationToTrue) 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 f109efd402d..e0db409711f 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 @@ -78,7 +78,7 @@ class UpdateApiVersionUseCaseTest { ) ) ) - withUpdateConfigApiVersion(serverConfig1, Either.Right(Unit)) + withUpdateConfigMetaData(serverConfig1, Either.Right(Unit)) } runTest { @@ -93,7 +93,7 @@ class UpdateApiVersionUseCaseTest { verify(arrangement.serverConfigRepository1) - .suspendFunction(arrangement.serverConfigRepository1::updateConfigApiVersion) + .suspendFunction(arrangement.serverConfigRepository1::updateConfigMetaData) .with(eq(serverConfig1)) .wasInvoked(exactly = once) } @@ -117,7 +117,7 @@ class UpdateApiVersionUseCaseTest { ) ) ) - withUpdateConfigApiVersion(serverConfig1, Either.Right(Unit)) + withUpdateConfigMetaData(serverConfig1, Either.Right(Unit)) } runTest { @@ -132,7 +132,7 @@ class UpdateApiVersionUseCaseTest { verify(arrangement.serverConfigRepository1) - .suspendFunction(arrangement.serverConfigRepository1::updateConfigApiVersion) + .suspendFunction(arrangement.serverConfigRepository1::updateConfigMetaData) .with(any()) .wasInvoked(exactly = once) } @@ -156,7 +156,7 @@ class UpdateApiVersionUseCaseTest { ) ) ) - withUpdateConfigApiVersion(serverConfig1, Either.Right(Unit)) + withUpdateConfigMetaData(serverConfig1, Either.Right(Unit)) withProxyCredForUser(userId1.toDao(), ProxyCredentialsEntity("user", "pass")) } @@ -172,7 +172,7 @@ class UpdateApiVersionUseCaseTest { .wasInvoked(exactly = once) verify(arrangement.serverConfigRepository1) - .suspendFunction(arrangement.serverConfigRepository1::updateConfigApiVersion) + .suspendFunction(arrangement.serverConfigRepository1::updateConfigMetaData) .with(any()) .wasInvoked(exactly = once) } @@ -205,8 +205,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")) } @@ -227,12 +227,12 @@ class UpdateApiVersionUseCaseTest { .wasNotInvoked() verify(arrangement.serverConfigRepository1) - .suspendFunction(arrangement.serverConfigRepository1::updateConfigApiVersion) + .suspendFunction(arrangement.serverConfigRepository1::updateConfigMetaData) .with(any()) .wasInvoked(exactly = once) verify(arrangement.serverConfigRepository2) - .suspendFunction(arrangement.serverConfigRepository2::updateConfigApiVersion) + .suspendFunction(arrangement.serverConfigRepository2::updateConfigMetaData) .with(any()) .wasInvoked(exactly = once) @@ -270,18 +270,18 @@ class UpdateApiVersionUseCaseTest { .then { result } } - fun withUpdateConfigApiVersion( + suspend fun withUpdateConfigMetaData( serverConfig: ServerConfig, result: Either ) { when (serverConfig.id) { serverConfig1.id -> given(serverConfigRepository1) - .suspendFunction(serverConfigRepository1::updateConfigApiVersion) + .suspendFunction(serverConfigRepository1::updateConfigMetaData) .whenInvokedWith(any()) .then { result } serverConfig2.id -> given(serverConfigRepository2) - .suspendFunction(serverConfigRepository2::updateConfigApiVersion) + .suspendFunction(serverConfigRepository2::updateConfigMetaData) .whenInvokedWith(any()) .then { result } 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 d191f82694e..6de7995d374 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.GlobalDatabaseProvider 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 + ), + ) - db.serverConfigurationDAO.updateApiVersion(config1.id, newVersion.apiVersion) - val actual = db.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 6c851b449da..e61b90d982a 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.GlobalDatabaseProvider 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