From 7aaaa9c756433ef3e7e14b3014e4667a4de849f2 Mon Sep 17 00:00:00 2001 From: Pablo Pajuelo Cabezas Date: Wed, 2 Apr 2025 13:25:53 +0200 Subject: [PATCH 1/7] fix: [ANDROAPP-6669] ANR while fetching data in search screen --- app/build.gradle.kts | 2 +- .../searchTrackEntity/SearchTEIViewModel.kt | 46 +++++++++++++------ .../listView/SearchTEList.kt | 15 +++--- .../SearchTEIViewModelTest.kt | 46 ++++++++++--------- gradle/libs.versions.toml | 2 + 5 files changed, 69 insertions(+), 42 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b94a30b5b0..b3e8b92bad 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -284,7 +284,7 @@ dependencies { testImplementation(libs.test.truth) testImplementation(libs.test.kotlinCoroutines) testImplementation(libs.test.turbine) - + testImplementation(libs.test.androidx.paging) androidTestUtil(libs.test.orchestrator) androidTestImplementation(libs.test.testRunner) diff --git a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt index 46656dc1bc..782238b027 100644 --- a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt +++ b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt @@ -24,8 +24,16 @@ import kotlinx.coroutines.async import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.dhis2.R @@ -128,6 +136,26 @@ class SearchTEIViewModel( private var fetchJob: Job? = null + val onNewSearch = MutableSharedFlow(extraBufferCapacity = 1) + + val searchPagingData = onNewSearch.onStart { emit(Unit) } + .flatMapLatest { + flow { + CoroutineTracker.increment() + emitAll( + when { + searching -> loadSearchResults() + displayFrontPageList() -> loadDisplayInListResults() + else -> emptyFlow() + }, + ) + CoroutineTracker.decrement() + } + } + .flowOn(dispatchers.io()) + .cachedIn(viewModelScope) + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), PagingData.empty()) + init { viewModelScope.launch(dispatchers.io()) { createButtonScrollVisibility.postValue( @@ -217,8 +245,7 @@ class SearchTEIViewModel( previousSate = _screenState.value?.screenState ?: SearchScreenState.NONE, listType = SearchScreenState.LIST, displayFrontPageList = searchRepository.getProgram(initialProgramUid) - ?.displayFrontPageList() - ?: false, + ?.displayFrontPageList() == true, canCreateWithoutSearch = searchRepository.canCreateInProgramWithoutSearch(), isSearching = searching, searchForm = SearchForm( @@ -545,16 +572,8 @@ class SearchTEIViewModel( when (_screenState.value?.screenState) { SearchScreenState.LIST -> { setListScreen() - fetchListResults { flow -> - flow?.let { - fetchListResults { flow -> - flow?.let { - _refreshData.postValue(Unit) - CoroutineTracker.decrement() - } - } - } - } + onNewSearch.emit(Unit) + CoroutineTracker.decrement() } SearchScreenState.MAP -> { @@ -577,9 +596,10 @@ class SearchTEIViewModel( uiState.updateMinAttributeWarning(true) setSearchScreen() _refreshData.postValue(Unit) + onNewSearch.emit(Unit) } } catch (e: Exception) { - Timber.d(e.message) + Timber.d(e) } } } diff --git a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt index a0cab00b8e..63e0e6812d 100644 --- a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt +++ b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt @@ -18,7 +18,9 @@ import androidx.coordinatorlayout.widget.CoordinatorLayout import androidx.core.view.updateLayoutParams import androidx.fragment.app.activityViewModels import androidx.fragment.app.viewModels +import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle import androidx.lifecycle.viewModelScope import androidx.paging.LoadState import androidx.recyclerview.widget.ConcatAdapter @@ -286,9 +288,10 @@ class SearchTEList : FragmentGlobalAbstract() { } private fun observeNewData() { + initData() viewModel.refreshData.observe(viewLifecycleOwner) { restoreAdapters() - initData() +// initData() } viewModel.dataResult.observe(viewLifecycleOwner) { @@ -348,14 +351,14 @@ class SearchTEList : FragmentGlobalAbstract() { private fun initData() { displayLoadingData() - viewModel.fetchListResults { - lifecycleScope.launch { - it?.takeIf { view != null }?.collectLatest { + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.STARTED) { + viewModel.searchPagingData.collect { data -> liveAdapter.addOnPagesUpdatedListener { onInitDataLoaded() } - liveAdapter.submitData(lifecycle, it) - } ?: onInitDataLoaded() + liveAdapter.submitData(lifecycle, data) + } } } } diff --git a/app/src/test/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModelTest.kt b/app/src/test/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModelTest.kt index ff65adb426..a2b5c08954 100644 --- a/app/src/test/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModelTest.kt +++ b/app/src/test/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModelTest.kt @@ -6,11 +6,13 @@ import androidx.compose.material.icons.automirrored.filled.List import androidx.compose.material.icons.automirrored.outlined.List import androidx.compose.material.icons.filled.Map import androidx.compose.material.icons.outlined.Map +import androidx.paging.testing.asSnapshot import app.cash.turbine.test import com.mapbox.geojson.BoundingBox import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.take import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest @@ -208,7 +210,7 @@ class SearchTEIViewModelTest { fun `Should return local results LiveData if not searching and displayInList is true`() { val testingProgram = testingProgram() setCurrentProgram(testingProgram) - viewModel.fetchListResults {} +// viewModel.fetchListResults {} testingDispatcher.scheduler.advanceUntilIdle() verify(repositoryKt).searchTrackedEntities( SearchParametersModel( @@ -220,33 +222,33 @@ class SearchTEIViewModelTest { } @Test - fun `Should return null if not searching and displayInList is false`() { + fun `Should return null if not searching and displayInList is false`() = runTest { val testingProgram = testingProgram(displayFrontPageList = false) setCurrentProgram(testingProgram) - viewModel.fetchListResults {} - - verify(repositoryKt, times(0)).searchTrackedEntities( - SearchParametersModel( - selectedProgram = testingProgram, - queryData = mutableMapOf(), - ), - true, - ) + viewModel.searchPagingData.test { + awaitItem() + verify(repositoryKt, times(0)).searchTrackedEntities( + SearchParametersModel( + selectedProgram = testingProgram, + queryData = mutableMapOf(), + ), + true, + ) - verify(repositoryKt, times(0)).searchTrackedEntities( - SearchParametersModel( - selectedProgram = testingProgram, - queryData = mutableMapOf(), - ), - false, - ) + verify(repositoryKt, times(0)).searchTrackedEntities( + SearchParametersModel( + selectedProgram = testingProgram, + queryData = mutableMapOf(), + ), + false, + ) + } } @Test - fun `Should return null global results if not searching`() { - viewModel.fetchListResults { - assertTrue(it == null) - } + fun `Should return empty global results if not searching`() = runTest { + val result = viewModel.searchPagingData.take(1).asSnapshot() + assertTrue(result.isEmpty()) } @ExperimentalCoroutinesApi diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index f74bf8607f..70e421897e 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -29,6 +29,7 @@ recyclerview = "1.3.1" compose = "1.5.4" composePlugin = "1.7.3" composePaging = "3.3.0" +pagingTesting = "3.3.0" composeLifecycle = "2.8.1" composeConstraintLayout = "1.0.1" activityCompose = "1.8.2" @@ -119,6 +120,7 @@ androidx-compose-uitooling = { group = "androidx.compose.ui", name = "ui-tooling androidx-compose-preview = { group = "androidx.compose.ui", name = "ui-tooling-preview", version.ref = "compose" } androidx-compose-viewbinding = { group = "androidx.compose.ui", name = "ui-viewbinding", version.ref = "compose" } androidx-compose-paging = { group = "androidx.paging", name = "paging-compose", version.ref = "composePaging" } +test-androidx-paging = { group = "androidx.paging", name = "paging-testing", version.ref = "pagingTesting" } androidx-compose-lifecycle = { group = "androidx.lifecycle", name = "lifecycle-runtime-compose", version.ref = "composeLifecycle" } androidx-coreKtx = { group = "androidx.core", name = "core-ktx", version.ref = "corektx" } androidx-activity-compose = { group = "androidx.activity", name = "activity-compose", version.ref = "activityCompose" } From e5915b3de19c26a057d090ec85d47ed6c0e844c9 Mon Sep 17 00:00:00 2001 From: Pablo Pajuelo Cabezas Date: Thu, 3 Apr 2025 14:37:28 +0200 Subject: [PATCH 2/7] fix: [ANDROAPP-6669] ANR in search paging --- .../searchTrackEntity/SearchTEIViewModel.kt | 20 ----------- .../SearchTEIViewModelTest.kt | 35 +++++++++++-------- 2 files changed, 21 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt index 782238b027..8d4b1e57c2 100644 --- a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt +++ b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt @@ -411,26 +411,6 @@ class SearchTEIViewModel( uiState = uiState.copy(searchEnabled = queryData.isNotEmpty()) } - fun fetchListResults(onPagedListReady: (Flow>?) -> Unit) { - CoroutineTracker.increment() - viewModelScope.launch(dispatchers.io()) { - val resultPagedList = async { - when { - searching -> loadSearchResults().cachedIn(viewModelScope) - displayFrontPageList() -> loadDisplayInListResults().cachedIn(viewModelScope) - else -> null - } - } - try { - onPagedListReady(resultPagedList.await()) - } catch (e: Exception) { - Timber.e(e) - } finally { - CoroutineTracker.decrement() - } - } - } - private suspend fun loadSearchResults() = withContext(dispatchers.io()) { val searchParametersModel = SearchParametersModel( selectedProgram = searchRepository.getProgram(initialProgramUid), diff --git a/app/src/test/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModelTest.kt b/app/src/test/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModelTest.kt index a2b5c08954..d1b69a3eb0 100644 --- a/app/src/test/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModelTest.kt +++ b/app/src/test/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModelTest.kt @@ -6,12 +6,14 @@ import androidx.compose.material.icons.automirrored.filled.List import androidx.compose.material.icons.automirrored.outlined.List import androidx.compose.material.icons.filled.Map import androidx.compose.material.icons.outlined.Map +import androidx.paging.PagingData import androidx.paging.testing.asSnapshot import app.cash.turbine.test import com.mapbox.geojson.BoundingBox import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.take import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.resetMain @@ -40,6 +42,7 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test +import org.mockito.kotlin.any import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import org.mockito.kotlin.times @@ -55,7 +58,9 @@ class SearchTEIViewModelTest { private val initialProgram = "programUid" private val initialQuery = mutableMapOf() private val repository: SearchRepository = mock() - private val repositoryKt: SearchRepositoryKt = mock() + private val repositoryKt: SearchRepositoryKt = mock { + on { searchTrackedEntities(any(), any()) } doReturn flowOf(PagingData.empty()) + } private val pageConfigurator: SearchPageConfigurator = mock() private val mapDataRepository: MapDataRepository = mock() private val networkUtils: NetworkUtils = mock() @@ -207,19 +212,21 @@ class SearchTEIViewModelTest { @ExperimentalCoroutinesApi @Test - fun `Should return local results LiveData if not searching and displayInList is true`() { - val testingProgram = testingProgram() - setCurrentProgram(testingProgram) -// viewModel.fetchListResults {} - testingDispatcher.scheduler.advanceUntilIdle() - verify(repositoryKt).searchTrackedEntities( - SearchParametersModel( - selectedProgram = testingProgram, - queryData = mutableMapOf(), - ), - false, - ) - } + fun `Should return local results LiveData if not searching and displayInList is true`() = + runTest { + val testingProgram = testingProgram() + setCurrentProgram(testingProgram) + + viewModel.searchPagingData.take(1).asSnapshot() + + verify(repositoryKt).searchTrackedEntities( + SearchParametersModel( + selectedProgram = testingProgram, + queryData = mutableMapOf(), + ), + false, + ) + } @Test fun `Should return null if not searching and displayInList is false`() = runTest { From 9b983d56c45854087bee7e8c18c141bec10b91c7 Mon Sep 17 00:00:00 2001 From: Xavier Molloy Date: Wed, 9 Apr 2025 07:20:51 +0200 Subject: [PATCH 3/7] fix: [ANDROAPP-6669] Remove commented code --- .../dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt index 63e0e6812d..dadd1b70f6 100644 --- a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt +++ b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt @@ -291,7 +291,6 @@ class SearchTEList : FragmentGlobalAbstract() { initData() viewModel.refreshData.observe(viewLifecycleOwner) { restoreAdapters() -// initData() } viewModel.dataResult.observe(viewLifecycleOwner) { From a9d1c0aa4b47b05562a0111285a2c444279168e2 Mon Sep 17 00:00:00 2001 From: Xavier Molloy Date: Wed, 9 Apr 2025 08:49:39 +0200 Subject: [PATCH 4/7] fix: [ANDROAPP-6669] fix ui tests --- .../java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt | 1 + .../dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt | 1 + 2 files changed, 2 insertions(+) diff --git a/app/src/androidTest/java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt b/app/src/androidTest/java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt index ea47e1bca5..efa8bf65ba 100644 --- a/app/src/androidTest/java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt +++ b/app/src/androidTest/java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt @@ -49,6 +49,7 @@ class TeiFlowRobot(val composeTestRule: ComposeTestRule) : BaseRobot() { enrollmentRobot(composeTestRule) { typeOnDateParameterWithLabel("LMP Date *", incidentDate) + pressBack() clickOnSaveEnrollment() } } diff --git a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt index dadd1b70f6..7ca3e11709 100644 --- a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt +++ b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/listView/SearchTEList.kt @@ -355,6 +355,7 @@ class SearchTEList : FragmentGlobalAbstract() { viewModel.searchPagingData.collect { data -> liveAdapter.addOnPagesUpdatedListener { onInitDataLoaded() + CoroutineTracker.decrement() } liveAdapter.submitData(lifecycle, data) } From 22646caec634f8998e97ccfbbb148c5f30890293 Mon Sep 17 00:00:00 2001 From: Xavier Molloy Date: Thu, 10 Apr 2025 08:52:32 +0200 Subject: [PATCH 5/7] fix: [ANDROAPP-6669] fix test --- .../java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt | 1 - .../dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/androidTest/java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt b/app/src/androidTest/java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt index efa8bf65ba..ea47e1bca5 100644 --- a/app/src/androidTest/java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt +++ b/app/src/androidTest/java/org/dhis2/usescases/flow/teiFlow/TeiFlowRobot.kt @@ -49,7 +49,6 @@ class TeiFlowRobot(val composeTestRule: ComposeTestRule) : BaseRobot() { enrollmentRobot(composeTestRule) { typeOnDateParameterWithLabel("LMP Date *", incidentDate) - pressBack() clickOnSaveEnrollment() } } diff --git a/app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt b/app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt index b2d2351a07..f629b7e653 100644 --- a/app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt +++ b/app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt @@ -7,6 +7,7 @@ import androidx.compose.ui.test.junit4.ComposeTestRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performImeAction import androidx.compose.ui.test.performTextReplacement import androidx.test.espresso.Espresso.onView import androidx.test.espresso.action.ViewActions.click @@ -108,6 +109,10 @@ class EnrollmentRobot(val composeTestRule: ComposeTestRule) : BaseRobot() { hasTestTag("INPUT_DATE_TIME_TEXT_FIELD") and hasAnySibling(hasText(label)), useUnmergedTree = true, ).performTextReplacement(dateValue) + onNode( + hasTestTag("INPUT_DATE_TIME_TEXT_FIELD") and hasAnySibling(hasText(label)), + useUnmergedTree = true, + ).performImeAction() } } From 96a4d7a075266fe8e2689092f3a174ee131c1da3 Mon Sep 17 00:00:00 2001 From: Xavier Molloy Date: Thu, 10 Apr 2025 09:38:10 +0200 Subject: [PATCH 6/7] fix: [ANDROAPP-6669] make Mutable shared flow private, fix sonar smell --- .../org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt index 8d4b1e57c2..8bef932fdb 100644 --- a/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt +++ b/app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEIViewModel.kt @@ -136,7 +136,7 @@ class SearchTEIViewModel( private var fetchJob: Job? = null - val onNewSearch = MutableSharedFlow(extraBufferCapacity = 1) + private val onNewSearch = MutableSharedFlow(extraBufferCapacity = 1) val searchPagingData = onNewSearch.onStart { emit(Unit) } .flatMapLatest { From 4e869e99725ef45e64cf0866a2ab162ade22e04d Mon Sep 17 00:00:00 2001 From: Xavier Molloy Date: Thu, 10 Apr 2025 13:19:19 +0200 Subject: [PATCH 7/7] fix: [ANDROAPP-6669] fix test --- .../dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt b/app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt index f629b7e653..de2dd21549 100644 --- a/app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt +++ b/app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/EnrollmentRobot.kt @@ -131,6 +131,14 @@ class EnrollmentRobot(val composeTestRule: ComposeTestRule) : BaseRobot() { ), useUnmergedTree = true, ).performTextReplacement(dateValue) + onNode( + hasTestTag( + "INPUT_DATE_TIME_TEXT_FIELD" + ) and hasAnySibling( + hasText(title) + ), + useUnmergedTree = true, + ).performImeAction() } }