commit bda8be382de60dfe232fe7f649c722f9a0643ed4 parent 191be0b960022b24e3e187332a13b3d2c3d9511a Author: Titouan Thibaud <tthibaud@mozilla.com> Date: Thu, 16 Oct 2025 16:15:30 +0000 Bug 1957987 - When user tries to download an existing file, ask if they want to open the existing file or download it again. r=android-reviewers,android-l10n-reviewers,delphine,giorga Differential Revision: https://phabricator.services.mozilla.com/D268380 Diffstat:
14 files changed, 680 insertions(+), 65 deletions(-)
diff --git a/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/EngineObserver.kt b/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/EngineObserver.kt @@ -37,6 +37,7 @@ import mozilla.components.concept.engine.translate.TranslationEngineState import mozilla.components.concept.engine.translate.TranslationError import mozilla.components.concept.engine.translate.TranslationOperation import mozilla.components.concept.engine.window.WindowRequest +import mozilla.components.concept.fetch.Headers.Names.E_TAG import mozilla.components.concept.fetch.Response import mozilla.components.lib.state.Store @@ -241,6 +242,7 @@ internal class EngineObserver( skipConfirmation = skipConfirmation, openInApp = openInApp, response = response, + etag = response?.headers?.get(E_TAG), ) store.dispatch( diff --git a/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/EngineObserverTest.kt b/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/EngineObserverTest.kt @@ -42,6 +42,9 @@ import mozilla.components.concept.engine.translate.TranslationError import mozilla.components.concept.engine.translate.TranslationOperation import mozilla.components.concept.engine.translate.TranslationOptions import mozilla.components.concept.engine.window.WindowRequest +import mozilla.components.concept.fetch.Header +import mozilla.components.concept.fetch.Headers.Names.E_TAG +import mozilla.components.concept.fetch.MutableHeaders import mozilla.components.concept.fetch.Response import mozilla.components.support.test.libstate.ext.waitUntilIdle import mozilla.components.support.test.middleware.CaptureActionsMiddleware @@ -1578,7 +1581,9 @@ class EngineObserverTest { @Test fun `onExternalResource will update the store`() { - val response = mock<Response>() + val response = mock<Response> { + `when`(headers).thenReturn(MutableHeaders(listOf(Header(E_TAG, "12345")))) + } val store = BrowserStore( initialState = BrowserState( @@ -1614,6 +1619,7 @@ class EngineObserverTest { assertEquals("file.txt", tab.content.download?.fileName) assertEquals("userAgent", tab.content.download?.userAgent) assertEquals("text/plain", tab.content.download?.contentType) + assertEquals("12345", tab.content.download?.etag) assertEquals(100L, tab.content.download?.contentLength) assertEquals(true, tab.content.download?.private) assertEquals(response, tab.content.download?.response) diff --git a/mobile/android/android-components/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt b/mobile/android/android-components/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt @@ -51,6 +51,13 @@ import mozilla.components.support.utils.Browsers value class Filename(val value: String) /** + * The name of a file that was already downloaded with the same ETag. + * The value will be `null` if no such file exists. + */ +@JvmInline +value class FileNameOfDuplicateIfAlreadyDownloaded(val value: String?) + +/** * The size of the file to be downloaded expressed as the number of `bytes`. * The value will be `0` if the size is unknown. */ @@ -82,6 +89,12 @@ value class PositiveActionCallback(val value: () -> Unit) value class NegativeActionCallback(val value: () -> Unit) /** + * Callback for when the open file button was tapped. + */ +@JvmInline +value class OpenFileCallback(val value: () -> Unit) + +/** * Feature implementation to provide download functionality for the selected * session. The feature will subscribe to the selected session and listen * for downloads. @@ -124,7 +137,14 @@ class DownloadsFeature( private val onDownloadStartedListener: ((String) -> Unit) = {}, private val shouldForwardToThirdParties: () -> Boolean = { false }, private val customFirstPartyDownloadDialog: ( - (Filename, ContentSize, PositiveActionCallback, NegativeActionCallback) -> Unit + ( + Filename, + ContentSize, + FileNameOfDuplicateIfAlreadyDownloaded, + PositiveActionCallback, + NegativeActionCallback, + OpenFileCallback, + ) -> Unit )? = null, private val customThirdPartyDownloadDialog: ( (ThirdPartyDownloaderApps, ThirdPartyDownloaderAppChosenCallback, NegativeActionCallback) -> Unit @@ -235,9 +255,11 @@ class DownloadsFeature( if (applicationContext.isPermissionGranted(downloadManager.permissions.asIterable())) { when { customFirstPartyDownloadDialog != null && !download.skipConfirmation -> { + val downloadWithSameEtag = findDownloadWithSameEtag(download) customFirstPartyDownloadDialog.invoke( Filename(download.realFilenameOrGuessed), ContentSize(download.contentLength ?: 0), + FileNameOfDuplicateIfAlreadyDownloaded(downloadWithSameEtag?.fileName), PositiveActionCallback { startDownload(download) useCases.consumeDownload.invoke(tab.id, download.id) @@ -245,6 +267,13 @@ class DownloadsFeature( NegativeActionCallback { useCases.cancelDownloadRequest.invoke(tab.id, download.id) }, + OpenFileCallback { + useCases.openAlreadyDownloadedFile.invoke( + tab.id, + download, + downloadWithSameEtag?.filePath, + ) + }, ) false } @@ -267,6 +296,19 @@ class DownloadsFeature( } @VisibleForTesting + internal fun findDownloadWithSameEtag(download: DownloadState): DownloadState? = + store.state + .downloads + .values + .filter { + it.url == download.url && + it.status == DownloadState.Status.COMPLETED && + it.etag == download.etag && + fileSystemHelper.fileExists(it.filePath) + } + .minByOrNull { it.createdTime } + + @VisibleForTesting internal fun startDownload(download: DownloadState): Boolean { fileSystemHelper.createDirectoryIfNotExists(download.directoryPath) diff --git a/mobile/android/android-components/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsUseCases.kt b/mobile/android/android-components/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsUseCases.kt @@ -4,8 +4,10 @@ package mozilla.components.feature.downloads +import android.content.Context import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.DownloadAction +import mozilla.components.browser.state.state.content.DownloadState import mozilla.components.browser.state.store.BrowserStore /** @@ -15,6 +17,7 @@ import mozilla.components.browser.state.store.BrowserStore */ class DownloadsUseCases( store: BrowserStore, + applicationContext: Context, ) { /** @@ -31,6 +34,33 @@ class DownloadsUseCases( } } + /** + * Use case that opens an already downloaded file. + * + * @property store + * @property applicationContext + */ + class OpenAlreadyDownloadedFileUseCase( + private val store: BrowserStore, + private val applicationContext: Context, + ) { + /** + * Opens the already downloaded file with the given [downloadId], and cancels the download + * request in the session with the given [tabId]. + */ + operator fun invoke(tabId: String, download: DownloadState, filePath: String?) { + store.dispatch(ContentAction.CancelDownloadAction(tabId, download.id)) + filePath ?: return + AbstractFetchDownloadService.openFile( + applicationContext, + applicationContext.packageName, + download.fileName, + filePath, + download.contentType, + ) + } + } + class ConsumeDownloadUseCase( private val store: BrowserStore, ) { @@ -85,6 +115,7 @@ class DownloadsUseCases( } val cancelDownloadRequest = CancelDownloadRequestUseCase(store) + val openAlreadyDownloadedFile = OpenAlreadyDownloadedFileUseCase(store, applicationContext) val consumeDownload = ConsumeDownloadUseCase(store) val restoreDownloads = RestoreDownloadsUseCase(store) val removeDownload = RemoveDownloadUseCase(store) diff --git a/mobile/android/android-components/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/FileSystemHelper.kt b/mobile/android/android-components/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/FileSystemHelper.kt @@ -36,6 +36,15 @@ interface FileSystemHelper { * @return The number of available bytes, or 0 if the path does not exist or is not a directory. */ fun availableBytesInDirectory(path: String): Long + + /** + * Checks if the given file path exists. + * + * @param filePath the path of the file to check + * @return <code>true</code> if and only if the file denoted by this abstract pathname exists; + * <code>false</code> otherwise + */ + fun fileExists(filePath: String): Boolean } /** @@ -79,4 +88,14 @@ class DefaultFileSystemHelper : FileSystemHelper { val file = File(path) return file.exists() && file.isDirectory } + + /** + * Checks if the given file path exists. + * + * @param filePath the path of the file to check + * @return <code>true</code> if and only if the file denoted by this abstract pathname exists; + * <code>false</code> otherwise + */ + override fun fileExists(filePath: String) = + File(filePath).exists() } diff --git a/mobile/android/android-components/components/feature/downloads/src/main/res/values/strings.xml b/mobile/android/android-components/components/feature/downloads/src/main/res/values/strings.xml @@ -19,21 +19,25 @@ <!-- Alert dialog confirmation before downloading a previously downloaded file, this is the title. %1$s will be replaced with the size of the file. --> - <string name="mozac_feature_downloads_again_dialog_title" tools:ignore="UnusedResources">Download file again? (%1$s)</string> + <string name="mozac_feature_downloads_again_dialog_title">Download file again? (%1$s)</string> <!-- Alert dialog confirmation before downloading a file, this is the title. %1$s will be replaced with the size of the file. --> <string name="mozac_feature_downloads_dialog_title_3">Download file? (%1$s)</string> <!-- This string is used as the title for the download confirmation dialog when the size of the file is not known. --> <string name="mozac_feature_downloads_dialog_title_with_unknown_size">Download file?</string> + <!-- Alert dialog confirmation before downloading a previously downloaded file, this is the title when the size of the file is not known. --> + <string name="mozac_feature_downloads_again_dialog_title_with_unknown_size">Download file again?</string> <!-- Alert dialog confirmation before download a file, this is the positive action. --> <string name="mozac_feature_downloads_dialog_download">Download</string> <!-- Alert dialog confirmation before downloading a file again, this is the positive action. --> - <string name="mozac_feature_downloads_dialog_download_again" tools:ignore="UnusedResources">Download again</string> + <string name="mozac_feature_downloads_dialog_download_again">Download again</string> <!-- Alert dialog confirmation before download a file, this is the negative action. --> <string name="mozac_feature_downloads_dialog_cancel">Cancel</string> <!-- Alert dialog confirmation before downloading a previously downloaded file, this is the description. %1$s will be replaced with the name of the file. --> - <string name="mozac_feature_downloads_already_exists_dialog_title" tools:ignore="UnusedResources">%1$s already exists.</string> + <string name="mozac_feature_downloads_already_exists_dialog_title">%1$s already exists.</string> + <!-- Alert dialog confirmation before downloading a previously downloaded file, this is the neutral action that proposes to open the file instead. --> + <string name="mozac_feature_downloads_open_existing_file">Open existing file</string> <!-- Alert dialog confirmation before downloading a file without network connection, this is the description. %1$s will be replaced with the name of the file. --> <string name="mozac_feature_downloads_file_failure_no_connection" tools:ignore="UnusedResources">%1$s wasn’t downloaded.</string> diff --git a/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadUseCasesTest.kt b/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadUseCasesTest.kt @@ -16,7 +16,7 @@ class DownloadUseCasesTest { @Test fun consumeDownloadUseCase() { val store: BrowserStore = mock() - val useCases = DownloadsUseCases(store) + val useCases = DownloadsUseCases(store, mock()) useCases.consumeDownload("tabId", "downloadId") verify(store).dispatch(ContentAction.ConsumeDownloadAction("tabId", "downloadId")) @@ -25,7 +25,7 @@ class DownloadUseCasesTest { @Test fun restoreDownloadsUseCase() { val store: BrowserStore = mock() - val useCases = DownloadsUseCases(store) + val useCases = DownloadsUseCases(store, mock()) useCases.restoreDownloads() verify(store).dispatch(DownloadAction.RestoreDownloadsStateAction) @@ -34,7 +34,7 @@ class DownloadUseCasesTest { @Test fun removeDownloadUseCase() { val store: BrowserStore = mock() - val useCases = DownloadsUseCases(store) + val useCases = DownloadsUseCases(store, mock()) useCases.removeDownload("downloadId") verify(store).dispatch(DownloadAction.RemoveDownloadAction("downloadId")) @@ -43,7 +43,7 @@ class DownloadUseCasesTest { @Test fun removeAllDownloadsUseCase() { val store: BrowserStore = mock() - val useCases = DownloadsUseCases(store) + val useCases = DownloadsUseCases(store, mock()) useCases.removeAllDownloads() verify(store).dispatch(DownloadAction.RemoveAllDownloadsAction) diff --git a/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt b/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt @@ -57,7 +57,6 @@ import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.times import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` import org.robolectric.annotation.Config import org.robolectric.shadows.ShadowToast @@ -170,7 +169,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - useCases = DownloadsUseCases(store), + useCases = DownloadsUseCases(store, mock()), downloadManager = downloadManager, ), ) @@ -206,7 +205,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - useCases = DownloadsUseCases(store), + useCases = DownloadsUseCases(store, mock()), fragmentManager = fragmentManager, downloadManager = downloadManager, ), @@ -273,7 +272,7 @@ class DownloadsFeatureTest { @Test fun `WHEN dismissing a download dialog THEN the download stream should be closed`() { - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val closeDownloadResponseUseCase = mock<CancelDownloadRequestUseCase>() val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") val dialogFragment = spy(object : DownloadDialogFragment() {}) @@ -304,7 +303,7 @@ class DownloadsFeatureTest { @Test fun `onPermissionsResult will start download if permissions were granted and thirdParty enabled`() { - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val consumeDownloadUseCase = mock<ConsumeDownloadUseCase>() val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") val downloadManager: DownloadManager = mock() @@ -340,7 +339,7 @@ class DownloadsFeatureTest { @Test fun `onPermissionsResult will process download if permissions were granted and thirdParty disabled`() { - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val consumeDownloadUseCase = mock<ConsumeDownloadUseCase>() val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") val downloadManager: DownloadManager = mock() @@ -383,7 +382,7 @@ class DownloadsFeatureTest { selectedTabId = "test-tab", ), ) - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) doReturn(closeDownloadResponseUseCase).`when`(downloadsUseCases).cancelDownloadRequest @@ -457,7 +456,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - useCases = DownloadsUseCases(store), + useCases = DownloadsUseCases(store, mock()), downloadManager = downloadManager, ), ) @@ -588,7 +587,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - DownloadsUseCases(store), + DownloadsUseCases(store, mock()), downloadManager = downloadManager, shouldForwardToThirdParties = { false }, ), @@ -617,7 +616,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - DownloadsUseCases(store), + DownloadsUseCases(store, mock()), downloadManager = downloadManager, shouldForwardToThirdParties = { true }, ), @@ -647,7 +646,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - DownloadsUseCases(store), + DownloadsUseCases(store, mock()), downloadManager = downloadManager, shouldForwardToThirdParties = { true }, ), @@ -681,11 +680,11 @@ class DownloadsFeatureTest { val feature = spy( DownloadsFeature( applicationContext = testContext, - store = mock(), + store = store, useCases = usecases, downloadManager = downloadManager, shouldForwardToThirdParties = { true }, - customFirstPartyDownloadDialog = { filename, contentSize, positiveActionCallback, negativeActionCallback -> + customFirstPartyDownloadDialog = { filename, contentSize, _, positiveActionCallback, negativeActionCallback, _ -> delegateFilename = filename.value delegateContentSize = contentSize.value delegatePositiveActionCallback = positiveActionCallback.value @@ -709,6 +708,130 @@ class DownloadsFeatureTest { } @Test + fun `GIVEN file with same etag was already downloaded WHEN processing download request THEN the existing file name should be provided to the download dialog`() { + val tab = createTab("https://www.mozilla.org", id = "test-tab") + val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab", id = "test", etag = "12345") + val usecases: DownloadsUseCases = mock() + val consumeDownloadUseCase: ConsumeDownloadUseCase = mock() + val cancelDownloadUseCase: CancelDownloadRequestUseCase = mock() + val openAlreadyDownloadedFileUseCase: DownloadsUseCases.OpenAlreadyDownloadedFileUseCase = mock() + doReturn(consumeDownloadUseCase).`when`(usecases).consumeDownload + doReturn(cancelDownloadUseCase).`when`(usecases).cancelDownloadRequest + doReturn(openAlreadyDownloadedFileUseCase).`when`(usecases).openAlreadyDownloadedFile + val downloadManager: DownloadManager = mock() + var delegateFilename = "" + var delegateContentSize: Long = -1 + var delegateFileNameIsAlreadyDownloaded: String? = null + var delegatePositiveActionCallback: (() -> Unit)? = null + var delegateNegativeActionCallback: (() -> Unit)? = null + var delegateOpenFileCallback: (() -> Unit)? = null + grantPermissions() + doReturn(arrayOf(INTERNET, WRITE_EXTERNAL_STORAGE)).`when`(downloadManager).permissions + + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = mapOf( + "test" to DownloadState( + fileName = "original.txt", + url = "https://www.mozilla.org/file.txt", + directoryPath = "/downloads", + etag = "12345", + status = DownloadState.Status.COMPLETED, + ), + ), + ), + ) + + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = usecases, + downloadManager = downloadManager, + shouldForwardToThirdParties = { true }, + customFirstPartyDownloadDialog = { filename, contentSize, fileNameIfAlreadyDownloaded, positiveActionCallback, negativeActionCallback, openFileAction -> + delegateFilename = filename.value + delegateContentSize = contentSize.value + delegatePositiveActionCallback = positiveActionCallback.value + delegateNegativeActionCallback = negativeActionCallback.value + delegateOpenFileCallback = openFileAction.value + delegateFileNameIsAlreadyDownloaded = fileNameIfAlreadyDownloaded.value + }, + fileSystemHelper = FakeFileSystemHelper(existingFiles = listOf("/downloads/original.txt")), + ), + ) + + doReturn(false).`when`(feature).isDownloadBiggerThanAvailableSpace(download) + + feature.processDownload(tab, download) + + assertEquals("file.txt", delegateFilename) + assertEquals(0, delegateContentSize) + assertEquals("original.txt", delegateFileNameIsAlreadyDownloaded) + assertNotNull(delegatePositiveActionCallback) + delegatePositiveActionCallback?.invoke() + verify(consumeDownloadUseCase).invoke(tab.id, download.id) + assertNotNull(delegateNegativeActionCallback) + delegateNegativeActionCallback?.invoke() + verify(cancelDownloadUseCase).invoke(tab.id, download.id) + assertNotNull(delegateOpenFileCallback) + delegateOpenFileCallback?.invoke() + verify(openAlreadyDownloadedFileUseCase).invoke(eq(tab.id), eq(download), eq("/downloads/original.txt")) + } + + @Test + fun `GIVEN file to be downloaded for the first time WHEN processing download request THEN the download dialog should be triggered with null existing file`() { + val tab = createTab("https://www.mozilla.org", id = "test-tab") + val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab", id = "test", etag = "12345") + val usecases: DownloadsUseCases = mock() + val consumeDownloadUseCase: ConsumeDownloadUseCase = mock() + val cancelDownloadUseCase: CancelDownloadRequestUseCase = mock() + val openAlreadyDownloadedFileUseCase: DownloadsUseCases.OpenAlreadyDownloadedFileUseCase = mock() + doReturn(consumeDownloadUseCase).`when`(usecases).consumeDownload + doReturn(cancelDownloadUseCase).`when`(usecases).cancelDownloadRequest + doReturn(openAlreadyDownloadedFileUseCase).`when`(usecases).openAlreadyDownloadedFile + val downloadManager: DownloadManager = mock() + var delegateFilename = "" + var delegateContentSize: Long = -1 + var delegateFileNameIsAlreadyDownloaded: String? = null + grantPermissions() + doReturn(arrayOf(INTERNET, WRITE_EXTERNAL_STORAGE)).`when`(downloadManager).permissions + + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = emptyMap(), + ), + ) + + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = usecases, + downloadManager = downloadManager, + shouldForwardToThirdParties = { true }, + customFirstPartyDownloadDialog = { filename, contentSize, fileNameIfAlreadyDownloaded, positiveActionCallback, negativeActionCallback, openFileAction -> + delegateFilename = filename.value + delegateContentSize = contentSize.value + delegateFileNameIsAlreadyDownloaded = fileNameIfAlreadyDownloaded.value + }, + ), + ) + + doReturn(false).`when`(feature).isDownloadBiggerThanAvailableSpace(download) + + feature.processDownload(tab, download) + + assertEquals("file.txt", delegateFilename) + assertEquals(0, delegateContentSize) + assertNull(delegateFileNameIsAlreadyDownloaded) + } + + @Test fun `GIVEN download should be forwarded to third party apps and a custom delegate is set WHEN processing a download request THEN forward it to the delegate`() { val tab = createTab("https://www.mozilla.org", id = "test-tab") val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab", id = "test") @@ -791,7 +914,7 @@ class DownloadsFeatureTest { val feature = DownloadsFeature( context, store, - DownloadsUseCases(store), + DownloadsUseCases(store, mock()), downloadManager = downloadManager, shouldForwardToThirdParties = { true }, ) @@ -847,7 +970,7 @@ class DownloadsFeatureTest { val feature = DownloadsFeature( context, store, - DownloadsUseCases(store), + DownloadsUseCases(store, mock()), downloadManager = downloadManager, shouldForwardToThirdParties = { true }, ) @@ -872,7 +995,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - DownloadsUseCases(store), + DownloadsUseCases(store, mock()), downloadManager = mock(), shouldForwardToThirdParties = { true }, fragmentManager = fragmentManager, @@ -889,7 +1012,7 @@ class DownloadsFeatureTest { @Test fun `WHEN dismissing a downloader app dialog THEN the download should be canceled`() { - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val cancelDownloadRequestUseCase = mock<CancelDownloadRequestUseCase>() val tab = createTab("https://www.mozilla.org", id = "test-tab") val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab") @@ -930,7 +1053,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - DownloadsUseCases(store), + DownloadsUseCases(store, mock()), downloadManager = mock(), shouldForwardToThirdParties = { true }, fragmentManager = fragmentManager, @@ -951,7 +1074,7 @@ class DownloadsFeatureTest { @Test fun `when our app is selected for downloading and permission granted then we should perform the download`() { val spyContext = spy(testContext) - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val consumeDownloadUseCase = mock<ConsumeDownloadUseCase>() val tab = createTab("https://www.mozilla.org", id = "test-tab") val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab") @@ -1122,7 +1245,7 @@ class DownloadsFeatureTest { fun `when an app third party is selected for downloading we MUST forward the download`() { val spyContext = spy(testContext) val tab = createTab("https://www.mozilla.org", id = "test-tab") - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val consumeDownloadUseCase = mock<ConsumeDownloadUseCase>() val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab") val ourApp = DownloaderApp(name = "app", packageName = "thridparty.app", resolver = mock(), activityName = "", url = "", contentType = null) @@ -1157,7 +1280,7 @@ class DownloadsFeatureTest { fun `None exception is thrown when unable to open an app third party for downloading`() { val spyContext = spy(testContext) val tab = createTab("https://www.mozilla.org", id = "test-tab") - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val consumeDownloadUseCase = mock<ConsumeDownloadUseCase>() val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab") val ourApp = DownloaderApp(name = "app", packageName = "thridparty.app", resolver = mock(), activityName = "", url = "", contentType = null) @@ -1193,7 +1316,7 @@ class DownloadsFeatureTest { fun `when the appChooserDialog is dismissed THEN the download must be canceled`() { val spyContext = spy(testContext) val tab = createTab("https://www.mozilla.org", id = "test-tab") - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val cancelDownloadRequestUseCase = mock<CancelDownloadRequestUseCase>() val download = DownloadState(url = "https://www.mozilla.org/file.txt", sessionId = "test-tab") val ourApp = mock<DownloaderApp>() @@ -1245,7 +1368,7 @@ class DownloadsFeatureTest { @Test fun `previous dialogs MUST be dismissed when navigating to another website`() { - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val cancelDownloadRequestUseCase = mock<CancelDownloadRequestUseCase>() val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download = download)) @@ -1282,7 +1405,7 @@ class DownloadsFeatureTest { @Test fun `previous dialogs must NOT be dismissed when navigating on the same website`() { - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val cancelDownloadRequestUseCase = mock<CancelDownloadRequestUseCase>() val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download = download)) @@ -1327,7 +1450,7 @@ class DownloadsFeatureTest { val downloadManager: DownloadManager = mock() var permissionsRequested = false val dialog = DownloadAppChooserDialog() - val downloadsUseCases = spy(DownloadsUseCases(store)) + val downloadsUseCases = spy(DownloadsUseCases(store, mock())) val consumeDownloadUseCase = mock<ConsumeDownloadUseCase>() val fragmentManager: FragmentManager = mockFragmentManager() @@ -1395,6 +1518,97 @@ class DownloadsFeatureTest { } @Test + fun `GIVEN file with same ETag was already downloaded WHEN starting download THEN show call download dialog with the already downloaded file name`() { + val downloadsUseCases: DownloadsUseCases = mock() + val consumeDownloadUseCase: ConsumeDownloadUseCase = mock() + val openAlreadyDownloadedFileUseCase: DownloadsUseCases.OpenAlreadyDownloadedFileUseCase = mock() + var fileNameIfAlreadyDownloaded: String? = null + var delegateOpenFileCallback: (() -> Unit)? = null + + doReturn(consumeDownloadUseCase).`when`(downloadsUseCases).consumeDownload + doReturn(openAlreadyDownloadedFileUseCase).`when`(downloadsUseCases).openAlreadyDownloadedFile + + val tab = createTab("https://www.mozilla.org", id = "test-tab") + val download = DownloadState( + url = "https://www.mozilla.org/file.txt", + sessionId = "test-tab", + id = "test", + fileName = "file.txt", + etag = "12345", + directoryPath = "/downloads", + ) + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = mapOf("test" to download.copy(status = DownloadState.Status.COMPLETED)), + ), + ) + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = downloadsUseCases, + customFirstPartyDownloadDialog = { _, _, fileName, _, _, openFileAction -> + fileNameIfAlreadyDownloaded = fileName.value + delegateOpenFileCallback = openFileAction.value + }, + fileSystemHelper = FakeFileSystemHelper(existingFiles = listOf("/downloads/file.txt")), + ), + ) + + grantPermissions() + + feature.processDownload(tab, download) + + assertEquals(fileNameIfAlreadyDownloaded, "file.txt") + + delegateOpenFileCallback?.invoke() + verify(openAlreadyDownloadedFileUseCase).invoke(eq(tab.id), eq(download), eq("/downloads/file.txt")) + } + + @Test + fun `GIVEN file is downloaded for the first time WHEN starting download THEN call download dialog with no alreadyDownloadedFile`() { + val downloadsUseCases: DownloadsUseCases = mock() + val consumeDownloadUseCase: ConsumeDownloadUseCase = mock() + var fileNameIfAlreadyDownloaded: String? = null + + doReturn(consumeDownloadUseCase).`when`(downloadsUseCases).consumeDownload + + val tab = createTab("https://www.mozilla.org", id = "test-tab") + val download = DownloadState( + url = "https://www.mozilla.org/file.txt", + sessionId = "test-tab", + id = "test", + fileName = "file.txt", + etag = "12345", + directoryPath = "/downloads", + ) + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = emptyMap(), + ), + ) + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = downloadsUseCases, + customFirstPartyDownloadDialog = { _, _, fileName, _, _, openFileAction -> + fileNameIfAlreadyDownloaded = fileName.value + }, + ), + ) + + grantPermissions() + + feature.processDownload(tab, download) + assertNull(fileNameIfAlreadyDownloaded) + } + + @Test fun `GIVEN content length is 0L WHEN calling isDownloadBiggerThanAvailableSpace THEN it returns false`() { val directoryPath = "/valid/path" @@ -1516,7 +1730,7 @@ class DownloadsFeatureTest { DownloadsFeature( testContext, store, - useCases = DownloadsUseCases(store), + useCases = DownloadsUseCases(store, mock()), tabId = "id", downloadManager = downloadManager, onDownloadStartedListener = onDownloadStartedListener, @@ -1533,6 +1747,238 @@ class DownloadsFeatureTest { verify(onDownloadStartedListener).invoke("downloadId") } + + @Test + fun `WHEN file was already downloaded with same etag and url THEN findDownloadWithSameEtag returns the previous download`() { + val download = DownloadState( + url = "https://www.mozilla.org/file.txt", + sessionId = "test-tab", + id = "test", + fileName = "file.txt", + etag = "12345", + directoryPath = "/downloads", + ) + val previousDownload = DownloadState( + url = "https://www.mozilla.org/file.txt", + fileName = "previous.txt", + directoryPath = "/downloads", + etag = "12345", + status = DownloadState.Status.COMPLETED, + ) + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = mapOf("test" to previousDownload), + ), + ) + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = DownloadsUseCases(store, mock()), + fileSystemHelper = FakeFileSystemHelper(existingFiles = listOf("/downloads/previous.txt")), + ), + ) + + val foundDownload = feature.findDownloadWithSameEtag(download) + + assertEquals(previousDownload, foundDownload) + } + + @Test + fun `WHEN file was already downloaded several times with same etag and url THEN findDownloadWithSameEtag returns the oldest download`() { + val download = DownloadState( + url = "https://www.mozilla.org/file.txt", + sessionId = "test-tab", + id = "test", + fileName = "file.txt", + directoryPath = "/downloads", + etag = "12345", + ) + val previousDownloadOldest = DownloadState( + url = "https://www.mozilla.org/file.txt", + id = "oldest", + fileName = "previous.txt", + directoryPath = "/downloads", + etag = "12345", + status = DownloadState.Status.COMPLETED, + createdTime = 0, + ) + val previousDownloadNewest = DownloadState( + url = "https://www.mozilla.org/file.txt", + id = "newest", + fileName = "previous(1).txt", + directoryPath = "/downloads", + etag = "12345", + status = DownloadState.Status.COMPLETED, + createdTime = 1, + ) + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = mapOf("newest" to previousDownloadNewest, "oldest" to previousDownloadOldest), + ), + ) + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = DownloadsUseCases(store, mock()), + fileSystemHelper = FakeFileSystemHelper(existingFiles = listOf("/downloads/file.txt", "/downloads/previous.txt", "/downloads/previous(1).txt")), + ), + ) + + val foundDownload = feature.findDownloadWithSameEtag(download) + + assertEquals(previousDownloadOldest, foundDownload) + } + + @Test + fun `WHEN file is already being downloaded - IN PROGRESS - with same etag and url THEN findDownloadWithSameEtag returns null`() { + val download = DownloadState( + url = "https://www.mozilla.org/file.txt", + sessionId = "test-tab", + id = "test", + fileName = "file.txt", + etag = "12345", + directoryPath = "/downloads", + ) + val previousDownload = DownloadState( + url = "https://www.mozilla.org/file.txt", + fileName = "previous.txt", + etag = "12345", + status = DownloadState.Status.DOWNLOADING, + ) + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = mapOf("test" to previousDownload), + ), + ) + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = DownloadsUseCases(store, mock()), + ), + ) + + val foundDownload = feature.findDownloadWithSameEtag(download) + + assertNull(foundDownload) + } + + @Test + fun `WHEN file was already downloaded with same etag and different url THEN findDownloadWithSameEtag returns null`() { + val download = DownloadState( + url = "https://www.mozilla.org/file.txt", + sessionId = "test-tab", + id = "test", + fileName = "file.txt", + etag = "12345", + directoryPath = "/downloads", + ) + val previousDownload = DownloadState( + url = "https://www.mozilla.org/another-file.txt", + fileName = "previous.txt", + etag = "12345", + status = DownloadState.Status.COMPLETED, + ) + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = mapOf("test" to previousDownload), + ), + ) + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = DownloadsUseCases(store, mock()), + ), + ) + + val foundDownload = feature.findDownloadWithSameEtag(download) + + assertNull(foundDownload) + } + + @Test + fun `WHEN file was already downloaded with same url and different etag THEN findDownloadWithSameEtag returns null`() { + val download = DownloadState( + url = "https://www.mozilla.org/file.txt", + sessionId = "test-tab", + id = "test", + fileName = "file.txt", + etag = "12345", + directoryPath = "/downloads", + ) + val previousDownload = DownloadState( + url = "https://www.mozilla.org/file.txt", + fileName = "previous.txt", + etag = "123456", + status = DownloadState.Status.COMPLETED, + ) + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = mapOf("test" to previousDownload), + ), + ) + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = DownloadsUseCases(store, mock()), + ), + ) + + val foundDownload = feature.findDownloadWithSameEtag(download) + + assertNull(foundDownload) + } + + @Test + fun `GIVEN file was already downloaded with same url and etag but file was deleted WHEN calling findDownloadWithSameEtag THEN it returns null`() { + val download = DownloadState( + url = "https://www.mozilla.org/file.txt", + sessionId = "test-tab", + id = "test", + fileName = "file.txt", + etag = "12345", + directoryPath = "/downloads", + ) + val previousDownload = DownloadState( + url = "https://www.mozilla.org/file.txt", + fileName = "previous.txt", + etag = "12345", + status = DownloadState.Status.COMPLETED, + ) + val store = BrowserStore( + BrowserState( + tabs = listOf(createTab("https://www.mozilla.org", id = "test-tab")), + selectedTabId = "test-tab", + downloads = mapOf("test" to previousDownload), + ), + ) + val feature = spy( + DownloadsFeature( + applicationContext = testContext, + store = store, + useCases = DownloadsUseCases(store, mock()), + fileSystemHelper = FakeFileSystemHelper(existingFiles = emptyList()), + ), + ) + + val foundDownload = feature.findDownloadWithSameEtag(download) + + assertNull(foundDownload) + } } private fun grantPermissions() { diff --git a/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/fake/FakeFileSystemHelper.kt b/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/fake/FakeFileSystemHelper.kt @@ -9,10 +9,13 @@ import mozilla.components.feature.downloads.FileSystemHelper class FakeFileSystemHelper( private val availableBitesInDirectory: Long = 0L, private val existingDirectories: List<String> = emptyList(), + private val existingFiles: List<String> = emptyList(), ) : FileSystemHelper { override fun createDirectoryIfNotExists(path: String): Boolean = true override fun isDirectory(path: String): Boolean = path in existingDirectories override fun availableBytesInDirectory(path: String): Long = availableBitesInDirectory + + override fun fileExists(filePath: String): Boolean = filePath in existingFiles } diff --git a/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt b/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt @@ -474,7 +474,7 @@ open class DefaultComponents(private val applicationContext: Context) { } val tabsUseCases: TabsUseCases by lazy { TabsUseCases(store) } - val downloadsUseCases: DownloadsUseCases by lazy { DownloadsUseCases(store) } + val downloadsUseCases: DownloadsUseCases by lazy { DownloadsUseCases(store, applicationContext) } val contextMenuUseCases: ContextMenuUseCases by lazy { ContextMenuUseCases(store) } val crashReporter: CrashReporter by lazy { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt @@ -117,7 +117,7 @@ abstract class AddonPopupBaseFragment : Fragment(), EngineSession.Observer, User onNeedToRequestPermissions = { permissions -> requestPermissions(permissions, REQUEST_CODE_DOWNLOAD_PERMISSIONS) }, - customFirstPartyDownloadDialog = { filename, contentSize, positiveAction, negativeAction -> + customFirstPartyDownloadDialog = { filename, contentSize, _, positiveAction, negativeAction, _ -> run { if (downloadDialog == null) { val title = if (contentSize.value > 0L) { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -783,42 +783,104 @@ abstract class BaseBrowserFragment : onNeedToRequestPermissions = { permissions -> requestPermissions(permissions, REQUEST_CODE_DOWNLOAD_PERMISSIONS) }, - customFirstPartyDownloadDialog = { filename, contentSize, positiveAction, negativeAction -> + customFirstPartyDownloadDialog = { + filename, + contentSize, + fileNameIfAlreadyDownloaded, + positiveAction, + negativeAction, + openFileAction, + -> run { if (downloadDialog == null) { requireContext().components.analytics.crashReporter.recordCrashBreadcrumb( Breadcrumb("FirstPartyDownloadDialog created"), ) - val title = if (contentSize.value > 0L) { - val contentSizeInBytes = - requireComponents.core.fileSizeFormatter.formatSizeInBytes( - contentSize.value, + if (fileNameIfAlreadyDownloaded.value != null) { + val title = if (contentSize.value > 0L) { + val contentSizeInBytes = + requireComponents.core.fileSizeFormatter.formatSizeInBytes( + contentSize.value, + ) + getString( + downloadsR.string.mozac_feature_downloads_again_dialog_title, + contentSizeInBytes, ) - getString( - downloadsR.string.mozac_feature_downloads_dialog_title_3, - contentSizeInBytes, + } else { + getString( + downloadsR.string.mozac_feature_downloads_again_dialog_title_with_unknown_size, + ) + } + + val message = getString( + downloadsR.string.mozac_feature_downloads_already_exists_dialog_title, + fileNameIfAlreadyDownloaded.value, ) - } else { - getString(downloadsR.string.mozac_feature_downloads_dialog_title_with_unknown_size) - } - downloadDialog = MaterialAlertDialogBuilder(requireContext()) - .setTitle(title) - .setMessage(filename.value) - .setPositiveButton(downloadsR.string.mozac_feature_downloads_dialog_download) { dialog, _ -> - positiveAction.value.invoke() - dialog.dismiss() - } - .setNegativeButton(downloadsR.string.mozac_feature_downloads_dialog_cancel) { dialog, _ -> - negativeAction.value.invoke() - dialog.dismiss() - }.setOnDismissListener { - downloadDialog = null - context.components.analytics.crashReporter.recordCrashBreadcrumb( - Breadcrumb("FirstPartyDownloadDialog onDismiss"), + downloadDialog = MaterialAlertDialogBuilder(requireContext()) + .setTitle(title) + .setMessage(message) + .setNegativeButton( + downloadsR.string.mozac_feature_downloads_dialog_download_again, + ) { dialog, _ -> + dialog.dismiss() + positiveAction.value.invoke() + } + .setPositiveButton( + downloadsR.string.mozac_feature_downloads_open_existing_file, + ) { dialog, _ -> + openFileAction.value.invoke() + dialog.dismiss() + } + .setNeutralButton( + downloadsR.string.mozac_feature_downloads_dialog_cancel, + ) { dialog, _ -> + negativeAction.value.invoke() + dialog.dismiss() + }.setOnDismissListener { + downloadDialog = null + context.components.analytics.crashReporter.recordCrashBreadcrumb( + Breadcrumb("FirstPartyDownloadDialog onDismiss"), + ) + }.show() + } else { + val title = if (contentSize.value > 0L) { + val contentSizeInBytes = + requireComponents.core.fileSizeFormatter.formatSizeInBytes( + contentSize.value, + ) + getString( + downloadsR.string.mozac_feature_downloads_dialog_title_3, + contentSizeInBytes, + ) + } else { + getString( + downloadsR.string.mozac_feature_downloads_dialog_title_with_unknown_size, ) - }.show() + } + + downloadDialog = MaterialAlertDialogBuilder(requireContext()) + .setTitle(title) + .setMessage(filename.value) + .setPositiveButton( + downloadsR.string.mozac_feature_downloads_dialog_download, + ) { dialog, _ -> + positiveAction.value.invoke() + dialog.dismiss() + } + .setNegativeButton( + downloadsR.string.mozac_feature_downloads_dialog_cancel, + ) { dialog, _ -> + negativeAction.value.invoke() + dialog.dismiss() + }.setOnDismissListener { + downloadDialog = null + context.components.analytics.crashReporter.recordCrashBreadcrumb( + Breadcrumb("FirstPartyDownloadDialog onDismiss"), + ) + }.show() + } } } }, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/UseCases.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/UseCases.kt @@ -94,7 +94,7 @@ class UseCases( WebAppUseCases(context, store.value, shortcutManager.value) } - val downloadUseCases by lazyMonitored { DownloadsUseCases(store.value) } + val downloadUseCases by lazyMonitored { DownloadsUseCases(store.value, context.applicationContext) } val contextMenuUseCases by lazyMonitored { ContextMenuUseCases(store.value) } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/Components.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/Components.kt @@ -242,7 +242,7 @@ class Components( val contextMenuUseCases: ContextMenuUseCases by lazy { ContextMenuUseCases(store) } - val downloadsUseCases: DownloadsUseCases by lazy { DownloadsUseCases(store) } + val downloadsUseCases: DownloadsUseCases by lazy { DownloadsUseCases(store, context.applicationContext) } val appLinksUseCases: AppLinksUseCases by lazy { AppLinksUseCases(context.applicationContext) }