From 702aeda47bf6abb4844a270a521a384fb32603b6 Mon Sep 17 00:00:00 2001 From: MatthewTighe Date: Mon, 15 Aug 2022 11:46:43 -0700 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/26423: simplify wallpaper types to single data class --- .../org/mozilla/fenix/home/HomeFragment.kt | 2 +- .../org/mozilla/fenix/wallpapers/Wallpaper.kt | 107 +++++------------- .../fenix/wallpapers/WallpaperDownloader.kt | 6 +- .../fenix/wallpapers/WallpaperFileManager.kt | 12 +- .../fenix/wallpapers/WallpapersUseCases.kt | 99 ++++++++++------ .../wallpapers/WallpaperFileManagerTest.kt | 14 ++- .../wallpapers/WallpapersUseCasesTest.kt | 62 +++++++++- 7 files changed, 178 insertions(+), 124 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 512376a59..c0c0b94b1 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -931,7 +931,7 @@ class HomeFragment : Fragment() { // We only want to update the wallpaper when it's different from the default one // as the default is applied already on xml by default. when (val currentWallpaper = state.wallpaperState.currentWallpaper) { - is Wallpaper.Default -> { + Wallpaper.Default -> { binding.wallpaperImageView.visibility = View.GONE } else -> { 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 54934ce51..3cdb74e4a 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt @@ -4,88 +4,43 @@ package org.mozilla.fenix.wallpapers -import androidx.annotation.DrawableRes -import java.util.Calendar import java.util.Date /** - * Type hierarchy defining the various wallpapers that are available as home screen backgrounds. + * Type that represents wallpapers. + * * @property name The name of the wallpaper. + * @property collectionName The name of the collection the wallpaper belongs to. + * @property availableLocales The locales that this wallpaper is restricted to. If null, the wallpaper + * is not restricted. + * @property startDate The date the wallpaper becomes available in a promotion. If null, it is available + * from any date. + * @property endDate The date the wallpaper stops being available in a promotion. If null, + * the wallpaper will be available to any date. */ -sealed class Wallpaper { - abstract val name: String - - /** - * The default wallpaper. This uses the standard color resource to as a background, instead of - * loading a bitmap. - */ - object Default : Wallpaper() { - override val name = "default" - } - - /** - * If a user had previously selected a wallpaper, they are allowed to retain it even if - * the wallpaper is otherwise expired. This type exists as a wrapper around that current - * wallpaper. - */ - data class Expired(override val name: String) : Wallpaper() - - /** - * Wallpapers that are included directly in the shipped APK. - * - * @property drawableId The drawable bitmap used as the background. - */ - sealed class Local : Wallpaper() { - abstract val drawableId: Int - data class Firefox(override val name: String, @DrawableRes override val drawableId: Int) : Local() - } - - /** - * Wallpapers that need to be fetched from a network resource. - * - * @property expirationDate Optional date at which this wallpaper should no longer be available. - */ - sealed class Remote : Wallpaper() { - abstract val expirationDate: Date? - abstract val remoteParentDirName: String - @Suppress("MagicNumber") - /** - * A promotional partnered wallpaper. - */ - data class House( - override val name: String, - override val expirationDate: Date? = Calendar.getInstance().run { - set(2022, Calendar.APRIL, 30) - time - } - ) : Remote(), Promotional { - override val remoteParentDirName: String = "house" - override fun isAvailableInLocale(locale: String): Boolean = - listOf("en-US", "es-US").contains(locale) - } - - /** - * Wallpapers that are original Firefox creations. - */ - data class Firefox( - override val name: String, - ) : Remote() { - override val expirationDate: Date? = null - override val remoteParentDirName: String = "firefox" - } - } - - /** - * Designates whether a wallpaper is part of a promotion that is locale-restricted. - */ - interface Promotional { - /** - * Returns whether the wallpaper is available in [locale] or not. - */ - fun isAvailableInLocale(locale: String): Boolean - } - +data class Wallpaper( + val name: String, + val collectionName: String, + val availableLocales: List?, + val startDate: Date?, + val endDate: Date? +) { companion object { + const val amethystName = "amethyst" + const val ceruleanName = "cerulean" + const val sunriseName = "sunrise" + const val twilightHillsName = "twilight-hills" + const val beachVibeName = "beach-vibe" + const val firefoxCollectionName = "firefox" + const val defaultName = "default" + val Default = Wallpaper( + name = defaultName, + collectionName = defaultName, + availableLocales = null, + startDate = null, + endDate = null, + ) + /** * Defines the standard path at which a wallpaper resource is kept on disk. * 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 dfb5c3248..0016f73f8 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperDownloader.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperDownloader.kt @@ -33,7 +33,7 @@ class WallpaperDownloader( * found at a remote path in the form: * /////.png */ - suspend fun downloadWallpaper(wallpaper: Wallpaper.Remote) = withContext(Dispatchers.IO) { + suspend fun downloadWallpaper(wallpaper: Wallpaper) = withContext(Dispatchers.IO) { if (remoteHost.isNullOrEmpty()) { return@withContext } @@ -71,14 +71,14 @@ class WallpaperDownloader( private data class WallpaperMetadata(val remotePath: String, val localPath: String) - private fun Wallpaper.Remote.toMetadata(context: Context): List = + private fun Wallpaper.toMetadata(context: Context): List = listOf("landscape", "portrait").flatMap { orientation -> listOf("light", "dark").map { theme -> val localPath = "wallpapers/$orientation/$theme/$name.png" val remotePath = "${context.resolutionSegment()}/" + "$orientation/" + "$theme/" + - "$remoteParentDirName/" + + "$collectionName/" + "$name.png" WallpaperMetadata(remotePath, localPath) } 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 8485fc1f7..67e7dc663 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperFileManager.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperFileManager.kt @@ -24,9 +24,15 @@ class WallpaperFileManager( * files for each of the following orientation and theme combinations: * light/portrait - light/landscape - dark/portrait - dark/landscape */ - suspend fun lookupExpiredWallpaper(name: String): Wallpaper.Expired? = withContext(Dispatchers.IO) { + suspend fun lookupExpiredWallpaper(name: String): Wallpaper? = withContext(Dispatchers.IO) { if (getAllLocalWallpaperPaths(name).all { File(rootDirectory, it).exists() }) { - Wallpaper.Expired(name) + Wallpaper( + name = name, + collectionName = "", + availableLocales = null, + startDate = null, + endDate = null, + ) } else null } @@ -40,7 +46,7 @@ class WallpaperFileManager( /** * Remove all wallpapers that are not the [currentWallpaper] or in [availableWallpapers]. */ - fun clean(currentWallpaper: Wallpaper, availableWallpapers: List) { + fun clean(currentWallpaper: Wallpaper, availableWallpapers: List) { scope.launch { val wallpapersToKeep = (listOf(currentWallpaper) + availableWallpapers).map { it.name } cleanChildren(portraitDirectory, wallpapersToKeep) 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 b13fc1101..aab02182e 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpapersUseCases.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpapersUseCases.kt @@ -96,7 +96,7 @@ class WallpapersUseCases( // This should be cleaned up as improvements are made to the storage, file management, // and download utilities. withContext(Dispatchers.IO) { - val availableWallpapers = getAvailableWallpapers() + val availableWallpapers = possibleWallpapers.getAvailableWallpapers() val currentWallpaperName = settings.currentWallpaper val currentWallpaper = possibleWallpapers.find { it.name == currentWallpaperName } ?: fileManager.lookupExpiredWallpaper(currentWallpaperName) @@ -104,7 +104,7 @@ class WallpapersUseCases( fileManager.clean( currentWallpaper, - possibleWallpapers.filterIsInstance() + possibleWallpapers ) downloadAllRemoteWallpapers(availableWallpapers) store.dispatch(AppAction.WallpaperAction.UpdateAvailableWallpapers(availableWallpapers)) @@ -112,42 +112,63 @@ class WallpapersUseCases( } } - private fun getAvailableWallpapers() = possibleWallpapers - .filter { !it.isExpired() && it.isAvailableInLocale() } + private fun List.getAvailableWallpapers() = + this.filter { !it.isExpired() && it.isAvailableInLocale() } private suspend fun downloadAllRemoteWallpapers(availableWallpapers: List) { - for (wallpaper in availableWallpapers.filterIsInstance()) { - downloader.downloadWallpaper(wallpaper) + for (wallpaper in availableWallpapers) { + if (wallpaper != Wallpaper.Default) { + downloader.downloadWallpaper(wallpaper) + } } } - private fun Wallpaper.isExpired(): Boolean = when (this) { - is Wallpaper.Remote -> { - val expired = this.expirationDate?.let { Date().after(it) } ?: false - expired && this.name != settings.currentWallpaper - } - else -> false + private fun Wallpaper.isExpired(): Boolean { + val expired = this.endDate?.let { Date().after(it) } ?: false + return expired && this.name != settings.currentWallpaper } private fun Wallpaper.isAvailableInLocale(): Boolean = - if (this is Wallpaper.Promotional) { - this.isAvailableInLocale(currentLocale) - } else { - true - } + this.availableLocales?.contains(currentLocale) ?: true companion object { - private val localWallpapers: List = listOf( - Wallpaper.Local.Firefox("amethyst", R.drawable.amethyst), - Wallpaper.Local.Firefox("cerulean", R.drawable.cerulean), - Wallpaper.Local.Firefox("sunrise", R.drawable.sunrise), + private val localWallpapers: List = listOf( + Wallpaper( + name = Wallpaper.amethystName, + collectionName = Wallpaper.firefoxCollectionName, + availableLocales = null, + startDate = null, + endDate = null, + ), + Wallpaper( + name = Wallpaper.ceruleanName, + collectionName = Wallpaper.firefoxCollectionName, + availableLocales = null, + startDate = null, + endDate = null, + ), + Wallpaper( + name = Wallpaper.sunriseName, + collectionName = Wallpaper.firefoxCollectionName, + availableLocales = null, + startDate = null, + endDate = null, + ), ) - private val remoteWallpapers: List = listOf( - Wallpaper.Remote.Firefox( - "twilight-hills" + private val remoteWallpapers: List = listOf( + Wallpaper( + name = Wallpaper.twilightHillsName, + collectionName = Wallpaper.firefoxCollectionName, + availableLocales = null, + startDate = null, + endDate = null, ), - Wallpaper.Remote.Firefox( - "beach-vibe" + Wallpaper( + name = Wallpaper.beachVibeName, + collectionName = Wallpaper.firefoxCollectionName, + availableLocales = null, + startDate = null, + endDate = null, ), ) val allWallpapers = listOf(Wallpaper.Default) + localWallpapers + remoteWallpapers @@ -173,24 +194,34 @@ class WallpapersUseCases( * * @param wallpaper The wallpaper to load a bitmap for. */ - override suspend operator fun invoke(wallpaper: Wallpaper): Bitmap? = when (wallpaper) { - is Wallpaper.Local -> loadWallpaperFromDrawable(context, wallpaper) - is Wallpaper.Remote -> loadWallpaperFromDisk(context, wallpaper) + override suspend operator fun invoke(wallpaper: Wallpaper): Bitmap? = when (wallpaper.name) { + Wallpaper.amethystName, Wallpaper.ceruleanName, Wallpaper.sunriseName -> { + loadWallpaperFromDrawable(context, wallpaper) + } + Wallpaper.twilightHillsName, Wallpaper.beachVibeName -> { + loadWallpaperFromDisk(context, wallpaper) + } else -> null } private suspend fun loadWallpaperFromDrawable( context: Context, - wallpaper: Wallpaper.Local + wallpaper: Wallpaper ): Bitmap? = Result.runCatching { + val drawableRes = when (wallpaper.name) { + Wallpaper.amethystName -> R.drawable.amethyst + Wallpaper.ceruleanName -> R.drawable.cerulean + Wallpaper.sunriseName -> R.drawable.sunrise + else -> return@runCatching null + } withContext(Dispatchers.IO) { - BitmapFactory.decodeResource(context.resources, wallpaper.drawableId) + BitmapFactory.decodeResource(context.resources, drawableRes) } }.getOrNull() private suspend fun loadWallpaperFromDisk( context: Context, - wallpaper: Wallpaper.Remote + wallpaper: Wallpaper ): Bitmap? = Result.runCatching { val path = wallpaper.getLocalPathFromContext(context) withContext(Dispatchers.IO) { @@ -203,7 +234,7 @@ class WallpapersUseCases( * Get the expected local path on disk for a wallpaper. This will differ depending * on orientation and app theme. */ - private fun Wallpaper.Remote.getLocalPathFromContext(context: Context): String { + private fun Wallpaper.getLocalPathFromContext(context: Context): String { val orientation = if (context.isLandscape()) "landscape" else "portrait" val theme = if (context.isDark()) "dark" else "light" return Wallpaper.getBaseLocalPath(orientation, theme, name) @@ -247,7 +278,7 @@ class WallpapersUseCases( Wallpapers.wallpaperSelected.record( Wallpapers.WallpaperSelectedExtra( name = wallpaper.name, - themeCollection = wallpaper::class.simpleName + themeCollection = wallpaper.collectionName ) ) } 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 680db4584..f9ecbd7b9 100644 --- a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperFileManagerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperFileManagerTest.kt @@ -46,7 +46,7 @@ class WallpaperFileManagerTest { val result = fileManager.lookupExpiredWallpaper(wallpaperName) - val expected = Wallpaper.Expired(name = wallpaperName) + val expected = generateWallpaper(name = wallpaperName) assertEquals(expected, result) } @@ -64,9 +64,9 @@ class WallpaperFileManagerTest { @Test fun `WHEN cleaned THEN current wallpaper and available wallpapers kept`() { val currentName = "current" - val currentWallpaper = Wallpaper.Expired(currentName) + val currentWallpaper = generateWallpaper(name = currentName) val availableName = "available" - val available = Wallpaper.Remote.House(name = availableName) + val available = generateWallpaper(name = availableName) val unavailableName = "unavailable" createAllFiles(currentName) createAllFiles(availableName) @@ -93,4 +93,12 @@ class WallpaperFileManagerTest { File(landscapeDarkFolder, "$name.png"), ) } + + private fun generateWallpaper(name: String) = Wallpaper( + name = name, + collectionName = "", + availableLocales = null, + startDate = null, + endDate = null + ) } 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 915609608..4ca5788d4 100644 --- a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpapersUseCasesTest.kt +++ b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpapersUseCasesTest.kt @@ -42,6 +42,48 @@ class WallpapersUseCasesTest { every { clean(any(), any()) } just runs } + @Test + fun `WHEN initializing THEN the default wallpaper is not downloaded`() = runTest { + val fakeRemoteWallpapers = listOf("first", "second", "third").map { name -> + makeFakeRemoteWallpaper(TimeRelation.LATER, name) + } + every { mockSettings.currentWallpaper } returns "" + coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null + + WallpapersUseCases.DefaultInitializeWallpaperUseCase( + appStore, + mockDownloader, + mockFileManager, + mockSettings, + "en-US", + possibleWallpapers = listOf(Wallpaper.Default) + fakeRemoteWallpapers + ).invoke() + + appStore.waitUntilIdle() + coVerify(exactly = 0) { mockDownloader.downloadWallpaper(Wallpaper.Default) } + } + + @Test + fun `WHEN initializing THEN default wallpaper is included in available wallpapers`() = runTest { + val fakeRemoteWallpapers = listOf("first", "second", "third").map { name -> + makeFakeRemoteWallpaper(TimeRelation.LATER, name) + } + every { mockSettings.currentWallpaper } returns "" + coEvery { mockFileManager.lookupExpiredWallpaper(any()) } returns null + + WallpapersUseCases.DefaultInitializeWallpaperUseCase( + appStore, + mockDownloader, + mockFileManager, + mockSettings, + "en-US", + possibleWallpapers = listOf(Wallpaper.Default) + fakeRemoteWallpapers + ).invoke() + + appStore.waitUntilIdle() + assertTrue(appStore.state.wallpaperState.availableWallpapers.contains(Wallpaper.Default)) + } + @Test fun `GIVEN wallpapers that expired WHEN invoking initialize use case THEN expired wallpapers are filtered out and cleaned up`() = runTest { val fakeRemoteWallpapers = listOf("first", "second", "third").map { name -> @@ -154,7 +196,7 @@ class WallpapersUseCasesTest { ).invoke() appStore.waitUntilIdle() - assertTrue(appStore.state.wallpaperState.currentWallpaper is Wallpaper.Default) + assertTrue(appStore.state.wallpaperState.currentWallpaper == Wallpaper.Default) } @Test @@ -213,7 +255,7 @@ class WallpapersUseCasesTest { timeRelation: TimeRelation, name: String = "name", isInPromo: Boolean = true - ): Wallpaper.Remote { + ): Wallpaper { fakeCalendar.time = baseFakeDate when (timeRelation) { TimeRelation.BEFORE -> fakeCalendar.add(Calendar.DATE, -5) @@ -222,9 +264,21 @@ class WallpapersUseCasesTest { } val relativeTime = fakeCalendar.time return if (isInPromo) { - Wallpaper.Remote.House(name = name, expirationDate = relativeTime) + Wallpaper( + name = name, + collectionName = "", + availableLocales = listOf("en-US"), + startDate = null, + endDate = relativeTime + ) } else { - Wallpaper.Remote.Firefox(name = name) + Wallpaper( + name = name, + collectionName = "", + availableLocales = null, + startDate = null, + endDate = relativeTime + ) } } }