Resolved bookmark saving issue, which causes the bug when we try to r…

…etrieve the saved bookmarks. * Enhanced the `isBookMarkExist` method, addressing a bug that prevented the addition of new bookmarks for the same file. The method has been refactored for improved functionality. * In the debug version, added informative logs to provide developers with insights into the bookmark-saving functionality.
kiwix · Nov 3, 2023 · d7fd2c3 · d7fd2c3
1 parent a211bba
commit d7fd2c3
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 15 deletions.
diff --git a/core/detekt_baseline.xml b/core/detekt_baseline.xml
@@ -68,6 +68,7 @@
     <ID>ReturnCount:ToolbarScrollingKiwixWebView.kt$ToolbarScrollingKiwixWebView$@SuppressLint("ClickableViewAccessibility") override fun onTouchEvent(event: MotionEvent): Boolean</ID>
     <ID>TooGenericExceptionCaught:CompatFindActionModeCallback.kt$CompatFindActionModeCallback$exception: Exception</ID>
     <ID>TooGenericExceptionCaught:JNIInitialiser.kt$JNIInitialiser$e: Exception</ID>
+    <ID>TooGenericExceptionCaught:LibkiwixBookmarks.kt$LibkiwixBookmarks$exception: Exception</ID>
     <ID>TooGenericExceptionCaught:OnSwipeTouchListener.kt$OnSwipeTouchListener.GestureListener$exception: Exception</ID>
     <ID>TooGenericExceptionCaught:ZimFileReader.kt$ZimFileReader$exception: Exception</ID>
     <ID>TooGenericExceptionThrown:AdapterDelegateManager.kt$AdapterDelegateManager$throw RuntimeException("No delegate registered for $item")</ID>

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
@@ -20,6 +20,7 @@ package org.kiwix.kiwixmobile.core.dao
 
 import android.os.Build
 import android.util.Base64
+import android.util.Log
 import io.reactivex.BackpressureStrategy
 import io.reactivex.BackpressureStrategy.LATEST
 import io.reactivex.Flowable
@@ -28,6 +29,7 @@ import io.reactivex.subjects.BehaviorSubject
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import org.kiwix.kiwixmobile.core.BuildConfig
 import org.kiwix.kiwixmobile.core.extensions.isFileExist
 import org.kiwix.kiwixmobile.core.page.adapter.Page
 import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem
@@ -44,7 +46,7 @@ import javax.inject.Inject
 
 class LibkiwixBookmarks @Inject constructor(
   val library: Library,
-  val manager: Manager,
+  manager: Manager,
   val sharedPreferenceUtil: SharedPreferenceUtil
 ) : PageDao {
 
@@ -135,7 +137,17 @@ class LibkiwixBookmarks @Inject constructor(
   private fun addBookToLibraryIfNotExist(libKiwixBook: Book?) {
     libKiwixBook?.let { book ->
       if (!library.booksIds.any { it == book.id }) {
-        library.addBook(libKiwixBook)
+        library.addBook(libKiwixBook).also {
+          if (BuildConfig.DEBUG) {
+            Log.d(
+              TAG,
+              "Added Book to Library:\n" +
+                "ZIM File Path: ${book.path}\n" +
+                "Book ID: ${book.id}\n" +
+                "Book Title: ${book.title}"
+            )
+          }
+        }
       }
     }
   }
@@ -175,17 +187,40 @@ class LibkiwixBookmarks @Inject constructor(
       val book = if (library.booksIds.contains(bookmark.bookId)) {
         library.getBookById(bookmark.bookId)
       } else {
+        if (BuildConfig.DEBUG) {
+          Log.d(
+            TAG,
+            "Library does not contain the book for this bookmark:\n" +
+              "Book Title: ${bookmark.bookTitle}\n" +
+              "Bookmark URL: ${bookmark.url}"
+          )
+        }
         null
       }
 
       // Create an Archive object for the book's path, if it exists.
-      val archive: Archive? = book?.let { Archive(it.path) }
+      val archive: Archive? = book?.run {
+        try {
+          Archive(this.path)
+        } catch (exception: Exception) {
+          // to handle if zim file not found
+          // TODO should we delete bookmark if zim file not found?
+          // deleteBookmark(book.id, bookmark.url)
+          if (BuildConfig.DEBUG) {
+            Log.e(
+              TAG,
+              "Failed to create an archive for path: ${book.path}\n" +
+                "Exception: $exception"
+            )
+          }
+          null
+        }
+      }
 
       // Check if the Archive has an illustration of the specified size and encode it to Base64.
       val favicon = archive?.takeIf { it.hasIllustration(ILLUSTRATION_SIZE) }?.let {
         Base64.encodeToString(it.getIllustrationItem(ILLUSTRATION_SIZE).data.data, Base64.DEFAULT)
       }
-
       // Create a LibkiwixBookmarkItem object with bookmark, favicon, and book path.
       val libkiwixBookmarkItem = LibkiwixBookmarkItem(
         bookmark,
@@ -203,7 +238,10 @@ class LibkiwixBookmarks @Inject constructor(
 
   private fun isBookMarkExist(libkiwixBookmarkItem: LibkiwixBookmarkItem): Boolean =
     getBookmarksList()
-      .any { it.url == libkiwixBookmarkItem.bookmarkUrl && it.zimId == libkiwixBookmarkItem.zimId }
+      .any {
+        it.url == libkiwixBookmarkItem.bookmarkUrl &&
+          it.zimFilePath == libkiwixBookmarkItem.zimFilePath
+      }
 
   private fun flowableBookmarkList(
     backpressureStrategy: BackpressureStrategy = LATEST
@@ -226,4 +264,8 @@ class LibkiwixBookmarks @Inject constructor(
   private fun updateFlowableBookmarkList() {
     bookmarkListBehaviour?.onNext(getBookmarksList())
   }
+
+  companion object {
+    const val TAG = "LibkiwixBookmark"
+  }
 }
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
@@ -21,15 +21,12 @@ package org.kiwix.kiwixmobile.core.data.remote
 import io.objectbox.Box
 import io.objectbox.BoxStore
 import io.objectbox.kotlin.boxFor
-import io.objectbox.kotlin.query
-import io.objectbox.query.QueryBuilder
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import org.kiwix.kiwixmobile.core.CoreApp
 import org.kiwix.kiwixmobile.core.dao.LibkiwixBookmarks
 import org.kiwix.kiwixmobile.core.dao.entities.BookmarkEntity
-import org.kiwix.kiwixmobile.core.dao.entities.BookmarkEntity_
 import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem
 import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
 import org.kiwix.libkiwix.Book
@@ -56,14 +53,15 @@ class ObjectBoxToLibkiwixMigrator {
           update(Archive(bookmarkEntity.zimFilePath))
         }
         libkiwixBookmarks.saveBookmark(LibkiwixBookmarkItem(bookmarkEntity, libkiwixBook))
+        // TODO should we remove data from objectBox?
         // removing the single entity from the object box after migration.
-        box.query {
-          equal(
-            BookmarkEntity_.bookmarkUrl,
-            bookmarkEntity.bookmarkUrl,
-            QueryBuilder.StringOrder.CASE_INSENSITIVE
-          )
-        }.remove()
+        // box.query {
+        //   equal(
+        //     BookmarkEntity_.bookmarkUrl,
+        //     bookmarkEntity.bookmarkUrl,
+        //     QueryBuilder.StringOrder.CASE_INSENSITIVE
+        //   )
+        // }.remove()
       }
     }
     sharedPreferenceUtil.putPrefBookMarkMigrated(true)