Skip to content

Commit

Permalink
Merge pull request #3558 from kiwix/Issue#3556
Browse files Browse the repository at this point in the history
Fixed: Version 3.8.0 shuts down after searching for an entry.
  • Loading branch information
kelson42 authored Dec 1, 2023
2 parents acb9036 + c90c631 commit c3e46bb
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -219,7 +218,7 @@ class SearchFragment : BaseFragment() {
searchView?.setOnQueryTextListener(
SimpleTextListener {
if (it.isNotEmpty()) {
searchViewModel.actions.trySend(Filter(it)).isSuccess
searchViewModel.searchResults(it)
}
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -88,10 +92,24 @@ class SearchViewModel @Inject constructor(
val actions = Channel<Action>(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")
Expand Down Expand Up @@ -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?)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -57,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
Expand Down Expand Up @@ -109,6 +110,94 @@ 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)
delay(DEBOUNCE_DELAY)
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
Expand Down Expand Up @@ -252,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 {
Expand Down

0 comments on commit c3e46bb

Please sign in to comment.