From 57f9e664cafa4623a2a707f1f7bae72388e36132 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Mon, 19 Feb 2024 17:26:41 +0530 Subject: [PATCH 1/6] * Enhanced the `queryForActualPath()` method to retrieve the actual path for a URI. Users faced a `java.lang.UnsupportedOperationException: Unknown URI` exception when querying the contentResolver with an invalid URI. A check has been added to prevent such errors. * On older devices, the download URI contains the full file path, causing the `documentProviderContentQuery` to fail in retrieving the path, leading to unexpected behavior (failure to open the ZIM file). The code has been improved to handle this scenario. * In certain devices, the content URI prefix may differ, and using only `public_downloads` to obtain the actual path from the URI might not work if the device has a different URI prefix. The code has been enhanced to consider every possible content prefix when retrieving the actual path. --- .../kiwixmobile/core/utils/files/FileUtils.kt | 65 +++++++++++++++---- 1 file changed, 54 insertions(+), 11 deletions(-) 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..5e02319e07 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 @@ -49,6 +49,7 @@ import java.io.BufferedReader import java.io.File import java.io.FileNotFoundException import java.io.IOException +import java.lang.NumberFormatException object FileUtils { @@ -156,18 +157,58 @@ object FileUtils { return null } - private fun documentProviderContentQuery(context: Context, uri: Uri) = - contentQuery( - context, - ContentUris.withAppendedId( - Uri.parse("content://downloads/public_downloads"), - try { - DocumentsContract.getDocumentId(uri).toLong() - } catch (ignore: NumberFormatException) { - 0L - } - ) + private fun documentProviderContentQuery(context: Context, uri: Uri): String? { + // Extracting the document ID from the URI. + val documentId = extractDocumentId(uri) + + // Attempt to handle cases where the document ID is a direct path to a ZIM file. + if (isValidZimFile(documentId)) { + return documentId.substringAfter("raw:") + } + + // Try different content URI prefixes in some case download content prefix is different. + val contentUriPrefixes = arrayOf( + "content://downloads/public_downloads", + "content://downloads/my_downloads", + "content://downloads/all_downloads" ) + val actualDocumentId = try { + documentId.toLong() + } catch (ignore: NumberFormatException) { + 0L + } + return queryForActualPath(context, actualDocumentId, contentUriPrefixes) + } + + private fun queryForActualPath( + context: Context, + documentId: Long, + contentUriPrefixes: Array + ): String? { + try { + for (prefix in contentUriPrefixes) { + val contentUri = ContentUris.withAppendedId(Uri.parse(prefix), documentId) + val path = contentQuery(context, contentUri) + + if (path != null) { + return path + } + } + } catch (ignore: Exception) { + // do nothing + } + + return null + } + + private fun extractDocumentId(uri: Uri): String { + try { + return DocumentsContract.getDocumentId(uri) + } catch (ignore: Exception) { + // Log or handle the exception if needed + } + return "" + } private fun contentQuery( context: Context, @@ -185,6 +226,8 @@ object FileUtils { null } catch (ignore: NullPointerException) { null + } catch (ignore: UnsupportedOperationException) { + null } } From c830e1acf00047b208dd2086790bc687e102287d Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Mon, 19 Feb 2024 23:17:46 +0530 Subject: [PATCH 2/6] Added Test cases for `getLocalFilePathByUri` method that we are using for getting the filePath from the given uri. * This test ensure that every uri will return the expected filePath. e.g. for download uri, older device compatibility, internal/external/USB-STICK uri, and if a invalid uri is given then it will return the null. --- .../files/FileUtilsInstrumentationTest.kt | 111 +++++++++++++++++- 1 file changed, 108 insertions(+), 3 deletions(-) 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 index bbb899772c..586c6ca79d 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt @@ -18,18 +18,22 @@ package org.kiwix.kiwixmobile.utils.files import android.content.Context +import android.net.Uri +import android.os.Environment import androidx.test.platform.app.InstrumentationRegistry import org.junit.After import org.junit.Assert import org.junit.Before import org.junit.Test import org.junit.jupiter.api.Assertions -import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getAllZimParts -import org.kiwix.kiwixmobile.core.utils.files.FileUtils.hasPart import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity import org.kiwix.kiwixmobile.core.utils.files.FileUtils +import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getAllZimParts +import org.kiwix.kiwixmobile.core.utils.files.FileUtils.hasPart import java.io.File +import java.io.FileOutputStream import java.io.IOException +import java.io.OutputStream import java.util.Random class FileUtilsInstrumentationTest { @@ -246,5 +250,106 @@ class FileUtilsInstrumentationTest { } } - data class DummyUrlData(val url: String?, val expectedFileName: String?) + @Test + fun testGetLocalFilePathByUri() { + val loadFileStream = + FileUtilsInstrumentationTest::class.java.classLoader.getResourceAsStream("testzim.zim") + val zimFile = File(testDir, "testzim.zim") + if (zimFile.exists()) zimFile.delete() + zimFile.createNewFile() + loadFileStream.use { inputStream -> + val outputStream: OutputStream = FileOutputStream(zimFile) + outputStream.use { it -> + val buffer = ByteArray(inputStream.available()) + var length: Int + while (inputStream.read(buffer).also { length = it } > 0) { + it.write(buffer, 0, length) + } + } + } + val commonUri = "Download/beer.stackexchange.com_en_all_2023-05.zim" + val sdCardPath = context!!.getExternalFilesDirs("")[1].path.substringBefore("/Android") + val dummyUriData = listOf( + // test the download uri on older devices + DummyUrlData( + null, + "${Environment.getExternalStorageDirectory()}/$commonUri", + Uri.parse( + "content://com.android.providers.downloads.documents/document/" + + "raw%3A%2Fstorage%2Femulated%2F0%2FDownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + ) + ), + // test the download uri with new version of android + DummyUrlData( + null, + "${Environment.getExternalStorageDirectory()}/$commonUri", + Uri.parse( + "content://com.android.providers.downloads.documents/document/" + + "%2Fstorage%2Femulated%2F0%2FDownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + ) + ), + // test with file scheme + DummyUrlData( + null, + zimFile.path, + Uri.fromFile(zimFile) + ), + // test with internal storage uri + DummyUrlData( + null, + "${Environment.getExternalStorageDirectory()}/$commonUri", + Uri.parse( + "content://com.android.externalstorage.documents/document/" + + "primary%3ADownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + ) + ), + // test with SD card uri + DummyUrlData( + null, + "$sdCardPath/$commonUri", + Uri.parse( + "content://com.android.externalstorage.documents/document/" + + sdCardPath.substringAfter("storage/") + + "%3ADownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + ) + ), + // test with USB stick uri + DummyUrlData( + null, + "/mnt/media_rw/USB/$commonUri", + Uri.parse( + "content://com.android.externalstorage.documents/document/" + + "USB%3ADownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + ) + ), + // test with invalid uri + DummyUrlData( + null, + null, + Uri.parse( + "content://com.android.externalstorage.documents/document/" + ) + ), + // test with invalid download uri + DummyUrlData( + null, + null, + Uri.parse( + "content://media/external/downloads/0" + ) + ) + ) + context?.let { context -> + dummyUriData.forEach { dummyUrlData -> + dummyUrlData.uri?.let { uri -> + Assertions.assertEquals( + FileUtils.getLocalFilePathByUri(context, uri), + dummyUrlData.expectedFileName + ) + } + } + } + } + + data class DummyUrlData(val url: String?, val expectedFileName: String?, val uri: Uri? = null) } From abdd966c1da3f860180077c06bf639f3641ddfaf Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Tue, 20 Feb 2024 12:14:48 +0530 Subject: [PATCH 3/6] Added code to handle the exception when the provided download URI is invalid, as we encountered this issue in the test case for API level 24. --- .../files/FileUtilsInstrumentationTest.kt | 29 ++++++++++--------- .../kiwixmobile/core/utils/files/FileUtils.kt | 3 ++ 2 files changed, 19 insertions(+), 13 deletions(-) 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 index 586c6ca79d..c681ff3cbc 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt @@ -267,25 +267,28 @@ class FileUtilsInstrumentationTest { } } } - val commonUri = "Download/beer.stackexchange.com_en_all_2023-05.zim" - val sdCardPath = context!!.getExternalFilesDirs("")[1].path.substringBefore("/Android") + val commonPath = "Download/beer.stackexchange.com_en_all_2023-05.zim" + val commonUri = "Download%2Fbeer.stackexchange.com_en_all_2023-05.zim" + // get the SD card path + val sdCardPath = context?.getExternalFilesDirs("") + ?.get(1)?.path?.substringBefore("/Android") val dummyUriData = listOf( // test the download uri on older devices DummyUrlData( null, - "${Environment.getExternalStorageDirectory()}/$commonUri", + "${Environment.getExternalStorageDirectory()}/$commonPath", Uri.parse( "content://com.android.providers.downloads.documents/document/" + - "raw%3A%2Fstorage%2Femulated%2F0%2FDownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + "raw%3A%2Fstorage%2Femulated%2F0%2F$commonUri" ) ), // test the download uri with new version of android DummyUrlData( null, - "${Environment.getExternalStorageDirectory()}/$commonUri", + "${Environment.getExternalStorageDirectory()}/$commonPath", Uri.parse( "content://com.android.providers.downloads.documents/document/" + - "%2Fstorage%2Femulated%2F0%2FDownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + "%2Fstorage%2Femulated%2F0%2F$commonUri" ) ), // test with file scheme @@ -297,29 +300,29 @@ class FileUtilsInstrumentationTest { // test with internal storage uri DummyUrlData( null, - "${Environment.getExternalStorageDirectory()}/$commonUri", + "${Environment.getExternalStorageDirectory()}/$commonPath", Uri.parse( "content://com.android.externalstorage.documents/document/" + - "primary%3ADownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + "primary%3A$commonUri" ) ), // test with SD card uri DummyUrlData( null, - "$sdCardPath/$commonUri", + "$sdCardPath/$commonPath", Uri.parse( "content://com.android.externalstorage.documents/document/" + - sdCardPath.substringAfter("storage/") + - "%3ADownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + sdCardPath?.substringAfter("storage/") + + "%3A$commonUri" ) ), // test with USB stick uri DummyUrlData( null, - "/mnt/media_rw/USB/$commonUri", + "/mnt/media_rw/USB/$commonPath", Uri.parse( "content://com.android.externalstorage.documents/document/" + - "USB%3ADownload%2Fbeer.stackexchange.com_en_all_2023-05.zim" + "USB%3A$commonUri" ) ), // test with invalid uri 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 5e02319e07..65d1674e7d 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 @@ -228,6 +228,9 @@ object FileUtils { null } catch (ignore: UnsupportedOperationException) { null + } catch (ignore: IllegalStateException) { + // to handle the scenario if the given uri is UNKNOWN + null } } From a64e58ea166e9398898b5ba1351c5527340c952e Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Tue, 27 Feb 2024 00:33:13 +0530 Subject: [PATCH 4/6] Added test cases for `extractDocumentId` and `documentProviderContentQuery` to comprehensively test all scenarios for URIs. * Implemented `DocumentResolverWrapper` to facilitate mocking of values for `DocumentsContract` and `ContentResolver` methods, considering the inability to directly mock these methods in our test class. * Included test cases covering various types of download URIs utilized in our `documentProviderContentQuery` method. * Removed `getAssetFileDescriptorFromUri` method from FileUtils class as now it is unused. * Removed the `getAssetFileDescriptorFromUri` method from FileUtils class as now it is unused. --- .../files/FileUtilsInstrumentationTest.kt | 154 +++++++++++++++--- .../utils/files/DocumentResolverWrapper.kt | 53 ++++++ .../kiwixmobile/core/utils/files/FileUtils.kt | 92 ++++++----- 3 files changed, 229 insertions(+), 70 deletions(-) create mode 100644 core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/DocumentResolverWrapper.kt 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 index c681ff3cbc..636c15f52d 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt @@ -19,15 +19,21 @@ package org.kiwix.kiwixmobile.utils.files import android.content.Context import android.net.Uri +import android.os.Build import android.os.Environment import androidx.test.platform.app.InstrumentationRegistry +import io.mockk.every +import io.mockk.mockk import org.junit.After import org.junit.Assert import org.junit.Before import org.junit.Test import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Assertions.assertEquals import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity +import org.kiwix.kiwixmobile.core.utils.files.DocumentResolverWrapper import org.kiwix.kiwixmobile.core.utils.files.FileUtils +import org.kiwix.kiwixmobile.core.utils.files.FileUtils.documentProviderContentQuery import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getAllZimParts import org.kiwix.kiwixmobile.core.utils.files.FileUtils.hasPart import java.io.File @@ -39,6 +45,14 @@ import java.util.Random class FileUtilsInstrumentationTest { private var context: Context? = null private var testDir: File? = null + private val commonPath = "Download/beer.stackexchange.com_en_all_2023-05.zim" + private val commonUri = "Download%2Fbeer.stackexchange.com_en_all_2023-05.zim" + private val downloadDocumentUriPrefix = + "content://com.android.providers.downloads.documents/document/" + private val primaryStorageUriPrefix = + "content://com.android.externalstorage.documents/document/" + private val downloadUriPrefix = "content://media/external/downloads/" + private val expectedFilePath = "${Environment.getExternalStorageDirectory()}/$commonPath" @Before fun executeBefore() { @@ -267,8 +281,6 @@ class FileUtilsInstrumentationTest { } } } - val commonPath = "Download/beer.stackexchange.com_en_all_2023-05.zim" - val commonUri = "Download%2Fbeer.stackexchange.com_en_all_2023-05.zim" // get the SD card path val sdCardPath = context?.getExternalFilesDirs("") ?.get(1)?.path?.substringBefore("/Android") @@ -276,20 +288,14 @@ class FileUtilsInstrumentationTest { // test the download uri on older devices DummyUrlData( null, - "${Environment.getExternalStorageDirectory()}/$commonPath", - Uri.parse( - "content://com.android.providers.downloads.documents/document/" + - "raw%3A%2Fstorage%2Femulated%2F0%2F$commonUri" - ) + expectedFilePath, + Uri.parse("${downloadDocumentUriPrefix}raw%3A%2Fstorage%2Femulated%2F0%2F$commonUri") ), // test the download uri with new version of android DummyUrlData( null, - "${Environment.getExternalStorageDirectory()}/$commonPath", - Uri.parse( - "content://com.android.providers.downloads.documents/document/" + - "%2Fstorage%2Femulated%2F0%2F$commonUri" - ) + expectedFilePath, + Uri.parse("$downloadDocumentUriPrefix%2Fstorage%2Femulated%2F0%2F$commonUri") ), // test with file scheme DummyUrlData( @@ -300,18 +306,15 @@ class FileUtilsInstrumentationTest { // test with internal storage uri DummyUrlData( null, - "${Environment.getExternalStorageDirectory()}/$commonPath", - Uri.parse( - "content://com.android.externalstorage.documents/document/" + - "primary%3A$commonUri" - ) + expectedFilePath, + Uri.parse("${primaryStorageUriPrefix}primary%3A$commonUri") ), - // test with SD card uri + // // test with SD card uri DummyUrlData( null, "$sdCardPath/$commonPath", Uri.parse( - "content://com.android.externalstorage.documents/document/" + + primaryStorageUriPrefix + sdCardPath?.substringAfter("storage/") + "%3A$commonUri" ) @@ -320,25 +323,20 @@ class FileUtilsInstrumentationTest { DummyUrlData( null, "/mnt/media_rw/USB/$commonPath", - Uri.parse( - "content://com.android.externalstorage.documents/document/" + - "USB%3A$commonUri" - ) + Uri.parse("${primaryStorageUriPrefix}USB%3A$commonUri") ), // test with invalid uri DummyUrlData( null, null, - Uri.parse( - "content://com.android.externalstorage.documents/document/" - ) + Uri.parse(primaryStorageUriPrefix) ), // test with invalid download uri DummyUrlData( null, null, Uri.parse( - "content://media/external/downloads/0" + "${downloadUriPrefix}0" ) ) ) @@ -354,5 +352,107 @@ class FileUtilsInstrumentationTest { } } + @Test + fun testExtractDocumentId() { + val dummyDownloadUriData = arrayOf( + DummyUrlData( + null, + "raw:$expectedFilePath", + Uri.parse("${downloadDocumentUriPrefix}raw%3A%2Fstorage%2Femulated%2F0%2F$commonUri") + ), + DummyUrlData( + null, + expectedFilePath, + Uri.parse("$downloadDocumentUriPrefix%2Fstorage%2Femulated%2F0%2F$commonUri") + ), + DummyUrlData( + null, + "", + Uri.parse(downloadUriPrefix) + ) + ) + + dummyDownloadUriData.forEach { dummyUrlData -> + dummyUrlData.uri?.let { uri -> + Assertions.assertEquals( + FileUtils.extractDocumentId(uri, DocumentResolverWrapper()), + dummyUrlData.expectedFileName + ) + } + } + + // Testing with a dynamically generated URI. This URI creates at runtime, + // and passing it statically would result in an `IllegalArgumentException` exception. + // Therefore, we simulate this scenario using the `DocumentsContractWrapper` + // to conduct the test. + val mockDocumentsContractWrapper: DocumentResolverWrapper = mockk() + val expectedDocumentId = "1000020403" + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + val mockedUri = Uri.parse("$downloadUriPrefix$expectedDocumentId") + every { mockDocumentsContractWrapper.getDocumentId(mockedUri) } returns expectedDocumentId + val actualDocumentId = FileUtils.extractDocumentId(mockedUri, mockDocumentsContractWrapper) + assertEquals(expectedDocumentId, actualDocumentId) + } + } + + @Test + fun testDocumentProviderContentQuery() { + // test to get the download uri on old device + testWithDownloadUri( + Uri.parse("${downloadDocumentUriPrefix}raw%3A%2Fstorage%2Femulated%2F0%2F$commonUri"), + expectedFilePath + ) + + // test to get the download uri on new device + testWithDownloadUri( + Uri.parse("$downloadDocumentUriPrefix%2Fstorage%2Femulated%2F0%2F$commonUri"), + expectedFilePath + ) + + // test with all possible download uris + val contentUriPrefixes = arrayOf( + "content://downloads/public_downloads", + "content://downloads/my_downloads", + "content://downloads/all_downloads" + ) + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + contentUriPrefixes.forEach { + val mockDocumentsContractWrapper: DocumentResolverWrapper = mockk() + val expectedDocumentId = "1000020403" + val mockedUri = Uri.parse("$it/$expectedDocumentId") + every { mockDocumentsContractWrapper.getDocumentId(mockedUri) } returns expectedDocumentId + every { + mockDocumentsContractWrapper.query( + context!!, + mockedUri, + "_data", + null, + null, + null + ) + } returns expectedFilePath + testWithDownloadUri( + mockedUri, + expectedFilePath, + mockDocumentsContractWrapper + ) + } + } + } + + private fun testWithDownloadUri( + uri: Uri, + expectedPath: String, + documentsContractWrapper: DocumentResolverWrapper = DocumentResolverWrapper() + ) { + context?.let { context -> + assertEquals( + expectedPath, + documentProviderContentQuery(context, uri, documentsContractWrapper) + ) + } + } + data class DummyUrlData(val url: String?, val expectedFileName: String?, val uri: Uri? = null) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/DocumentResolverWrapper.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/DocumentResolverWrapper.kt new file mode 100644 index 0000000000..ce2c20d23a --- /dev/null +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/DocumentResolverWrapper.kt @@ -0,0 +1,53 @@ +/* + * Kiwix Android + * Copyright (c) 2024 Kiwix + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package org.kiwix.kiwixmobile.core.utils.files + +import android.content.Context +import android.net.Uri +import android.provider.DocumentsContract +import org.kiwix.kiwixmobile.core.extensions.get + +/** + * Wrapper class created for `DocumentsContract` and `ContentResolver` methods. + * This class facilitates the usage of these methods in test cases where direct + * mocking is not feasible. + */ +class DocumentResolverWrapper { + fun getDocumentId(uri: Uri): String = DocumentsContract.getDocumentId(uri) + + @Suppress("LongParameterList") + fun query( + context: Context, + uri: Uri, + columnName: String, + selection: String?, + selectionArgs: Array?, + sortOrder: String? + ): String? = context.contentResolver.query( + uri, + arrayOf(columnName), + selection, + selectionArgs, + sortOrder + )?.use { + return@query if (it.moveToFirst() && it.getColumnIndex(columnName) != -1) { + it[columnName] + } else null + } +} 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 65d1674e7d..1ea410feeb 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 @@ -22,7 +22,6 @@ import android.app.Activity import android.content.ContentUris import android.content.Context import android.content.Intent -import android.content.res.AssetFileDescriptor import android.net.Uri import android.os.Build import android.os.Environment @@ -40,14 +39,12 @@ import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.core.downloader.ChunkUtils import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity.Book import org.kiwix.kiwixmobile.core.extensions.deleteFile -import org.kiwix.kiwixmobile.core.extensions.get import org.kiwix.kiwixmobile.core.extensions.isFileExist import org.kiwix.kiwixmobile.core.extensions.toast 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 import java.lang.NumberFormatException @@ -120,6 +117,7 @@ object FileUtils { context: Context, uri: Uri ): String? { + Log.e("DOWNLOAD_URI", "getLocalFilePathByUri: $uri") if (DocumentsContract.isDocumentUri(context, uri)) { if ("com.android.externalstorage.documents" == uri.authority) { val documentId = DocumentsContract.getDocumentId(uri) @@ -157,9 +155,13 @@ object FileUtils { return null } - private fun documentProviderContentQuery(context: Context, uri: Uri): String? { + fun documentProviderContentQuery( + context: Context, + uri: Uri, + documentsContractWrapper: DocumentResolverWrapper = DocumentResolverWrapper() + ): String? { // Extracting the document ID from the URI. - val documentId = extractDocumentId(uri) + val documentId = extractDocumentId(uri, documentsContractWrapper) // Attempt to handle cases where the document ID is a direct path to a ZIM file. if (isValidZimFile(documentId)) { @@ -177,59 +179,75 @@ object FileUtils { } catch (ignore: NumberFormatException) { 0L } - return queryForActualPath(context, actualDocumentId, contentUriPrefixes) + return queryForActualPath( + context, + actualDocumentId, + contentUriPrefixes, + documentsContractWrapper + ) } private fun queryForActualPath( context: Context, documentId: Long, - contentUriPrefixes: Array + contentUriPrefixes: Array, + documentsContractWrapper: DocumentResolverWrapper ): String? { try { for (prefix in contentUriPrefixes) { - val contentUri = ContentUris.withAppendedId(Uri.parse(prefix), documentId) - val path = contentQuery(context, contentUri) - - if (path != null) { - return path + contentQuery( + context, + ContentUris.withAppendedId(Uri.parse(prefix), documentId), + documentsContractWrapper + )?.let { + return@queryForActualPath it } } } catch (ignore: Exception) { - // do nothing + Log.e( + "kiwix", "Error in getting path for documentId = $documentId \n" + + "Exception = $ignore" + ) } return null } - private fun extractDocumentId(uri: Uri): String { + fun extractDocumentId( + uri: Uri, + documentsContractWrapper: DocumentResolverWrapper + ): String { try { - return DocumentsContract.getDocumentId(uri) + return documentsContractWrapper.getDocumentId(uri) } catch (ignore: Exception) { - // Log or handle the exception if needed + Log.e( + "kiwix", "Unable to get documentId for uri = $uri \n" + + "Exception = $ignore" + ) } return "" } private fun contentQuery( context: Context, - uri: Uri + uri: Uri, + documentsContractWrapper: DocumentResolverWrapper = DocumentResolverWrapper() ): String? { val columnName = "_data" return try { - context.contentResolver.query(uri, arrayOf(columnName), null, null, null) - ?.use { - if (it.moveToFirst() && it.getColumnIndex(columnName) != -1) { - it[columnName] - } else null - } - } catch (ignore: SecurityException) { - null - } catch (ignore: NullPointerException) { - null - } catch (ignore: UnsupportedOperationException) { - null - } catch (ignore: IllegalStateException) { - // to handle the scenario if the given uri is UNKNOWN + documentsContractWrapper.query( + context, + uri, + columnName, + null, + null, + null + ) + } catch (ignore: Exception) { + Log.e( + "kiwix", "Could not get path for uri = $uri \n" + + "Exception = $ignore" + ) null } } @@ -455,16 +473,4 @@ object FileUtils { @JvmStatic fun getDemoFilePathForCustomApp(context: Context) = "${ContextCompat.getExternalFilesDirs(context, null)[0]}/demo.zim" - - @JvmStatic - fun getAssetFileDescriptorFromUri( - context: Context, - uri: Uri - ): AssetFileDescriptor? { - return try { - context.contentResolver.openAssetFileDescriptor(uri, "r") - } catch (ignore: FileNotFoundException) { - null - } - } } From 58b423b06d071c423afd14a9ec79d610194f731b Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Tue, 27 Feb 2024 00:47:02 +0530 Subject: [PATCH 5/6] Fixed code formatting issue. --- .../kiwix/kiwixmobile/core/utils/files/FileUtils.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 1ea410feeb..b78f97b2c2 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 @@ -205,8 +205,8 @@ object FileUtils { } } catch (ignore: Exception) { Log.e( - "kiwix", "Error in getting path for documentId = $documentId \n" + - "Exception = $ignore" + "kiwix", + "Error in getting path for documentId = $documentId \nException = $ignore" ) } @@ -221,8 +221,8 @@ object FileUtils { return documentsContractWrapper.getDocumentId(uri) } catch (ignore: Exception) { Log.e( - "kiwix", "Unable to get documentId for uri = $uri \n" + - "Exception = $ignore" + "kiwix", + "Unable to get documentId for uri = $uri \nException = $ignore" ) } return "" @@ -245,8 +245,8 @@ object FileUtils { ) } catch (ignore: Exception) { Log.e( - "kiwix", "Could not get path for uri = $uri \n" + - "Exception = $ignore" + "kiwix", + "Could not get path for uri = $uri \nException = $ignore" ) null } From 7671a3be7b6d21a1563b9ef6c4f62ef9e423ff3a Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Tue, 27 Feb 2024 12:28:24 +0530 Subject: [PATCH 6/6] Added necessary comments and removed unnecessary logs. --- .../utils/files/FileUtilsInstrumentationTest.kt | 8 ++++++++ .../org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt | 2 -- 2 files changed, 8 insertions(+), 2 deletions(-) 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 index 636c15f52d..9ea250a816 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt @@ -388,6 +388,10 @@ class FileUtilsInstrumentationTest { val mockDocumentsContractWrapper: DocumentResolverWrapper = mockk() val expectedDocumentId = "1000020403" if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + // Below Android 10, download URI schemes are of the content type, such as: + // content://com.android.chrome.FileProvider/downloads/alpinelinux_en_all_nopic_2022-12.zim + // As a result, this method will not be called during that time. We are not testing it on + // Android versions below 10, as it would result in an "IllegalArgumentException" exception. val mockedUri = Uri.parse("$downloadUriPrefix$expectedDocumentId") every { mockDocumentsContractWrapper.getDocumentId(mockedUri) } returns expectedDocumentId val actualDocumentId = FileUtils.extractDocumentId(mockedUri, mockDocumentsContractWrapper) @@ -417,6 +421,10 @@ class FileUtilsInstrumentationTest { ) if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + // Below Android 10, download URI schemes are of the content type, such as: + // content://com.android.chrome.FileProvider/downloads/alpinelinux_en_all_nopic_2022-12.zim + // As a result, this method will not be called during that time. We are not testing it on + // Android versions below 10, as it would result in an "IllegalArgumentException" exception. contentUriPrefixes.forEach { val mockDocumentsContractWrapper: DocumentResolverWrapper = mockk() val expectedDocumentId = "1000020403" 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 b78f97b2c2..f954433421 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 @@ -46,7 +46,6 @@ import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import java.io.BufferedReader import java.io.File import java.io.IOException -import java.lang.NumberFormatException object FileUtils { @@ -117,7 +116,6 @@ object FileUtils { context: Context, uri: Uri ): String? { - Log.e("DOWNLOAD_URI", "getLocalFilePathByUri: $uri") if (DocumentsContract.isDocumentUri(context, uri)) { if ("com.android.externalstorage.documents" == uri.authority) { val documentId = DocumentsContract.getDocumentId(uri)