Fixed: Create only one reader and use it everywhere.

[MohitMaliFtechiz]

Fixes #3536

• Now we are creating only one reader for both zimFile, and assetFileDescriptor and using it for all the features. We have created a helper class to handle all the creation of filehandler whether we have opened the ZIM File via FileDescriptor or FilePath. It handles all the related details inside it and avoids the rewriting of code(one for filePath, and the second for fileDescriptor) which is bug-prone like we have faced in Fixed, the search is not working in dwds app. #3535.
• Also we have introduced a new method in this helper class called canOpenInLibkiwix() which ensures that the zim file can be opened in libkiwix or not before passing it to the libkiwix to avoid any crash thrown by the libkiwix see Added file picker in play store variant #3636 (comment).

[codecov]

Codecov Report

Attention: Patch coverage is 42.85714% with 184 lines in your changes missing coverage. Please review.

Project coverage is 54.95%. Comparing base (4a9be43) to head (2d0bc48).
Report is 20 commits behind head on main.

Files	Patch %	Lines
...g/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt	29.62%	28 Missing and 10 partials ⚠️
...rg/kiwix/kiwixmobile/core/utils/files/FileUtils.kt	0.00%	16 Missing ⚠️
...rg/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt	18.75%	12 Missing and 1 partial ⚠️
.../kiwix/kiwixmobile/core/main/CoreReaderFragment.kt	29.41%	5 Missing and 7 partials ⚠️
.../java/org/kiwix/kiwixmobile/core/dao/NewBookDao.kt	54.16%	9 Missing and 2 partials ⚠️
...g/kiwix/kiwixmobile/core/data/KiwixRoomDatabase.kt	25.00%	9 Missing ⚠️
.../java/org/kiwix/kiwixmobile/core/dao/HistoryDao.kt	0.00%	8 Missing ⚠️
.../org/kiwix/kiwixmobile/core/dao/NewBookmarksDao.kt	0.00%	8 Missing ⚠️
.../java/org/kiwix/kiwixmobile/core/dao/NewNoteDao.kt	0.00%	8 Missing ⚠️
...bile/nav/destination/reader/KiwixReaderFragment.kt	70.83%	1 Missing and 6 partials ⚠️
... and 21 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3624      +/-   ##
============================================
+ Coverage     54.91%   54.95%   +0.03%     
- Complexity     1481     1501      +20     
============================================
  Files           309      310       +1     
  Lines         12424    12547     +123     
  Branches       1560     1590      +30     
============================================
+ Hits           6823     6895      +72     
- Misses         4639     4678      +39     
- Partials        962      974      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kelson42]

@MohitMaliFtechiz I had only a quick overview and my feelings are a bit mixed... I don't have a definitive opinion, but none of history, notes or bookmarks should actually still deal - at this level fd/path - with the ZIM files.

The ZIM UUID (or an other internal ID) should be alone to attach notes/bookmarks/... to the rest and then get - from a central interface - all other book related details (like the filehandler, reader, ...)

The central library should probably be handled as singleton or similar.

[kelson42]

The architecture is weak, you don't need to open a file with libkiwix to know if it will be a success or not. So don't do it. If you really need a method canOpenInLibkiwix(), which I really doubt, don't use libkiwix to do that. You already have all the information you need to know if it will work or not.

Exactly the same kind of misconception like I have explained in length in #3649 (which has not been updated following my instructions). We are running out of time.

in addition, I see no reason why we have two different class to open a handle a ZIM via path or fd, it should be one class with two differrent constructors.

[MohitMaliFtechiz]

The architecture is weak, you don't need to open a file with libkiwix to know if it will be a success or not. So don't do it. If you really need a method canOpenInLibkiwix(), which I really doubt, don't use libkiwix to do that. You already have all the information you need to know if it will work or not.

@kelson42 We are not opening the file/fileDescriptor in libkiwix to check if it can open or not in libkiwix. canOpenInLibkiwix() is an Android method that is created by us, it is not a libkiwix function. We know how libkiwix opens a file or fileDescriptor, so in this function, we are validating that. e.g. for a file, it should be readable, and fileDecriptor should able to open via path. so this function is checking this for both before passing anything to libkiwix.

in addition, I see no reason why we have two different class to open a handle a ZIM via path or fd, it should be one class with two differrent constructors.

I have done this to make it a well-structured/separate concern and reduce the complexity of methods inside the class since both have different ways to open and manage the ZIM files. If you want to make it one class so okay I am making the changes.

[gouri-panda]

@MohitMaliDeveloper small conflicts are here. @kelson42 what do you think about the @MohitMaliDeveloper latest comment?

[MohitMaliFtechiz]

@gouri-panda All the conflicts and project compilation errors have been resolved.

[kelson42]

@MohitMaliFtechiz Please renase, this time I would really like to meege