From 3d68233723f67d08a6c6bbd819bed5d12d4c5cca Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Fri, 9 Dec 2022 14:51:23 +0300 Subject: [PATCH 1/6] Support retrieving account data whose key starts with a string. --- .../accountdata/SessionAccountDataService.kt | 13 +++++++++++++ .../user/accountdata/UserAccountDataDataSource.kt | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/accountdata/SessionAccountDataService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/accountdata/SessionAccountDataService.kt index a22dd33774..8addb0782e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/accountdata/SessionAccountDataService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/accountdata/SessionAccountDataService.kt @@ -63,4 +63,17 @@ interface SessionAccountDataService { * Update the account data with the provided type and the provided account data content. */ suspend fun updateUserAccountData(type: String, content: Content) + + /** + * Retrieve user account data list whose type starts with the given type. + * @param type the type or the starting part of a type + * @return list of account data whose type starts with the given type + */ + fun getUserAccountDataEventsStartWith(type: String): List + + /** + * Deletes user account data of the given type. + * @param type the type to delete from user account data + */ + suspend fun deleteUserAccountData(type: String) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/UserAccountDataDataSource.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/UserAccountDataDataSource.kt index 39f155096a..2e66f4513b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/UserAccountDataDataSource.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/UserAccountDataDataSource.kt @@ -60,6 +60,16 @@ internal class UserAccountDataDataSource @Inject constructor( ) } + fun getAccountDataEventsStartWith(type: String): List { + return realmSessionProvider.withRealm { realm -> + realm + .where(UserAccountDataEntity::class.java) + .contains(UserAccountDataEntityFields.TYPE, type) + .findAll() + .map(accountDataMapper::map) + } + } + private fun accountDataEventsQuery(realm: Realm, types: Set): RealmQuery { val query = realm.where(UserAccountDataEntity::class.java) if (types.isNotEmpty()) { From 8206b534f9d132a02318436549b05b59bd3853d3 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Fri, 9 Dec 2022 14:52:27 +0300 Subject: [PATCH 2/6] Create a task to delete an event data with a given type. --- .../user/accountdata/AccountDataModule.kt | 3 ++ .../DefaultSessionAccountDataService.kt | 11 ++++- .../accountdata/DeleteUserAccountDataTask.kt | 43 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DeleteUserAccountDataTask.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/AccountDataModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/AccountDataModule.kt index 3173686a27..463292b9c6 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/AccountDataModule.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/AccountDataModule.kt @@ -42,4 +42,7 @@ internal abstract class AccountDataModule { @Binds abstract fun bindUpdateBreadcrumbsTask(task: DefaultUpdateBreadcrumbsTask): UpdateBreadcrumbsTask + + @Binds + abstract fun bindDeleteUserAccountDataTask(task: DefaultDeleteUserAccountDataTask): DeleteUserAccountDataTask } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DefaultSessionAccountDataService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DefaultSessionAccountDataService.kt index c73446cf25..304a586a79 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DefaultSessionAccountDataService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DefaultSessionAccountDataService.kt @@ -34,10 +34,11 @@ import javax.inject.Inject internal class DefaultSessionAccountDataService @Inject constructor( @SessionDatabase private val monarchy: Monarchy, private val updateUserAccountDataTask: UpdateUserAccountDataTask, + private val deleteUserAccountDataTask: DeleteUserAccountDataTask, private val userAccountDataSyncHandler: UserAccountDataSyncHandler, private val userAccountDataDataSource: UserAccountDataDataSource, private val roomAccountDataDataSource: RoomAccountDataDataSource, - private val taskExecutor: TaskExecutor + private val taskExecutor: TaskExecutor, ) : SessionAccountDataService { override fun getUserAccountDataEvent(type: String): UserAccountDataEvent? { @@ -78,4 +79,12 @@ internal class DefaultSessionAccountDataService @Inject constructor( userAccountDataSyncHandler.handleGenericAccountData(realm, type, content) } } + + override fun getUserAccountDataEventsStartWith(type: String): List { + return userAccountDataDataSource.getAccountDataEventsStartWith(type) + } + + override suspend fun deleteUserAccountData(type: String) { + deleteUserAccountDataTask.execute(DeleteUserAccountDataTask.Params(type)) + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DeleteUserAccountDataTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DeleteUserAccountDataTask.kt new file mode 100644 index 0000000000..8d155e32cb --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DeleteUserAccountDataTask.kt @@ -0,0 +1,43 @@ +/* + * Copyright 2022 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.session.user.accountdata + +import org.matrix.android.sdk.internal.di.UserId +import org.matrix.android.sdk.internal.network.GlobalErrorReceiver +import org.matrix.android.sdk.internal.network.executeRequest +import org.matrix.android.sdk.internal.task.Task +import javax.inject.Inject + +internal interface DeleteUserAccountDataTask : Task { + + data class Params( + val type: String, + ) +} + +internal class DefaultDeleteUserAccountDataTask @Inject constructor( + private val accountDataApi: AccountDataAPI, + @UserId private val userId: String, + private val globalErrorReceiver: GlobalErrorReceiver, +) : DeleteUserAccountDataTask { + + override suspend fun execute(params: DeleteUserAccountDataTask.Params) { + return executeRequest(globalErrorReceiver) { + accountDataApi.deleteAccountData(userId, params.type) + } + } +} From 22cce30e353523b4a52085a6201a592fc5a259b2 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Fri, 9 Dec 2022 14:53:27 +0300 Subject: [PATCH 3/6] Create use case to detect and delete unnecessary account data of client information. --- .../DeleteUnusedClientInformationUseCase.kt | 39 +++++++++++++++++++ .../settings/devices/v2/DevicesViewModel.kt | 10 +++++ 2 files changed, 49 insertions(+) create mode 100644 vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt new file mode 100644 index 0000000000..ae77cf7493 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.devices.v2 + +import im.vector.app.core.di.ActiveSessionHolder +import im.vector.app.core.session.clientinfo.MATRIX_CLIENT_INFO_KEY_PREFIX +import javax.inject.Inject + +class DeleteUnusedClientInformationUseCase @Inject constructor( + private val activeSessionHolder: ActiveSessionHolder, +) { + + suspend fun execute(deviceFullInfoList: List) { + val expectedClientInfoKeyList = deviceFullInfoList.map { MATRIX_CLIENT_INFO_KEY_PREFIX + it.deviceInfo.deviceId } + activeSessionHolder + .getSafeActiveSession() + ?.accountDataService() + ?.getUserAccountDataEventsStartWith(MATRIX_CLIENT_INFO_KEY_PREFIX) + ?.map { it.type } + ?.subtract(expectedClientInfoKeyList.toSet()) + ?.forEach { userAccountDataKeyToDelete -> + activeSessionHolder.getSafeActiveSession()?.accountDataService()?.deleteUserAccountData(userAccountDataKeyToDelete) + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt index b7a6c5df30..d8aeefa377 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt @@ -51,6 +51,7 @@ class DevicesViewModel @AssistedInject constructor( refreshDevicesUseCase: RefreshDevicesUseCase, private val vectorPreferences: VectorPreferences, private val toggleIpAddressVisibilityUseCase: ToggleIpAddressVisibilityUseCase, + private val deleteUnusedClientInformationUseCase: DeleteUnusedClientInformationUseCase, ) : VectorSessionsListViewModel(initialState, activeSessionHolder, refreshDevicesUseCase), @@ -112,6 +113,9 @@ class DevicesViewModel @AssistedInject constructor( val deviceFullInfoList = async.invoke() val unverifiedSessionsCount = deviceFullInfoList.count { !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse() } val inactiveSessionsCount = deviceFullInfoList.count { it.isInactive } + + deleteUnusedClientInformation(deviceFullInfoList) + copy( devices = async, unverifiedSessionsCount = unverifiedSessionsCount, @@ -125,6 +129,12 @@ class DevicesViewModel @AssistedInject constructor( } } + private fun deleteUnusedClientInformation(deviceFullInfoList: List) { + viewModelScope.launch { + deleteUnusedClientInformationUseCase.execute(deviceFullInfoList) + } + } + private fun refreshDevicesOnCryptoDevicesChange() { viewModelScope.launch { refreshDevicesOnCryptoDevicesChangeUseCase.execute() From 7a667b513e8f21ca1a58ed58fd6f717dfa52bd67 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Fri, 9 Dec 2022 15:47:28 +0300 Subject: [PATCH 4/6] Execute use case from a better place. --- .../home/UnknownDeviceDetectorSharedViewModel.kt | 12 ++++++++++++ .../v2/DeleteUnusedClientInformationUseCase.kt | 5 +++-- .../features/settings/devices/v2/DevicesViewModel.kt | 9 --------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt b/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt index 21c7bd6ea1..040ffa7b3f 100644 --- a/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt @@ -32,11 +32,13 @@ import im.vector.app.core.platform.EmptyViewEvents import im.vector.app.core.platform.VectorViewModel import im.vector.app.core.platform.VectorViewModelAction import im.vector.app.core.time.Clock +import im.vector.app.features.settings.devices.v2.DeleteUnusedClientInformationUseCase import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.sample +import kotlinx.coroutines.launch import org.matrix.android.sdk.api.NoOpMatrixCallback import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.session.Session @@ -66,6 +68,7 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor( private val setUnverifiedSessionsAlertShownUseCase: SetUnverifiedSessionsAlertShownUseCase, private val isNewLoginAlertShownUseCase: IsNewLoginAlertShownUseCase, private val setNewLoginAlertShownUseCase: SetNewLoginAlertShownUseCase, + private val deleteUnusedClientInformationUseCase: DeleteUnusedClientInformationUseCase, ) : VectorViewModel(initialState) { sealed class Action : VectorViewModelAction { @@ -102,6 +105,9 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor( ) { cryptoList, infoList, pInfo -> // Timber.v("## Detector trigger ${cryptoList.map { "${it.deviceId} ${it.trustLevel}" }}") // Timber.v("## Detector trigger canCrossSign ${pInfo.get().selfSigned != null}") + + deleteUnusedClientInformation(infoList) + infoList .filter { info -> // filter verified session, by checking the crypto device info @@ -143,6 +149,12 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor( session.cryptoService().fetchDevicesList(NoOpMatrixCallback()) } + private fun deleteUnusedClientInformation(deviceFullInfoList: List) { + viewModelScope.launch { + deleteUnusedClientInformationUseCase.execute(deviceFullInfoList) + } + } + override fun handle(action: Action) { when (action) { is Action.IgnoreDevice -> { diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt index ae77cf7493..9cbca9664a 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt @@ -18,14 +18,15 @@ package im.vector.app.features.settings.devices.v2 import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.session.clientinfo.MATRIX_CLIENT_INFO_KEY_PREFIX +import org.matrix.android.sdk.api.session.crypto.model.DeviceInfo import javax.inject.Inject class DeleteUnusedClientInformationUseCase @Inject constructor( private val activeSessionHolder: ActiveSessionHolder, ) { - suspend fun execute(deviceFullInfoList: List) { - val expectedClientInfoKeyList = deviceFullInfoList.map { MATRIX_CLIENT_INFO_KEY_PREFIX + it.deviceInfo.deviceId } + suspend fun execute(deviceInfoList: List) { + val expectedClientInfoKeyList = deviceInfoList.map { MATRIX_CLIENT_INFO_KEY_PREFIX + it.deviceId } activeSessionHolder .getSafeActiveSession() ?.accountDataService() diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt index d8aeefa377..232fcd50f7 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt @@ -51,7 +51,6 @@ class DevicesViewModel @AssistedInject constructor( refreshDevicesUseCase: RefreshDevicesUseCase, private val vectorPreferences: VectorPreferences, private val toggleIpAddressVisibilityUseCase: ToggleIpAddressVisibilityUseCase, - private val deleteUnusedClientInformationUseCase: DeleteUnusedClientInformationUseCase, ) : VectorSessionsListViewModel(initialState, activeSessionHolder, refreshDevicesUseCase), @@ -114,8 +113,6 @@ class DevicesViewModel @AssistedInject constructor( val unverifiedSessionsCount = deviceFullInfoList.count { !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse() } val inactiveSessionsCount = deviceFullInfoList.count { it.isInactive } - deleteUnusedClientInformation(deviceFullInfoList) - copy( devices = async, unverifiedSessionsCount = unverifiedSessionsCount, @@ -129,12 +126,6 @@ class DevicesViewModel @AssistedInject constructor( } } - private fun deleteUnusedClientInformation(deviceFullInfoList: List) { - viewModelScope.launch { - deleteUnusedClientInformationUseCase.execute(deviceFullInfoList) - } - } - private fun refreshDevicesOnCryptoDevicesChange() { viewModelScope.launch { refreshDevicesOnCryptoDevicesChangeUseCase.execute() From 85a6c8c6f26bdb4bd61e9ca664e3f0be09125fd5 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Fri, 9 Dec 2022 19:53:20 +0300 Subject: [PATCH 5/6] Write unit tests for the use case. --- .../DeleteUnusedClientInformationUseCase.kt | 3 + ...eleteUnusedClientInformationUseCaseTest.kt | 137 ++++++++++++++++++ .../fakes/FakeSessionAccountDataService.kt | 4 + 3 files changed, 144 insertions(+) create mode 100644 vector/src/test/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCaseTest.kt diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt index 9cbca9664a..d98e839b4f 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt @@ -26,6 +26,9 @@ class DeleteUnusedClientInformationUseCase @Inject constructor( ) { suspend fun execute(deviceInfoList: List) { + // A defensive approach against local storage reports an empty device list (although it is not a seen situation). + if (deviceInfoList.isEmpty()) return + val expectedClientInfoKeyList = deviceInfoList.map { MATRIX_CLIENT_INFO_KEY_PREFIX + it.deviceId } activeSessionHolder .getSafeActiveSession() diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCaseTest.kt new file mode 100644 index 0000000000..68c9204208 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCaseTest.kt @@ -0,0 +1,137 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.devices.v2 + +import im.vector.app.core.session.clientinfo.MATRIX_CLIENT_INFO_KEY_PREFIX +import im.vector.app.test.fakes.FakeActiveSessionHolder +import io.mockk.coVerify +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import org.matrix.android.sdk.api.session.accountdata.UserAccountDataEvent +import org.matrix.android.sdk.api.session.crypto.model.DeviceInfo + +private const val A_CURRENT_DEVICE_ID = "current-device-id" +private const val A_DEVICE_ID_1 = "a-device-id-1" +private const val A_DEVICE_ID_2 = "a-device-id-2" +private const val A_DEVICE_ID_3 = "a-device-id-3" +private const val A_DEVICE_ID_4 = "a-device-id-4" + +private val A_DEVICE_INFO_1 = DeviceInfo(deviceId = A_DEVICE_ID_1) +private val A_DEVICE_INFO_2 = DeviceInfo(deviceId = A_DEVICE_ID_2) + +class DeleteUnusedClientInformationUseCaseTest { + + private val fakeActiveSessionHolder = FakeActiveSessionHolder() + + private val deleteUnusedClientInformationUseCase = DeleteUnusedClientInformationUseCase( + activeSessionHolder = fakeActiveSessionHolder.instance, + ) + + @Before + fun setup() { + fakeActiveSessionHolder.fakeSession.givenSessionId(A_CURRENT_DEVICE_ID) + } + + @Test + fun `given a device list that account data has all of them and extra devices then use case deletes the unused ones`() = runTest { + // Given + val devices = listOf(A_DEVICE_INFO_1, A_DEVICE_INFO_2) + val userAccountDataEventList = listOf( + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_1, mapOf("key" to "value")), + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_2, mapOf("key" to "value")), + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_3, mapOf("key" to "value")), + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_4, mapOf("key" to "value")), + ) + fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.givenGetUserAccountDataEventsStartWith( + type = MATRIX_CLIENT_INFO_KEY_PREFIX, + userAccountDataEventList = userAccountDataEventList, + ) + + // When + deleteUnusedClientInformationUseCase.execute(devices) + + // Then + coVerify { fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.deleteUserAccountData(MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_3) } + coVerify { fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.deleteUserAccountData(MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_4) } + } + + @Test + fun `given a device list that account data has exactly all of them then use case does nothing`() = runTest { + // Given + val devices = listOf(A_DEVICE_INFO_1, A_DEVICE_INFO_2) + val userAccountDataEventList = listOf( + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_1, mapOf("key" to "value")), + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_2, mapOf("key" to "value")), + ) + fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.givenGetUserAccountDataEventsStartWith( + type = MATRIX_CLIENT_INFO_KEY_PREFIX, + userAccountDataEventList = userAccountDataEventList, + ) + + // When + deleteUnusedClientInformationUseCase.execute(devices) + + // Then + coVerify(exactly = 0) { + fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.deleteUserAccountData(any()) + } + } + + @Test + fun `given a device list that account data has missing some of them then use case does nothing`() = runTest { + // Given + val devices = listOf(A_DEVICE_INFO_1, A_DEVICE_INFO_2) + val userAccountDataEventList = listOf( + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_1, mapOf("key" to "value")), + ) + fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.givenGetUserAccountDataEventsStartWith( + type = MATRIX_CLIENT_INFO_KEY_PREFIX, + userAccountDataEventList = userAccountDataEventList, + ) + + // When + deleteUnusedClientInformationUseCase.execute(devices) + + // Then + coVerify(exactly = 0) { + fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.deleteUserAccountData(any()) + } + } + + @Test + fun `given an empty device list that account data has some devices then use case does nothing`() = runTest { + // Given + val devices = emptyList() + val userAccountDataEventList = listOf( + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_1, mapOf("key" to "value")), + UserAccountDataEvent(type = MATRIX_CLIENT_INFO_KEY_PREFIX + A_DEVICE_ID_2, mapOf("key" to "value")), + ) + fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.givenGetUserAccountDataEventsStartWith( + type = MATRIX_CLIENT_INFO_KEY_PREFIX, + userAccountDataEventList = userAccountDataEventList, + ) + + // When + deleteUnusedClientInformationUseCase.execute(devices) + + // Then + coVerify(exactly = 0) { + fakeActiveSessionHolder.fakeSession.fakeSessionAccountDataService.deleteUserAccountData(any()) + } + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSessionAccountDataService.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSessionAccountDataService.kt index f1a0ae7452..cd357ec85a 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeSessionAccountDataService.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSessionAccountDataService.kt @@ -54,4 +54,8 @@ class FakeSessionAccountDataService : SessionAccountDataService by mockk(relaxed fun verifyUpdateUserAccountDataEventSucceeds(type: String, content: Content, inverse: Boolean = false) { coVerify(inverse = inverse) { updateUserAccountData(type, content) } } + + fun givenGetUserAccountDataEventsStartWith(type: String, userAccountDataEventList: List) { + every { getUserAccountDataEventsStartWith(type) } returns userAccountDataEventList + } } From 8c6c2dd5c2f5ed1777f8da128c82adddf268a589 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Mon, 12 Dec 2022 16:36:40 +0300 Subject: [PATCH 6/6] Code review fixes. --- changelog.d/7754.feature | 1 + .../accountdata/UserAccountDataDataSource.kt | 2 +- .../DefaultDeleteUserAccountDataTaskTest.kt | 53 +++++++++++++++++++ .../sdk/test/fakes/FakeAccountDataApi.kt | 32 +++++++++++ .../DeleteUnusedClientInformationUseCase.kt | 7 ++- .../UnknownDeviceDetectorSharedViewModel.kt | 2 +- ...eleteUnusedClientInformationUseCaseTest.kt | 3 +- 7 files changed, 92 insertions(+), 8 deletions(-) create mode 100644 changelog.d/7754.feature create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/user/accountdata/DefaultDeleteUserAccountDataTaskTest.kt create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeAccountDataApi.kt rename vector/src/main/java/im/vector/app/{features/settings/devices/v2 => core/session/clientinfo}/DeleteUnusedClientInformationUseCase.kt (87%) rename vector/src/test/java/im/vector/app/{features/settings/devices/v2 => core/session/clientinfo}/DeleteUnusedClientInformationUseCaseTest.kt (97%) diff --git a/changelog.d/7754.feature b/changelog.d/7754.feature new file mode 100644 index 0000000000..0e1b6d0961 --- /dev/null +++ b/changelog.d/7754.feature @@ -0,0 +1 @@ +Delete unused client information from account data diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/UserAccountDataDataSource.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/UserAccountDataDataSource.kt index 2e66f4513b..01f5d9f708 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/UserAccountDataDataSource.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/user/accountdata/UserAccountDataDataSource.kt @@ -64,7 +64,7 @@ internal class UserAccountDataDataSource @Inject constructor( return realmSessionProvider.withRealm { realm -> realm .where(UserAccountDataEntity::class.java) - .contains(UserAccountDataEntityFields.TYPE, type) + .beginsWith(UserAccountDataEntityFields.TYPE, type) .findAll() .map(accountDataMapper::map) } diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/user/accountdata/DefaultDeleteUserAccountDataTaskTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/user/accountdata/DefaultDeleteUserAccountDataTaskTest.kt new file mode 100644 index 0000000000..86580127dc --- /dev/null +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/user/accountdata/DefaultDeleteUserAccountDataTaskTest.kt @@ -0,0 +1,53 @@ +/* + * Copyright 2022 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.session.user.accountdata + +import io.mockk.coVerify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.matrix.android.sdk.test.fakes.FakeAccountDataApi +import org.matrix.android.sdk.test.fakes.FakeGlobalErrorReceiver + +private const val A_TYPE = "a-type" +private const val A_USER_ID = "a-user-id" + +@ExperimentalCoroutinesApi +class DefaultDeleteUserAccountDataTaskTest { + + private val fakeGlobalErrorReceiver = FakeGlobalErrorReceiver() + private val fakeAccountDataApi = FakeAccountDataApi() + + private val deleteUserAccountDataTask = DefaultDeleteUserAccountDataTask( + accountDataApi = fakeAccountDataApi.instance, + userId = A_USER_ID, + globalErrorReceiver = fakeGlobalErrorReceiver + ) + + @Test + fun `given parameters when executing the task then api is called`() = runTest { + // Given + val params = DeleteUserAccountDataTask.Params(type = A_TYPE) + fakeAccountDataApi.givenParamsToDeleteAccountData(A_USER_ID, A_TYPE) + + // When + deleteUserAccountDataTask.execute(params) + + // Then + coVerify { fakeAccountDataApi.instance.deleteAccountData(A_USER_ID, A_TYPE) } + } +} diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeAccountDataApi.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeAccountDataApi.kt new file mode 100644 index 0000000000..f3acc02458 --- /dev/null +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeAccountDataApi.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2022 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.test.fakes + +import io.mockk.coEvery +import io.mockk.just +import io.mockk.mockk +import io.mockk.runs +import org.matrix.android.sdk.internal.session.user.accountdata.AccountDataAPI + +internal class FakeAccountDataApi { + + val instance: AccountDataAPI = mockk() + + fun givenParamsToDeleteAccountData(userId: String, type: String) { + coEvery { instance.deleteAccountData(userId, type) } just runs + } +} diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt b/vector/src/main/java/im/vector/app/core/session/clientinfo/DeleteUnusedClientInformationUseCase.kt similarity index 87% rename from vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt rename to vector/src/main/java/im/vector/app/core/session/clientinfo/DeleteUnusedClientInformationUseCase.kt index d98e839b4f..dcd5c58480 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/session/clientinfo/DeleteUnusedClientInformationUseCase.kt @@ -14,10 +14,9 @@ * limitations under the License. */ -package im.vector.app.features.settings.devices.v2 +package im.vector.app.core.session.clientinfo import im.vector.app.core.di.ActiveSessionHolder -import im.vector.app.core.session.clientinfo.MATRIX_CLIENT_INFO_KEY_PREFIX import org.matrix.android.sdk.api.session.crypto.model.DeviceInfo import javax.inject.Inject @@ -25,9 +24,9 @@ class DeleteUnusedClientInformationUseCase @Inject constructor( private val activeSessionHolder: ActiveSessionHolder, ) { - suspend fun execute(deviceInfoList: List) { + suspend fun execute(deviceInfoList: List): Result = runCatching { // A defensive approach against local storage reports an empty device list (although it is not a seen situation). - if (deviceInfoList.isEmpty()) return + if (deviceInfoList.isEmpty()) return Result.success(Unit) val expectedClientInfoKeyList = deviceInfoList.map { MATRIX_CLIENT_INFO_KEY_PREFIX + it.deviceId } activeSessionHolder diff --git a/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt b/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt index 040ffa7b3f..de9e496ee1 100644 --- a/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt @@ -31,8 +31,8 @@ import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.core.platform.EmptyViewEvents import im.vector.app.core.platform.VectorViewModel import im.vector.app.core.platform.VectorViewModelAction +import im.vector.app.core.session.clientinfo.DeleteUnusedClientInformationUseCase import im.vector.app.core.time.Clock -import im.vector.app.features.settings.devices.v2.DeleteUnusedClientInformationUseCase import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.launchIn diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCaseTest.kt b/vector/src/test/java/im/vector/app/core/session/clientinfo/DeleteUnusedClientInformationUseCaseTest.kt similarity index 97% rename from vector/src/test/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCaseTest.kt rename to vector/src/test/java/im/vector/app/core/session/clientinfo/DeleteUnusedClientInformationUseCaseTest.kt index 68c9204208..8acb7b404b 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/DeleteUnusedClientInformationUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/core/session/clientinfo/DeleteUnusedClientInformationUseCaseTest.kt @@ -14,9 +14,8 @@ * limitations under the License. */ -package im.vector.app.features.settings.devices.v2 +package im.vector.app.core.session.clientinfo -import im.vector.app.core.session.clientinfo.MATRIX_CLIENT_INFO_KEY_PREFIX import im.vector.app.test.fakes.FakeActiveSessionHolder import io.mockk.coVerify import kotlinx.coroutines.test.runTest