Merge pull request #7754 from vector-im/feature/ons/remove_client_information_account_data

Delete unused client information from account data (PSG-871)
This commit is contained in:
Onuray Sahin 2022-12-13 11:10:41 +03:00 committed by GitHub
commit 250bd9c620
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 360 additions and 1 deletions

1
changelog.d/7754.feature Normal file
View file

@ -0,0 +1 @@
Delete unused client information from account data

View file

@ -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<UserAccountDataEvent>
/**
* Deletes user account data of the given type.
* @param type the type to delete from user account data
*/
suspend fun deleteUserAccountData(type: String)
}

View file

@ -42,4 +42,7 @@ internal abstract class AccountDataModule {
@Binds
abstract fun bindUpdateBreadcrumbsTask(task: DefaultUpdateBreadcrumbsTask): UpdateBreadcrumbsTask
@Binds
abstract fun bindDeleteUserAccountDataTask(task: DefaultDeleteUserAccountDataTask): DeleteUserAccountDataTask
}

View file

@ -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<UserAccountDataEvent> {
return userAccountDataDataSource.getAccountDataEventsStartWith(type)
}
override suspend fun deleteUserAccountData(type: String) {
deleteUserAccountDataTask.execute(DeleteUserAccountDataTask.Params(type))
}
}

View file

@ -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<DeleteUserAccountDataTask.Params, Unit> {
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)
}
}
}

View file

@ -60,6 +60,16 @@ internal class UserAccountDataDataSource @Inject constructor(
)
}
fun getAccountDataEventsStartWith(type: String): List<UserAccountDataEvent> {
return realmSessionProvider.withRealm { realm ->
realm
.where(UserAccountDataEntity::class.java)
.beginsWith(UserAccountDataEntityFields.TYPE, type)
.findAll()
.map(accountDataMapper::map)
}
}
private fun accountDataEventsQuery(realm: Realm, types: Set<String>): RealmQuery<UserAccountDataEntity> {
val query = realm.where(UserAccountDataEntity::class.java)
if (types.isNotEmpty()) {

View file

@ -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) }
}
}

View file

@ -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
}
}

View file

@ -0,0 +1,42 @@
/*
* 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.core.session.clientinfo
import im.vector.app.core.di.ActiveSessionHolder
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(deviceInfoList: List<DeviceInfo>): Result<Unit> = runCatching {
// A defensive approach against local storage reports an empty device list (although it is not a seen situation).
if (deviceInfoList.isEmpty()) return Result.success(Unit)
val expectedClientInfoKeyList = deviceInfoList.map { MATRIX_CLIENT_INFO_KEY_PREFIX + it.deviceId }
activeSessionHolder
.getSafeActiveSession()
?.accountDataService()
?.getUserAccountDataEventsStartWith(MATRIX_CLIENT_INFO_KEY_PREFIX)
?.map { it.type }
?.subtract(expectedClientInfoKeyList.toSet())
?.forEach { userAccountDataKeyToDelete ->
activeSessionHolder.getSafeActiveSession()?.accountDataService()?.deleteUserAccountData(userAccountDataKeyToDelete)
}
}
}

View file

@ -31,12 +31,14 @@ 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 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<UnknownDevicesState, UnknownDeviceDetectorSharedViewModel.Action, EmptyViewEvents>(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 out verified sessions or those which do not support encryption (i.e. without crypto info)
@ -143,6 +149,12 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor(
session.cryptoService().fetchDevicesList(NoOpMatrixCallback())
}
private fun deleteUnusedClientInformation(deviceFullInfoList: List<DeviceInfo>) {
viewModelScope.launch {
deleteUnusedClientInformationUseCase.execute(deviceFullInfoList)
}
}
override fun handle(action: Action) {
when (action) {
is Action.IgnoreDevice -> {

View file

@ -112,6 +112,7 @@ class DevicesViewModel @AssistedInject constructor(
val deviceFullInfoList = async.invoke()
val unverifiedSessionsCount = deviceFullInfoList.count { !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse() }
val inactiveSessionsCount = deviceFullInfoList.count { it.isInactive }
copy(
devices = async,
unverifiedSessionsCount = unverifiedSessionsCount,

View file

@ -0,0 +1,136 @@
/*
* 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.core.session.clientinfo
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<DeviceInfo>()
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())
}
}
}

View file

@ -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<UserAccountDataEvent>) {
every { getUserAccountDataEventsStartWith(type) } returns userAccountDataEventList
}
}