From c4dbe478b80452961db006fc3d790514e243e36a Mon Sep 17 00:00:00 2001 From: MohitMali Date: Tue, 8 Aug 2023 16:15:14 +0530 Subject: [PATCH] Use `SuggestionSearch` instead of `Search` for better search functionality. * Since `Search` is not compatible with those zim files which does not have Xapian index but `SuggestionSearch` have this functionality to search inside those zim files so we have used this. * Update test cases for test new search functionality. --- .../kiwix/kiwixmobile/core/reader/ZimFileReader.kt | 11 +++++------ .../core/search/viewmodel/SearchResultGenerator.kt | 4 ++-- .../kiwixmobile/core/search/viewmodel/SearchState.kt | 2 +- .../core/search/viewmodel/SearchViewModel.kt | 4 ++-- .../core/search/viewmodel/SearchStateTest.kt | 12 ++++++------ .../core/search/viewmodel/SearchViewModelTest.kt | 12 ++++++------ .../{EntryWrapper.kt => SuggestionItemWrapper.kt} | 4 ++-- ...eratorWrapper.kt => SuggestionIteratorWrapper.kt} | 7 +++---- .../{SearchWrapper.kt => SuggestionSearchWrapper.kt} | 9 ++++----- .../search/viewmodel/ZimSearchResultGeneratorTest.kt | 6 +++--- 10 files changed, 34 insertions(+), 37 deletions(-) rename core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/{EntryWrapper.kt => SuggestionItemWrapper.kt} (90%) rename core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/{SearchIteratorWrapper.kt => SuggestionIteratorWrapper.kt} (76%) rename core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/{SearchWrapper.kt => SuggestionSearchWrapper.kt} (72%) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt index e2dcb861b..6c48300af 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt @@ -37,9 +37,8 @@ import org.kiwix.libkiwix.JNIKiwixException import org.kiwix.libzim.Archive import org.kiwix.libzim.DirectAccessInfo import org.kiwix.libzim.Item -import org.kiwix.libzim.Query -import org.kiwix.libzim.Search -import org.kiwix.libzim.Searcher +import org.kiwix.libzim.SuggestionSearch +import org.kiwix.libzim.SuggestionSearcher import java.io.File import java.io.FileInputStream import java.io.IOException @@ -56,7 +55,7 @@ class ZimFileReader constructor( val zimFile: File, private val jniKiwixReader: Archive, private val nightModeConfig: NightModeConfig, - private val searcher: Searcher = Searcher(jniKiwixReader) + private val searcher: SuggestionSearcher = SuggestionSearcher(jniKiwixReader) ) { interface Factory { fun create(file: File): ZimFileReader? @@ -130,9 +129,9 @@ class ZimFileReader constructor( null } - fun searchSuggestions(prefix: String): Search? = + fun searchSuggestions(prefix: String): SuggestionSearch? = try { - searcher.search(Query(prefix)) + searcher.suggest(prefix) } catch (ignore: Exception) { // to handled the exception if there is no FT Xapian index found in the current zim file null diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchResultGenerator.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchResultGenerator.kt index 65d2b2980..75d4dd374 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchResultGenerator.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchResultGenerator.kt @@ -22,14 +22,14 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import kotlinx.coroutines.yield import org.kiwix.kiwixmobile.core.reader.ZimFileReader -import org.kiwix.libzim.Search +import org.kiwix.libzim.SuggestionSearch import javax.inject.Inject interface SearchResultGenerator { suspend fun generateSearchResults( searchTerm: String, zimFileReader: ZimFileReader? - ): Search? + ): SuggestionSearch? } class ZimSearchResultGenerator @Inject constructor() : SearchResultGenerator { diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt index 9fd480d44..ef745ae6d 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt @@ -30,7 +30,7 @@ data class SearchState( if (searchTerm.isEmpty()) { recentResults } else { - searchResultsWithTerm.search?.let { + searchResultsWithTerm.suggestionSearch?.let { val maximumResults = it.estimatedMatches val safeEndIndex = if (startIndex + 100 < maximumResults) startIndex + 100 else maximumResults 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 6b0b62c2e..c890fbf9c 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 @@ -59,7 +59,7 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.effects.SearchInPreviousScree import org.kiwix.kiwixmobile.core.search.viewmodel.effects.ShowDeleteSearchDialog import org.kiwix.kiwixmobile.core.search.viewmodel.effects.ShowToast import org.kiwix.kiwixmobile.core.search.viewmodel.effects.StartSpeechInput -import org.kiwix.libzim.Search +import org.kiwix.libzim.SuggestionSearch import javax.inject.Inject @OptIn(ExperimentalCoroutinesApi::class) @@ -174,4 +174,4 @@ class SearchViewModel @Inject constructor( } } -data class SearchResultsWithTerm(val searchTerm: String, val search: Search?) +data class SearchResultsWithTerm(val searchTerm: String, val suggestionSearch: SuggestionSearch?) diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt index ab0c1f14e..b1a0c9528 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt @@ -30,11 +30,11 @@ internal class SearchStateTest { @Test internal fun `visibleResults use searchResults when searchTerm is not empty`() { val searchTerm = "notEmpty" - val searchWrapper: SearchWrapper = mockk() - val searchIteratorWrapper: SearchIteratorWrapper = mockk() - val entryWrapper: EntryWrapper = mockk() + val suggestionSearchWrapper: SuggestionSearchWrapper = mockk() + val searchIteratorWrapper: SuggestionIteratorWrapper = mockk() + val entryWrapper: SuggestionItemWrapper = mockk() val estimatedMatches = 1 - every { searchWrapper.estimatedMatches } returns estimatedMatches.toLong() + every { suggestionSearchWrapper.estimatedMatches } returns estimatedMatches.toLong() // Settings list to hasNext() to ensure it returns true only for the first call. // Otherwise, if we do not set this, the method will always return true when checking if the iterator has a next value, // causing our test case to get stuck in an infinite loop due to this explicit setting. @@ -42,7 +42,7 @@ internal class SearchStateTest { every { searchIteratorWrapper.next() } returns entryWrapper every { entryWrapper.title } returns searchTerm every { - searchWrapper.getResults( + suggestionSearchWrapper.getResults( 0, estimatedMatches ) @@ -50,7 +50,7 @@ internal class SearchStateTest { assertThat( SearchState( searchTerm, - SearchResultsWithTerm("", searchWrapper), + SearchResultsWithTerm("", suggestionSearchWrapper), emptyList(), FromWebView ).getVisibleResults(0) 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 e234fea30..f847b62f8 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 @@ -74,7 +74,7 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.effects.SearchInPreviousScree import org.kiwix.kiwixmobile.core.search.viewmodel.effects.ShowDeleteSearchDialog import org.kiwix.kiwixmobile.core.search.viewmodel.effects.ShowToast import org.kiwix.kiwixmobile.core.search.viewmodel.effects.StartSpeechInput -import org.kiwix.libzim.Search +import org.kiwix.libzim.SuggestionSearch @OptIn(ExperimentalCoroutinesApi::class) internal class SearchViewModelTest { @@ -122,12 +122,12 @@ internal class SearchViewModelTest { val item = ZimSearchResultListItem("") val searchTerm = "searchTerm" val searchOrigin = FromWebView - val search: Search = mockk() + val suggestionSearch: SuggestionSearch = mockk() viewModel.state.test(this) .also { emissionOf( searchTerm = searchTerm, - search = search, + suggestionSearch = suggestionSearch, databaseResults = listOf(RecentSearchListItem("")), searchOrigin = searchOrigin ) @@ -135,7 +135,7 @@ internal class SearchViewModelTest { .assertValue( SearchState( searchTerm, - SearchResultsWithTerm(searchTerm, search), + SearchResultsWithTerm(searchTerm, suggestionSearch), listOf(RecentSearchListItem("")), searchOrigin ) @@ -243,14 +243,14 @@ internal class SearchViewModelTest { private fun TestScope.emissionOf( searchTerm: String, - search: Search, + suggestionSearch: SuggestionSearch, databaseResults: List, searchOrigin: SearchOrigin ) { coEvery { searchResultGenerator.generateSearchResults(searchTerm, zimFileReader) - } returns search + } returns suggestionSearch viewModel.actions.trySend(Filter(searchTerm)).isSuccess recentsFromDb.trySend(databaseResults).isSuccess viewModel.actions.trySend(ScreenWasStartedFrom(searchOrigin)).isSuccess diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/EntryWrapper.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionItemWrapper.kt similarity index 90% rename from core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/EntryWrapper.kt rename to core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionItemWrapper.kt index 4b9833306..bc5942e66 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/EntryWrapper.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionItemWrapper.kt @@ -18,8 +18,8 @@ package org.kiwix.kiwixmobile.core.search.viewmodel -import org.kiwix.libzim.Entry +import org.kiwix.libzim.SuggestionItem -internal class EntryWrapper : Entry() { +class SuggestionItemWrapper : SuggestionItem() { override fun getTitle(): String = super.getTitle() } diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchIteratorWrapper.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionIteratorWrapper.kt similarity index 76% rename from core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchIteratorWrapper.kt rename to core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionIteratorWrapper.kt index b26760899..85340cd69 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchIteratorWrapper.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionIteratorWrapper.kt @@ -18,12 +18,11 @@ package org.kiwix.kiwixmobile.core.search.viewmodel -import org.kiwix.libzim.SearchIterator +import org.kiwix.libzim.SuggestionIterator -// Create this as a helper class, as we can not directly use libkiwix/libzim functions in testing -internal class SearchIteratorWrapper : SearchIterator() { +class SuggestionIteratorWrapper : SuggestionIterator() { override fun remove() {} override fun hasNext(): Boolean = super.hasNext() - override fun next(): EntryWrapper = super.next() as EntryWrapper + override fun next(): SuggestionItemWrapper = super.next() as SuggestionItemWrapper } diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchWrapper.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionSearchWrapper.kt similarity index 72% rename from core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchWrapper.kt rename to core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionSearchWrapper.kt index d9c1f878c..80db06685 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchWrapper.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionSearchWrapper.kt @@ -18,12 +18,11 @@ package org.kiwix.kiwixmobile.core.search.viewmodel -import org.kiwix.libzim.Search +import org.kiwix.libzim.SuggestionSearch -// Create this as a helper class, as we can not directly use libkiwix/libzim functions in testing -internal class SearchWrapper : Search() { +class SuggestionSearchWrapper : SuggestionSearch() { override fun getEstimatedMatches(): Long = super.getEstimatedMatches() - override fun getResults(start: Int, maxResults: Int): SearchIteratorWrapper = - super.getResults(start, maxResults) as SearchIteratorWrapper + override fun getResults(start: Int, maxResults: Int): SuggestionIteratorWrapper = + super.getResults(start, maxResults) as SuggestionIteratorWrapper } diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/ZimSearchResultGeneratorTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/ZimSearchResultGeneratorTest.kt index caba9daf7..287e4bd61 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/ZimSearchResultGeneratorTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/ZimSearchResultGeneratorTest.kt @@ -44,11 +44,11 @@ internal class ZimSearchResultGeneratorTest { @Test internal fun `suggestion results are distinct`() { val searchTerm = " " - val searchWrapper: SearchWrapper = mockk() - every { zimFileReader.searchSuggestions(searchTerm) } returns searchWrapper + val suggestionSearchWrapper: SuggestionSearchWrapper = mockk() + every { zimFileReader.searchSuggestions(searchTerm) } returns suggestionSearchWrapper runBlocking { assertThat(zimSearchResultGenerator.generateSearchResults(searchTerm, zimFileReader)) - .isEqualTo(searchWrapper) + .isEqualTo(suggestionSearchWrapper) verify { zimFileReader.searchSuggestions(searchTerm) }