Merge pull request #4218 from kiwix/Fixes#4216

Fixed: Kiwix cannot import ZIM files from the filesystem.
kiwix · Feb 13, 2025 · 2017a4e · 2017a4e
2 parents 50eeeec + 4735933
commit 2017a4e
Show file tree

Hide file tree

Showing 119 changed files with 335 additions and 250 deletions.
diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt
@@ -18,7 +18,6 @@
 
 package org.kiwix.kiwixmobile
 
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.lifecycle.Lifecycle
 import androidx.preference.PreferenceManager
@@ -136,7 +135,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
     val loadFileStream =
       ObjectBoxToLibkiwixMigratorTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim.zim"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/localLibrary/CopyMoveFileHandlerTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/localLibrary/CopyMoveFileHandlerTest.kt
@@ -20,7 +20,6 @@ package org.kiwix.kiwixmobile.localLibrary
 
 import android.net.Uri
 import android.os.Build
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.documentfile.provider.DocumentFile
 import androidx.lifecycle.Lifecycle
@@ -227,7 +226,7 @@ class CopyMoveFileHandlerTest : BaseActivityTest() {
     val loadFileStream =
       CopyMoveFileHandlerTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim.zim"
     )
     if (zimFile.exists()) zimFile.delete()
@@ -247,7 +246,7 @@ class CopyMoveFileHandlerTest : BaseActivityTest() {
 
   private fun getInvalidZimFileUri(extension: String): Uri {
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim$extension"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/localLibrary/OpeningFilesFromStorageTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/localLibrary/OpeningFilesFromStorageTest.kt
@@ -26,7 +26,6 @@ import android.net.Uri
 import android.os.Build
 import android.provider.MediaStore
 import android.util.Log
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.lifecycle.Lifecycle
 import androidx.preference.PreferenceManager
@@ -228,7 +227,7 @@ class OpeningFilesFromStorageTest : BaseActivityTest() {
     val loadFileStream =
       CopyMoveFileHandlerTest::class.java.classLoader.getResourceAsStream(fileName)
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       fileName
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/main/DarkModeViewPainterTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/main/DarkModeViewPainterTest.kt
@@ -18,7 +18,6 @@
 
 package org.kiwix.kiwixmobile.main
 
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
@@ -161,7 +160,7 @@ class DarkModeViewPainterTest : BaseActivityTest() {
     val loadFileStream =
       DarkModeViewPainterTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim.zim"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/mimetype/MimeTypeTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/mimetype/MimeTypeTest.kt
@@ -18,7 +18,6 @@
 
 package org.kiwix.kiwixmobile.mimetype
 
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.lifecycle.Lifecycle
 import androidx.preference.PreferenceManager
@@ -72,7 +71,7 @@ class MimeTypeTest : BaseActivityTest() {
   fun testMimeType() = runBlocking {
     val loadFileStream = MimeTypeTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim.zim"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryTest.kt
@@ -18,7 +18,6 @@
 
 package org.kiwix.kiwixmobile.nav.destination.library
 
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.lifecycle.Lifecycle
 import androidx.preference.PreferenceManager
@@ -122,7 +121,7 @@ class LocalLibraryTest : BaseActivityTest() {
       LocalLibraryTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     val zimFile =
       File(
-        ContextCompat.getExternalFilesDirs(context, null)[0],
+        context.getExternalFilesDirs(null)[0],
         "testzim.zim"
       )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/note/NoteFragmentTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/note/NoteFragmentTest.kt
@@ -19,7 +19,6 @@
 package org.kiwix.kiwixmobile.note
 
 import android.os.Build
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
@@ -267,7 +266,7 @@ class NoteFragmentTest : BaseActivityTest() {
 
     val loadFileStream =
       NoteFragmentTest::class.java.classLoader.getResourceAsStream(zimFileName)
-    val zimFile = File(ContextCompat.getExternalFilesDirs(context, null)[0], zimFileName)
+    val zimFile = File(context.getExternalFilesDirs(null)[0], zimFileName)
     if (zimFile.exists()) zimFile.delete()
     zimFile.createNewFile()
     loadFileStream.use { inputStream ->

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt
@@ -18,7 +18,6 @@
 
 package org.kiwix.kiwixmobile.page.bookmarks
 
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
@@ -204,7 +203,7 @@ class LibkiwixBookmarkTest : BaseActivityTest() {
     val loadFileStream =
       LibkiwixBookmarkTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim.zim"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/page/history/NavigationHistoryTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/page/history/NavigationHistoryTest.kt
@@ -19,7 +19,6 @@
 package org.kiwix.kiwixmobile.page.history
 
 import android.os.Build
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
@@ -115,7 +114,7 @@ class NavigationHistoryTest : BaseActivityTest() {
     val loadFileStream =
       NavigationHistoryTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim.zim"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/EncodedUrlTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/EncodedUrlTest.kt
@@ -18,7 +18,6 @@
 
 package org.kiwix.kiwixmobile.reader
 
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.lifecycle.Lifecycle
 import androidx.preference.PreferenceManager
@@ -79,7 +78,7 @@ class EncodedUrlTest : BaseActivityTest() {
     val loadFileStream =
       EncodedUrlTest::class.java.classLoader.getResourceAsStream("characters_encoding.zim")
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "characters_encoding.zim"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/KiwixReaderFragmentTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/KiwixReaderFragmentTest.kt
@@ -19,7 +19,6 @@
 package org.kiwix.kiwixmobile.reader
 
 import android.os.Build
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
@@ -113,7 +112,7 @@ class KiwixReaderFragmentTest : BaseActivityTest() {
     val loadFileStream =
       KiwixReaderFragmentTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim.zim"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/...src/androidTest/java/org/kiwix/kiwixmobile/reader/ZimFileReaderWithSplittedZimFileTest.kt b/...src/androidTest/java/org/kiwix/kiwixmobile/reader/ZimFileReaderWithSplittedZimFileTest.kt
@@ -18,7 +18,6 @@
 
 package org.kiwix.kiwixmobile.reader
 
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
@@ -154,7 +153,7 @@ class ZimFileReaderWithSplittedZimFileTest : BaseActivityTest() {
   private fun createAndGetSplitedZimFile(shouldCreateExtraZeroSizeFile: Boolean = false): File? {
     val loadFileStream =
       EncodedUrlTest::class.java.classLoader.getResourceAsStream("testzim.zim")
-    val storageDir = ContextCompat.getExternalFilesDirs(context, null)[0]
+    val storageDir = context.getExternalFilesDirs(null)[0]
 
     // Delete existing parts if they exist
     (1..3)

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt
@@ -18,7 +18,6 @@
 package org.kiwix.kiwixmobile.search
 
 import android.os.Build
-import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
@@ -342,7 +341,7 @@ class SearchFragmentTest : BaseActivityTest() {
     val loadFileStream =
       SearchFragmentTest::class.java.classLoader.getResourceAsStream("testzim.zim")
     val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
+      context.getExternalFilesDirs(null)[0],
       "testzim.zim"
     )
     if (zimFile.exists()) zimFile.delete()

diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt
@@ -340,7 +340,7 @@ class FileUtilsInstrumentationTest {
     // get the SD card path
     val sdCardPath = context?.getExternalFilesDirs("")
       ?.get(1)?.path?.substringBefore("/Android")
-    val dummyUriData = listOf(
+    val dummyUriData = arrayListOf(
       // test the download uri on older devices
       DummyUrlData(
         null,
@@ -385,14 +385,6 @@ class FileUtilsInstrumentationTest {
             "%3A$commonUri"
         )
       ),
-      // test with USB stick uri
-      DummyUrlData(
-        null,
-        null,
-        "/mnt/media_rw/USB/$commonPath",
-        null,
-        Uri.parse("${primaryStorageUriPrefix}USB%3A$commonUri")
-      ),
       // test with invalid uri
       DummyUrlData(
         null,
@@ -421,6 +413,18 @@ class FileUtilsInstrumentationTest {
         )
       )
     )
+    // test with USB stick uri
+    if (Build.VERSION.SDK_INT > Build.VERSION_CODES.N_MR1) {
+      dummyUriData.add(
+        DummyUrlData(
+          null,
+          null,
+          "/mnt/media_rw/USB/$commonPath",
+          null,
+          Uri.parse("${primaryStorageUriPrefix}USB%3A$commonUri")
+        )
+      )
+    }
     context?.let { context ->
       CoroutineScope(Dispatchers.Main).launch {
         dummyUriData.forEach { dummyUrlData ->

diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt
@@ -410,7 +410,7 @@ class CopyMoveFileHandler @Inject constructor(
     FileUtils.isSplittedZimFile(destinationFile.name) || validateZimFileValid(destinationFile)
 
   fun handleInvalidZimFile(destinationFile: File, sourceUri: Uri) {
-    val errorMessage = activity.getString(R.string.error_file_invalid)
+    val errorMessage = activity.getString(R.string.error_file_invalid, destinationFile.path)
     if (isMoveOperation) {
       val moveSuccessful = tryMoveWithDocumentContract(
         destinationFile.toUri(),

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
@@ -27,6 +27,7 @@ import android.net.Uri
 import android.os.Build
 import android.os.Bundle
 import android.os.Environment
+import android.util.Log
 import android.view.LayoutInflater
 import android.view.Menu
 import android.view.MenuInflater
@@ -90,6 +91,7 @@ import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
 import org.kiwix.kiwixmobile.core.utils.SimpleRecyclerViewScrollListener
 import org.kiwix.kiwixmobile.core.utils.SimpleRecyclerViewScrollListener.Companion.SCROLL_DOWN
 import org.kiwix.kiwixmobile.core.utils.SimpleRecyclerViewScrollListener.Companion.SCROLL_UP
+import org.kiwix.kiwixmobile.core.utils.TAG_KIWIX
 import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower
 import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog
 import org.kiwix.kiwixmobile.core.utils.files.FileUtils
@@ -425,7 +427,7 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal
         // and we will handle this later.
         val fileName = documentFile?.name
         if (fileName != null && !isValidZimFile(fileName)) {
-          activity.toast(string.error_file_invalid)
+          activity.toast(getString(string.error_file_invalid, "$uri"))
           return@launch
         }
         copyMoveFileHandler?.showMoveFileToPublicDirectoryDialog(
@@ -456,12 +458,18 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal
       requireActivity().applicationContext, uri
     )
     if (filePath == null || !File(filePath).isFileExist()) {
-      activity.toast(string.error_file_not_found)
+      Log.e(
+        TAG_KIWIX,
+        "The Selected ZIM file not found in the storage. File Uri = $uri\n" +
+          "Retrieved Path = $filePath"
+      )
+      activity.toast(getString(string.error_file_not_found, "$uri"))
       return null
     }
     val file = File(filePath)
     return if (!FileUtils.isValidZimFile(file.path)) {
-      activity.toast(string.error_file_invalid)
+      Log.e(TAG_KIWIX, "Selected ZIM file is not a valid ZIM file. File path = ${file.path}")
+      activity.toast(getString(string.error_file_invalid, file.path))
       null
     } else {
       file

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
@@ -144,7 +144,7 @@ class KiwixReaderFragment : CoreReaderFragment() {
       // it will attempt to open the last opened ZIM file with the last loaded URL,
       // which is inside the non-existing ZIM file. This leads to unexpected behavior.
       exitBook()
-      activity.toast(string.error_file_not_found)
+      activity.toast(getString(string.error_file_not_found, zimFileUri))
       return
     }
     val zimReaderSource = ZimReaderSource(File(filePath))

diff --git a/...in/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt b/...in/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt
@@ -41,7 +41,9 @@ data class OpenFileWithNavigation(private val bookOnDisk: BooksOnDiskListItem.Bo
         zimReaderSource.canOpenInLibkiwix()
       }
       if (!canOpenInLibkiwix) {
-        activity.toast(R.string.error_file_not_found)
+        activity.toast(
+          activity.getString(R.string.error_file_not_found, zimReaderSource.toDatabase())
+        )
       } else {
         activity.navigate(
           actionNavigationLibraryToNavigationReader().apply {

diff --git a/app/src/test/java/org/kiwix/kiwixmobile/localLibrary/CopyMoveFileHandlerTest.kt b/app/src/test/java/org/kiwix/kiwixmobile/localLibrary/CopyMoveFileHandlerTest.kt
@@ -339,6 +339,7 @@ class CopyMoveFileHandlerTest {
   fun `notifyFileOperationSuccess should handle invalid ZIM file`() = runTest {
     fileHandler = spyk(fileHandler)
     fileHandler.shouldValidateZimFile = true
+    every { destinationFile.path } returns ""
     coEvery { fileHandler.isValidZimFile(destinationFile) } returns false
     fileHandler.notifyFileOperationSuccess(destinationFile, sourceUri)
 
@@ -350,17 +351,26 @@ class CopyMoveFileHandlerTest {
     fileHandler = spyk(fileHandler)
     every { fileHandler.tryMoveWithDocumentContract(any(), any(), any()) } returns true
     every { destinationFile.parentFile } returns mockk()
+    every { destinationFile.path } returns ""
     fileHandler.isMoveOperation = true
 
     fileHandler.handleInvalidZimFile(destinationFile, sourceUri)
 
     verify { fileHandler.dismissProgressDialog() }
-    verify { fileCopyMoveCallback.onError(activity.getString(R.string.error_file_invalid)) }
+    verify {
+      fileCopyMoveCallback.onError(
+        activity.getString(
+          R.string.error_file_invalid,
+          destinationFile.path
+        )
+      )
+    }
   }
 
   @Test
   fun `handleInvalidZimFile should delete file and show error if move fails`() {
     fileHandler = spyk(fileHandler)
+    every { destinationFile.path } returns ""
     every { fileHandler.tryMoveWithDocumentContract(any(), any(), any()) } returns false
     every { destinationFile.parentFile } returns mockk()
     fileHandler.isMoveOperation = true
@@ -369,7 +379,7 @@ class CopyMoveFileHandlerTest {
 
     verify {
       fileHandler.handleFileOperationError(
-        activity.getString(R.string.error_file_invalid),
+        activity.getString(R.string.error_file_invalid, destinationFile.path),
         destinationFile
       )
     }