From d7fd2c3531904174dc9a0847874b7daac9006e11 Mon Sep 17 00:00:00 2001 From: MohitMali Date: Thu, 28 Sep 2023 17:50:10 +0530 Subject: [PATCH] Resolved bookmark saving issue, which causes the bug when we try to retrieve 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. --- core/detekt_baseline.xml | 1 + .../kiwixmobile/core/dao/LibkiwixBookmarks.kt | 52 +++++++++++++++++-- .../remote/ObjectBoxToLibkiwixMigrator.kt | 18 +++---- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/core/detekt_baseline.xml b/core/detekt_baseline.xml index d3886eedb9..9d11100032 100644 --- a/core/detekt_baseline.xml +++ b/core/detekt_baseline.xml @@ -68,6 +68,7 @@ ReturnCount:ToolbarScrollingKiwixWebView.kt$ToolbarScrollingKiwixWebView$@SuppressLint("ClickableViewAccessibility") override fun onTouchEvent(event: MotionEvent): Boolean TooGenericExceptionCaught:CompatFindActionModeCallback.kt$CompatFindActionModeCallback$exception: Exception TooGenericExceptionCaught:JNIInitialiser.kt$JNIInitialiser$e: Exception + TooGenericExceptionCaught:LibkiwixBookmarks.kt$LibkiwixBookmarks$exception: Exception TooGenericExceptionCaught:OnSwipeTouchListener.kt$OnSwipeTouchListener.GestureListener$exception: Exception TooGenericExceptionCaught:ZimFileReader.kt$ZimFileReader$exception: Exception TooGenericExceptionThrown:AdapterDelegateManager.kt$AdapterDelegateManager$throw RuntimeException("No delegate registered for $item") 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 index 54e3ea0a9c..b65fbde58c 100644 --- 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 index a8884ad44a..036891feb8 100644 --- 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)