From 43f0e4019f5fed3cdf226999fe0bbdfd6f61a93e Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Fri, 8 Nov 2024 19:35:35 -0500 Subject: [PATCH 01/14] Initial implementation --- connect/build.gradle | 1 - connect/res/values/strings.xml | 1 + .../webview/StripeConnectWebViewClient.kt | 3 ++ .../connect/webview/StripeDownloadListener.kt | 46 +++++++++++++++++++ 4 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt diff --git a/connect/build.gradle b/connect/build.gradle index 8b27e4155c4..085855c5851 100644 --- a/connect/build.gradle +++ b/connect/build.gradle @@ -53,7 +53,6 @@ dependencies { testImplementation testLibs.mockito.kotlin testImplementation testLibs.robolectric testImplementation testLibs.truth - testImplementation project(':stripe-core') androidTestImplementation testLibs.androidx.composeUi androidTestImplementation testLibs.androidx.coreKtx diff --git a/connect/res/values/strings.xml b/connect/res/values/strings.xml index 709af97caf0..143a2f96d05 100644 --- a/connect/res/values/strings.xml +++ b/connect/res/values/strings.xml @@ -1,4 +1,5 @@ Not yet built… + Downloading file diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewClient.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewClient.kt index b84a31faece..a8fb32f3e7e 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewClient.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewClient.kt @@ -40,6 +40,9 @@ internal class StripeConnectWebViewClient( useWideViewPort = true userAgentString = "$userAgentString - stripe-android/${StripeSdkVersion.VERSION_NAME}" } + + setDownloadListener(StripeDownloadListener(webView.context)) + addJavascriptInterface(StripeJsInterface(), ANDROID_JS_INTERFACE) addJavascriptInterface(StripeJsInterfaceInternal(), ANDROID_JS_INTERNAL_INTERFACE) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt new file mode 100644 index 00000000000..c13e17f786f --- /dev/null +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -0,0 +1,46 @@ +package com.stripe.android.connect.webview + +import android.app.DownloadManager +import android.content.Context +import android.net.Uri +import android.os.Environment +import android.webkit.DownloadListener +import android.webkit.URLUtil +import com.stripe.android.connect.R +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch + +class StripeDownloadListener( + private val context: Context, + private val ioScope: CoroutineScope = CoroutineScope(Dispatchers.IO), +) : DownloadListener { + + private val downloadManager: DownloadManager by lazy { + context.getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager + } + + override fun onDownloadStart( + url: String?, + userAgent: String?, + contentDisposition: String?, + mimetype: String?, + contentLength: Long + ) { + url ?: return + val request = DownloadManager.Request(Uri.parse(url)) + + request.setDestinationInExternalPublicDir( + Environment.DIRECTORY_DOWNLOADS, + URLUtil.guessFileName(url, contentDisposition, mimetype) + ) + request.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + request.setTitle(URLUtil.guessFileName(url, contentDisposition, mimetype)) + request.setDescription(context.getString(R.string.stripe_downloading_file)) + request.setMimeType(mimetype) + + ioScope.launch { + downloadManager.enqueue(request) + } + } +} From 0871c100a683675c6a53fe2bf36a162e1bc8737d Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Fri, 8 Nov 2024 19:52:10 -0500 Subject: [PATCH 02/14] minor refactors --- .../connect/webview/StripeDownloadListener.kt | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index c13e17f786f..1671c68275c 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -1,9 +1,10 @@ package com.stripe.android.connect.webview import android.app.DownloadManager +import android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED import android.content.Context import android.net.Uri -import android.os.Environment +import android.os.Environment.DIRECTORY_DOWNLOADS import android.webkit.DownloadListener import android.webkit.URLUtil import com.stripe.android.connect.R @@ -28,14 +29,12 @@ class StripeDownloadListener( contentLength: Long ) { url ?: return - val request = DownloadManager.Request(Uri.parse(url)) - request.setDestinationInExternalPublicDir( - Environment.DIRECTORY_DOWNLOADS, - URLUtil.guessFileName(url, contentDisposition, mimetype) - ) - request.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED) - request.setTitle(URLUtil.guessFileName(url, contentDisposition, mimetype)) + val request = DownloadManager.Request(Uri.parse(url)) + val fileName = URLUtil.guessFileName(url, contentDisposition, mimetype) + request.setDestinationInExternalPublicDir(DIRECTORY_DOWNLOADS, fileName) + request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + request.setTitle(fileName) request.setDescription(context.getString(R.string.stripe_downloading_file)) request.setMimeType(mimetype) From a816820a4ed6415957a9e6db1515d04ab89e4b97 Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Fri, 8 Nov 2024 19:59:39 -0500 Subject: [PATCH 03/14] Fix strict mode violations --- .../connect/webview/StripeDownloadListener.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index 1671c68275c..1a31a8fff95 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -30,15 +30,15 @@ class StripeDownloadListener( ) { url ?: return - val request = DownloadManager.Request(Uri.parse(url)) - val fileName = URLUtil.guessFileName(url, contentDisposition, mimetype) - request.setDestinationInExternalPublicDir(DIRECTORY_DOWNLOADS, fileName) - request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) - request.setTitle(fileName) - request.setDescription(context.getString(R.string.stripe_downloading_file)) - request.setMimeType(mimetype) - ioScope.launch { + val request = DownloadManager.Request(Uri.parse(url)) + val fileName = URLUtil.guessFileName(url, contentDisposition, mimetype) + request.setDestinationInExternalPublicDir(DIRECTORY_DOWNLOADS, fileName) + request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + request.setTitle(fileName) + request.setDescription(context.getString(R.string.stripe_downloading_file)) + request.setMimeType(mimetype) + downloadManager.enqueue(request) } } From c536794856874c15311c565de09597255197918c Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Fri, 8 Nov 2024 20:08:20 -0500 Subject: [PATCH 04/14] implement basic download finished toast --- .../connect/webview/StripeDownloadListener.kt | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index 1a31a8fff95..b45a7c45a09 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -3,14 +3,20 @@ package com.stripe.android.connect.webview import android.app.DownloadManager import android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED import android.content.Context +import android.content.Intent import android.net.Uri import android.os.Environment.DIRECTORY_DOWNLOADS import android.webkit.DownloadListener import android.webkit.URLUtil +import android.widget.Toast +import androidx.core.content.FileProvider import com.stripe.android.connect.R import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import java.io.File class StripeDownloadListener( private val context: Context, @@ -39,7 +45,42 @@ class StripeDownloadListener( request.setDescription(context.getString(R.string.stripe_downloading_file)) request.setMimeType(mimetype) - downloadManager.enqueue(request) + val downloadId = downloadManager.enqueue(request) + + // Monitor the download progress and show a toast when done + val query = DownloadManager.Query().setFilterById(downloadId) + while (true) { + val cursor = downloadManager.query(query) + cursor.moveToFirst() + val index = cursor.getColumnIndex(DownloadManager.COLUMN_STATUS) + if (index < 0) break // status does not exist - abort + val status = cursor.getInt(index) + if (status == DownloadManager.STATUS_SUCCESSFUL) { + showOpenFileToast(fileName, mimetype) + break // download complete - exit the loop + } + cursor.close() + delay(1000) + } + } + } + + private fun showOpenFileToast(fileName: String, mimeType: String?) { + MainScope().launch { + val toast = Toast.makeText(context, "Download complete", Toast.LENGTH_LONG) + toast.view?.setOnClickListener { + openFile(fileName, mimeType) + } + toast.show() } } + + private fun openFile(fileName: String, mimeType: String?) { + val file = File(context.getExternalFilesDir(DIRECTORY_DOWNLOADS), fileName) + val uri = FileProvider.getUriForFile(context, "${context.packageName}.fileprovider", file) + val intent = Intent(Intent.ACTION_VIEW) + intent.setDataAndType(uri, mimeType) + intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) + context.startActivity(intent) + } } From 6b770cb01a6bebd43a9e8da3e448dc3e8cd253d0 Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Sun, 10 Nov 2024 13:23:06 -0500 Subject: [PATCH 05/14] Remove file opening code (not working), make internal --- .../connect/webview/StripeDownloadListener.kt | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index b45a7c45a09..7cdec35e2ef 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -18,7 +18,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.launch import java.io.File -class StripeDownloadListener( +internal class StripeDownloadListener( private val context: Context, private val ioScope: CoroutineScope = CoroutineScope(Dispatchers.IO), ) : DownloadListener { @@ -56,31 +56,23 @@ class StripeDownloadListener( if (index < 0) break // status does not exist - abort val status = cursor.getInt(index) if (status == DownloadManager.STATUS_SUCCESSFUL) { - showOpenFileToast(fileName, mimetype) + showOpenFileToast() break // download complete - exit the loop } cursor.close() - delay(1000) + delay(DOWNLOAD_DELAY_MS) } } } - private fun showOpenFileToast(fileName: String, mimeType: String?) { + private fun showOpenFileToast() { MainScope().launch { val toast = Toast.makeText(context, "Download complete", Toast.LENGTH_LONG) - toast.view?.setOnClickListener { - openFile(fileName, mimeType) - } toast.show() } } - private fun openFile(fileName: String, mimeType: String?) { - val file = File(context.getExternalFilesDir(DIRECTORY_DOWNLOADS), fileName) - val uri = FileProvider.getUriForFile(context, "${context.packageName}.fileprovider", file) - val intent = Intent(Intent.ACTION_VIEW) - intent.setDataAndType(uri, mimeType) - intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) - context.startActivity(intent) + internal companion object { + private const val DOWNLOAD_DELAY_MS = 1_000L } } From cd77613f56af67651dac9f873d9961a3432927d4 Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Sun, 10 Nov 2024 14:15:46 -0500 Subject: [PATCH 06/14] Add test --- connect/res/values/strings.xml | 2 + .../connect/webview/StripeDownloadListener.kt | 35 +++++--- .../webview/StripeDownloadListenerTest.kt | 80 +++++++++++++++++++ 3 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt diff --git a/connect/res/values/strings.xml b/connect/res/values/strings.xml index 143a2f96d05..0a2b0fd287f 100644 --- a/connect/res/values/strings.xml +++ b/connect/res/values/strings.xml @@ -2,4 +2,6 @@ Not yet built… Downloading file + Download complete + Unable to download file diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index 7cdec35e2ef..9bfe085287a 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -2,31 +2,29 @@ package com.stripe.android.connect.webview import android.app.DownloadManager import android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED +import android.app.DownloadManager.STATUS_PAUSED +import android.app.DownloadManager.STATUS_PENDING +import android.app.DownloadManager.STATUS_RUNNING import android.content.Context -import android.content.Intent import android.net.Uri import android.os.Environment.DIRECTORY_DOWNLOADS import android.webkit.DownloadListener import android.webkit.URLUtil import android.widget.Toast -import androidx.core.content.FileProvider import com.stripe.android.connect.R import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.MainScope import kotlinx.coroutines.delay import kotlinx.coroutines.launch -import java.io.File internal class StripeDownloadListener( private val context: Context, + private val downloadManager: DownloadManager? = + context.getSystemService(Context.DOWNLOAD_SERVICE) as? DownloadManager, private val ioScope: CoroutineScope = CoroutineScope(Dispatchers.IO), ) : DownloadListener { - private val downloadManager: DownloadManager by lazy { - context.getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager - } - override fun onDownloadStart( url: String?, userAgent: String?, @@ -34,7 +32,10 @@ internal class StripeDownloadListener( mimetype: String?, contentLength: Long ) { - url ?: return + if (url == null || downloadManager == null) { + showErrorToast() + return + } ioScope.launch { val request = DownloadManager.Request(Uri.parse(url)) @@ -49,15 +50,16 @@ internal class StripeDownloadListener( // Monitor the download progress and show a toast when done val query = DownloadManager.Query().setFilterById(downloadId) - while (true) { + var isDownloading = true + while (isDownloading) { val cursor = downloadManager.query(query) cursor.moveToFirst() val index = cursor.getColumnIndex(DownloadManager.COLUMN_STATUS) if (index < 0) break // status does not exist - abort val status = cursor.getInt(index) - if (status == DownloadManager.STATUS_SUCCESSFUL) { + if (status !in listOf(STATUS_PENDING, STATUS_RUNNING, STATUS_PAUSED)) { showOpenFileToast() - break // download complete - exit the loop + isDownloading = false // download complete - exit the loop } cursor.close() delay(DOWNLOAD_DELAY_MS) @@ -65,9 +67,18 @@ internal class StripeDownloadListener( } } + private fun showErrorToast() { + MainScope().launch { + val errorString = context.getString(R.string.stripe_unable_to_download_file) + val toast = Toast.makeText(context, errorString, Toast.LENGTH_LONG) + toast.show() + } + } + private fun showOpenFileToast() { MainScope().launch { - val toast = Toast.makeText(context, "Download complete", Toast.LENGTH_LONG) + val downloadCompleteString = context.getString(R.string.stripe_download_complete) + val toast = Toast.makeText(context, downloadCompleteString, Toast.LENGTH_LONG) toast.show() } } diff --git a/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt b/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt new file mode 100644 index 00000000000..266fc85e5e0 --- /dev/null +++ b/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt @@ -0,0 +1,80 @@ +package com.stripe.android.connect.webview + +import android.app.DownloadManager +import android.content.Context +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.Before +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions + +@ExperimentalCoroutinesApi +class StripeDownloadListenerTest { + + private val context: Context = mock { + on { getSystemService(Context.DOWNLOAD_SERVICE) } doReturn downloadManager + on { getString(any()) } doReturn "Placeholder string" + } + private val downloadManager: DownloadManager = mock { + + } + private val dispatcher = UnconfinedTestDispatcher() + + private lateinit var stripeDownloadListener: StripeDownloadListener + + @Before + fun setup() { + Dispatchers.setMain(dispatcher) + stripeDownloadListener = StripeDownloadListener( + context = context, + downloadManager = downloadManager, + ioScope = MainScope(), + ) + } + + @Test + fun `onDownloadStart creates download request with correct parameters`() = runTest(dispatcher) { + val url = "https://example.com/file.pdf" + val userAgent = "Mozilla/5.0" + val contentDisposition = "attachment; filename=file.pdf" + val mimeType = "application/pdf" + val contentLength = 1024L + + stripeDownloadListener.onDownloadStart(url, userAgent, contentDisposition, mimeType, contentLength) + + verify(downloadManager).enqueue(any()) + } + + @Test + fun `onDownloadStart does nothing when URL is null`() = runTest { + stripeDownloadListener.onDownloadStart(null, "", "", "", 0) + + verifyNoInteractions(downloadManager) + } + + @Test + fun `onDownloadStart does nothing when download manager doesn't exist`() = runTest { + stripeDownloadListener = StripeDownloadListener( + context = context, + downloadManager = null, + ioScope = MainScope(), + ) + val url = "https://example.com/file.pdf" + val userAgent = "Mozilla/5.0" + val contentDisposition = "attachment; filename=file.pdf" + val mimeType = "application/pdf" + val contentLength = 1024L + + stripeDownloadListener.onDownloadStart(url, userAgent, contentDisposition, mimeType, contentLength) + + verifyNoInteractions(downloadManager) + } +} From d9f389863d69da19bd20626075f123fd2e22fcde Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Sun, 10 Nov 2024 14:51:28 -0500 Subject: [PATCH 07/14] Fix test --- .../connect/webview/StripeDownloadListener.kt | 25 +++++++--- .../webview/StripeConnectWebViewClientTest.kt | 1 + .../webview/StripeDownloadListenerTest.kt | 49 +++++++++++++------ 3 files changed, 52 insertions(+), 23 deletions(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index 9bfe085287a..543633c9cbb 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -12,6 +12,7 @@ import android.webkit.DownloadListener import android.webkit.URLUtil import android.widget.Toast import com.stripe.android.connect.R +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.MainScope @@ -23,6 +24,18 @@ internal class StripeDownloadListener( private val downloadManager: DownloadManager? = context.getSystemService(Context.DOWNLOAD_SERVICE) as? DownloadManager, private val ioScope: CoroutineScope = CoroutineScope(Dispatchers.IO), + // methods below exposed for mocking in tests + private val getDownloadManagerRequest: (Uri) -> DownloadManager.Request = { uri -> + DownloadManager.Request(uri) + }, + private val getFileName: (url: String, contentDisposition: String?, mimeType: String?) -> String = + { url, contentDisposition, mimeType -> + URLUtil.guessFileName(url, contentDisposition, mimeType) + }, + private val parseUri: (String) -> Uri = { Uri.parse(it) }, + private val showToast: (String) -> Unit = { toastString -> + Toast.makeText(context, toastString, Toast.LENGTH_LONG).show() + } ) : DownloadListener { override fun onDownloadStart( @@ -38,8 +51,8 @@ internal class StripeDownloadListener( } ioScope.launch { - val request = DownloadManager.Request(Uri.parse(url)) - val fileName = URLUtil.guessFileName(url, contentDisposition, mimetype) + val request = getDownloadManagerRequest(parseUri(url)) + val fileName = getFileName(url, contentDisposition, mimetype) request.setDestinationInExternalPublicDir(DIRECTORY_DOWNLOADS, fileName) request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) request.setTitle(fileName) @@ -69,17 +82,13 @@ internal class StripeDownloadListener( private fun showErrorToast() { MainScope().launch { - val errorString = context.getString(R.string.stripe_unable_to_download_file) - val toast = Toast.makeText(context, errorString, Toast.LENGTH_LONG) - toast.show() + showToast(context.getString(R.string.stripe_unable_to_download_file)) } } private fun showOpenFileToast() { MainScope().launch { - val downloadCompleteString = context.getString(R.string.stripe_download_complete) - val toast = Toast.makeText(context, downloadCompleteString, Toast.LENGTH_LONG) - toast.show() + showToast(context.getString(R.string.stripe_download_complete)) } } diff --git a/connect/src/test/java/com/stripe/android/connect/webview/StripeConnectWebViewClientTest.kt b/connect/src/test/java/com/stripe/android/connect/webview/StripeConnectWebViewClientTest.kt index 00c99c48c23..ba96c6207f5 100644 --- a/connect/src/test/java/com/stripe/android/connect/webview/StripeConnectWebViewClientTest.kt +++ b/connect/src/test/java/com/stripe/android/connect/webview/StripeConnectWebViewClientTest.kt @@ -25,6 +25,7 @@ class StripeConnectWebViewClientTest { } private val mockWebView: WebView = mock { on { settings } doReturn mockSettings + on { context } doReturn mock() } private lateinit var webViewClient: StripeConnectWebViewClient diff --git a/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt b/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt index 266fc85e5e0..01f0c6183a4 100644 --- a/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt +++ b/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt @@ -2,9 +2,9 @@ package com.stripe.android.connect.webview import android.app.DownloadManager import android.content.Context +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.MainScope +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain @@ -16,32 +16,51 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions -@ExperimentalCoroutinesApi class StripeDownloadListenerTest { + private val downloadManager: DownloadManager = mock { + on { enqueue(any()) } doReturn 123L + } + private val downloadManagerRequest: DownloadManager.Request = mock { + // mock all builder methods to return this mock, for proper chaining + on { setDestinationInExternalPublicDir(any(), any()) } doReturn mock + on { setNotificationVisibility(any()) } doReturn mock + on { setTitle(any()) } doReturn mock + on { setDescription(any()) } doReturn mock + on { setMimeType(any()) } doReturn mock + } private val context: Context = mock { on { getSystemService(Context.DOWNLOAD_SERVICE) } doReturn downloadManager on { getString(any()) } doReturn "Placeholder string" } - private val downloadManager: DownloadManager = mock { - - } - private val dispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope() private lateinit var stripeDownloadListener: StripeDownloadListener @Before fun setup() { - Dispatchers.setMain(dispatcher) + Dispatchers.setMain(UnconfinedTestDispatcher()) + initDownloadListener() + } + + private fun initDownloadListener( + context: Context = this.context, + downloadManager: DownloadManager? = this.downloadManager, + ioScope: CoroutineScope = testScope, + ) { stripeDownloadListener = StripeDownloadListener( context = context, downloadManager = downloadManager, - ioScope = MainScope(), + ioScope = ioScope, + getDownloadManagerRequest = { downloadManagerRequest }, // mock request + getFileName = { _, _, _ -> "file.pdf" }, // fake file name for testing + parseUri = { mock() }, // mock Uri.parse + showToast = { }, // no-op toast for testing ) } @Test - fun `onDownloadStart creates download request with correct parameters`() = runTest(dispatcher) { + fun `onDownloadStart creates download request`() = runTest { val url = "https://example.com/file.pdf" val userAgent = "Mozilla/5.0" val contentDisposition = "attachment; filename=file.pdf" @@ -49,6 +68,7 @@ class StripeDownloadListenerTest { val contentLength = 1024L stripeDownloadListener.onDownloadStart(url, userAgent, contentDisposition, mimeType, contentLength) + testScope.testScheduler.advanceUntilIdle() verify(downloadManager).enqueue(any()) } @@ -56,17 +76,15 @@ class StripeDownloadListenerTest { @Test fun `onDownloadStart does nothing when URL is null`() = runTest { stripeDownloadListener.onDownloadStart(null, "", "", "", 0) + testScope.testScheduler.advanceUntilIdle() verifyNoInteractions(downloadManager) } @Test fun `onDownloadStart does nothing when download manager doesn't exist`() = runTest { - stripeDownloadListener = StripeDownloadListener( - context = context, - downloadManager = null, - ioScope = MainScope(), - ) + initDownloadListener(downloadManager = null) + val url = "https://example.com/file.pdf" val userAgent = "Mozilla/5.0" val contentDisposition = "attachment; filename=file.pdf" @@ -74,6 +92,7 @@ class StripeDownloadListenerTest { val contentLength = 1024L stripeDownloadListener.onDownloadStart(url, userAgent, contentDisposition, mimeType, contentLength) + testScope.testScheduler.advanceUntilIdle() verifyNoInteractions(downloadManager) } From 6f012e77fc19704a6daadc1b4155f9b14158085c Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Sun, 10 Nov 2024 15:15:14 -0500 Subject: [PATCH 08/14] Fix lint (unused import) --- .../com/stripe/android/connect/webview/StripeDownloadListener.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index 543633c9cbb..b5ce18938f1 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -12,7 +12,6 @@ import android.webkit.DownloadListener import android.webkit.URLUtil import android.widget.Toast import com.stripe.android.connect.R -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.MainScope From e667558288b76dff6fb3ff65df8e1f01207e69ce Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Sun, 10 Nov 2024 21:44:42 -0500 Subject: [PATCH 09/14] Close resource --- .../connect/webview/StripeDownloadListener.kt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index b5ce18938f1..3b319f57e16 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -64,16 +64,16 @@ internal class StripeDownloadListener( val query = DownloadManager.Query().setFilterById(downloadId) var isDownloading = true while (isDownloading) { - val cursor = downloadManager.query(query) - cursor.moveToFirst() - val index = cursor.getColumnIndex(DownloadManager.COLUMN_STATUS) - if (index < 0) break // status does not exist - abort - val status = cursor.getInt(index) - if (status !in listOf(STATUS_PENDING, STATUS_RUNNING, STATUS_PAUSED)) { - showOpenFileToast() - isDownloading = false // download complete - exit the loop + downloadManager.query(query).use { cursor -> + cursor.moveToFirst() + val index = cursor.getColumnIndex(DownloadManager.COLUMN_STATUS) + if (index < 0) return@launch // status does not exist - abort + val status = cursor.getInt(index) + if (status !in listOf(STATUS_PENDING, STATUS_RUNNING, STATUS_PAUSED)) { + showOpenFileToast() + isDownloading = false // download complete - exit the loop + } } - cursor.close() delay(DOWNLOAD_DELAY_MS) } } From bc6da3b669b462e205d7a34a8eb10946da52a4ab Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Tue, 12 Nov 2024 00:07:34 -0500 Subject: [PATCH 10/14] Refactor into interfaces --- .../connect/webview/StripeDownloadListener.kt | 55 ++++++++-------- .../connect/webview/StripeDownloadManager.kt | 62 +++++++++++++++++++ .../connect/webview/StripeToastManager.kt | 17 +++++ .../webview/StripeDownloadListenerTest.kt | 45 +++++++++----- 4 files changed, 133 insertions(+), 46 deletions(-) create mode 100644 connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt create mode 100644 connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index 3b319f57e16..0ddd1c22136 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -6,11 +6,8 @@ import android.app.DownloadManager.STATUS_PAUSED import android.app.DownloadManager.STATUS_PENDING import android.app.DownloadManager.STATUS_RUNNING import android.content.Context -import android.net.Uri import android.os.Environment.DIRECTORY_DOWNLOADS import android.webkit.DownloadListener -import android.webkit.URLUtil -import android.widget.Toast import com.stripe.android.connect.R import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -20,21 +17,10 @@ import kotlinx.coroutines.launch internal class StripeDownloadListener( private val context: Context, - private val downloadManager: DownloadManager? = - context.getSystemService(Context.DOWNLOAD_SERVICE) as? DownloadManager, + private val stripeDownloadManager: StripeDownloadManager = StripeDownloadManagerImpl(context), + private val stripeToastManager: StripeToastManager = StripeToastManagerImpl(context), private val ioScope: CoroutineScope = CoroutineScope(Dispatchers.IO), - // methods below exposed for mocking in tests - private val getDownloadManagerRequest: (Uri) -> DownloadManager.Request = { uri -> - DownloadManager.Request(uri) - }, - private val getFileName: (url: String, contentDisposition: String?, mimeType: String?) -> String = - { url, contentDisposition, mimeType -> - URLUtil.guessFileName(url, contentDisposition, mimeType) - }, - private val parseUri: (String) -> Uri = { Uri.parse(it) }, - private val showToast: (String) -> Unit = { toastString -> - Toast.makeText(context, toastString, Toast.LENGTH_LONG).show() - } + private val mainScope: CoroutineScope = MainScope(), ) : DownloadListener { override fun onDownloadStart( @@ -44,31 +30,40 @@ internal class StripeDownloadListener( mimetype: String?, contentLength: Long ) { - if (url == null || downloadManager == null) { + if (url == null) { showErrorToast() return } ioScope.launch { - val request = getDownloadManagerRequest(parseUri(url)) - val fileName = getFileName(url, contentDisposition, mimetype) + val request = stripeDownloadManager.getDownloadRequest(url) + val fileName = stripeDownloadManager.getFileName(url, contentDisposition, mimetype) request.setDestinationInExternalPublicDir(DIRECTORY_DOWNLOADS, fileName) request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) request.setTitle(fileName) request.setDescription(context.getString(R.string.stripe_downloading_file)) request.setMimeType(mimetype) - val downloadId = downloadManager.enqueue(request) + val downloadId = stripeDownloadManager.enqueue(request) + if (downloadId == null) { + showErrorToast() + return@launch + } // Monitor the download progress and show a toast when done - val query = DownloadManager.Query().setFilterById(downloadId) + val query = stripeDownloadManager.getQueryById(downloadId) var isDownloading = true while (isDownloading) { - downloadManager.query(query).use { cursor -> - cursor.moveToFirst() - val index = cursor.getColumnIndex(DownloadManager.COLUMN_STATUS) + val cursor = stripeDownloadManager.query(query) + if (cursor == null) { + showErrorToast() + return@launch + } + cursor.use { resource -> + resource.moveToFirst() + val index = resource.getColumnIndex(DownloadManager.COLUMN_STATUS) if (index < 0) return@launch // status does not exist - abort - val status = cursor.getInt(index) + val status = resource.getInt(index) if (status !in listOf(STATUS_PENDING, STATUS_RUNNING, STATUS_PAUSED)) { showOpenFileToast() isDownloading = false // download complete - exit the loop @@ -80,14 +75,14 @@ internal class StripeDownloadListener( } private fun showErrorToast() { - MainScope().launch { - showToast(context.getString(R.string.stripe_unable_to_download_file)) + mainScope.launch { + stripeToastManager.showToast(context.getString(R.string.stripe_unable_to_download_file)) } } private fun showOpenFileToast() { - MainScope().launch { - showToast(context.getString(R.string.stripe_download_complete)) + mainScope.launch { + stripeToastManager.showToast(context.getString(R.string.stripe_download_complete)) } } diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt new file mode 100644 index 00000000000..ebe50fd3b98 --- /dev/null +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt @@ -0,0 +1,62 @@ +package com.stripe.android.connect.webview + +import android.app.DownloadManager +import android.content.Context +import android.database.Cursor +import android.net.Uri +import android.webkit.URLUtil + +/** + * Provides an interface for various download and file operations. Useful for mocking in tests. + */ +interface StripeDownloadManager { + /** + * Returns the ID of the download, or null if the download could not be started. + */ + fun enqueue(request: DownloadManager.Request): Long? + + /** + * Returns a [DownloadManager.Request] for the given URI. + */ + fun getDownloadRequest(uri: String): DownloadManager.Request + + /** + * Returns a best-guess file name for the given URL, content disposition, and MIME type. + */ + fun getFileName(url: String, contentDisposition: String? = null, mimeType: String? = null): String + + /** + * Returns a [DownloadManager.Query] for the given download ID. + */ + fun getQueryById(id: Long): DownloadManager.Query + + /** + * Queries the download manager for downloads matching the given query, or null if downloads can't be queried. + */ + fun query(query: DownloadManager.Query): Cursor? +} + +class StripeDownloadManagerImpl(context: Context) : StripeDownloadManager { + private val downloadManager: DownloadManager? = + context.getSystemService(Context.DOWNLOAD_SERVICE) as? DownloadManager + + override fun enqueue(request: DownloadManager.Request): Long? { + return downloadManager?.enqueue(request) + } + + override fun getDownloadRequest(uri: String): DownloadManager.Request { + return DownloadManager.Request(Uri.parse(uri)) + } + + override fun getFileName(url: String, contentDisposition: String?, mimeType: String?): String { + return URLUtil.guessFileName(url, contentDisposition, mimeType) + } + + override fun getQueryById(id: Long): DownloadManager.Query { + return DownloadManager.Query().setFilterById(id) + } + + override fun query(query: DownloadManager.Query): Cursor? { + return downloadManager?.query(query) + } +} diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt new file mode 100644 index 00000000000..5e71b91f627 --- /dev/null +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt @@ -0,0 +1,17 @@ +package com.stripe.android.connect.webview + +import android.content.Context +import android.widget.Toast + +/** + * Provides an interface for various download and file operations. Useful for mocking in tests. + */ +interface StripeToastManager { + fun showToast(toastString: String) +} + +class StripeToastManagerImpl(private val context: Context) : StripeToastManager { + override fun showToast(toastString: String) { + Toast.makeText(context, toastString, Toast.LENGTH_LONG).show() + } +} diff --git a/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt b/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt index 01f0c6183a4..7cf1d338f8f 100644 --- a/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt +++ b/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt @@ -2,6 +2,7 @@ package com.stripe.android.connect.webview import android.app.DownloadManager import android.content.Context +import android.database.Cursor import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.test.TestScope @@ -15,14 +16,11 @@ import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.whenever class StripeDownloadListenerTest { - private val downloadManager: DownloadManager = mock { - on { enqueue(any()) } doReturn 123L - } private val downloadManagerRequest: DownloadManager.Request = mock { - // mock all builder methods to return this mock, for proper chaining on { setDestinationInExternalPublicDir(any(), any()) } doReturn mock on { setNotificationVisibility(any()) } doReturn mock on { setTitle(any()) } doReturn mock @@ -30,9 +28,14 @@ class StripeDownloadListenerTest { on { setMimeType(any()) } doReturn mock } private val context: Context = mock { - on { getSystemService(Context.DOWNLOAD_SERVICE) } doReturn downloadManager on { getString(any()) } doReturn "Placeholder string" } + private val stripeDownloadManager: StripeDownloadManager = mock { + on { getDownloadRequest(any()) } doReturn downloadManagerRequest + on { getFileName(any(), any(), any()) } doReturn "file.pdf" + on { enqueue(any()) } doReturn 123L + } + private val stripeToastManager: StripeToastManager = mock() private val testScope = TestScope() private lateinit var stripeDownloadListener: StripeDownloadListener @@ -45,17 +48,17 @@ class StripeDownloadListenerTest { private fun initDownloadListener( context: Context = this.context, - downloadManager: DownloadManager? = this.downloadManager, + stripeDownloadManager: StripeDownloadManager = this.stripeDownloadManager, + stripeToastManager: StripeToastManager = this.stripeToastManager, ioScope: CoroutineScope = testScope, + mainScope: CoroutineScope = testScope, ) { stripeDownloadListener = StripeDownloadListener( context = context, - downloadManager = downloadManager, + stripeDownloadManager = stripeDownloadManager, + stripeToastManager = stripeToastManager, ioScope = ioScope, - getDownloadManagerRequest = { downloadManagerRequest }, // mock request - getFileName = { _, _, _ -> "file.pdf" }, // fake file name for testing - parseUri = { mock() }, // mock Uri.parse - showToast = { }, // no-op toast for testing + mainScope = mainScope, ) } @@ -67,10 +70,19 @@ class StripeDownloadListenerTest { val mimeType = "application/pdf" val contentLength = 1024L + val cursor: Cursor = mock { + on { moveToFirst() } doReturn true + on { getColumnIndex(any()) } doReturn 0 + on { getInt(any()) } doReturn DownloadManager.STATUS_SUCCESSFUL + } + whenever(stripeDownloadManager.getQueryById(any())).thenReturn(mock()) + whenever(stripeDownloadManager.query(any())).thenReturn(cursor) + stripeDownloadListener.onDownloadStart(url, userAgent, contentDisposition, mimeType, contentLength) testScope.testScheduler.advanceUntilIdle() - verify(downloadManager).enqueue(any()) + verify(stripeDownloadManager).enqueue(any()) + verify(stripeToastManager).showToast(any()) } @Test @@ -78,12 +90,13 @@ class StripeDownloadListenerTest { stripeDownloadListener.onDownloadStart(null, "", "", "", 0) testScope.testScheduler.advanceUntilIdle() - verifyNoInteractions(downloadManager) + verifyNoInteractions(stripeDownloadManager) + verify(stripeToastManager).showToast(any()) } @Test - fun `onDownloadStart does nothing when download manager doesn't exist`() = runTest { - initDownloadListener(downloadManager = null) + fun `onDownloadStart shows error toast when enqueue returns null`() = runTest { + whenever(stripeDownloadManager.enqueue(any())).thenReturn(null) val url = "https://example.com/file.pdf" val userAgent = "Mozilla/5.0" @@ -94,6 +107,6 @@ class StripeDownloadListenerTest { stripeDownloadListener.onDownloadStart(url, userAgent, contentDisposition, mimeType, contentLength) testScope.testScheduler.advanceUntilIdle() - verifyNoInteractions(downloadManager) + verify(stripeToastManager).showToast(any()) } } From 8b79aa85941661e5d76838617cc9e0f94f942a89 Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Tue, 12 Nov 2024 10:35:02 -0500 Subject: [PATCH 11/14] Make interfaces internal --- .../stripe/android/connect/webview/StripeDownloadManager.kt | 4 ++-- .../com/stripe/android/connect/webview/StripeToastManager.kt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt index ebe50fd3b98..f1c3e33e142 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt @@ -9,7 +9,7 @@ import android.webkit.URLUtil /** * Provides an interface for various download and file operations. Useful for mocking in tests. */ -interface StripeDownloadManager { +internal interface StripeDownloadManager { /** * Returns the ID of the download, or null if the download could not be started. */ @@ -36,7 +36,7 @@ interface StripeDownloadManager { fun query(query: DownloadManager.Query): Cursor? } -class StripeDownloadManagerImpl(context: Context) : StripeDownloadManager { +internal class StripeDownloadManagerImpl(context: Context) : StripeDownloadManager { private val downloadManager: DownloadManager? = context.getSystemService(Context.DOWNLOAD_SERVICE) as? DownloadManager diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt index 5e71b91f627..ddbd8154340 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt @@ -6,11 +6,11 @@ import android.widget.Toast /** * Provides an interface for various download and file operations. Useful for mocking in tests. */ -interface StripeToastManager { +internal interface StripeToastManager { fun showToast(toastString: String) } -class StripeToastManagerImpl(private val context: Context) : StripeToastManager { +internal class StripeToastManagerImpl(private val context: Context) : StripeToastManager { override fun showToast(toastString: String) { Toast.makeText(context, toastString, Toast.LENGTH_LONG).show() } From 77a715759463b8389be9a92a353db370f8ad198f Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Tue, 12 Nov 2024 19:10:36 -0500 Subject: [PATCH 12/14] Move scope into toast manager --- .../connect/webview/StripeDownloadListener.kt | 9 ++------- .../android/connect/webview/StripeToastManager.kt | 12 ++++++++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index 0ddd1c22136..4fb00b21699 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -20,7 +20,6 @@ internal class StripeDownloadListener( private val stripeDownloadManager: StripeDownloadManager = StripeDownloadManagerImpl(context), private val stripeToastManager: StripeToastManager = StripeToastManagerImpl(context), private val ioScope: CoroutineScope = CoroutineScope(Dispatchers.IO), - private val mainScope: CoroutineScope = MainScope(), ) : DownloadListener { override fun onDownloadStart( @@ -75,15 +74,11 @@ internal class StripeDownloadListener( } private fun showErrorToast() { - mainScope.launch { - stripeToastManager.showToast(context.getString(R.string.stripe_unable_to_download_file)) - } + stripeToastManager.showToast(context.getString(R.string.stripe_unable_to_download_file)) } private fun showOpenFileToast() { - mainScope.launch { - stripeToastManager.showToast(context.getString(R.string.stripe_download_complete)) - } + stripeToastManager.showToast(context.getString(R.string.stripe_download_complete)) } internal companion object { diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt index ddbd8154340..e690729b867 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeToastManager.kt @@ -2,6 +2,9 @@ package com.stripe.android.connect.webview import android.content.Context import android.widget.Toast +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.launch /** * Provides an interface for various download and file operations. Useful for mocking in tests. @@ -10,8 +13,13 @@ internal interface StripeToastManager { fun showToast(toastString: String) } -internal class StripeToastManagerImpl(private val context: Context) : StripeToastManager { +internal class StripeToastManagerImpl( + private val context: Context, + private val scope: CoroutineScope = MainScope() +) : StripeToastManager { override fun showToast(toastString: String) { - Toast.makeText(context, toastString, Toast.LENGTH_LONG).show() + scope.launch { + Toast.makeText(context, toastString, Toast.LENGTH_LONG).show() + } } } From 97107aa1ca829b07278fd082e225144dbb6cd657 Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Tue, 12 Nov 2024 19:25:29 -0500 Subject: [PATCH 13/14] Better abstractions --- .../connect/webview/StripeDownloadListener.kt | 31 ++-------- .../connect/webview/StripeDownloadManager.kt | 59 +++++++++++-------- 2 files changed, 39 insertions(+), 51 deletions(-) diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt index 4fb00b21699..0ec0a03f041 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadListener.kt @@ -1,17 +1,13 @@ package com.stripe.android.connect.webview -import android.app.DownloadManager -import android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED import android.app.DownloadManager.STATUS_PAUSED import android.app.DownloadManager.STATUS_PENDING import android.app.DownloadManager.STATUS_RUNNING import android.content.Context -import android.os.Environment.DIRECTORY_DOWNLOADS import android.webkit.DownloadListener import com.stripe.android.connect.R import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.MainScope import kotlinx.coroutines.delay import kotlinx.coroutines.launch @@ -35,15 +31,7 @@ internal class StripeDownloadListener( } ioScope.launch { - val request = stripeDownloadManager.getDownloadRequest(url) - val fileName = stripeDownloadManager.getFileName(url, contentDisposition, mimetype) - request.setDestinationInExternalPublicDir(DIRECTORY_DOWNLOADS, fileName) - request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) - request.setTitle(fileName) - request.setDescription(context.getString(R.string.stripe_downloading_file)) - request.setMimeType(mimetype) - - val downloadId = stripeDownloadManager.enqueue(request) + val downloadId = stripeDownloadManager.enqueueDownload(url, contentDisposition, mimetype) if (downloadId == null) { showErrorToast() return@launch @@ -53,20 +41,13 @@ internal class StripeDownloadListener( val query = stripeDownloadManager.getQueryById(downloadId) var isDownloading = true while (isDownloading) { - val cursor = stripeDownloadManager.query(query) - if (cursor == null) { + val status = stripeDownloadManager.getDownloadStatus(query) + if (status == null) { showErrorToast() return@launch - } - cursor.use { resource -> - resource.moveToFirst() - val index = resource.getColumnIndex(DownloadManager.COLUMN_STATUS) - if (index < 0) return@launch // status does not exist - abort - val status = resource.getInt(index) - if (status !in listOf(STATUS_PENDING, STATUS_RUNNING, STATUS_PAUSED)) { - showOpenFileToast() - isDownloading = false // download complete - exit the loop - } + } else if (status !in listOf(STATUS_PENDING, STATUS_RUNNING, STATUS_PAUSED)) { + showOpenFileToast() + isDownloading = false // download complete - exit the loop } delay(DOWNLOAD_DELAY_MS) } diff --git a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt index f1c3e33e142..02ea08eb77f 100644 --- a/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt +++ b/connect/src/main/java/com/stripe/android/connect/webview/StripeDownloadManager.kt @@ -1,62 +1,69 @@ package com.stripe.android.connect.webview import android.app.DownloadManager +import android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED import android.content.Context -import android.database.Cursor import android.net.Uri +import android.os.Environment.DIRECTORY_DOWNLOADS import android.webkit.URLUtil +import com.stripe.android.connect.R /** * Provides an interface for various download and file operations. Useful for mocking in tests. */ internal interface StripeDownloadManager { - /** - * Returns the ID of the download, or null if the download could not be started. - */ - fun enqueue(request: DownloadManager.Request): Long? - - /** - * Returns a [DownloadManager.Request] for the given URI. - */ - fun getDownloadRequest(uri: String): DownloadManager.Request /** - * Returns a best-guess file name for the given URL, content disposition, and MIME type. + * Enqueues a download for the given URL, content disposition, and MIME type. + * Returns null if a download could not be started. */ - fun getFileName(url: String, contentDisposition: String? = null, mimeType: String? = null): String + fun enqueueDownload(url: String, contentDisposition: String? = null, mimeType: String? = null): Long? /** - * Returns a [DownloadManager.Query] for the given download ID. + * Returns a [DownloadManager.Query] for the given download ID, used for monitoring the status of a download. */ fun getQueryById(id: Long): DownloadManager.Query /** - * Queries the download manager for downloads matching the given query, or null if downloads can't be queried. + * Returns the status of the download represented by [query]. Maps to [DownloadManager.COLUMN_STATUS], ie. + * [DownloadManager.STATUS_PENDING], [DownloadManager.STATUS_RUNNING], [DownloadManager.STATUS_PAUSED], etc. + * + * Returns null if the status could not be determined. This operation should not be retried if null is returned. */ - fun query(query: DownloadManager.Query): Cursor? + fun getDownloadStatus(query: DownloadManager.Query): Int? } -internal class StripeDownloadManagerImpl(context: Context) : StripeDownloadManager { +internal class StripeDownloadManagerImpl(private val context: Context) : StripeDownloadManager { private val downloadManager: DownloadManager? = context.getSystemService(Context.DOWNLOAD_SERVICE) as? DownloadManager - override fun enqueue(request: DownloadManager.Request): Long? { - return downloadManager?.enqueue(request) - } + override fun enqueueDownload(url: String, contentDisposition: String?, mimeType: String?): Long? { + downloadManager ?: return null - override fun getDownloadRequest(uri: String): DownloadManager.Request { - return DownloadManager.Request(Uri.parse(uri)) - } + val request = DownloadManager.Request(Uri.parse(url)) + val fileName = URLUtil.guessFileName(url, contentDisposition, mimeType) + request.setDestinationInExternalPublicDir(DIRECTORY_DOWNLOADS, fileName) + request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + request.setTitle(fileName) + request.setDescription(context.getString(R.string.stripe_downloading_file)) + request.setMimeType(mimeType) - override fun getFileName(url: String, contentDisposition: String?, mimeType: String?): String { - return URLUtil.guessFileName(url, contentDisposition, mimeType) + return downloadManager.enqueue(request) } override fun getQueryById(id: Long): DownloadManager.Query { return DownloadManager.Query().setFilterById(id) } - override fun query(query: DownloadManager.Query): Cursor? { - return downloadManager?.query(query) + override fun getDownloadStatus(query: DownloadManager.Query): Int? { + downloadManager ?: return null + + val cursor = downloadManager.query(query) ?: return null + return cursor.use { resource -> + resource.moveToFirst() + val index = resource.getColumnIndex(DownloadManager.COLUMN_STATUS) + if (index < 0) return null // status does not exist - abort + resource.getInt(index) + } } } From 59eea10e8571c0640046d0af6ebd8aba506621d0 Mon Sep 17 00:00:00 2001 From: Simon Duchastel Date: Tue, 12 Nov 2024 20:00:31 -0500 Subject: [PATCH 14/14] Fix test --- .../webview/StripeDownloadListenerTest.kt | 29 ++++--------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt b/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt index 7cf1d338f8f..c88572550a2 100644 --- a/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt +++ b/connect/src/test/java/com/stripe/android/connect/webview/StripeDownloadListenerTest.kt @@ -2,7 +2,6 @@ package com.stripe.android.connect.webview import android.app.DownloadManager import android.content.Context -import android.database.Cursor import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.test.TestScope @@ -12,6 +11,7 @@ import kotlinx.coroutines.test.setMain import org.junit.Before import org.junit.Test import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import org.mockito.kotlin.verify @@ -20,20 +20,13 @@ import org.mockito.kotlin.whenever class StripeDownloadListenerTest { - private val downloadManagerRequest: DownloadManager.Request = mock { - on { setDestinationInExternalPublicDir(any(), any()) } doReturn mock - on { setNotificationVisibility(any()) } doReturn mock - on { setTitle(any()) } doReturn mock - on { setDescription(any()) } doReturn mock - on { setMimeType(any()) } doReturn mock - } private val context: Context = mock { on { getString(any()) } doReturn "Placeholder string" } private val stripeDownloadManager: StripeDownloadManager = mock { - on { getDownloadRequest(any()) } doReturn downloadManagerRequest - on { getFileName(any(), any(), any()) } doReturn "file.pdf" - on { enqueue(any()) } doReturn 123L + on { enqueueDownload(any(), anyOrNull(), anyOrNull()) } doReturn 123L + on { getQueryById(any()) } doReturn mock() + on { getDownloadStatus(any()) } doReturn DownloadManager.STATUS_SUCCESSFUL } private val stripeToastManager: StripeToastManager = mock() private val testScope = TestScope() @@ -51,14 +44,12 @@ class StripeDownloadListenerTest { stripeDownloadManager: StripeDownloadManager = this.stripeDownloadManager, stripeToastManager: StripeToastManager = this.stripeToastManager, ioScope: CoroutineScope = testScope, - mainScope: CoroutineScope = testScope, ) { stripeDownloadListener = StripeDownloadListener( context = context, stripeDownloadManager = stripeDownloadManager, stripeToastManager = stripeToastManager, ioScope = ioScope, - mainScope = mainScope, ) } @@ -70,18 +61,10 @@ class StripeDownloadListenerTest { val mimeType = "application/pdf" val contentLength = 1024L - val cursor: Cursor = mock { - on { moveToFirst() } doReturn true - on { getColumnIndex(any()) } doReturn 0 - on { getInt(any()) } doReturn DownloadManager.STATUS_SUCCESSFUL - } - whenever(stripeDownloadManager.getQueryById(any())).thenReturn(mock()) - whenever(stripeDownloadManager.query(any())).thenReturn(cursor) - stripeDownloadListener.onDownloadStart(url, userAgent, contentDisposition, mimeType, contentLength) testScope.testScheduler.advanceUntilIdle() - verify(stripeDownloadManager).enqueue(any()) + verify(stripeDownloadManager).enqueueDownload(url, contentDisposition, mimeType) verify(stripeToastManager).showToast(any()) } @@ -96,7 +79,7 @@ class StripeDownloadListenerTest { @Test fun `onDownloadStart shows error toast when enqueue returns null`() = runTest { - whenever(stripeDownloadManager.enqueue(any())).thenReturn(null) + whenever(stripeDownloadManager.enqueueDownload(any(), anyOrNull(), anyOrNull())).thenReturn(null) val url = "https://example.com/file.pdf" val userAgent = "Mozilla/5.0"