From cdeccfd9b232cd78a49e449f5bf1b91adfb626d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Tue, 10 Dec 2024 18:26:05 +0100 Subject: [PATCH 1/5] feat: add query profiling and cached paginated list [WPB-14826] --- .../com/wire/android/WireApplication.kt | 8 + .../android/debug/DatabaseProfilingManager.kt | 56 ++++++ .../ConversationListViewModel.kt | 6 +- .../debug/DatabaseProfilingManagerTest.kt | 187 ++++++++++++++++++ kalium | 2 +- 5 files changed, 256 insertions(+), 3 deletions(-) create mode 100644 app/src/main/kotlin/com/wire/android/debug/DatabaseProfilingManager.kt create mode 100644 app/src/test/kotlin/com/wire/android/debug/DatabaseProfilingManagerTest.kt diff --git a/app/src/main/kotlin/com/wire/android/WireApplication.kt b/app/src/main/kotlin/com/wire/android/WireApplication.kt index 9b2d2823d17..d67de8e4c97 100644 --- a/app/src/main/kotlin/com/wire/android/WireApplication.kt +++ b/app/src/main/kotlin/com/wire/android/WireApplication.kt @@ -29,6 +29,7 @@ import co.touchlab.kermit.platformLogWriter import com.wire.android.analytics.ObserveCurrentSessionAnalyticsUseCase import com.wire.android.datastore.GlobalDataStore import com.wire.android.datastore.UserDataStoreProvider +import com.wire.android.debug.DatabaseProfilingManager import com.wire.android.di.ApplicationScope import com.wire.android.di.KaliumCoreLogic import com.wire.android.feature.analytics.AnonymousAnalyticsManagerImpl @@ -89,6 +90,9 @@ class WireApplication : BaseApp() { @Inject lateinit var currentScreenManager: CurrentScreenManager + @Inject + lateinit var databaseProfilingManager: DatabaseProfilingManager + override val workManagerConfiguration: Configuration get() = Configuration.Builder() .setWorkerFactory(wireWorkerFactory.get()) @@ -183,6 +187,10 @@ class WireApplication : BaseApp() { logDeviceInformation() // 5. Verify if we can initialize Anonymous Analytics initializeAnonymousAnalytics() + // 6. Observe and update profiling when needed + globalAppScope.launch { + databaseProfilingManager.observeAndUpdateProfiling() + } } private fun initializeAnonymousAnalytics() { diff --git a/app/src/main/kotlin/com/wire/android/debug/DatabaseProfilingManager.kt b/app/src/main/kotlin/com/wire/android/debug/DatabaseProfilingManager.kt new file mode 100644 index 00000000000..e6fcc1aa28c --- /dev/null +++ b/app/src/main/kotlin/com/wire/android/debug/DatabaseProfilingManager.kt @@ -0,0 +1,56 @@ +/* + * Wire + * Copyright (C) 2024 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.debug + +import com.wire.android.datastore.GlobalDataStore +import com.wire.android.di.KaliumCoreLogic +import com.wire.kalium.logic.CoreLogic +import com.wire.kalium.logic.data.user.UserId +import com.wire.kalium.logic.functional.mapToRightOr +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.scan +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class DatabaseProfilingManager @Inject constructor( + @KaliumCoreLogic private val coreLogic: CoreLogic, + private val globalDataStore: GlobalDataStore, +) { + + suspend fun observeAndUpdateProfiling() { + globalDataStore.isLoggingEnabled() + .flatMapLatest { isLoggingEnabled -> + coreLogic.getGlobalScope().sessionRepository.allValidSessionsFlow() + .mapToRightOr(emptyList()) + .map { it.map { it.userId } } + .scan(emptyList()) { previousList, currentList -> currentList - previousList.toSet() } + .map { userIds -> isLoggingEnabled to userIds } + } + .filter { (_, userIds) -> userIds.isNotEmpty() } + .distinctUntilChanged() + .collect { (isLoggingEnabled, userIds) -> + userIds.forEach { userId -> + coreLogic.getSessionScope(userId).debug.changeProfiling(isLoggingEnabled) + } + } + } +} diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversationslist/ConversationListViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversationslist/ConversationListViewModel.kt index 3ec4d460b94..092c94f5660 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversationslist/ConversationListViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversationslist/ConversationListViewModel.kt @@ -24,6 +24,7 @@ import androidx.compose.runtime.setValue import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import androidx.paging.PagingData +import androidx.paging.cachedIn import androidx.paging.insertSeparators import androidx.paging.map import com.wire.android.BuildConfig @@ -210,10 +211,11 @@ class ConversationListViewModelImpl @AssistedInject constructor( } } .flowOn(dispatcher.io()) + .cachedIn(viewModelScope) private var notPaginatedConversationListState by mutableStateOf(ConversationListState.NotPaginated()) - override val conversationListState: ConversationListState - get() = if (usePagination) { + override val conversationListState: ConversationListState = + if (usePagination) { ConversationListState.Paginated( conversations = conversationsPaginatedFlow, domain = currentAccount.domain diff --git a/app/src/test/kotlin/com/wire/android/debug/DatabaseProfilingManagerTest.kt b/app/src/test/kotlin/com/wire/android/debug/DatabaseProfilingManagerTest.kt new file mode 100644 index 00000000000..c94111f0d72 --- /dev/null +++ b/app/src/test/kotlin/com/wire/android/debug/DatabaseProfilingManagerTest.kt @@ -0,0 +1,187 @@ +/* + * Wire + * Copyright (C) 2024 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.debug + +import com.wire.android.config.CoroutineTestExtension +import com.wire.android.datastore.GlobalDataStore +import com.wire.kalium.logic.CoreLogic +import com.wire.kalium.logic.StorageFailure +import com.wire.kalium.logic.data.auth.AccountInfo +import com.wire.kalium.logic.data.user.UserId +import com.wire.kalium.logic.functional.Either +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.impl.annotations.MockK +import io.mockk.mockk +import kotlinx.collections.immutable.PersistentMap +import kotlinx.collections.immutable.persistentMapOf +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.amshove.kluent.internal.assertEquals +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith + +@ExtendWith(CoroutineTestExtension::class) +class DatabaseProfilingManagerTest { + + @Test + fun `given valid session and logging enabled, when observing, then profiling should be enabled`() = + runTest { + // given + val account = AccountInfo.Valid(UserId("user", "domain")) + val (arrangement, databaseProfilingManager) = Arrangement() + .withAllValidSessions(flowOf(Either.Right(listOf(account)))) + .withIsLoggingEnabled(flowOf(true)) + .arrange() + + // when + val job = launch { + databaseProfilingManager.observeAndUpdateProfiling() + } + advanceUntilIdle() + // then + assertEquals(true, arrangement.profilingValues[account.userId]) + job.cancel() + } + + @Test + fun `given valid session and logging disabled, when observing, then profiling is disabled`() = + runTest { + // given + val account = AccountInfo.Valid(UserId("user", "domain")) + val (arrangement, databaseProfilingManager) = Arrangement() + .withAllValidSessions(flowOf(Either.Right(listOf(account)))) + .withIsLoggingEnabled(flowOf(false)) + .arrange() + // when + val job = launch { + databaseProfilingManager.observeAndUpdateProfiling() + } + advanceUntilIdle() + // then + assertEquals(false, arrangement.profilingValues[account.userId]) + job.cancel() + } + + @Test + fun `given valid session, when observing and logging changes from disabled to enabled, then profiling is enabled`() = + runTest { + // given + val account = AccountInfo.Valid(UserId("user", "domain")) + val (arrangement, databaseProfilingManager) = Arrangement() + .withAllValidSessions(flowOf(Either.Right(listOf(account)))) + .withIsLoggingEnabled(flowOf(false)) + .arrange() + // when + val job = launch { + databaseProfilingManager.observeAndUpdateProfiling() + } + arrangement.withIsLoggingEnabled(flowOf(true)) + advanceUntilIdle() + // then + assertEquals(true, arrangement.profilingValues[account.userId]) + job.cancel() + } + + @Test + fun `given two valid sessions, when observing and logging changes from disabled to enabled, then profiling is enabled for both`() = + runTest { + // given + val account1 = AccountInfo.Valid(UserId("user1", "domain")) + val account2 = AccountInfo.Valid(UserId("user2", "domain")) + val (arrangement, databaseProfilingManager) = Arrangement() + .withAllValidSessions(flowOf(Either.Right(listOf(account1, account2)))) + .withIsLoggingEnabled(flowOf(false)) + .arrange() + // when + val job = launch { + databaseProfilingManager.observeAndUpdateProfiling() + } + arrangement.withIsLoggingEnabled(flowOf(true)) + advanceUntilIdle() + // then + assertEquals(true, arrangement.profilingValues[account1.userId]) + assertEquals(true, arrangement.profilingValues[account2.userId]) + job.cancel() + } + + @Test + fun `given valid session and logging enabled, when observing and new session appears, then profiling is enabled for both`() = + runTest { + // given + val account1 = AccountInfo.Valid(UserId("user1", "domain")) + val account2 = AccountInfo.Valid(UserId("user2", "domain")) + val validSessionsFlow = MutableStateFlow(Either.Right(listOf(account1))) + val (arrangement, databaseProfilingManager) = Arrangement() + .withAllValidSessions(validSessionsFlow) + .withIsLoggingEnabled(flowOf(true)) + .arrange() + // when + val job = launch { + databaseProfilingManager.observeAndUpdateProfiling() + } + validSessionsFlow.value = Either.Right(listOf(account1, account2)) + advanceUntilIdle() + // then + assertEquals(true, arrangement.profilingValues[account1.userId]) + assertEquals(true, arrangement.profilingValues[account2.userId]) + job.cancel() + } + + private class Arrangement { + + @MockK + lateinit var coreLogic: CoreLogic + + @MockK + private lateinit var globalDataStore: GlobalDataStore + + var profilingValues: PersistentMap = persistentMapOf() + private set + + init { + MockKAnnotations.init(this, relaxed = true, relaxUnitFun = true) + coEvery { coreLogic.getSessionScope(any()).debug.changeProfiling(any()) } answers { + profilingValues = profilingValues.put(firstArg(), secondArg()) + } + coEvery { coreLogic.getSessionScope(any()) } answers { + val userId = firstArg() + mockk() { + coEvery { debug.changeProfiling(any()) } answers { + val profilingValue = firstArg() + profilingValues = profilingValues.put(userId, profilingValue) + } + } + } + } + + fun withIsLoggingEnabled(isLoggingEnabledFlow: Flow) = apply { + coEvery { globalDataStore.isLoggingEnabled() } returns isLoggingEnabledFlow + } + + fun withAllValidSessions(allValidSessionsFlow: Flow>>) = apply { + coEvery { coreLogic.getGlobalScope().sessionRepository.allValidSessionsFlow() } returns allValidSessionsFlow + } + + fun arrange() = this to DatabaseProfilingManager(coreLogic, globalDataStore) + } +} diff --git a/kalium b/kalium index d8b69f1202e..85de0cfb590 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit d8b69f1202e0ea88889c98bf1e2f9cd5016d197c +Subproject commit 85de0cfb5906f5fbf5e9ce903f09b3edd92bc22d From 11bca8b4cc1a69179f6ccbdbb66a7c43ac92d738 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Tue, 10 Dec 2024 18:46:18 +0100 Subject: [PATCH 2/5] fix detekt --- .../com/wire/android/debug/DatabaseProfilingManagerTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/test/kotlin/com/wire/android/debug/DatabaseProfilingManagerTest.kt b/app/src/test/kotlin/com/wire/android/debug/DatabaseProfilingManagerTest.kt index c94111f0d72..b0dc851cc0d 100644 --- a/app/src/test/kotlin/com/wire/android/debug/DatabaseProfilingManagerTest.kt +++ b/app/src/test/kotlin/com/wire/android/debug/DatabaseProfilingManagerTest.kt @@ -165,7 +165,7 @@ class DatabaseProfilingManagerTest { } coEvery { coreLogic.getSessionScope(any()) } answers { val userId = firstArg() - mockk() { + mockk { coEvery { debug.changeProfiling(any()) } answers { val profilingValue = firstArg() profilingValues = profilingValues.put(userId, profilingValue) From ff095db9699c7e026cdb4a909fde1a977dfbc49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Tue, 10 Dec 2024 18:47:27 +0100 Subject: [PATCH 3/5] update kalium ref --- kalium | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kalium b/kalium index 85de0cfb590..3917dad4a1f 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit 85de0cfb5906f5fbf5e9ce903f09b3edd92bc22d +Subproject commit 3917dad4a1fc809a2f3e25d98a7ac2b9552e72f0 From 29c49c3f9138732c8d869409ffd3bf7c59a1ebb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Wed, 11 Dec 2024 18:15:18 +0100 Subject: [PATCH 4/5] update kalium ref --- kalium | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kalium b/kalium index 3917dad4a1f..0667f9b780a 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit 3917dad4a1fc809a2f3e25d98a7ac2b9552e72f0 +Subproject commit 0667f9b780a8262768b0c37af3d49d4f83c55701 From bec03014813d17f455db2633affe6d8f5c05cb18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Fri, 13 Dec 2024 15:39:36 +0100 Subject: [PATCH 5/5] fix failing tests --- .../kotlin/com/wire/android/ui/CallActivityViewModelTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt index 1c7345672a2..448d1f987fb 100644 --- a/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt @@ -117,6 +117,7 @@ class CallActivityViewModelTest { .arrange() viewModel.switchAccountIfNeeded(userId, arrangement.switchAccountActions) + advanceUntilIdle() coVerify(inverse = true) { arrangement.accountSwitch(any()) } } @@ -132,6 +133,7 @@ class CallActivityViewModelTest { .arrange() viewModel.switchAccountIfNeeded(UserId("anotherUser", "domain"), arrangement.switchAccountActions) + advanceUntilIdle() coVerify(exactly = if (switchedToAnotherAccountCalled) 1 else 0) { arrangement.switchAccountActions.switchedToAnotherAccount()