Rebased and resolved conflicts.

* Improved the test cases for saving/migrating bookmarks in libkiwix. * Improved libkiwix bookmark saving/retrieving according to one reader.
kiwix · Feb 22, 2024 · a386cfc · a386cfc
1 parent f0de7b7
commit a386cfc
Show file tree

Hide file tree

Showing 11 changed files with 67 additions and 36 deletions.
diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt
@@ -46,6 +46,7 @@ import org.kiwix.kiwixmobile.core.dao.entities.BookmarkEntity
 import org.kiwix.kiwixmobile.core.data.remote.ObjectBoxToLibkiwixMigrator
 import org.kiwix.kiwixmobile.core.di.modules.DatabaseModule
 import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem
+import org.kiwix.kiwixmobile.core.reader.ZimReaderSource
 import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
 import org.kiwix.kiwixmobile.main.KiwixMainActivity
 import org.kiwix.kiwixmobile.testutils.RetryRule
@@ -76,6 +77,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
       expectedZimId,
       expectedZimName,
       expectedZimFilePath,
+      ZimReaderSource(File(expectedZimFilePath)),
       expectedBookmarkUrl,
       expectedTitle,
       expectedFavicon
@@ -153,6 +155,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
       expectedZimId,
       expectedZimName,
       expectedZimFilePath,
+      ZimReaderSource(File(expectedZimFilePath)),
       expectedBookmarkUrl,
       expectedTitle,
       expectedFavicon
@@ -169,7 +172,10 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
       .subscribe(
         { actualDataAfterMigration ->
           assertEquals(1, actualDataAfterMigration.size)
-          assertEquals(actualDataAfterMigration[0].zimFilePath, expectedZimFilePath)
+          assertEquals(
+            actualDataAfterMigration[0].zimReaderSource?.toDatabase(),
+            expectedZimFilePath
+          )
           assertEquals(actualDataAfterMigration[0].zimId, expectedZimId)
           assertEquals(actualDataAfterMigration[0].title, expectedTitle)
           assertEquals(actualDataAfterMigration[0].url, expectedBookmarkUrl)
@@ -213,6 +219,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
       expectedZimId,
       expectedZimName,
       expectedZimFilePath,
+      ZimReaderSource(File(expectedZimFilePath)),
       existingBookmarkUrl,
       existingTitle,
       expectedFavicon
@@ -269,6 +276,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
           expectedZimId,
           expectedZimName,
           expectedZimFilePath,
+          ZimReaderSource(File(expectedZimFilePath)),
           "https://alpine_linux/search_$it",
           "title_$it",
           expectedFavicon

diff --git a/...in/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt b/...in/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt
@@ -31,7 +31,7 @@ data class OpenFileWithNavigation(private val bookOnDisk: BooksOnDiskListItem.Bo
 
   override fun invokeWith(activity: AppCompatActivity) {
     val zimReaderSource = bookOnDisk.zimReaderSource
-    if (!zimReaderSource.exists()) {
+    if (!zimReaderSource.canOpenInLibkiwix()) {
       activity.toast(R.string.error_file_not_found)
     } else {
       activity.navigate(

diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt
@@ -253,7 +253,7 @@ class LibkiwixBookmarks @Inject constructor(
     getBookmarksList()
       .any {
         it.url == libkiwixBookmarkItem.bookmarkUrl &&
-          it.zimFilePath == libkiwixBookmarkItem.zimFilePath
+          it.zimReaderSource?.toDatabase() == libkiwixBookmarkItem.zimReaderSource?.toDatabase()
       }
 
   private fun flowableBookmarkList(

diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/data/remote/ObjectBoxToLibkiwixMigrator.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/data/remote/ObjectBoxToLibkiwixMigrator.kt
@@ -77,7 +77,7 @@ class ObjectBoxToLibkiwixMigrator {
           Log.e(
             "MIGRATING_BOOKMARKS",
             "there is an error while migrating the bookmark for\n" +
-              " ZIM file = ${bookmarkEntity.zimReaderSource.toDatabase()} \n" +
+              " ZIM file = ${bookmarkEntity.zimFilePath} \n" +
               "Bookmark Title = ${bookmarkEntity.bookmarkTitle} \n" +
               "Original exception is = $ignore"
           )

diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/LibkiwixBookmarkItem.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/LibkiwixBookmarkItem.kt
@@ -22,14 +22,14 @@ import org.kiwix.kiwixmobile.core.dao.entities.BookmarkEntity
 import org.kiwix.kiwixmobile.core.page.adapter.Page
 import org.kiwix.kiwixmobile.core.reader.ZimFileReader
 import org.kiwix.kiwixmobile.core.reader.ZimReaderSource
+import org.kiwix.kiwixmobile.core.reader.ZimReaderSource.Companion.fromDatabaseValue
 import org.kiwix.libkiwix.Book
 import org.kiwix.libkiwix.Bookmark
 
 data class LibkiwixBookmarkItem(
   val databaseId: Long = 0L,
   override val zimId: String,
   val zimName: String,
-  val zimFilePath: String?,
   override val zimReaderSource: ZimReaderSource?,
   val bookmarkUrl: String,
   override val title: String,
@@ -46,8 +46,7 @@ data class LibkiwixBookmarkItem(
   ) : this(
     zimId = libkiwixBookmark.bookId,
     zimName = libkiwixBookmark.bookTitle,
-    zimFilePath = zimFilePath,
-    zimReaderSource = null,
+    zimReaderSource = fromDatabaseValue(zimFilePath),
     bookmarkUrl = libkiwixBookmark.url,
     title = libkiwixBookmark.title,
     favicon = favicon,
@@ -60,8 +59,7 @@ data class LibkiwixBookmarkItem(
     zimFileReader: ZimFileReader,
     libKiwixBook: Book
   ) : this(
-    zimFilePath = zimFileReader.zimReaderSource.toDatabase(),
-    zimReaderSource = null,
+    zimReaderSource = zimFileReader.zimReaderSource,
     zimId = libKiwixBook.id,
     zimName = libKiwixBook.name,
     bookmarkUrl = articleUrl,
@@ -75,7 +73,6 @@ data class LibkiwixBookmarkItem(
     libkiwixBook: Book
   ) : this(
     zimId = bookmarkEntity.zimId,
-    zimFilePath = bookmarkEntity.zimReaderSource.toDatabase(),
     zimReaderSource = bookmarkEntity.zimReaderSource,
     zimName = bookmarkEntity.zimName,
     bookmarkUrl = bookmarkEntity.bookmarkUrl,

diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt
@@ -36,7 +36,7 @@ class ZimReaderContainer @Inject constructor(private val zimFileReaderFactory: F
       return
     }
     zimFileReader =
-      if (zimReaderSource?.exists() == true && zimReaderSource.canOpenInLibkiwix())
+      if (zimReaderSource?.exists() == true)
         zimFileReaderFactory.create(zimReaderSource)
       else null
   }

diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt
@@ -72,15 +72,19 @@ class ZimReaderSource(
   }
 
   fun createArchive(): Archive? {
-    return file?.let {
-      Archive(it.canonicalPath)
-    } ?: assetFileDescriptor?.let {
-      Archive(
-        it.parcelFileDescriptor.dup().fileDescriptor,
-        it.startOffset,
-        it.length
-      )
+    if (canOpenInLibkiwix()) {
+      return file?.let {
+        Archive(it.canonicalPath)
+      } ?: assetFileDescriptor?.let {
+        Archive(
+          it.parcelFileDescriptor.dup().fileDescriptor,
+          it.startOffset,
+          it.length
+        )
+      }
     }
+
+    return null
   }
 
   fun toDatabase(): String = file?.canonicalPath ?: uri.toString()

diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/page/PageTestHelpers.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/page/PageTestHelpers.kt
@@ -19,6 +19,7 @@
 package org.kiwix.kiwixmobile.core.page
 
 import org.kiwix.kiwixmobile.core.page.adapter.Page
+import org.kiwix.kiwixmobile.core.page.bookmark.adapter.BookmarkItem
 import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem
 import org.kiwix.kiwixmobile.core.page.bookmark.viewmodel.BookmarkState
 import org.kiwix.kiwixmobile.core.page.history.adapter.HistoryListItem
@@ -83,13 +84,34 @@ fun bookmark(
   zimReaderSource: ZimReaderSource,
   bookmarkUrl: String = "bookmarkUrl",
   favicon: String = "favicon"
+): BookmarkItem {
+  return BookmarkItem(
+    id = id,
+    zimId = zimId,
+    zimName = zimName,
+    zimReaderSource = zimReaderSource,
+    bookmarkUrl = bookmarkUrl,
+    title = bookmarkTitle,
+    isSelected = isSelected,
+    favicon = favicon
+  )
+}
+
+fun libkiwixBookmarkItem(
+  bookmarkTitle: String = "bookmarkTitle",
+  isSelected: Boolean = false,
+  id: Long = 2,
+  zimId: String = "zimId",
+  zimName: String = "zimName",
+  zimReaderSource: ZimReaderSource,
+  bookmarkUrl: String = "bookmarkUrl",
+  favicon: String = "favicon"
 ): LibkiwixBookmarkItem {
   return LibkiwixBookmarkItem(
     id = id,
     zimId = zimId,
     zimName = zimName,
-    zimFilePath = zimReaderSource.toDatabase(),
-    zimReaderSource = null,
+    zimReaderSource = zimReaderSource,
     bookmarkUrl = bookmarkUrl,
     title = bookmarkTitle,
     isSelected = isSelected,

diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/page/bookmark/viewmodel/BookmarkStateTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/page/bookmark/viewmodel/BookmarkStateTest.kt
@@ -21,8 +21,8 @@ package org.kiwix.kiwixmobile.core.page.bookmark.viewmodel
 import io.mockk.mockk
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
-import org.kiwix.kiwixmobile.core.page.bookmark
 import org.kiwix.kiwixmobile.core.page.bookmarkState
+import org.kiwix.kiwixmobile.core.page.libkiwixBookmarkItem
 import org.kiwix.kiwixmobile.core.reader.ZimReaderSource
 
 internal class BookmarkStateTest {
@@ -31,10 +31,10 @@ internal class BookmarkStateTest {
     val zimReaderSource: ZimReaderSource = mockk()
     assertThat(
       bookmarkState(emptyList()).copy(
-        listOf(bookmark(zimReaderSource = zimReaderSource))
+        listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource))
       ).pageItems
     ).isEqualTo(
-      listOf(bookmark(zimReaderSource = zimReaderSource))
+      listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource))
     )
   }
 }
diff --git a/...src/test/java/org/kiwix/kiwixmobile/core/page/bookmark/viewmodel/BookmarkViewModelTest.kt b/...src/test/java/org/kiwix/kiwixmobile/core/page/bookmark/viewmodel/BookmarkViewModelTest.kt
@@ -30,10 +30,10 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
 import org.kiwix.kiwixmobile.core.dao.LibkiwixBookmarks
 import org.kiwix.kiwixmobile.core.page.adapter.Page
-import org.kiwix.kiwixmobile.core.page.bookmark
 import org.kiwix.kiwixmobile.core.page.bookmark.viewmodel.effects.ShowDeleteBookmarksDialog
 import org.kiwix.kiwixmobile.core.page.bookmark.viewmodel.effects.UpdateAllBookmarksPreference
 import org.kiwix.kiwixmobile.core.page.bookmarkState
+import org.kiwix.kiwixmobile.core.page.libkiwixBookmarkItem
 import org.kiwix.kiwixmobile.core.page.viewmodel.Action
 import org.kiwix.kiwixmobile.core.page.viewmodel.Action.Filter
 import org.kiwix.kiwixmobile.core.page.viewmodel.Action.UpdatePages
@@ -90,10 +90,10 @@ internal class BookmarkViewModelTest {
     assertThat(
       viewModel.updatePages(
         bookmarkState(),
-        UpdatePages(listOf(bookmark(zimReaderSource = zimReaderSource)))
+        UpdatePages(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
       )
     ).isEqualTo(
-      bookmarkState(listOf(bookmark(zimReaderSource = zimReaderSource)))
+      bookmarkState(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
     )
   }
 
@@ -122,10 +122,10 @@ internal class BookmarkViewModelTest {
     assertThat(
       viewModel.updatePages(
         bookmarkState(),
-        UpdatePages(listOf(bookmark(zimReaderSource = zimReaderSource)))
+        UpdatePages(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
       )
     ).isEqualTo(
-      bookmarkState(listOf(bookmark(zimReaderSource = zimReaderSource)))
+      bookmarkState(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
     )
   }
 
@@ -136,7 +136,7 @@ internal class BookmarkViewModelTest {
       viewModel.deselectAllPages(
         bookmarkState(
           bookmarks = listOf(
-            bookmark(
+            libkiwixBookmarkItem(
               isSelected = true,
               zimReaderSource = zimReaderSource
             )
@@ -146,7 +146,7 @@ internal class BookmarkViewModelTest {
     ).isEqualTo(
       bookmarkState(
         bookmarks = listOf(
-          bookmark(
+          libkiwixBookmarkItem(
             isSelected = false,
             zimReaderSource = zimReaderSource
           )
@@ -170,10 +170,10 @@ internal class BookmarkViewModelTest {
     assertThat(
       viewModel.copyWithNewItems(
         bookmarkState(),
-        listOf(bookmark(zimReaderSource = zimReaderSource))
+        listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource))
       )
     ).isEqualTo(
-      bookmarkState(listOf(bookmark(zimReaderSource = zimReaderSource)))
+      bookmarkState(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
     )
   }
 }
diff --git a/...g/kiwix/kiwixmobile/core/page/bookmark/viewmodel/effects/ShowDeleteBookmarksDialogTest.kt b/...g/kiwix/kiwixmobile/core/page/bookmark/viewmodel/effects/ShowDeleteBookmarksDialogTest.kt
@@ -27,8 +27,8 @@ import org.junit.jupiter.api.Test
 import org.kiwix.kiwixmobile.core.base.SideEffect
 import org.kiwix.kiwixmobile.core.dao.NewBookmarksDao
 import org.kiwix.kiwixmobile.core.main.CoreMainActivity
-import org.kiwix.kiwixmobile.core.page.bookmark
 import org.kiwix.kiwixmobile.core.page.bookmarkState
+import org.kiwix.kiwixmobile.core.page.libkiwixBookmarkItem
 import org.kiwix.kiwixmobile.core.page.viewmodel.effects.DeletePageItems
 import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower
 import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog.DeleteAllBookmarks
@@ -64,7 +64,7 @@ internal class ShowDeleteBookmarksDialogTest {
     val showDeleteBookmarksDialog =
       ShowDeleteBookmarksDialog(
         effects,
-        bookmarkState(listOf(bookmark(isSelected = true, zimReaderSource = mockk()))),
+        bookmarkState(listOf(libkiwixBookmarkItem(isSelected = true, zimReaderSource = mockk()))),
         newBookmarksDao
       )
     mockkActivityInjection(showDeleteBookmarksDialog)
@@ -77,7 +77,7 @@ internal class ShowDeleteBookmarksDialogTest {
     val showDeleteBookmarksDialog =
       ShowDeleteBookmarksDialog(
         effects,
-        bookmarkState(listOf(bookmark(zimReaderSource = mockk()))),
+        bookmarkState(listOf(libkiwixBookmarkItem(zimReaderSource = mockk()))),
         newBookmarksDao
       )
     mockkActivityInjection(showDeleteBookmarksDialog)