Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use libkiwix to store bookmarks #3109

Closed
gouri-panda opened this issue Oct 9, 2022 · 9 comments · Fixed by #3653
Closed

Use libkiwix to store bookmarks #3109

gouri-panda opened this issue Oct 9, 2022 · 9 comments · Fixed by #3653
Assignees
Milestone

Comments

@gouri-panda
Copy link
Collaborator

This is a subtask for #3102 . Write migration test and new database test for Bookmarks.

@gouri-panda gouri-panda added this to the 3.7.0 milestone Oct 9, 2022
@gouri-panda gouri-panda self-assigned this Oct 9, 2022
@kelson42
Copy link
Collaborator

After discussion with @mgautierfr, it seems clear to me we should not use Room for the bookmarks. Read kiwix/libkiwix#840. The libkiwix provides already all the primitives.

@gouri-panda
Copy link
Collaborator Author

@kelson42 Sounds good to me!

@kelson42 kelson42 changed the title Create database test for Bookmarks Use libkiwix to store bookmarks Nov 4, 2022
@kelson42 kelson42 pinned this issue Nov 30, 2022
@kelson42 kelson42 modified the milestones: 3.7.0, 3.8.0 May 19, 2023
@kelson42 kelson42 unpinned this issue Jun 9, 2023
@MohitMaliFtechiz MohitMaliFtechiz self-assigned this Aug 24, 2023
@MohitMaliFtechiz
Copy link
Collaborator

@kelson42, Now we have the latest libkiwix which has the functionality to store the bookmarks in Library, Library has inbuilt functions to write bookmarks in a file (this is for write the bookmark in a file which we will use later for retrieving the saved bookmark), write a library to file (this holds the books in it which has the zimFilePath, favicon, etc).
We had to save the bookmarks and library to their respective files(bookmark.txt, library.txt) since we read bookmarks and their filePath, and favicon from these files.

In #3474 we have implemented this functionality.

we have successfully implemented the functionality of bookmarks with libkiwix (Save, retrieve, delete) while saving or deleting the bookmarks we are updating the corresponding files with the updated data to avoid any data loss.
We have implemented the migration functionality from ObjectBox to libkiwix to keep the previous bookmarks, to properly test the migration functionality we have written the test cases for it.
Now the only thing left to do is write test cases for saving, deleting, and retrieving functionality, As for testing these functionalities we need to use the libkiwix classes and we need to create wrapper classes as we created in java-libkiwix https://github.com/kiwix/java-libkiwix/tree/main/lib/src/test/org/kiwix/test/libkiwix here is the tricky part , to summarise functionality to save bookmark using libkiwix is done while creating test cases we required the wrapper which is taking time. So I am working on it.

Please let me know your thought on this.

@kelson42
Copy link
Collaborator

kelson42 commented Sep 18, 2023

@MohitMaliFtechiz Testing reading/writting bookmarks should be tested in java-libkiwix.

It does not make sense to test here because there is no additional logic.

Here, you need to test the bookmark related UI and migration.

@kelson42
Copy link
Collaborator

@MohitMaliFtechiz Testing reading/writting bookmarks should be tests in java-libkiwix. It does not make sense to test here because there is no additional logic.

Here, you need to test the bookmark related UI and migration.

Please confirm you don't write anything to fs or create custom bookmark related structures/objects.

@MohitMaliFtechiz
Copy link
Collaborator

Please confirm you don't write anything to fs or create custom bookmark related structures/objects.

@kelson42 We are saving bookmarks and library in the file system because libkiwix stores bookmark and books in library in the current session(e.g. when the user is using the application) if a user close the application then the libkiwix session is expired and it has not anything in memory. so we need to save bookmarks, and books in the filesystem to access them later as @mgautierfr mentioned there are some methods to store the bookmarks and library in a file which is our current approach to save the bookmarks and book in a file and retrieve those later via Manager to read saved bookmarks and their filePath, favicon.

So we are using write bookmarks in a file and write a library to file to deal with filesystem , we are not using any native android filesystem methods.

Here, you need to test the bookmark related UI and migration.

If this is the approach then we just need to create an instrumentation test case for testing the bookmarks-related UI. The migration test case is already written.

@kelson42
Copy link
Collaborator

kelson42 commented Sep 18, 2023

@MohitMaliFtechiz I meant, you don't write to the fs outside the libkiwix

@MohitMaliFtechiz
Copy link
Collaborator

@MohitMaliFtechiz I meant, you don't write to rhe fs outside the libkiwix

Yes nothing outside the libkiwix.

@kelson42
Copy link
Collaborator

kelson42 commented Oct 8, 2023

Last comment seems to me to clarify the situation appropriatly #3109 (comment). PR shozld be completed, tested and reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants