Use libkiwix to store bookmarks #3474
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MohitMaliFtechiz
force-pushed
the
Issue#3109
branch
from
1ee4bb2
to
40de9d0
Compare
|
@MohitMaliFtechiz Have you secured that the bookmark related Objectbox is only there for the google play build? |
Yes google play build also works since we are using libkiwix to store bookmark instead of Objectbox, Old playstore users who are using Objectbox will automatically migrate to new libkiwix boomark. |
|
@MohitMaliFtechiz Your answer is not what i ask for. I ask for non-PS builds (apk, fdroid) |
@kelson42 Yes it will work for non-playstore builds as well. e.g. for nightly builds(APK) and fdroid builds but for now, we can not publish our apk on fdroid since we have |
|
@MohitMaliFtechiz yes, but still secure that the objectbox bookmark related code is already not included in those builds. |
1 similar comment
|
@MohitMaliFtechiz yes, but still secure that the objectbox bookmark related code is already not included in those builds. |
@kelson42 We need the ObjectBox-related code for the migration of existing bookmarks to libkiwix. For the upcoming version of our application, we can only remove some functions of the ObjectBox bookmarks e.g. save/delete but we need the implementation of the bookmarks as we need the existing list of the bookmarks from objectbox for migration. |
|
no migration for fdroid (objectbox is forbidden there), but ok for our apk (not sure what was decided with gouri). |
MohitMaliFtechiz
force-pushed
the
Issue#3109
branch
from
ff68e88
to
c8e614e
Compare
MohitMaliFtechiz
force-pushed
the
Issue#3109
branch
from
c8e614e
to
d7fd2c3
Compare
|
@mgautierfr Sorry to be late on this I was busy with other tickets, but now I figured out the problem I was facing while reading the books from the library object.
Logs When writing a file Logs when reading the same file manager.readFile method LibraryIssue.mp4So it is not probably not writing a library to file properly. Apart from this method, I am facing one more issue, the library has the book object and it has an illustration for the book, but when I try to get the library contains this data in it. public native Illustration get illustration(int size);Error it is showing or i am missing something? |
|
Let's do one issue at the time.
This is the first thing to fix/understand. |
This is the content of the library.
Previously, added books are missing it is only showing books for the last added zim file.
I am writing it inside the |
Can you loop over the books in the library and print its name/id/path before you write the library (where you are printing
Are we speaking about bookmarks or book ? |
|
@mgautierfr While reading books from library.
Yes we are here talking about the bookmark and book, we are saving bookmark/library inside |
|
@MohitMaliFtechiz This is a side comment. I understand you write the library.xml only for the bookmarks for the moment, but this won't be like this anymore. So please just write both libraty and bookmark XML files directly at the root directory/kiwix |
@kelson42 For now we are writing the bookmarks at root directory of the device.
or are you talking about the root directory of our app-specific directory? |
|
Where is the code loading and writing the library (with the log message) ? In this PR ? |
|
@mgautierfr Here is the code that loads the data from the file and this code writes the data into the file |
Yes, I see no reason to have a "bookmark" folder.... in particular if you write library.xml in it. |
The path The path should be And be careful Are you sure you are checking the right |
Hoo! Thanks, I have corrected this, somehow my storage is creating the XML file in the same directory. Now library.xml looks like Bu error is the same after changing this.
Yes, we are checking reading/writing on the same file. |
|
Ok, I have found the problem. C++ Strangely, C++ We have to:
|
@mgautierfr Ohh! Thanks for finding the actual bug.
Yes, exactly this is the same scenario I am facing.
I assume this will be fixed when we make these changes.
Now we have another issue related to the |
Please open a new specific issue for this. We will lost our mind if we discuss about several issues in a PR discussion. |
mgautierfr
added a commit
to kiwix/java-libkiwix
that referenced
this pull request
Nov 9, 2023
The readOnly argument of `readFile` is to put the loaded library in readOnly mode. Which kinda means "Do not write again what is loaded". We don't want that for android as we want to save again our (potentially modified) library. See kiwix/kiwix-android#3474
…ted flowables to perform bookmarks operation on background thread for better user experience.
…n, improved the delete functionality in the bookmark screen, and also refactored the add/delete bookmark functionality in the reader fragment.
…ow written in a file with libkiwix
* We introduced a Flowable list in LibkiwixBookmarks to observe live changes on the UI. Previously, the UI was not updating after deleting bookmarks on the BookmarkScreen, and toggling bookmarks in the reader screen was also not reflecting the changes. To address this, we created a Flowable list and are now observing it whenever bookmarks are added or removed in the library.
…save the `library`, which contains information about books, their file paths, and favicons, into a file. * We now save this library information into a file named `library.txt` and subsequently read from it to retrieve file paths and favicons. * The test cases have been refactored to accommodate this new functionality. * The `ObjectBoxToLibkiwiMigrator` code has also been enhanced. With this change, we now save books in the library for their favicon and zimFilePath, resulting in a refactor and improvement of this class's functionality and its associated test cases. * The process of writing bookmarks and library data to file has been enhanced. Now, this is performed asynchronously in a background thread to mitigate potential data loss. * Additionally, several other improvements have been made throughout the codebase.
… since we don't need these classes as saving/deleting functionality will be tested in java-libkiwix. * Added instrumentation test case for testing the UI part with libkiwix bookmark functionality.
…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.
…e current book has been removed because adding the book to the library is unnecessary, as we have not saved any bookmarks yet. * We now write bookmarks and the library on the main thread instead of saving them in the background thread to prevent any data loss.
…d logs before writing the library in file.
…he favicon from it, we are now speeding up the process by directly taking the favicon from the book, as the issue at kiwix/java-libkiwix#73 has been resolved.
MohitMaliFtechiz
force-pushed
the
Issue#3109
branch
from
9eb17b5
to
8c46646
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3109