From 81b6a2b19f6ceb303dac61d5a9488da033998b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Fri, 3 Nov 2023 16:34:36 +0100 Subject: [PATCH 1/5] fix: persisting team members connection state [WPB-5338] --- .../src/commonMain/db_user/com/wire/kalium/persistence/Users.sq | 2 +- .../kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq index c2e564040b2..e8990275a47 100644 --- a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq +++ b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq @@ -63,7 +63,7 @@ WHERE qualified_id = ?; updateTeamMemberUser: UPDATE User -SET name = ?, handle = ?, email = ?, phone = ?, accent_id = ?, team = ?, preview_asset_id = ?, complete_asset_id = ?, bot_service = ?, incomplete_metadata = 0 +SET name = ?, handle = ?, email = ?, phone = ?, accent_id = ?, team = ?, connection_status = ?, preview_asset_id = ?, complete_asset_id = ?, bot_service = ?, incomplete_metadata = 0 WHERE qualified_id = ?; updateTeamMemberType: diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt index eda6d32457a..f26721a988b 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt @@ -185,6 +185,7 @@ class UserDAOImpl internal constructor( phone = user.phone, accent_id = user.accentId, team = user.team, + connection_status = user.connectionStatus, preview_asset_id = user.previewAssetId, complete_asset_id = user.completeAssetId, bot_service = user.botService, From c714a617616484d77e185f0fbcc79326a435b9fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Fri, 3 Nov 2023 17:21:07 +0100 Subject: [PATCH 2/5] format code --- .../com/wire/kalium/persistence/Users.sq | 52 ++++++++++++++++--- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq index e8990275a47..b3f0a7c20ec 100644 --- a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq +++ b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq @@ -58,27 +58,57 @@ VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?); updateUser: UPDATE User -SET name = ?, handle = ?, email = ?, phone = ?, accent_id = ?, team = ?, preview_asset_id = ?, complete_asset_id = ?, user_type = ?, bot_service = ?, incomplete_metadata = ?, expires_at = ? +SET name = ?, + handle = ?, + email = ?, + phone = ?, + accent_id = ?, + team = ?, + preview_asset_id = ?, + complete_asset_id = ?, + user_type = ?, + bot_service = ?, + incomplete_metadata = ?, + expires_at = ? WHERE qualified_id = ?; updateTeamMemberUser: UPDATE User -SET name = ?, handle = ?, email = ?, phone = ?, accent_id = ?, team = ?, connection_status = ?, preview_asset_id = ?, complete_asset_id = ?, bot_service = ?, incomplete_metadata = 0 +SET name = ?, + handle = ?, + email = ?, + phone = ?, + accent_id = ?, + team = ?, + connection_status = ?, + preview_asset_id = ?, + complete_asset_id = ?, + bot_service = ?, + incomplete_metadata = 0 WHERE qualified_id = ?; updateTeamMemberType: UPDATE User -SET team = ?, connection_status = ?, user_type = ? +SET team = ?, + connection_status = ?, + user_type = ? WHERE qualified_id = ?; markUserAsDeleted: UPDATE User -SET team = NULL , preview_asset_id = NULL, complete_asset_id = NULL, user_type = ?, deleted = 1 +SET team = NULL , + preview_asset_id = NULL, + complete_asset_id = NULL, + user_type = ?, + deleted = 1 WHERE qualified_id = ?; markUserAsDefederated: UPDATE User -SET team = NULL , preview_asset_id = NULL, complete_asset_id = NULL, defederated = 1 +SET team = NULL, + preview_asset_id = NULL, + complete_asset_id = NULL, + defederated = 1 WHERE qualified_id = ?; insertOrIgnoreUserId: @@ -87,7 +117,12 @@ VALUES(?, 1); updateSelfUser: UPDATE User -SET name = ?, handle = ?, email = ?, accent_id = ?, preview_asset_id = ?, complete_asset_id = ? +SET name = ?, + handle = ?, + email = ?, + accent_id = ?, + preview_asset_id = ?, + complete_asset_id = ? WHERE qualified_id = ?; insertOrIgnoreUserIdWithConnectionStatus: @@ -136,7 +171,10 @@ updateUserhandle: UPDATE User SET handle = ? WHERE qualified_id = ?; updateUserAsset: -UPDATE User SET complete_asset_id = ?, preview_asset_id = ? WHERE complete_asset_id = ?; +UPDATE User +SET complete_asset_id = ?, + preview_asset_id = ? +WHERE complete_asset_id = ?; selectChanges: SELECT changes(); From 4f6aac40a2e2a87facc91ec0b24388473fd08ac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Fri, 3 Nov 2023 17:57:38 +0100 Subject: [PATCH 3/5] add test for upsertTeamMembers and update doc --- .../wire/kalium/persistence/dao/UserDAO.kt | 9 ++- .../kalium/persistence/dao/UserDAOTest.kt | 63 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt index 7ab3603e9be..067f8e1fa62 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt @@ -164,7 +164,14 @@ interface UserDAO { suspend fun upsertTeamMembersTypes(users: List) /** - * This will update all columns, except [UserEntity.userType] or insert a new record with default values + * This will update all columns, except: + * - [UserEntity.availabilityStatus] + * - [UserEntity.userType] + * - [UserEntity.deleted] + * - [UserEntity.hasIncompleteMetadata] + * - [UserEntity.expiresAt] + * - [UserEntity.defederated] + * or insert a new record with default values * An upsert operation is a one that tries to update a record and if fails (not rows affected by change) inserts instead. * In this case as the transaction can be executed many times, we need to take care for not deleting old data. */ diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt index e71a7255450..4f8b33104bc 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.take import kotlinx.coroutines.test.TestResult import kotlinx.coroutines.test.runTest +import kotlinx.datetime.Clock import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertContains @@ -762,6 +763,68 @@ class UserDAOTest : BaseDatabaseTest() { assertEquals(false, result.defederated) } + @Test + fun givenExistingTeamMemberUser_whenUpdatingIt_thenAllImportantFieldsAreProperlyUpdated() = runTest(dispatcher) { + val user = user1.copy( + name = "Name", + handle = "Handle", + email = "Email", + phone = "Phone", + accentId = 1, + team = "Team", + connectionStatus = ConnectionEntity.State.ACCEPTED, + previewAssetId = UserAssetIdEntity("PreviewAssetId", "PreviewAssetDomain"), + completeAssetId = UserAssetIdEntity("CompleteAssetId", "CompleteAssetDomain"), + availabilityStatus = UserAvailabilityStatusEntity.AVAILABLE, + userType = UserTypeEntity.STANDARD, + botService = BotIdEntity("BotService", "BotServiceDomain"), + deleted = false, + hasIncompleteMetadata = false, + expiresAt = null, + defederated = false, + ) + db.userDAO.insertUser(user) + val updatedTeamMemberUser = user1.copy( + name = "newName", + handle = "newHandle", + email = "newEmail", + phone = "newPhone", + accentId = 2, + team = "newTeam", + connectionStatus = ConnectionEntity.State.PENDING, + previewAssetId = UserAssetIdEntity("newPreviewAssetId", "newPreviewAssetDomain"), + completeAssetId = UserAssetIdEntity("newCompleteAssetId", "newCompleteAssetDomain"), + availabilityStatus = UserAvailabilityStatusEntity.BUSY, + userType = UserTypeEntity.EXTERNAL, + botService = BotIdEntity("newBotService", "newBotServiceDomain"), + deleted = true, + hasIncompleteMetadata = true, + expiresAt = Clock.System.now(), + defederated = true, + ) + db.userDAO.upsertTeamMembers(listOf(updatedTeamMemberUser)) + val result = db.userDAO.getUserByQualifiedID(user1.id).first() + assertTrue { + result != null && + result.name == updatedTeamMemberUser.name && + result.handle == updatedTeamMemberUser.handle && + result.email == updatedTeamMemberUser.email && + result.phone == updatedTeamMemberUser.phone && + result.accentId == updatedTeamMemberUser.accentId && + result.team == updatedTeamMemberUser.team && + result.connectionStatus == updatedTeamMemberUser.connectionStatus && + result.previewAssetId == updatedTeamMemberUser.previewAssetId && + result.completeAssetId == updatedTeamMemberUser.completeAssetId && + result.availabilityStatus != updatedTeamMemberUser.availabilityStatus && // should not be updated + result.userType != updatedTeamMemberUser.userType && // should not be updated + result.botService == updatedTeamMemberUser.botService && + result.deleted != updatedTeamMemberUser.deleted && // should not be updated + result.hasIncompleteMetadata != updatedTeamMemberUser.hasIncompleteMetadata && // should not be updated + result.expiresAt != updatedTeamMemberUser.expiresAt && // should not be updated + result.defederated != updatedTeamMemberUser.defederated // should not be updated + } + } + private companion object { val USER_ENTITY_1 = newUserEntity(QualifiedIDEntity("1", "wire.com")) val USER_ENTITY_2 = newUserEntity(QualifiedIDEntity("2", "wire.com")) From bd36a08c466e27f47c3ac754667d5e73a21ea8ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Fri, 3 Nov 2023 18:09:04 +0100 Subject: [PATCH 4/5] change name --- .../kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt index 4f8b33104bc..1c05cc4f88b 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt @@ -764,7 +764,7 @@ class UserDAOTest : BaseDatabaseTest() { } @Test - fun givenExistingTeamMemberUser_whenUpdatingIt_thenAllImportantFieldsAreProperlyUpdated() = runTest(dispatcher) { + fun givenExistingTeamMemberUser_whenUpsertingIt_thenAllImportantFieldsAreProperlyUpdated() = runTest(dispatcher) { val user = user1.copy( name = "Name", handle = "Handle", From 65c03d38360c88fb41d4eb6dacbb915e0ea5b2e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Mon, 6 Nov 2023 13:15:47 +0100 Subject: [PATCH 5/5] upgrade test asserts --- .../kalium/persistence/dao/UserDAOTest.kt | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt index 1c05cc4f88b..5ecb8524d47 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt @@ -34,6 +34,7 @@ import kotlin.test.Test import kotlin.test.assertContains import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -804,25 +805,23 @@ class UserDAOTest : BaseDatabaseTest() { ) db.userDAO.upsertTeamMembers(listOf(updatedTeamMemberUser)) val result = db.userDAO.getUserByQualifiedID(user1.id).first() - assertTrue { - result != null && - result.name == updatedTeamMemberUser.name && - result.handle == updatedTeamMemberUser.handle && - result.email == updatedTeamMemberUser.email && - result.phone == updatedTeamMemberUser.phone && - result.accentId == updatedTeamMemberUser.accentId && - result.team == updatedTeamMemberUser.team && - result.connectionStatus == updatedTeamMemberUser.connectionStatus && - result.previewAssetId == updatedTeamMemberUser.previewAssetId && - result.completeAssetId == updatedTeamMemberUser.completeAssetId && - result.availabilityStatus != updatedTeamMemberUser.availabilityStatus && // should not be updated - result.userType != updatedTeamMemberUser.userType && // should not be updated - result.botService == updatedTeamMemberUser.botService && - result.deleted != updatedTeamMemberUser.deleted && // should not be updated - result.hasIncompleteMetadata != updatedTeamMemberUser.hasIncompleteMetadata && // should not be updated - result.expiresAt != updatedTeamMemberUser.expiresAt && // should not be updated - result.defederated != updatedTeamMemberUser.defederated // should not be updated - } + assertNotNull(result) + assertEquals(updatedTeamMemberUser.name, result.name) + assertEquals(updatedTeamMemberUser.handle, result.handle) + assertEquals(updatedTeamMemberUser.email, result.email) + assertEquals(updatedTeamMemberUser.phone, result.phone) + assertEquals(updatedTeamMemberUser.accentId, result.accentId) + assertEquals(updatedTeamMemberUser.team, result.team) + assertEquals(updatedTeamMemberUser.connectionStatus, result.connectionStatus) + assertEquals(updatedTeamMemberUser.previewAssetId, result.previewAssetId) + assertEquals(updatedTeamMemberUser.completeAssetId, result.completeAssetId) + assertEquals(updatedTeamMemberUser.botService, result.botService) + assertNotEquals(updatedTeamMemberUser.availabilityStatus, result.availabilityStatus) + assertNotEquals(updatedTeamMemberUser.userType, result.userType) + assertNotEquals(updatedTeamMemberUser.deleted, result.deleted) + assertNotEquals(updatedTeamMemberUser.hasIncompleteMetadata, result.hasIncompleteMetadata) + assertNotEquals(updatedTeamMemberUser.expiresAt, result.expiresAt) + assertNotEquals(updatedTeamMemberUser.defederated, result.defederated) } private companion object {