From a1df0fedfeb19116c91f294975cdc5f07bc1d187 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 29 Nov 2023 14:56:55 +0530 Subject: [PATCH 1/3] Implemented debouncing mechanism for the search functionality. * Implemented a debouncing mechanism to enhance the handling of search functionalities. This approach is employed to prevent unnecessary data loads from libkiwix, addressing the issue of crashes when users rapidly type and search for results. --- .../kiwixmobile/core/search/SearchFragment.kt | 3 +-- .../core/search/viewmodel/SearchViewModel.kt | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt index 55dbecc41f..d0fa06af50 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt @@ -61,7 +61,6 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.Action import org.kiwix.kiwixmobile.core.search.viewmodel.Action.ActivityResultReceived import org.kiwix.kiwixmobile.core.search.viewmodel.Action.ClickedSearchInText import org.kiwix.kiwixmobile.core.search.viewmodel.Action.ExitedSearch -import org.kiwix.kiwixmobile.core.search.viewmodel.Action.Filter import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemClick import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemLongClick import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnOpenInNewTabClick @@ -219,7 +218,7 @@ class SearchFragment : BaseFragment() { searchView?.setOnQueryTextListener( SimpleTextListener { if (it.isNotEmpty()) { - searchViewModel.actions.trySend(Filter(it)).isSuccess + searchViewModel.searchResults(it) } } ) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt index f284a0422f..e835ee27fa 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt @@ -31,6 +31,8 @@ import kotlinx.coroutines.channels.trySendBlocking import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch @@ -65,6 +67,8 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.effects.StartSpeechInput import org.kiwix.libzim.SuggestionSearch import javax.inject.Inject +const val DEBOUNCE_DELAY = 500L + @OptIn(ExperimentalCoroutinesApi::class) class SearchViewModel @Inject constructor( private val recentSearchDao: NewRecentSearchDao, @@ -88,10 +92,24 @@ class SearchViewModel @Inject constructor( val actions = Channel(Channel.UNLIMITED) private val filter = ConflatedBroadcastChannel("") private val searchOrigin = ConflatedBroadcastChannel(FromWebView) + private val debouncedSearchQuery = MutableStateFlow("") init { viewModelScope.launch { reducer() } viewModelScope.launch { actionMapper() } + viewModelScope.launch { debouncedSearchQuery() } + } + + private suspend fun debouncedSearchQuery() { + // Observe and collect the debounced search query + debouncedSearchQuery + // Applying debouncing to delay the emission of consecutive search queries + .debounce(DEBOUNCE_DELAY) + // Ensuring that only distinct search queries are processed + .distinctUntilChanged() + .collect { query -> + actions.trySend(Filter(query)).isSuccess + } } @Suppress("DEPRECATION") @@ -201,6 +219,10 @@ class SearchViewModel @Inject constructor( }, LATEST) .subscribeOn(Schedulers.io()) } + + fun searchResults(query: String) { + debouncedSearchQuery.value = query + } } data class SearchResultsWithTerm(val searchTerm: String, val suggestionSearch: SuggestionSearch?) From c3da77b5966098ff27aa5978cb391ecd1ae44cf5 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 29 Nov 2023 18:48:40 +0530 Subject: [PATCH 2/3] Added unit test cases to properly test the search functionalities scenario. --- .../search/viewmodel/SearchViewModelTest.kt | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt index 7fbf391906..36eb2c6857 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt @@ -29,10 +29,12 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.consumeAsFlow import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestCoroutineDispatcher +import kotlinx.coroutines.test.TestCoroutineScheduler import kotlinx.coroutines.test.TestCoroutineScope import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.resetMain @@ -109,6 +111,93 @@ internal class SearchViewModelTest { viewModel = SearchViewModel(recentSearchDao, zimReaderContainer, searchResultGenerator) } + @Nested + inner class DebouncedTest { + @Test + fun `Search action is debounced`() = runTest { + val searchTerm1 = "query1" + val searchTerm2 = "query2" + val searchTerm3 = "query3" + val searchOrigin = FromWebView + val suggestionSearch: SuggestionSearch = mockk() + + viewModel.state + .test(this) + .also { + searchResult(searchTerm1, suggestionSearch, testScheduler) + delay(100) + searchResult(searchTerm2, suggestionSearch, testScheduler) + delay(100) + searchResult(searchTerm3, suggestionSearch, testScheduler) + it.assertValue( + SearchState( + searchTerm3, + SearchResultsWithTerm(searchTerm3, suggestionSearch), + emptyList(), + searchOrigin + ) + ) + } + .finish() + } + + @Test + fun `Search action is not debounced if time hasn't passed`() = runTest { + val searchTerm1 = "query1" + val searchTerm2 = "query2" + val searchTerm3 = "query3" + val searchOrigin = FromWebView + val suggestionSearch: SuggestionSearch = mockk() + + viewModel.state + .test(this) + .also { + searchResult(searchTerm1, suggestionSearch, testScheduler) + delay(50) // assume user rapidly typing + searchResult(searchTerm2, suggestionSearch, testScheduler) + delay(50) + // test value is not passed to searchResult as time has not passed and user still typing + // Match if it is initial `SearchState` + it.assertValue( + SearchState( + "", + SearchResultsWithTerm("", null), + emptyList(), + searchOrigin + ) + ) + searchResult(searchTerm3, suggestionSearch, testScheduler) + delay(DEBOUNCE_DELAY) + it.assertValue( + SearchState( + searchTerm3, + SearchResultsWithTerm(searchTerm3, suggestionSearch), + emptyList(), + searchOrigin + ) + ) + } + .finish() + } + + private fun searchResult( + searchTerm: String, + suggestionSearch: SuggestionSearch, + testScheduler: TestCoroutineScheduler + ) { + coEvery { + searchResultGenerator.generateSearchResults(searchTerm, zimFileReader) + } returns suggestionSearch + viewModel.searchResults(searchTerm) + recentsFromDb.trySend(emptyList()).isSuccess + viewModel.actions.trySend(ScreenWasStartedFrom(FromWebView)).isSuccess + testScheduler.apply { + advanceTimeBy(400) + runCurrent() + } + } + } + @Nested inner class StateTests { @Test From c90c6312f4421cf1b8c6484ac52c39f51bce6093 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 29 Nov 2023 19:29:20 +0530 Subject: [PATCH 3/3] Fixed unit test cases. * Fixed: `Search action is debounced` test. * Fixed: `SearchState combines sources from inputs` test. --- .../kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt index 36eb2c6857..6d19e066d2 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt @@ -59,7 +59,6 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.Action.ClickedSearchInText import org.kiwix.kiwixmobile.core.search.viewmodel.Action.ConfirmedDelete import org.kiwix.kiwixmobile.core.search.viewmodel.Action.CreatedWithArguments import org.kiwix.kiwixmobile.core.search.viewmodel.Action.ExitedSearch -import org.kiwix.kiwixmobile.core.search.viewmodel.Action.Filter import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemClick import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemLongClick import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnOpenInNewTabClick @@ -129,6 +128,7 @@ internal class SearchViewModelTest { searchResult(searchTerm2, suggestionSearch, testScheduler) delay(100) searchResult(searchTerm3, suggestionSearch, testScheduler) + delay(DEBOUNCE_DELAY) it.assertValue( SearchState( searchTerm3, @@ -341,7 +341,7 @@ internal class SearchViewModelTest { coEvery { searchResultGenerator.generateSearchResults(searchTerm, zimFileReader) } returns suggestionSearch - viewModel.actions.trySend(Filter(searchTerm)).isSuccess + viewModel.searchResults(searchTerm) recentsFromDb.trySend(databaseResults).isSuccess viewModel.actions.trySend(ScreenWasStartedFrom(searchOrigin)).isSuccess testScheduler.apply {