From ab38adf19cc481e1e801224e5c01ba629e5f3b01 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 21 Dec 2023 18:36:18 +0530 Subject: [PATCH 1/9] Fixed: Create only one reader and use it everywhere. * Now we are creating only one reader for both `zimFile`, `assetFileDescriptor` and using for all the features. --- .../destination/reader/KiwixReaderFragment.kt | 12 ++-- .../fileselectView/effects/DeleteFiles.kt | 2 +- .../core/main/CoreReaderFragment.kt | 35 +++++------ .../page/bookmark/adapter/BookmarkItem.kt | 2 +- .../page/history/adapter/HistoryListItem.kt | 4 +- .../core/page/notes/adapter/NoteListItem.kt | 2 +- .../viewmodel/effects/ShowOpenNoteDialog.kt | 2 +- .../kiwixmobile/core/reader/ZimFileReader.kt | 62 +++++++++++++++---- .../core/reader/ZimReaderContainer.kt | 41 ++++++------ .../custom/main/CustomReaderFragment.kt | 6 +- 10 files changed, 96 insertions(+), 72 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index ab638536ee..dac02f2ded 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -134,7 +134,7 @@ class KiwixReaderFragment : CoreReaderFragment() { } private fun closeZimBook() { - zimReaderContainer?.setZimFile(null) + zimReaderContainer?.setZimFileOrFileDescriptor(null, null, null) } override fun openHomeScreen() { @@ -205,9 +205,7 @@ class KiwixReaderFragment : CoreReaderFragment() { override fun onResume() { super.onResume() - if (zimReaderContainer?.zimFile == null && - zimReaderContainer?.zimFileReader?.assetFileDescriptor == null - ) { + if (zimReaderContainer?.isValidZimFileReader == false) { exitBook() } if (isFullScreenVideo) { @@ -299,7 +297,11 @@ class KiwixReaderFragment : CoreReaderFragment() { // pass this uri to zimFileReader, which is necessary for saving // notes, bookmarks, history, and reopening the same ZIM file after the app closes. getAssetFileDescriptorFromUri(activity, it)?.let { assetFileDescriptor -> - openZimFile(null, assetFileDescriptor = assetFileDescriptor, filePath = "$it") + openZimFile( + null, + assetFileDescriptor = assetFileDescriptor, + assetFileDescriptorPath = "$it" + ) } ?: kotlin.run { activity.toast(R.string.cannot_open_file) } diff --git a/app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/DeleteFiles.kt b/app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/DeleteFiles.kt index 4c75a9c8e3..2021cc4356 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/DeleteFiles.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/DeleteFiles.kt @@ -60,7 +60,7 @@ data class DeleteFiles(private val booksOnDiskListItems: List) : return fold(true) { acc, book -> acc && deleteSpecificZimFile(book).also { if (it && book.file.canonicalPath == zimReaderContainer.zimCanonicalPath) { - zimReaderContainer.setZimFile(null) + zimReaderContainer.setZimFileOrFileDescriptor(null) } } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 4c40461228..befef99d57 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -1228,7 +1228,7 @@ abstract class CoreReaderFragment : reopenBook() } tempWebViewForUndo?.let { - zimReaderContainer?.setZimFile(tempZimFileForUndo) + zimReaderContainer?.setZimFileOrFileDescriptor(tempZimFileForUndo) webViewList.add(index, it) tabsAdapter?.notifyDataSetChanged() snackBarRoot?.let { root -> @@ -1479,17 +1479,11 @@ abstract class CoreReaderFragment : file: File?, isCustomApp: Boolean = false, assetFileDescriptor: AssetFileDescriptor? = null, - filePath: String? = null + assetFileDescriptorPath: String? = null ) { if (hasPermission(Manifest.permission.READ_EXTERNAL_STORAGE) || isCustomApp) { - if (file?.isFileExist() == true) { - openAndSetInContainer(file = file) - updateTitle() - } else if (assetFileDescriptor != null) { - openAndSetInContainer( - assetFileDescriptor = assetFileDescriptor, - filePath = filePath - ) + if (file?.isFileExist() == true || assetFileDescriptor != null) { + openAndSetInContainer(file = file, assetFileDescriptor, assetFileDescriptorPath) updateTitle() } else { Log.w(TAG_KIWIX, "ZIM file doesn't exist at " + file?.absolutePath) @@ -1522,24 +1516,23 @@ abstract class CoreReaderFragment : private fun openAndSetInContainer( file: File? = null, assetFileDescriptor: AssetFileDescriptor? = null, - filePath: String? = null + assetFileDescriptorPath: String? = null ) { try { - if (isNotPreviouslyOpenZim(file?.canonicalPath)) { + if (isNotPreviouslyOpenZim(file?.canonicalPath) && + isNotPreviouslyOpenZim(assetFileDescriptorPath) + ) { webViewList.clear() } } catch (e: IOException) { e.printStackTrace() } zimReaderContainer?.let { zimReaderContainer -> - if (assetFileDescriptor != null) { - zimReaderContainer.setZimFileDescriptor( - assetFileDescriptor, - filePath = filePath - ) - } else { - zimReaderContainer.setZimFile(file) - } + zimReaderContainer.setZimFileOrFileDescriptor( + file, + assetFileDescriptor, + assetFileDescriptorPath + ) val zimFileReader = zimReaderContainer.zimFileReader zimFileReader?.let { zimFileReader -> @@ -1632,7 +1625,7 @@ abstract class CoreReaderFragment : private fun restoreDeletedTabs() { if (tempWebViewListForUndo.isNotEmpty()) { - zimReaderContainer?.setZimFile(tempZimFileForUndo) + zimReaderContainer?.setZimFileOrFileDescriptor(tempZimFileForUndo) webViewList.addAll(tempWebViewListForUndo) tabsAdapter?.notifyDataSetChanged() snackBarRoot?.let { root -> diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/BookmarkItem.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/BookmarkItem.kt index 2bf0670e77..dcfe58096c 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/BookmarkItem.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/BookmarkItem.kt @@ -51,7 +51,7 @@ data class BookmarkItem( ) : this( zimId = zimFileReader.id, zimName = zimFileReader.name, - zimFilePath = zimFileReader.zimFile?.canonicalPath ?: zimFileReader.assetDescriptorFilePath, + zimFilePath = zimFileReader.zimCanonicalPath, bookmarkUrl = url, title = title, favicon = zimFileReader.favicon diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/history/adapter/HistoryListItem.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/history/adapter/HistoryListItem.kt index e74b228b77..9450be4af6 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/history/adapter/HistoryListItem.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/history/adapter/HistoryListItem.kt @@ -48,9 +48,7 @@ sealed class HistoryListItem : PageRelated { ) : this( zimId = zimFileReader.id, zimName = zimFileReader.name, - zimFilePath = zimFileReader.zimFile?.canonicalPath - ?: zimFileReader.assetDescriptorFilePath - ?: "", + zimFilePath = zimFileReader.zimCanonicalPath ?: "", favicon = zimFileReader.favicon, historyUrl = url, title = title, diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/adapter/NoteListItem.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/adapter/NoteListItem.kt index 4ab8d16699..229711ad62 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/adapter/NoteListItem.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/adapter/NoteListItem.kt @@ -35,7 +35,7 @@ data class NoteListItem( ) : this( zimId = zimFileReader.id, title = title, - zimFilePath = zimFileReader.zimFile?.canonicalPath ?: zimFileReader.assetDescriptorFilePath, + zimFilePath = zimFileReader.zimCanonicalPath, zimUrl = url, favicon = zimFileReader.favicon, noteFilePath = noteFilePath diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt index 8adaa9b4ae..181fa77f29 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt @@ -50,7 +50,7 @@ data class ShowOpenNoteDialog( // which is already set in zimReaderContainer, so there's no need to set it again. item.zimFilePath?.let { val file = File(it) - zimReaderContainer.setZimFile(file) + zimReaderContainer.setZimFileOrFileDescriptor(file) } effects.offer(OpenNote(item.noteFilePath, item.zimUrl, item.title)) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt index 2707a5034a..26722c1285 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt @@ -29,6 +29,7 @@ import io.reactivex.schedulers.Schedulers import org.kiwix.kiwixmobile.core.CoreApp import org.kiwix.kiwixmobile.core.NightModeConfig import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity.Book +import org.kiwix.kiwixmobile.core.extensions.isFileExist import org.kiwix.kiwixmobile.core.main.UNINITIALISER_ADDRESS import org.kiwix.kiwixmobile.core.main.UNINITIALISE_HTML import org.kiwix.kiwixmobile.core.reader.ZimFileReader.Companion.CONTENT_PREFIX @@ -61,34 +62,57 @@ class ZimFileReader constructor( private val searcher: SuggestionSearcher = SuggestionSearcher(jniKiwixReader) ) { interface Factory { - fun create(file: File): ZimFileReader? fun create( - assetFileDescriptor: AssetFileDescriptor, + file: File? = null, + assetFileDescriptor: AssetFileDescriptor? = null, filePath: String? = null ): ZimFileReader? class Impl @Inject constructor(private val nightModeConfig: NightModeConfig) : Factory { - override fun create(file: File) = + override fun create( + file: File?, + assetFileDescriptor: AssetFileDescriptor?, + filePath: String? + ) = try { + when { + file != null -> createArchiveWithFile(file) + assetFileDescriptor != null -> { + createArchiveWithAssetFileDescriptor(assetFileDescriptor, filePath) + } + + else -> null + } + } catch (ignore: JNIKiwixException) { + null + } catch (ignore: Exception) { // for handing the error, if any zim file is corrupted + null + } + + private fun createArchiveWithFile(file: File): ZimFileReader? = + if (file.isFileExist()) { ZimFileReader( - file, + zimFile = file, nightModeConfig = nightModeConfig, jniKiwixReader = Archive(file.canonicalPath) ).also { Log.e(TAG, "create: ${file.path}") } - } catch (ignore: JNIKiwixException) { - null - } catch (ignore: Exception) { // for handing the error, if any zim file is corrupted + } else { + Log.e( + TAG, + "Error in creating ZimFileReader," + + " because file does not exist on path: ${file.path}" + ) null } - override fun create( + private fun createArchiveWithAssetFileDescriptor( assetFileDescriptor: AssetFileDescriptor, filePath: String? ): ZimFileReader? = - try { + if (assetFileDescriptor.parcelFileDescriptor.dup().fileDescriptor.valid()) { ZimFileReader( null, assetFileDescriptor, @@ -100,11 +124,14 @@ class ZimFileReader constructor( assetFileDescriptor.length ) ).also { - Log.e(TAG, "create: with fileDescriptor") + Log.e(TAG, "created: with assetFileDescriptor $assetFileDescriptor}") } - } catch (ignore: JNIKiwixException) { - null - } catch (ignore: Exception) { // for handing the error, if any zim file is corrupted + } else { + Log.e( + TAG, + "Error in creating ZimFileReader," + + " because provided assetFileDescriptor is not valid" + ) null } } @@ -163,6 +190,15 @@ class ZimFileReader constructor( null } + /** + * Return the zimFile path if opened from file else return the filePath of assetFileDescriptor + */ + val zimCanonicalPath + get() = zimFile?.canonicalPath ?: assetDescriptorFilePath + + val isValidZimFileReader + get() = zimFile != null || assetFileDescriptor != null + fun searchSuggestions(prefix: String): SuggestionSearch? = try { searcher.suggest(prefix) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt index d896c1a9b4..6582d2c761 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt @@ -19,7 +19,6 @@ package org.kiwix.kiwixmobile.core.reader import android.content.res.AssetFileDescriptor import android.webkit.WebResourceResponse -import org.kiwix.kiwixmobile.core.extensions.isFileExist import org.kiwix.kiwixmobile.core.reader.ZimFileReader.Factory import java.io.File import java.net.HttpURLConnection @@ -34,23 +33,18 @@ class ZimReaderContainer @Inject constructor(private val zimFileReaderFactory: F field = value } - fun setZimFile(file: File?) { - if (file?.canonicalPath == zimFileReader?.zimFile?.canonicalPath) { - return - } - zimFileReader = - if (file?.isFileExist() == true) zimFileReaderFactory.create(file) - else null - } - - fun setZimFileDescriptor( - assetFileDescriptor: AssetFileDescriptor, + fun setZimFileOrFileDescriptor( + file: File? = null, + assetFileDescriptor: AssetFileDescriptor? = null, filePath: String? = null ) { + if (isValidZimFileReader == true && + file?.canonicalPath == zimFileReader?.zimFile?.canonicalPath + ) { + return + } zimFileReader = - if (assetFileDescriptor.parcelFileDescriptor.dup().fileDescriptor.valid()) - zimFileReaderFactory.create(assetFileDescriptor, filePath) - else null + zimFileReaderFactory.create(file, assetFileDescriptor, filePath) } fun getPageUrlFromTitle(title: String) = zimFileReader?.getPageUrlFrom(title) @@ -83,18 +77,23 @@ class ZimReaderContainer @Inject constructor(private val zimFileReaderFactory: F } } - fun copyReader(): ZimFileReader? = zimFile?.let(zimFileReaderFactory::create) - ?: assetFileDescriptor?.let(zimFileReaderFactory::create) + fun copyReader(): ZimFileReader? = + zimFileReaderFactory.create( + file = zimFile, + assetFileDescriptor = assetFileDescriptor, + filePath = assetDescriptorPath + ) val zimFile get() = zimFileReader?.zimFile val assetFileDescriptor get() = zimFileReader?.assetFileDescriptor - /** - * Return the zimFile path if opened from file else return the filePath of assetFileDescriptor - */ + val assetDescriptorPath get() = zimFileReader?.assetDescriptorFilePath + + val isValidZimFileReader get() = zimFileReader?.isValidZimFileReader + val zimCanonicalPath - get() = zimFileReader?.zimFile?.canonicalPath ?: zimFileReader?.assetDescriptorFilePath + get() = zimFileReader?.zimCanonicalPath val zimFileTitle get() = zimFileReader?.title val mainPage get() = zimFileReader?.mainPage val id get() = zimFileReader?.id diff --git a/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt b/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt index c15fb0d72d..eb932e2a49 100644 --- a/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt +++ b/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt @@ -181,11 +181,7 @@ class CustomReaderFragment : CoreReaderFragment() { onFilesFound = { when (it) { is ValidationState.HasFile -> { - if (it.assetFileDescriptor != null) { - openZimFile(null, true, it.assetFileDescriptor) - } else { - openZimFile(it.file, true) - } + openZimFile(it.file, true, it.assetFileDescriptor) // Save book in the database to display it in `ZimHostFragment`. zimReaderContainer?.zimFileReader?.let { zimFileReader -> // Check if the file is not null. If the file is null, From 5d97055a6aeb6019ed2e51fb4b1b6baa2f5e7ef2 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 27 Dec 2023 15:13:03 +0530 Subject: [PATCH 2/9] Re-enabled the file picker button in play store button. --- .../library/LocalLibraryFragment.kt | 57 ++++--------------- 1 file changed, 10 insertions(+), 47 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt index 8e22466209..5610fdf24f 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt @@ -78,7 +78,6 @@ import org.kiwix.kiwixmobile.core.utils.SimpleRecyclerViewScrollListener.Compani import org.kiwix.kiwixmobile.core.utils.SimpleRecyclerViewScrollListener.Companion.SCROLL_UP import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog -import org.kiwix.kiwixmobile.core.utils.files.FileUtils import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BookOnDiskDelegate import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskAdapter import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskListItem @@ -90,7 +89,6 @@ import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel.FileSelectActions.Req import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel.FileSelectActions.RequestNavigateTo import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel.FileSelectActions.RequestSelect import org.kiwix.kiwixmobile.zimManager.fileselectView.FileSelectListState -import java.io.File import javax.inject.Inject private const val WAS_IN_ACTION_MODE = "WAS_IN_ACTION_MODE" @@ -245,7 +243,7 @@ class LocalLibraryFragment : BaseFragment() { offerAction(FileSelectActions.UserClickedDownloadBooksButton) } } - hideFilePickerButton() + setUpFilePickerButton() fragmentDestinationLibraryBinding?.zimfilelist?.addOnScrollListener( SimpleRecyclerViewScrollListener { _, newState -> @@ -303,26 +301,16 @@ class LocalLibraryFragment : BaseFragment() { } } - private fun hideFilePickerButton() { - if (sharedPreferenceUtil.isPlayStoreBuild) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { - fragmentDestinationLibraryBinding?.selectFile?.visibility = View.GONE - } - } - + private fun setUpFilePickerButton() { fragmentDestinationLibraryBinding?.selectFile?.setOnClickListener { - if (!requireActivity().isManageExternalStoragePermissionGranted(sharedPreferenceUtil)) { - showManageExternalStoragePermissionDialog() - } else { - showFileChooser() - } + showFileChooser() } } private fun showFileChooser() { val intent = Intent().apply { - action = Intent.ACTION_GET_CONTENT - type = "*/*" + action = Intent.ACTION_OPEN_DOCUMENT + type = "application/octet-stream" addCategory(Intent.CATEGORY_OPENABLE) } try { @@ -335,40 +323,15 @@ class LocalLibraryFragment : BaseFragment() { private val fileSelectLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> if (result.resultCode == RESULT_OK) { - result.data?.data?.let { uri -> - getZimFileFromUri(uri)?.let(::navigateToReaderFragment) - } + result.data?.data?.let(::navigateToReaderFragment) } } - private fun getZimFileFromUri( - uri: Uri - ): File? { - val filePath = FileUtils.getLocalFilePathByUri( - requireActivity().applicationContext, uri + private fun navigateToReaderFragment(uri: Uri) { + activity?.navigate( + LocalLibraryFragmentDirections.actionNavigationLibraryToNavigationReader() + .apply { zimFileUri = "$uri" } ) - if (filePath == null || !File(filePath).exists()) { - activity.toast(R.string.error_file_not_found) - return null - } - val file = File(filePath) - return if (!FileUtils.isValidZimFile(file.path)) { - activity.toast(R.string.error_file_invalid) - null - } else { - file - } - } - - private fun navigateToReaderFragment(file: File) { - if (!file.canRead()) { - activity.toast(R.string.unable_to_read_zim_file) - } else { - activity?.navigate( - LocalLibraryFragmentDirections.actionNavigationLibraryToNavigationReader() - .apply { zimFileUri = file.toUri().toString() } - ) - } } override fun onResume() { From b3b6c06a0e2a84fbc0c1c58e5c8387ec1b2f72ac Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 27 Dec 2023 15:20:12 +0530 Subject: [PATCH 3/9] Open file with `AssetFileDescriptor` instead of `File` since we have restriction to directly access the files via filePath so instead of a file now we are acquiring the fileDescriptor approach since we can directly open ZIM files via file descriptor without any special permission. Therefore, we are now starting to utilize the fileDescriptor instead of a file. --- .../destination/reader/KiwixReaderFragment.kt | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index dac02f2ded..de11b68106 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -19,7 +19,6 @@ package org.kiwix.kiwixmobile.nav.destination.reader import android.content.Intent -import android.net.Uri import android.os.Bundle import android.os.Handler import android.os.Looper @@ -33,6 +32,7 @@ import android.view.View.VISIBLE import android.view.ViewGroup import androidx.appcompat.app.AppCompatActivity import androidx.core.net.toFile +import androidx.core.net.toUri import androidx.drawerlayout.widget.DrawerLayout import com.google.android.material.bottomnavigation.BottomNavigationView import org.kiwix.kiwixmobile.R @@ -55,7 +55,6 @@ import org.kiwix.kiwixmobile.core.main.ToolbarScrollingKiwixWebView import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import org.kiwix.kiwixmobile.core.utils.TAG_CURRENT_FILE import org.kiwix.kiwixmobile.core.utils.TAG_KIWIX -import org.kiwix.kiwixmobile.core.utils.files.FileUtils import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getAssetFileDescriptorFromUri import java.io.File @@ -108,15 +107,18 @@ class KiwixReaderFragment : CoreReaderFragment() { } private fun tryOpeningZimFile(zimFileUri: String) { - val filePath = FileUtils.getLocalFilePathByUri( - requireActivity().applicationContext, Uri.parse(zimFileUri) - ) - - if (filePath == null || !File(filePath).isFileExist()) { - activity.toast(R.string.error_file_not_found) - return + getAssetFileDescriptorFromUri( + requireActivity(), + zimFileUri.toUri() + )?.let { assetFileDescriptor -> + openZimFile( + null, + assetFileDescriptor = assetFileDescriptor, + assetFileDescriptorPath = zimFileUri + ) + } ?: kotlin.run { + activity.toast(R.string.cannot_open_file) } - openZimFile(File(filePath)) } override fun loadDrawerViews() { From 4da41bc9ecc69eb8f24988ed5af80a6cce452ee5 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 27 Dec 2023 15:22:55 +0530 Subject: [PATCH 4/9] Taking `takePersistableUriPermission` for uris that user try to open via file picker. Since we need access of this uri to open the same file again, if user tries to open notes, history etc. --- .../kiwix/kiwixmobile/core/utils/files/FileUtils.kt | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt index b5222394bc..8896e39012 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt @@ -26,6 +26,7 @@ import android.content.res.AssetFileDescriptor import android.net.Uri import android.os.Build import android.os.Environment +import android.os.ParcelFileDescriptor import android.provider.DocumentsContract import android.util.Log import android.webkit.URLUtil @@ -416,7 +417,16 @@ object FileUtils { uri: Uri ): AssetFileDescriptor? { return try { - context.contentResolver.openAssetFileDescriptor(uri, "r") + context.contentResolver.takePersistableUriPermission( + uri, + Intent.FLAG_GRANT_READ_URI_PERMISSION + ) + context.contentResolver.openFileDescriptor(uri, "r").use { + AssetFileDescriptor( + ParcelFileDescriptor.dup(it?.fileDescriptor), + 0, AssetFileDescriptor.UNKNOWN_LENGTH + ) + } } catch (ignore: FileNotFoundException) { null } From 8e59d4390e104a2a425e7f602de09925a4b201ee Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 27 Dec 2023 15:53:15 +0530 Subject: [PATCH 5/9] Fixed zim file not opening that is previous loaded, if we close the application or navigate to the other fragments. * Improved the error logging if due to some condition we are unable to open the assetFileDescriptor. --- .../destination/reader/KiwixReaderFragment.kt | 33 ++++++++++++------- .../kiwixmobile/core/utils/files/FileUtils.kt | 7 ++-- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index de11b68106..f611e559d7 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -43,7 +43,6 @@ import org.kiwix.kiwixmobile.core.base.FragmentActivityExtensions.Super import org.kiwix.kiwixmobile.core.base.FragmentActivityExtensions.Super.ShouldCall import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.setupDrawerToggle import org.kiwix.kiwixmobile.core.extensions.coreMainActivity -import org.kiwix.kiwixmobile.core.extensions.isFileExist import org.kiwix.kiwixmobile.core.extensions.setBottomMarginToFragmentContainerView import org.kiwix.kiwixmobile.core.extensions.setImageDrawableCompat import org.kiwix.kiwixmobile.core.extensions.snack @@ -56,7 +55,6 @@ import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import org.kiwix.kiwixmobile.core.utils.TAG_CURRENT_FILE import org.kiwix.kiwixmobile.core.utils.TAG_KIWIX import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getAssetFileDescriptorFromUri -import java.io.File private const val HIDE_TAB_SWITCHER_DELAY: Long = 300 @@ -226,15 +224,28 @@ class KiwixReaderFragment : CoreReaderFragment() { currentTab: Int ) { val settings = requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0) - val zimFile = settings.getString(TAG_CURRENT_FILE, null) - - if (zimFile != null && File(zimFile).isFileExist()) { - if (zimReaderContainer?.zimFile == null) { - openZimFile(File(zimFile)) - Log.d( - TAG_KIWIX, - "Kiwix normal start, Opened last used zimFile: -> $zimFile" - ) + val zimFileUri = settings.getString(TAG_CURRENT_FILE, null) + + if (zimFileUri != null && + getAssetFileDescriptorFromUri(requireActivity(), zimFileUri.toUri()) != null + ) { + if (zimReaderContainer?.assetFileDescriptor == null) { + getAssetFileDescriptorFromUri( + requireActivity(), + zimFileUri.toUri() + )?.let { assetFileDescriptor -> + openZimFile( + null, + assetFileDescriptor = assetFileDescriptor, + assetFileDescriptorPath = zimFileUri + ) + Log.d( + TAG_KIWIX, + "Kiwix normal start, Opened last used zimFile: -> $zimFileUri" + ) + } ?: kotlin.run { + activity.toast(R.string.cannot_open_file) + } } else { zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt index 8896e39012..b841af3555 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt @@ -48,7 +48,6 @@ import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import java.io.BufferedReader import java.io.File -import java.io.FileNotFoundException import java.io.IOException object FileUtils { @@ -427,7 +426,11 @@ object FileUtils { 0, AssetFileDescriptor.UNKNOWN_LENGTH ) } - } catch (ignore: FileNotFoundException) { + } catch (ignore: Exception) { + Log.e( + "GET_FILE_DESCRIPTOR", + "Unable to get the file descriptor for uri = $uri\n original exception = $ignore" + ) null } } From 1f8c4e8ad069bdd68fcc807681ab0f5c1cad515f Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 27 Dec 2023 16:36:59 +0530 Subject: [PATCH 6/9] Moved `takePersistableUriPermission` permission for only we have selected via file picker since we can only take this permission for those file that we have selected via file picker. * exist the previous book, if any open in reader screen when there is some error in opening new file in the reader screen. --- .../nav/destination/library/LocalLibraryFragment.kt | 11 ++++++++++- .../nav/destination/reader/KiwixReaderFragment.kt | 2 ++ .../kiwix/kiwixmobile/core/utils/files/FileUtils.kt | 4 ---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt index 5610fdf24f..d09f7c93d0 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt @@ -323,7 +323,16 @@ class LocalLibraryFragment : BaseFragment() { private val fileSelectLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> if (result.resultCode == RESULT_OK) { - result.data?.data?.let(::navigateToReaderFragment) + result.data?.data?.let { + // Taking `takePersistableUriPermission` for uris that user try to open via file picker. + // Since we need access of this uri to open the same file again, + // if user tries to open notes, history etc. + requireActivity().contentResolver.takePersistableUriPermission( + it, + Intent.FLAG_GRANT_READ_URI_PERMISSION + ) + navigateToReaderFragment(it) + } } } diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index f611e559d7..8455e42590 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -115,6 +115,8 @@ class KiwixReaderFragment : CoreReaderFragment() { assetFileDescriptorPath = zimFileUri ) } ?: kotlin.run { + // exit the previous if any loaded + exitBook() activity.toast(R.string.cannot_open_file) } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt index b841af3555..4be853e7b7 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt @@ -416,10 +416,6 @@ object FileUtils { uri: Uri ): AssetFileDescriptor? { return try { - context.contentResolver.takePersistableUriPermission( - uri, - Intent.FLAG_GRANT_READ_URI_PERMISSION - ) context.contentResolver.openFileDescriptor(uri, "r").use { AssetFileDescriptor( ParcelFileDescriptor.dup(it?.fileDescriptor), From d2505b7ca3c8d831b898c6c97723870799d6b536 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 27 Dec 2023 17:30:44 +0530 Subject: [PATCH 7/9] Resolved the application crash occurring when clicking the 'OPEN_NOTE' button in the NotesFragment. * Now, we primarily utilize the `FileDescriptor` instead of the file in conjunction with the URI for various functionalities. Consequently, we have refactored our note-opening functionality accordingly. --- .../viewmodel/effects/ShowOpenNoteDialog.kt | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt index 181fa77f29..8b39b99aad 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt @@ -19,6 +19,7 @@ package org.kiwix.kiwixmobile.core.page.notes.viewmodel.effects import androidx.appcompat.app.AppCompatActivity +import androidx.core.net.toUri import io.reactivex.processors.PublishProcessor import org.kiwix.kiwixmobile.core.base.SideEffect import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.cachedComponent @@ -29,7 +30,7 @@ import org.kiwix.kiwixmobile.core.page.viewmodel.effects.OpenPage import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog.ShowNoteDialog -import java.io.File +import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getAssetFileDescriptorFromUri import javax.inject.Inject data class ShowOpenNoteDialog( @@ -49,8 +50,19 @@ data class ShowOpenNoteDialog( // For custom apps, we are currently using fileDescriptor, and they only have a single file in them, // which is already set in zimReaderContainer, so there's no need to set it again. item.zimFilePath?.let { - val file = File(it) - zimReaderContainer.setZimFileOrFileDescriptor(file) + // Obtain the asset file descriptor for the provided URI. + // Given that we now exclusively use the fileDescriptor for all our functionalities, + // we have modified this method to assign the fileDescriptor instead of the file + // to address the crash triggered when clicking the 'OPEN NOTE' button. + getAssetFileDescriptorFromUri( + activity, + it.toUri() + )?.let { assetFileDescriptor -> + zimReaderContainer.setZimFileOrFileDescriptor( + assetFileDescriptor = assetFileDescriptor, + filePath = it + ) + } } effects.offer(OpenNote(item.noteFilePath, item.zimUrl, item.title)) } From 7c59193bd891fc7feac649112c2dc78cd583e2f0 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 27 Dec 2023 18:38:19 +0530 Subject: [PATCH 8/9] Improved creation of `ZimFileReader`. * If current file is same that is previously opened in the `ZimFileReader` then it will not create the reader again. * Improved the variable naming. --- .../viewmodel/effects/ShowOpenNoteDialog.kt | 2 +- .../kiwixmobile/core/reader/ZimFileReader.kt | 10 +++---- .../core/reader/ZimReaderContainer.kt | 26 ++++++++++++++----- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt index 8b39b99aad..8c27e4a760 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt @@ -60,7 +60,7 @@ data class ShowOpenNoteDialog( )?.let { assetFileDescriptor -> zimReaderContainer.setZimFileOrFileDescriptor( assetFileDescriptor = assetFileDescriptor, - filePath = it + assetDescriptorFilePath = it ) } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt index 26722c1285..306da24124 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt @@ -65,7 +65,7 @@ class ZimFileReader constructor( fun create( file: File? = null, assetFileDescriptor: AssetFileDescriptor? = null, - filePath: String? = null + assetDescriptorFilePath: String? = null ): ZimFileReader? class Impl @Inject constructor(private val nightModeConfig: NightModeConfig) : @@ -73,13 +73,13 @@ class ZimFileReader constructor( override fun create( file: File?, assetFileDescriptor: AssetFileDescriptor?, - filePath: String? + assetDescriptorFilePath: String? ) = try { when { file != null -> createArchiveWithFile(file) assetFileDescriptor != null -> { - createArchiveWithAssetFileDescriptor(assetFileDescriptor, filePath) + createArchiveWithAssetFileDescriptor(assetFileDescriptor, assetDescriptorFilePath) } else -> null @@ -110,13 +110,13 @@ class ZimFileReader constructor( private fun createArchiveWithAssetFileDescriptor( assetFileDescriptor: AssetFileDescriptor, - filePath: String? + assetFileDescriptorPath: String? ): ZimFileReader? = if (assetFileDescriptor.parcelFileDescriptor.dup().fileDescriptor.valid()) { ZimFileReader( null, assetFileDescriptor, - assetDescriptorFilePath = filePath, + assetDescriptorFilePath = assetFileDescriptorPath, nightModeConfig = nightModeConfig, jniKiwixReader = Archive( assetFileDescriptor.parcelFileDescriptor.dup().fileDescriptor, diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt index 6582d2c761..5205cfed0b 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt @@ -36,17 +36,31 @@ class ZimReaderContainer @Inject constructor(private val zimFileReaderFactory: F fun setZimFileOrFileDescriptor( file: File? = null, assetFileDescriptor: AssetFileDescriptor? = null, - filePath: String? = null + assetDescriptorFilePath: String? = null ) { - if (isValidZimFileReader == true && - file?.canonicalPath == zimFileReader?.zimFile?.canonicalPath - ) { + if (shouldNotCreateZimFileReader(file, assetDescriptorFilePath)) { return } zimFileReader = - zimFileReaderFactory.create(file, assetFileDescriptor, filePath) + zimFileReaderFactory.create(file, assetFileDescriptor, assetDescriptorFilePath) } + private fun shouldNotCreateZimFileReader( + file: File?, + assetDescriptorFilePath: String? + ) = + when { + file != null -> { + file.canonicalPath == zimCanonicalPath + } + + assetDescriptorFilePath != null -> { + assetDescriptorFilePath == zimCanonicalPath + } + + else -> false + } + fun getPageUrlFromTitle(title: String) = zimFileReader?.getPageUrlFrom(title) fun getRandomArticleUrl() = zimFileReader?.getRandomArticleUrl() @@ -81,7 +95,7 @@ class ZimReaderContainer @Inject constructor(private val zimFileReaderFactory: F zimFileReaderFactory.create( file = zimFile, assetFileDescriptor = assetFileDescriptor, - filePath = assetDescriptorPath + assetDescriptorFilePath = assetDescriptorPath ) val zimFile get() = zimFileReader?.zimFile From 21b34e2b4480c2fb59a4fa63f084dc88cee69667 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Sat, 30 Dec 2023 20:53:37 +0530 Subject: [PATCH 9/9] Checking permission of fileDescriptor is it has the read access or not. --- .../library/LocalLibraryFragment.kt | 4 +- .../kiwixmobile/core/utils/files/FileUtils.kt | 37 ++++++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt index d09f7c93d0..e040399c28 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt @@ -309,8 +309,8 @@ class LocalLibraryFragment : BaseFragment() { private fun showFileChooser() { val intent = Intent().apply { - action = Intent.ACTION_OPEN_DOCUMENT - type = "application/octet-stream" + action = Intent.ACTION_GET_CONTENT + type = "*/*" addCategory(Intent.CATEGORY_OPENABLE) } try { diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt index 4be853e7b7..691de57fae 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt @@ -48,7 +48,10 @@ import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import java.io.BufferedReader import java.io.File +import java.io.FileDescriptor +import java.io.FileInputStream import java.io.IOException +import java.nio.ByteBuffer object FileUtils { @@ -416,7 +419,21 @@ object FileUtils { uri: Uri ): AssetFileDescriptor? { return try { - context.contentResolver.openFileDescriptor(uri, "r").use { + val documentFile = DocumentFile.fromSingleUri(context, uri) + if (documentFile?.uri == null) { + return null + } + Log.e( + "PERMISSION", + "getAssetFileDescriptorFromUri: can Read file = ${documentFile.canRead()}\n" + + " can right file = ${documentFile.canWrite()}" + ) + context.contentResolver.openFileDescriptor(documentFile.uri, "r", null).use { + Log.e( + "PERMISSION", + "getAssetFileDescriptorFromUri: check file descriptor permission = " + + "${checkReadFileDescriptorPermission(it?.fileDescriptor)}" + ) AssetFileDescriptor( ParcelFileDescriptor.dup(it?.fileDescriptor), 0, AssetFileDescriptor.UNKNOWN_LENGTH @@ -430,4 +447,22 @@ object FileUtils { null } } + + private fun checkReadFileDescriptorPermission(fileDescriptor: FileDescriptor?): Boolean { + if (fileDescriptor?.valid() == false) { + // The FileDescriptor is not valid + return false + } + + return try { + val channel = FileInputStream(fileDescriptor).channel + // Try to check read access + channel.position(0) + channel.read(ByteBuffer.allocate(1)) + true + } catch (ignore: Exception) { + // An exception occurred, indicating a lack of read permission + false + } + } }