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 36.55462% with 151 lines in your changes are missing coverage. Please review.

Project coverage is 52.98%. Comparing base (c9c4414) to head (bc1b46a).
Report is 14 commits behind head on main.

❗ Current head bc1b46a differs from pull request most recent head 78dc73f. Consider uploading reports for the commit 78dc73f to get more accurate results

Files	Patch %	Lines
...g/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt	36.17%	22 Missing and 8 partials ⚠️
...rg/kiwix/kiwixmobile/core/utils/files/FileUtils.kt	0.00%	16 Missing ⚠️
...bile/nav/destination/reader/KiwixReaderFragment.kt	25.00%	12 Missing and 3 partials ⚠️
...org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt	39.13%	13 Missing and 1 partial ⚠️
.../kiwix/kiwixmobile/core/main/CoreReaderFragment.kt	31.25%	5 Missing and 6 partials ⚠️
.../java/org/kiwix/kiwixmobile/core/dao/NewBookDao.kt	18.18%	6 Missing and 3 partials ⚠️
.../org/kiwix/kiwixmobile/core/dao/NewBookmarksDao.kt	0.00%	8 Missing ⚠️
...rg/kiwix/kiwixmobile/core/main/CoreMainActivity.kt	0.00%	7 Missing ⚠️
...r/fileselectView/effects/OpenFileWithNavigation.kt	0.00%	5 Missing ⚠️
.../java/org/kiwix/kiwixmobile/core/dao/HistoryDao.kt	37.50%	5 Missing ⚠️
... and 18 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3624      +/-   ##
============================================
- Coverage     53.03%   52.98%   -0.06%     
- Complexity     1307     1316       +9     
============================================
  Files           292      293       +1     
  Lines         10992    11065      +73     
  Branches       1458     1472      +14     
============================================
+ Hits           5830     5863      +33     
- Misses         4198     4231      +33     
- Partials        964      971       +7     

☔ 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.