Adapt latest java-libkiwix wrapper #3217
Merged
Conversation
This file contains hidden or 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#3216
branch
2 times, most recently
from
ac5a220 to
07bb13c
Compare
Collaborator
|
@MohitMaliFtechiz Does this PR fixes other tickezs in 0.8 milestone? If "yes", which one exactly? |
Collaborator
Author
|
@kelson42, Yes it seems to fix some other tickets of milestone
Some tickets may be fixed by this PR, but not tested yet. |
Collaborator
|
So why they are not in description? Maybe just because you are not over... but please put them when you are so far. |
This was referenced Jul 24, 2023
MohitMaliFtechiz
force-pushed
the
Issue#3216
branch
from
c11184a to
73ee0fd
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #3217 +/- ##
=============================================
+ Coverage 48.86% 49.31% +0.45%
- Complexity 1018 1027 +9
=============================================
Files 285 284 -1
Lines 10136 10189 +53
Branches 1353 1356 +3
=============================================
+ Hits 4953 5025 +72
+ Misses 4399 4356 -43
- Partials 784 808 +24
☔ View full report in Codecov by Sentry. |
MohitMaliFtechiz
force-pushed
the
Issue#3216
branch
from
2797600 to
6235ba6
Compare
MohitMaliFtechiz
force-pushed
the
Issue#3216
branch
from
c5a37d0 to
27008b7
Compare
This was referenced Aug 15, 2023
Collaborator
Author
|
@kelson42, @gouri-panda, This PR is ready for review now. But here we have fixed |
Collaborator
|
@MohitMaliDeveloper Ok, but it'll take some time to review the code since I'm not quite familiar with this end of the code and the PR is large and it also touches breakable changes. I want to make things right :) |
Collaborator
|
@MohitMaliFtechiz we need anyway to republish the binding with the right name first. please prepare everything for the release in a pr (see release ticket in java-libkiwix). So this pr depends on kiwix/java-libkiwix#42 |
MohitMaliFtechiz
force-pushed
the
Issue#3216
branch
from
27008b7 to
5b942e9
Compare
Collaborator
|
@gouri-panda Any news here? We really need your expert approval here to move forward! Can you please check if it fixes #3118 as well? |
Collaborator
|
org.kiwix/libkiwix has been released on Maven. Please update PR to use it and review with PRIO0. |
* As we can not search in that zim file which is not contains the FT Xapian index and testzim.zim file is not contain it so we have improved our test case. * Now we are downloading the `off the grid` zim file and performing the search functionality inside it.
* Fixed search unit test cases. * Improved the search functionality to perform the unit test case for search.
… been upgraded (as mentioned in kiwix/kiwix-build#544). However, to make this upgrade compatible, the minimum SDK version had to be updated to 24 since the NDK version is not compatible with Android API levels below 24. The README.md file has been updated to reflect the new minimum Android support version, which is now Android 7, due to the change in the minimum SDK version. Since the project's minimum SDK version is now 24, some conditions related to Android 23 and 24 have become unused. These unused conditions have been removed, and along with that, the drawable-night-23 and drawable-23 launch_screen.xml files, which were no longer in use, have also been removed.
* Now our minimum SDK version is 24 and we are using some conditions placed on behalf of this api level which are unused now, so we have removed those conditions. * Removed `CompatV21` file as now it is unused. * Renamed `CompatV23` to `CompatV24` and refactored the code to support our new minimum api level. * Refactored `NetworkUtilsTest` to support api level 24.
* Since our minimum API level is now 24, we have updated our CI configuration to run on API level 24, which aligns with the minimum API level required for our project.
* With the integration of libkiwix12, we now receive full results based on the search term. Previously, we were loading the entire list, but we have now implemented pagination to enhance the search functionality. * The libkiwix provides us with a Search object, which enables us to obtain the suggestion list using start and end indices. Consequently, we have modified our `ZimSearchResultGenerator` code to return the `Search` object instead of a list. * To accommodate the changes, we have updated the return type of `SearchResultGenerator` to a nullable Search. This change is necessary because we initialize the `SearchState` when the search is initialized in `SearchViewModel`, and initially, we do not have the Search object available. The nullable return type allows us to pass the Search object when it becomes available.
* Due to linking errors with libkiwix/libzim functions, direct usage in testing was not possible. To address this, helper classes were created, similar to those in `java-libkiwix`, for testing the search functionality.
…at the end of list and there is no data available to show
* We are previously checking `hasEntryByPath`, `hasEntryByTitle`, `mainEntry.isRedirect` which are internally calling the same function as we are calling after checking this condition so it would be better to directly use those function to avoid calling same function twice see more details kiwix/java-libkiwix#60.
* Updated proguard file to keep the `libkiwix/libzim` classes in release variant.
…ality. * Since `Search` is not compatible with those zim files which does not have Xapian index but `SuggestionSearch` have this functionality to search inside those zim files so we have used this. * Update test cases for test new search functionality.
After addressing the issue documented in kiwix/java-libkiwix#61, we now have the ability to search within zim files that do not have a Xapian index. As a result, we have enhanced our test to utilize pre-existing zim files. This improvement leads to reduced time consumption, improved memory efficiency, and minimized network usage impact.
* Added a loading progress bar at the end of the RecyclerView when loading more search results as the user scrolls to the bottom. This indicator informs users that additional results are being loaded. The progress bar appears if there are more results available for the search term, providing users with visibility into ongoing loading. * Enhanced the search loading process for larger ZIM files by introducing coroutines. This background threading approach prevents the UI thread from being impacted and ensures a smooth scrolling experience for users.
* A bug was introduced after enhancing the search functionality to align with the new `java-libkiwix` wrapper. Initially, when searching for any article in the ZIM file, the loading progress bar was not being displayed. This commit resolves this issue. * While searching within large ZIM files, the application used to freeze momentarily due to fetching data from the `libkiwix` on the UI thread. We have improved this functionality to provide a seamless user experience.
* Previously our test cases were launching the `KiwixMainActivity` twice (firstly it sets the values in preference and then relaunch the activity) before running the test case which was the cause of slow testing and sometimes it caused to memory leak in some api levels so we have refactored our test cases to launch `KiwixMainTest` once per test case. * `IntroFragmentTest` is failing on api level 24 due to a memory leak because after clicking on getStarted button is going to the LocalLibrary page but somehow `onStart` and `onStop` methods are calling of the `CoreReaderFragment` which stablish the `serviceConnection` but it was happening very quickly so before attaching the binder to `readAloudService` unbindService method called but at this point service was not created but creating was in progress so after going to library screen it allocate memory to `readAloudService` but `ReaderFragment` is no more visible to the user that's why memory leak happened. We have fixed it by unbinding the service in `onDestroyView` method.
…ral other enhancements: * Added logging for cases where the `ZimFileReader` failed to find an entry, failed to load an asset, and more. * Significantly improved the "load more" functionality: Now, if the user is near the end of the list, we start loading new search results. This enhancement enhances the user experience by reducing wait times for new search results. * Added references in the `KiwixServer` class to help developers understand why we are keeping the library object. * Implemented test cases to thoroughly test the `getMimeTypeFromUrl()` function in the ZimFileReader class. * Enhanced the search functionality by consolidating the use of a single viewModelScope instead of multiple coroutines, reducing the potential for future issues.
…t in the `SearchFragment`. We are now directly obtaining additional search data from the `SearchViewModel`, which already contains the `SearchState`.
kelson42
force-pushed
the
Issue#3216
branch
from
6530539 to
9ff91ba
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 #3122
Fixes #3094
Fixes #3410
Fixes #3442
Fixes #2767
Fixes #2768
Fixes #3098
Fixes #3216
Fixes #2683.
We now have a new wrapper for
java-libkiwix. This wrapper provides more direct access to libkiwix and libzim, enabling us to use the reader and searcher functionality through the natural methods of libzim and libkiwix. As a result, our wrapper introduces several new APIs, such as Archive and Searcher. To accommodate these changes in libkiwix, we have refactored our Android code to utilize the newjava-libkiwixAPIs.As part of the update,
java-libkiwixnow compiles with Android 24 and above, making it incompatible with versions below Android 24. Consequently, we have raised the minimum SDK version ofkiwix-androidto 24 to ensure compatibility withjava-libkiwix. For more information, refer to this link.Introduced an endless suggestion list in search "Endless" suggestion list #3122.
ZimSearchResultGeneratorcode to return theSearchobject instead of a list.SearchResultGeneratorto a nullable Search. This change is necessary because we initialize theSearchStatewhen the search is initialized inSearchViewModel, and initially, we do not have the Search object available. The nullable return type allows us to pass the Search object when it becomes available.Due to linking errors with libkiwix/libzim functions, direct testing usage was impossible. To address this, helper classes were created, similar to those in
java-libkiwix, for testing the search functionality.Since this pull request contains numerous changes, we have committed each small change with a detailed commit message. Please read the commit messages to understand the specific modifications made in this pull request.
This PR also fixes some other tickets of milestone
3.8.0:KiwixMainActivitytwice (firstly it sets the values in preference and then relaunches the activity) before running the test case which was the cause of slow testing and sometimes it caused memory leak in some API levels so we have refactored our test cases to launchKiwixMainActivityonce per test case.IntroFragmentTestis failing on API level 24 due to a memory leak because after clicking on the get-started button it goes to the LocalLibrary page but somehow CoreReaderFragment'sonStartandonStopmethods are calling, which establish theserviceConnection, but it was happening very quickly. So before attaching the binder to thereadAloudServiceunbindService method was called but at this point, the binder was not created, but creating was in progress so after going to the library screen it allocated memory toreadAloudService, butReaderFragmentnot visible to the user, that's why memory leak happened. We have fixed it by unbinding the service in theonDestroyViewmethod.