From 3dbb42988f08dd4247e6ba72e51a73da7431f542 Mon Sep 17 00:00:00 2001 From: MatthewTighe Date: Fri, 19 Aug 2022 15:47:55 -0700 Subject: [PATCH] [fenix] Closes https://github.com/mozilla-mobile/fenix/issues/26214: Download wallpaper thumbnails at app startup --- .../wallpapers/LegacyWallpaperFileManager.kt | 1 + .../org/mozilla/fenix/wallpapers/Wallpaper.kt | 15 +++- .../fenix/wallpapers/WallpaperDownloader.kt | 75 +++++++++------- .../fenix/wallpapers/WallpaperFileManager.kt | 1 + .../wallpapers/WallpaperMetadataFetcher.kt | 1 + .../fenix/wallpapers/WallpapersUseCases.kt | 12 ++- .../LegacyWallpaperFileManagerTest.kt | 3 +- .../wallpapers/WallpaperDownloaderTest.kt | 25 +++++- .../wallpapers/WallpaperFileManagerTest.kt | 1 + .../wallpapers/WallpapersUseCasesTest.kt | 85 +++++++++++++++++-- 10 files changed, 176 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/wallpapers/LegacyWallpaperFileManager.kt b/app/src/main/java/org/mozilla/fenix/wallpapers/LegacyWallpaperFileManager.kt index 71126eee4e..52efa79294 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/LegacyWallpaperFileManager.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/LegacyWallpaperFileManager.kt @@ -38,6 +38,7 @@ class LegacyWallpaperFileManager( collection = Wallpaper.DefaultCollection, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ) } else null } diff --git a/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt b/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt index 44e3a7e04e..6c70d44d5b 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt @@ -21,6 +21,7 @@ data class Wallpaper( val collection: Collection, val textColor: Long?, val cardColor: Long?, + val thumbnailFileState: ImageFileState, ) { /** * Type that represents a collection that a [Wallpaper] belongs to. @@ -66,6 +67,7 @@ data class Wallpaper( collection = DefaultCollection, textColor = null, cardColor = null, + thumbnailFileState = ImageFileState.Downloaded, ) /** @@ -100,7 +102,8 @@ data class Wallpaper( name = name, textColor = textColor, cardColor = cardColor, - collection = DefaultCollection + collection = DefaultCollection, + thumbnailFileState = ImageFileState.Downloaded, ) } else null } @@ -119,4 +122,14 @@ data class Wallpaper( */ fun lowercase(): String = this.name.lowercase() } + + /** + * Defines the download state of wallpaper asset. + */ + enum class ImageFileState { + NotAvailable, + Downloading, + Downloaded, + Error, + } } diff --git a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperDownloader.kt b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperDownloader.kt index c16c20afeb..4f304626aa 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperDownloader.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperDownloader.kt @@ -13,6 +13,7 @@ import mozilla.components.concept.fetch.isSuccess import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.wallpapers.Wallpaper.Companion.getLocalPath import java.io.File +import java.lang.IllegalStateException /** * Can download wallpapers from a remote host. @@ -37,40 +38,52 @@ class WallpaperDownloader( * wallpapers//.png */ suspend fun downloadWallpaper(wallpaper: Wallpaper) = withContext(dispatcher) { - for (metadata in wallpaper.toMetadata()) { - val localFile = File(storageRootDirectory.absolutePath, metadata.localPath) - // Don't overwrite an asset if it exists - if (localFile.exists()) continue - val request = Request( - url = "$remoteHost/${metadata.remotePath}", - method = Request.Method.GET - ) - Result.runCatching { - val response = client.fetch(request) - if (!response.isSuccess) { - return@withContext - } - File(localFile.path.substringBeforeLast("/")).mkdirs() - response.body.useStream { input -> - input.copyTo(localFile.outputStream()) - } - }.onFailure { - // This should clean up any partial downloads - Result.runCatching { - if (localFile.exists()) { - localFile.delete() - } - } - } + listOf(Wallpaper.ImageType.Portrait, Wallpaper.ImageType.Landscape).map { imageType -> + wallpaper.downloadAsset(imageType) } } - private data class WallpaperMetadata(val remotePath: String, val localPath: String) + /** + * Downloads a thumbnail for a wallpaper from the network. This is expected to be found remotely + * at: + * ///.png + * and stored locally at: + * wallpapers//.png + */ + suspend fun downloadThumbnail(wallpaper: Wallpaper): Wallpaper.ImageFileState = withContext(dispatcher) { + wallpaper.downloadAsset(Wallpaper.ImageType.Thumbnail) + } - private fun Wallpaper.toMetadata(): List = - listOf(Wallpaper.ImageType.Portrait, Wallpaper.ImageType.Landscape).map { orientation -> - val localPath = getLocalPath(this.name, orientation) - val remotePath = "${collection.name}/${this.name}/${orientation.lowercase()}.png" - WallpaperMetadata(remotePath, localPath) + private suspend fun Wallpaper.downloadAsset( + imageType: Wallpaper.ImageType + ): Wallpaper.ImageFileState = withContext(dispatcher) { + val localFile = File(storageRootDirectory, getLocalPath(name, imageType)) + if (localFile.exists()) return@withContext Wallpaper.ImageFileState.Downloaded + + val remotePath = "${collection.name}/${name}/${imageType.lowercase()}.png" + val request = Request( + url = "$remoteHost/$remotePath", + method = Request.Method.GET + ) + + return@withContext Result.runCatching { + val response = client.fetch(request) + if (!response.isSuccess) { + throw IllegalStateException() + } + File(localFile.path.substringBeforeLast("/")).mkdirs() + response.body.useStream { input -> + input.copyTo(localFile.outputStream()) + } + Wallpaper.ImageFileState.Downloaded + }.getOrElse { + // This should clean up any partial downloads + Result.runCatching { + if (localFile.exists()) { + localFile.delete() + } + } + Wallpaper.ImageFileState.Downloaded } + } } diff --git a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperFileManager.kt b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperFileManager.kt index 69ca7f8cea..9c73166458 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperFileManager.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperFileManager.kt @@ -37,6 +37,7 @@ class WallpaperFileManager( collection = Wallpaper.DefaultCollection, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.Downloaded, ) } else null } diff --git a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperMetadataFetcher.kt b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperMetadataFetcher.kt index 118f2e0c8d..cd6668ca0b 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperMetadataFetcher.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperMetadataFetcher.kt @@ -83,6 +83,7 @@ class WallpaperMetadataFetcher( textColor = getArgbValueAsLong("text-color"), cardColor = getArgbValueAsLong("card-color"), collection = collection, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ) } } diff --git a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpapersUseCases.kt b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpapersUseCases.kt index 0719e0cfd8..b7d51f1717 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpapersUseCases.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpapersUseCases.kt @@ -182,18 +182,21 @@ class WallpapersUseCases( collection = firefoxClassicCollection, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ), Wallpaper( name = Wallpaper.ceruleanName, collection = firefoxClassicCollection, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ), Wallpaper( name = Wallpaper.sunriseName, collection = firefoxClassicCollection, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ), ) private val remoteWallpapers: List = listOf( @@ -202,12 +205,14 @@ class WallpapersUseCases( collection = firefoxClassicCollection, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ), Wallpaper( name = Wallpaper.beachVibeName, collection = firefoxClassicCollection, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ), ) val allWallpapers = listOf(Wallpaper.Default) + localWallpapers + remoteWallpapers @@ -246,7 +251,12 @@ class WallpapersUseCases( possibleWallpapers.forEach { downloader.downloadWallpaper(it) } - val defaultIncluded = listOf(Wallpaper.Default) + possibleWallpapers + val wallpapersWithUpdatedThumbnailState = possibleWallpapers.map { wallpaper -> + val result = downloader.downloadThumbnail(wallpaper) + wallpaper.copy(thumbnailFileState = result) + } + + val defaultIncluded = listOf(Wallpaper.Default) + wallpapersWithUpdatedThumbnailState store.dispatch(AppAction.WallpaperAction.UpdateAvailableWallpapers(defaultIncluded)) } diff --git a/app/src/test/java/org/mozilla/fenix/wallpapers/LegacyWallpaperFileManagerTest.kt b/app/src/test/java/org/mozilla/fenix/wallpapers/LegacyWallpaperFileManagerTest.kt index c504aa10ab..c490f9ff70 100644 --- a/app/src/test/java/org/mozilla/fenix/wallpapers/LegacyWallpaperFileManagerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/wallpapers/LegacyWallpaperFileManagerTest.kt @@ -98,6 +98,7 @@ class LegacyWallpaperFileManagerTest { name = name, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, collection = Wallpaper.Collection( name = Wallpaper.defaultName, heading = null, @@ -105,7 +106,7 @@ class LegacyWallpaperFileManagerTest { availableLocales = null, startDate = null, endDate = null, - learnMoreUrl = null + learnMoreUrl = null, ), ) } diff --git a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperDownloaderTest.kt b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperDownloaderTest.kt index c9ea5cb79e..f804c056b6 100644 --- a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperDownloaderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperDownloaderTest.kt @@ -7,6 +7,7 @@ import kotlinx.coroutines.test.runTest import mozilla.components.concept.fetch.Client import mozilla.components.concept.fetch.Request import mozilla.components.concept.fetch.Response +import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before @@ -51,7 +52,7 @@ class WallpaperDownloaderTest { } @Test - fun `GIVEN that request is successful WHEN downloading THEN file is created in expected location`() = runTest { + fun `GIVEN that asset request is successful WHEN downloading assets THEN both files are created in expected location`() = runTest { val wallpaper = generateWallpaper() val portraitRequest = wallpaper.generateRequest("portrait") val landscapeRequest = wallpaper.generateRequest("landscape") @@ -70,6 +71,25 @@ class WallpaperDownloaderTest { assertTrue(expectedLandscapeFile.exists() && expectedLandscapeFile.readText() == wallpaperBytes) } + @Test + fun `GIVEN that thumbnail request is successful WHEN downloading THEN file is created in expected location`() = runTest { + val wallpaper = generateWallpaper() + val thumbnailRequest = Request( + url = "$remoteHost/${wallpaper.collection.name}/${wallpaper.name}/thumbnail.png", + method = Request.Method.GET + ) + val mockThumbnailResponse = mockk() + every { mockThumbnailResponse.status } returns 200 + every { mockThumbnailResponse.body } returns Response.Body(wallpaperBytes.byteInputStream()) + every { mockClient.fetch(thumbnailRequest) } returns mockThumbnailResponse + + val result = downloader.downloadThumbnail(wallpaper) + + val expectedThumbnailFile = File(tempFolder.root, "wallpapers/${wallpaper.name}/thumbnail.png") + assertTrue(expectedThumbnailFile.exists() && expectedThumbnailFile.readText() == wallpaperBytes) + assertEquals(Wallpaper.ImageFileState.Downloaded, result) + } + @Test fun `GIVEN that request fails WHEN downloading THEN file is not created`() = runTest { val wallpaper = generateWallpaper() @@ -111,7 +131,8 @@ class WallpaperDownloaderTest { name = name, collection = wallpaperCollection, textColor = null, - cardColor = null + cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable ) private fun Wallpaper.generateRequest(type: String) = Request( diff --git a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperFileManagerTest.kt b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperFileManagerTest.kt index e7874fac1f..ecda1aee56 100644 --- a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperFileManagerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperFileManagerTest.kt @@ -130,6 +130,7 @@ class WallpaperFileManagerTest { name = name, textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.Downloaded, collection = Wallpaper.DefaultCollection ) } diff --git a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpapersUseCasesTest.kt b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpapersUseCasesTest.kt index 2d5179c1d0..bdd5f04088 100644 --- a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpapersUseCasesTest.kt +++ b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpapersUseCasesTest.kt @@ -49,7 +49,9 @@ class WallpapersUseCasesTest { } private val mockMetadataFetcher = mockk() - private val mockDownloader = mockk(relaxed = true) + private val mockDownloader = mockk { + coEvery { downloadWallpaper(any()) } returns mockk() + } private val mockFileManager = mockk { coEvery { clean(any(), any()) } returns mockk() } @@ -243,6 +245,7 @@ class WallpapersUseCasesTest { every { mockSettings.currentWallpaperName } returns "" coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null coEvery { mockMetadataFetcher.downloadWallpaperList() } returns fakeRemoteWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded WallpapersUseCases.DefaultInitializeWallpaperUseCase( appStore, @@ -265,6 +268,7 @@ class WallpapersUseCasesTest { every { mockSettings.currentWallpaperName } returns "" coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null coEvery { mockMetadataFetcher.downloadWallpaperList() } returns fakeRemoteWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded WallpapersUseCases.DefaultInitializeWallpaperUseCase( appStore, @@ -291,6 +295,7 @@ class WallpapersUseCasesTest { every { mockSettings.currentWallpaperName } returns "" coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null coEvery { mockMetadataFetcher.downloadWallpaperList() } returns possibleWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded WallpapersUseCases.DefaultInitializeWallpaperUseCase( appStore, @@ -313,9 +318,11 @@ class WallpapersUseCasesTest { makeFakeRemoteWallpaper(TimeRelation.LATER, name) } val expiredWallpaper = makeFakeRemoteWallpaper(TimeRelation.BEFORE, "expired") - every { mockSettings.currentWallpaperName } returns expiredWallpaper.name + val allWallpapers = listOf(expiredWallpaper) + fakeRemoteWallpapers coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns expiredWallpaper - coEvery { mockMetadataFetcher.downloadWallpaperList() } returns listOf(expiredWallpaper) + fakeRemoteWallpapers + coEvery { mockMetadataFetcher.downloadWallpaperList() } returns allWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded + WallpapersUseCases.DefaultInitializeWallpaperUseCase( appStore, @@ -326,8 +333,11 @@ class WallpapersUseCasesTest { "en-US", ).invoke() + val expectedWallpaper = expiredWallpaper.copy( + thumbnailFileState = Wallpaper.ImageFileState.Downloaded + ) appStore.waitUntilIdle() - assertTrue(appStore.state.wallpaperState.availableWallpapers.contains(expiredWallpaper)) + assertTrue(appStore.state.wallpaperState.availableWallpapers.contains(expectedWallpaper)) assertEquals(expiredWallpaper, appStore.state.wallpaperState.currentWallpaper) } @@ -363,6 +373,7 @@ class WallpapersUseCasesTest { every { mockSettings.currentWallpaperName } returns "" coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null coEvery { mockMetadataFetcher.downloadWallpaperList() } returns fakeRemoteWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded WallpapersUseCases.DefaultInitializeWallpaperUseCase( appStore, @@ -378,6 +389,60 @@ class WallpapersUseCasesTest { } } + @Test + fun `GIVEN available wallpapers WHEN invoking initialize use case THEN thumbnails downloaded and store state reflects that`() = runTest { + val fakeRemoteWallpapers = listOf("first", "second", "third").map { name -> + makeFakeRemoteWallpaper(TimeRelation.LATER, name) + } + every { mockSettings.currentWallpaperName } returns "" + coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null + coEvery { mockMetadataFetcher.downloadWallpaperList() } returns fakeRemoteWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded + + WallpapersUseCases.DefaultInitializeWallpaperUseCase( + appStore, + mockDownloader, + mockFileManager, + mockMetadataFetcher, + mockSettings, + "en-US", + ).invoke() + + for (fakeRemoteWallpaper in fakeRemoteWallpapers) { + coVerify { mockDownloader.downloadThumbnail(fakeRemoteWallpaper) } + } + appStore.waitUntilIdle() + assertTrue(appStore.state.wallpaperState.availableWallpapers.all { + it.thumbnailFileState == Wallpaper.ImageFileState.Downloaded + }) + } + + @Test + fun `GIVEN thumbnail download fails WHEN invoking initialize use case THEN store state reflects that`() = runTest { + val fakeRemoteWallpapers = listOf("first", "second", "third").map { name -> + makeFakeRemoteWallpaper(TimeRelation.LATER, name) + } + val failedWallpaper = makeFakeRemoteWallpaper(TimeRelation.LATER, "failed") + every { mockSettings.currentWallpaperName } returns "" + coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null + coEvery { mockMetadataFetcher.downloadWallpaperList() } returns listOf(failedWallpaper) + fakeRemoteWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded + coEvery { mockDownloader.downloadThumbnail(failedWallpaper) } returns Wallpaper.ImageFileState.Error + + WallpapersUseCases.DefaultInitializeWallpaperUseCase( + appStore, + mockDownloader, + mockFileManager, + mockMetadataFetcher, + mockSettings, + "en-US", + ).invoke() + + val expectedWallpaper = failedWallpaper.copy(thumbnailFileState = Wallpaper.ImageFileState.Error) + appStore.waitUntilIdle() + assertTrue(appStore.state.wallpaperState.availableWallpapers.contains(expectedWallpaper)) + } + @Test fun `GIVEN a wallpaper has not been selected WHEN invoking initialize use case THEN store contains default`() = runTest { val fakeRemoteWallpapers = listOf("first", "second", "third").map { name -> @@ -386,6 +451,7 @@ class WallpapersUseCasesTest { every { mockSettings.currentWallpaperName } returns "" coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null coEvery { mockMetadataFetcher.downloadWallpaperList() } returns fakeRemoteWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded WallpapersUseCases.DefaultInitializeWallpaperUseCase( appStore, @@ -407,10 +473,10 @@ class WallpapersUseCasesTest { makeFakeRemoteWallpaper(TimeRelation.LATER, name) } val possibleWallpapers = listOf(selectedWallpaper) + fakeRemoteWallpapers - val allWallpapers = listOf(Wallpaper.Default) + possibleWallpapers every { mockSettings.currentWallpaperName } returns selectedWallpaper.name coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null coEvery { mockMetadataFetcher.downloadWallpaperList() } returns possibleWallpapers + coEvery { mockDownloader.downloadThumbnail(any()) } returns Wallpaper.ImageFileState.Downloaded WallpapersUseCases.DefaultInitializeWallpaperUseCase( appStore, @@ -421,9 +487,12 @@ class WallpapersUseCasesTest { "en-US", ).invoke() + val expectedWallpapers = (listOf(Wallpaper.Default) + possibleWallpapers).map { + it.copy(thumbnailFileState = Wallpaper.ImageFileState.Downloaded) + } appStore.waitUntilIdle() assertEquals(selectedWallpaper, appStore.state.wallpaperState.currentWallpaper) - assertEquals(allWallpapers, appStore.state.wallpaperState.availableWallpapers) + assertEquals(expectedWallpapers, appStore.state.wallpaperState.availableWallpapers) } @Test @@ -457,7 +526,7 @@ class WallpapersUseCasesTest { private fun makeFakeRemoteWallpaper( timeRelation: TimeRelation, name: String = "name", - isInPromo: Boolean = true + isInPromo: Boolean = true, ): Wallpaper { fakeCalendar.time = baseFakeDate when (timeRelation) { @@ -480,6 +549,7 @@ class WallpapersUseCasesTest { ), textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ) } else { Wallpaper( @@ -495,6 +565,7 @@ class WallpapersUseCasesTest { ), textColor = null, cardColor = null, + thumbnailFileState = Wallpaper.ImageFileState.NotAvailable, ) } }