Merge pull request #3514 from kiwix/Issue#3508

Fixed, the DWDS app is very slow when opening a search result.

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt

@@ -102,11 +102,14 @@ class KiwixReaderFragment : CoreReaderFragment() {
   }
 
   private fun openSearchItem(item: SearchItemToOpen) {
-    zimReaderContainer?.titleToUrl(item.pageTitle)?.let {
-      if (item.shouldOpenInNewTab) {
-        createNewTab()
+    item.pageUrl?.let(::loadUrlWithCurrentWebview) ?: kotlin.run {
+      // For handling the previously saved recent searches
+      zimReaderContainer?.titleToUrl(item.pageTitle)?.let {
+        if (item.shouldOpenInNewTab) {
+          createNewTab()
+        }
+        loadUrlWithCurrentWebview(zimReaderContainer?.urlSuffixToParsableUrl(it))
       }
-      loadUrlWithCurrentWebview(zimReaderContainer?.urlSuffixToParsableUrl(it))
     }
     requireActivity().consumeObservable<SearchItemToOpen>(TAG_FILE_SEARCHED)
   }

core/objectbox-models/default.json

@@ -221,7 +221,7 @@
     },
     {
       "id": "7:7635075139296819361",
-      "lastPropertyId": "3:3320858395373055542",
+      "lastPropertyId": "4:6431198268026321201",
       "name": "RecentSearchEntity",
       "properties": [
         {
@@ -239,6 +239,11 @@
           "id": "3:3320858395373055542",
           "name": "zimId",
           "type": 9
+        },
+        {
+          "id": "4:6431198268026321201",
+          "name": "url",
+          "type": 9
         }
       ],
       "relations": []

core/objectbox-models/default.json.bak

@@ -391,9 +391,7 @@
         {
           "id": "5:7998345745727500384",
           "name": "noteTitle",
-          "indexId": "5:7599267150617990347",
-          "type": 9,
-          "flags": 2080
+          "type": 9
         },
         {
           "id": "7:2160052450778801841",
@@ -429,7 +427,8 @@
     1293695782925933448,
     3655049272366703856,
     7576716732364166705,
-    4868787482832538530
+    4868787482832538530,
+    7599267150617990347
   ],
   "retiredPropertyUids": [
     4712434661554562781,

core/src/main/java/org/kiwix/kiwixmobile/core/dao/NewRecentSearchDao.kt

@@ -42,11 +42,11 @@ class NewRecentSearchDao @Inject constructor(
   ).map { searchEntities ->
     searchEntities.distinctBy(RecentSearchEntity::searchTerm)
       .take(NUM_RECENT_RESULTS)
-      .map { searchEntity -> RecentSearchListItem(searchEntity.searchTerm) }
+      .map { searchEntity -> RecentSearchListItem(searchEntity.searchTerm, searchEntity.url) }
   }
 
-  fun saveSearch(title: String, id: String) {
-    box.put(RecentSearchEntity(searchTerm = title, zimId = id))
+  fun saveSearch(title: String, id: String, url: String?) {
+    box.put(RecentSearchEntity(searchTerm = title, zimId = id, url = url))
   }
 
   fun deleteSearchString(searchTerm: String) {

core/src/main/java/org/kiwix/kiwixmobile/core/dao/entities/RecentSearchEntity.kt

@@ -24,5 +24,6 @@ import io.objectbox.annotation.Id
 data class RecentSearchEntity(
   @Id var id: Long = 0L,
   val searchTerm: String,
-  val zimId: String
+  val zimId: String,
+  val url: String?
 )

core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt

@@ -336,7 +336,18 @@ private val String.filePath: String
 
 // Decode the URL if it is encoded because libkiwix does not return the path for encoded paths.
 val String.decodeUrl: String
-  get() = URLDecoder.decode(this, "UTF-8")
+  get() = try {
+    URLDecoder.decode(this, "UTF-8")
+  } catch (illegalArgumentException: IllegalArgumentException) {
+    // Searched item already has the decoded URL,
+    // and if any URL contains % in it, then it will fail to decode that URL and will not load the page.
+    // e.g. https://kiwix.app/A/FT%, More info https://github.com/kiwix/kiwix-android/pull/3514
+    Log.e(
+      "ZimFileReader",
+      "Could not decode url $this \n original exception = $illegalArgumentException"
+    )
+    this
+  }
 
 // Truncate mime-type (everything after the first space and semi-colon(if exists)
 val String.truncateMimeType: String
@@ -352,3 +363,7 @@ private val DirectAccessInfo.parcelFileDescriptor: ParcelFileDescriptor?
 
 // Default illustration size for ZIM file favicons
 const val ILLUSTRATION_SIZE = 48
+
+// add content prefix to url since searched items return the url inside of zim without content prefix.
+val String.addContentPrefix: String
+  get() = if (startsWith(CONTENT_PREFIX)) this else CONTENT_PREFIX + this

core/src/main/java/org/kiwix/kiwixmobile/core/search/adapter/SearchListItem.kt

@@ -20,7 +20,13 @@ package org.kiwix.kiwixmobile.core.search.adapter
 
 sealed class SearchListItem {
   abstract val value: String
+  abstract val url: String?
 
-  data class RecentSearchListItem(override val value: String) : SearchListItem()
-  data class ZimSearchResultListItem constructor(override val value: String) : SearchListItem()
+  data class RecentSearchListItem(override val value: String, override val url: String?) :
+    SearchListItem()
+
+  data class ZimSearchResultListItem constructor(
+    override val value: String,
+    override val url: String?
+  ) : SearchListItem()
 }

core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt

@@ -39,7 +39,7 @@ data class SearchState(
         val searchResults = mutableListOf<SearchListItem.RecentSearchListItem>()
         while (searchIterator.hasNext()) {
           val entry = searchIterator.next()
-          searchResults.add(SearchListItem.RecentSearchListItem(entry.title))
+          searchResults.add(SearchListItem.RecentSearchListItem(entry.title, entry.path))
         }
         /**
          * Returns null if there are no suggestions left in the iterator.

core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt

@@ -170,7 +170,8 @@ class SearchViewModel @Inject constructor(
       SaveSearchToRecents(
         recentSearchDao,
         searchListItem,
-        zimReaderContainer.id
+        zimReaderContainer.id,
+        viewModelScope
       )
     ).isSuccess
     _effects.trySendBlocking(OpenSearchItem(searchListItem, openInNewTab))

core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItem.kt

@@ -24,6 +24,7 @@ import kotlinx.parcelize.Parcelize
 import org.kiwix.kiwixmobile.core.base.SideEffect
 import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.setNavigationResultOnCurrent
 import org.kiwix.kiwixmobile.core.main.CoreMainActivity
+import org.kiwix.kiwixmobile.core.reader.addContentPrefix
 import org.kiwix.kiwixmobile.core.search.adapter.SearchListItem
 import org.kiwix.kiwixmobile.core.utils.TAG_FILE_SEARCHED
 
@@ -35,7 +36,11 @@ data class OpenSearchItem(
     val readerFragmentResId = (activity as CoreMainActivity).readerFragmentResId
     activity.navigate(readerFragmentResId)
     activity.setNavigationResultOnCurrent(
-      SearchItemToOpen(searchListItem.value, openInNewTab),
+      SearchItemToOpen(
+        searchListItem.value,
+        openInNewTab,
+        searchListItem.url?.addContentPrefix
+      ),
       TAG_FILE_SEARCHED
     )
   }
@@ -44,5 +49,6 @@ data class OpenSearchItem(
 @Parcelize
 data class SearchItemToOpen(
   val pageTitle: String,
-  val shouldOpenInNewTab: Boolean
+  val shouldOpenInNewTab: Boolean,
+  val pageUrl: String?
 ) : Parcelable

core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/SaveSearchToRecents.kt

@@ -19,16 +19,29 @@
 package org.kiwix.kiwixmobile.core.search.viewmodel.effects
 
 import androidx.appcompat.app.AppCompatActivity
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
 import org.kiwix.kiwixmobile.core.base.SideEffect
 import org.kiwix.kiwixmobile.core.dao.NewRecentSearchDao
+import org.kiwix.kiwixmobile.core.reader.addContentPrefix
 import org.kiwix.kiwixmobile.core.search.adapter.SearchListItem
 
 data class SaveSearchToRecents(
   private val recentSearchDao: NewRecentSearchDao,
   private val searchListItem: SearchListItem,
-  private val id: String?
+  private val id: String?,
+  private val viewModelScope: CoroutineScope
 ) : SideEffect<Unit> {
   override fun invokeWith(activity: AppCompatActivity) {
-    id?.let { recentSearchDao.saveSearch(searchListItem.value, it) }
+    id?.let {
+      viewModelScope.launch(Dispatchers.IO) {
+        recentSearchDao.saveSearch(
+          searchListItem.value,
+          it,
+          searchListItem.url?.addContentPrefix
+        )
+      }
+    }
   }
 }

core/src/sharedTestFunctions/java/org/kiwix/sharedFunctions/TestModelFunctions.kt

@@ -165,8 +165,13 @@ fun libraryNetworkEntity(books: List<Book> = emptyList()) = LibraryNetworkEntity
   book = LinkedList(books)
 }
 
-fun recentSearchEntity(id: Long = 0L, searchTerm: String = "", zimId: String = "") =
-  RecentSearchEntity(id, searchTerm, zimId)
+fun recentSearchEntity(
+  id: Long = 0L,
+  searchTerm: String = "",
+  zimId: String = "",
+  url: String = ""
+) =
+  RecentSearchEntity(id, searchTerm, zimId, url)
 
 fun bookOnDiskEntity(
   id: Long = 0,

core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewRecentSearchDaoTest.kt

@@ -50,7 +50,7 @@ internal class NewRecentSearchDaoTest {
       newRecentSearchDao.recentSearches(zimId)
         .test(this)
         .assertValues(
-          queryResult.map { RecentSearchListItem(it.searchTerm) }
+          queryResult.map { RecentSearchListItem(it.searchTerm, it.url) }
         )
         .finish()
     }
@@ -62,7 +62,7 @@ internal class NewRecentSearchDaoTest {
       newRecentSearchDao.recentSearches(null)
         .test(this)
         .assertValues(
-          queryResult.map { RecentSearchListItem(it.searchTerm) }
+          queryResult.map { RecentSearchListItem(it.searchTerm, it.url) }
         )
         .finish()
     }
@@ -74,7 +74,7 @@ internal class NewRecentSearchDaoTest {
       newRecentSearchDao.recentSearches("")
         .test(this)
         .assertValues(
-          queryResult.take(1).map { RecentSearchListItem(it.searchTerm) }
+          queryResult.take(1).map { RecentSearchListItem(it.searchTerm, it.url) }
         )
         .finish()
     }
@@ -109,8 +109,16 @@ internal class NewRecentSearchDaoTest {
 
   @Test
   fun `saveSearch puts RecentSearchEntity into box`() {
-    newRecentSearchDao.saveSearch("title", "id")
-    verify { box.put(recentSearchEntity(searchTerm = "title", zimId = "id")) }
+    newRecentSearchDao.saveSearch("title", "id", "https://kiwix.app/mainPage")
+    verify {
+      box.put(
+        recentSearchEntity(
+          searchTerm = "title",
+          zimId = "id",
+          url = "https://kiwix.app/mainPage"
+        )
+      )
+    }
   }
 
   @Test

core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt

@@ -31,6 +31,7 @@ internal class SearchStateTest {
   @Test
   internal fun `visibleResults use searchResults when searchTerm is not empty`() = runTest {
     val searchTerm = "notEmpty"
+    val pageUrl = ""
     val suggestionSearchWrapper: SuggestionSearchWrapper = mockk()
     val searchIteratorWrapper: SuggestionIteratorWrapper = mockk()
     val entryWrapper: SuggestionItemWrapper = mockk()
@@ -42,6 +43,7 @@ internal class SearchStateTest {
     every { searchIteratorWrapper.hasNext() } returnsMany listOf(true, false)
     every { searchIteratorWrapper.next() } returns entryWrapper
     every { entryWrapper.title } returns searchTerm
+    every { entryWrapper.path } returns pageUrl
     every {
       suggestionSearchWrapper.getResults(
         0,
@@ -55,12 +57,12 @@ internal class SearchStateTest {
         emptyList(),
         FromWebView
       ).getVisibleResults(0)
-    ).isEqualTo(listOf(RecentSearchListItem(searchTerm)))
+    ).isEqualTo(listOf(RecentSearchListItem(searchTerm, "")))
   }
 
   @Test
   internal fun `visibleResults use recentResults when searchTerm is empty`() = runTest {
-    val results = listOf(RecentSearchListItem(""))
+    val results = listOf(RecentSearchListItem("", ""))
     assertThat(
       SearchState(
         "",

core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt

@@ -19,6 +19,7 @@
 package org.kiwix.kiwixmobile.core.search.viewmodel
 
 import android.os.Bundle
+import androidx.lifecycle.viewModelScope
 import io.mockk.clearAllMocks
 import io.mockk.coEvery
 import io.mockk.every
@@ -119,7 +120,7 @@ internal class SearchViewModelTest {
 
     @Test
     fun `SearchState combines sources from inputs`() = runTest {
-      val item = ZimSearchResultListItem("")
+      val item = ZimSearchResultListItem("", "")
       val searchTerm = "searchTerm"
       val searchOrigin = FromWebView
       val suggestionSearch: SuggestionSearch = mockk()
@@ -128,15 +129,15 @@ internal class SearchViewModelTest {
           emissionOf(
             searchTerm = searchTerm,
             suggestionSearch = suggestionSearch,
-            databaseResults = listOf(RecentSearchListItem("")),
+            databaseResults = listOf(RecentSearchListItem("", "")),
             searchOrigin = searchOrigin
           )
         }
         .assertValue(
           SearchState(
             searchTerm,
             SearchResultsWithTerm(searchTerm, suggestionSearch),
-            listOf(RecentSearchListItem("")),
+            listOf(RecentSearchListItem("", "")),
             searchOrigin
           )
         )
@@ -154,27 +155,27 @@ internal class SearchViewModelTest {
 
     @Test
     fun `OnItemClick offers Saves and Opens`() = runBlockingTest {
-      val searchListItem = RecentSearchListItem("")
+      val searchListItem = RecentSearchListItem("", "")
       actionResultsInEffects(
         OnItemClick(searchListItem),
-        SaveSearchToRecents(recentSearchDao, searchListItem, "id"),
+        SaveSearchToRecents(recentSearchDao, searchListItem, "id", viewModel.viewModelScope),
         OpenSearchItem(searchListItem, false)
       )
     }
 
     @Test
     fun `OnOpenInNewTabClick offers Saves and Opens in new tab`() = runBlockingTest {
-      val searchListItem = RecentSearchListItem("")
+      val searchListItem = RecentSearchListItem("", "")
       actionResultsInEffects(
         OnOpenInNewTabClick(searchListItem),
-        SaveSearchToRecents(recentSearchDao, searchListItem, "id"),
+        SaveSearchToRecents(recentSearchDao, searchListItem, "id", viewModel.viewModelScope),
         OpenSearchItem(searchListItem, true)
       )
     }
 
     @Test
     fun `OnItemLongClick offers Saves and Opens`() = runBlockingTest {
-      val searchListItem = RecentSearchListItem("")
+      val searchListItem = RecentSearchListItem("", "")
       actionResultsInEffects(
         OnItemLongClick(searchListItem),
         ShowDeleteSearchDialog(searchListItem, viewModel.actions)
@@ -188,7 +189,7 @@ internal class SearchViewModelTest {
 
     @Test
     fun `ConfirmedDelete offers Delete and Toast`() = runBlockingTest {
-      val searchListItem = RecentSearchListItem("")
+      val searchListItem = RecentSearchListItem("", "")
       actionResultsInEffects(
         ConfirmedDelete(searchListItem),
         DeleteRecentSearch(searchListItem, recentSearchDao),

core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SuggestionItemWrapper.kt

@@ -22,4 +22,5 @@ import org.kiwix.libzim.SuggestionItem
 
 class SuggestionItemWrapper : SuggestionItem() {
   override fun getTitle(): String = super.getTitle()
+  override fun getPath(): String = super.getPath()
 }

core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/DeleteRecentSearchTest.kt

@@ -30,7 +30,7 @@ internal class DeleteRecentSearchTest {
 
   @Test
   fun `invoke with deletes a search`() {
-    val searchListItem: SearchListItem = RecentSearchListItem("")
+    val searchListItem: SearchListItem = RecentSearchListItem("", "")
     val recentSearchDao: NewRecentSearchDao = mockk()
     val activity: AppCompatActivity = mockk()
     DeleteRecentSearch(searchListItem, recentSearchDao).invokeWith(activity)