diff --git a/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt b/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt index 66da7913..ba762d5c 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt @@ -3,6 +3,7 @@ package com.subtracks.data.db import androidx.room3.Dao import androidx.room3.Query import androidx.room3.Upsert +import com.subtracks.data.model.DownloadArtwork import com.subtracks.data.model.SongDownload import kotlinx.coroutines.flow.Flow @@ -14,6 +15,15 @@ interface DownloadDao { @Query("SELECT * FROM song_downloads") suspend fun all(): List + @Query( + "SELECT d.sourceId AS sourceId, al.coverArt AS albumCoverArt, ar.coverArt AS artistCoverArt " + + "FROM song_downloads d " + + "LEFT JOIN songs s ON s.sourceId = d.sourceId AND s.id = d.songId " + + "LEFT JOIN albums al ON al.sourceId = s.sourceId AND al.id = s.albumId " + + "LEFT JOIN artists ar ON ar.sourceId = s.sourceId AND ar.id = s.artistId", + ) + suspend fun artwork(): List + @Query("SELECT * FROM song_downloads WHERE sourceId = :sourceId AND songId = :songId") suspend fun find( sourceId: Long, diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 83916dd3..ade304dc 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -357,6 +357,18 @@ interface LibraryDao { albumId: String, ): Flow + @Query("SELECT * FROM albums WHERE sourceId = :sourceId AND id = :albumId") + suspend fun albumOnce( + sourceId: Long, + albumId: String, + ): Album? + + @Query("SELECT * FROM artists WHERE sourceId = :sourceId AND id = :artistId") + suspend fun artistOnce( + sourceId: Long, + artistId: String, + ): Artist? + @Query("SELECT * FROM artists WHERE sourceId = :sourceId AND id = :artistId") fun artist( sourceId: Long, diff --git a/app/src/main/kotlin/com/subtracks/data/download/ArtworkStore.kt b/app/src/main/kotlin/com/subtracks/data/download/ArtworkStore.kt new file mode 100644 index 00000000..495b52fb --- /dev/null +++ b/app/src/main/kotlin/com/subtracks/data/download/ArtworkStore.kt @@ -0,0 +1,81 @@ +package com.subtracks.data.download + +import android.net.Uri +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.OkHttpClient +import okhttp3.Request +import java.io.File +import java.io.IOException +import java.util.Base64 + +/** + * Cover art downloaded alongside the media, stored per source under `downloads//art`. + * + * Cover art ids come from the server, so keys are base64url encoded rather than used as file + * names: a key can contain separators or spaces and must never reach outside the art directory. + */ +class ArtworkStore( + private val downloadsDir: File, +) { + fun file( + sourceId: Long, + cacheKey: String, + ): File = artDir(sourceId).resolve(fileName(cacheKey)) + + fun uri( + sourceId: Long, + cacheKey: String, + ): String? = file(sourceId, cacheKey).takeIf { it.exists() }?.let { Uri.fromFile(it).toString() } + + fun write( + sourceId: Long, + cacheKey: String, + bytes: ByteArray, + ): Boolean { + val target = file(sourceId, cacheKey) + return runCatching { + target.parentFile?.mkdirs() + val partial = File(target.parentFile, target.name + PARTIAL_SUFFIX) + partial.writeBytes(bytes) + partial.renameTo(target) + }.getOrDefault(false) + } + + fun sweep( + sourceId: Long, + keep: Set, + ) { + val keepNames = keep.mapTo(HashSet(), ::fileName) + artDir(sourceId).listFiles().orEmpty().forEach { file -> + if (file.isFile && file.name !in keepNames) file.delete() + } + } + + private fun artDir(sourceId: Long): File = downloadsDir.resolve(sourceId.toString()).resolve(ART_DIR) + + private fun fileName(cacheKey: String): String = + Base64.getUrlEncoder().withoutPadding().encodeToString(cacheKey.toByteArray(Charsets.UTF_8)) + + private companion object { + const val ART_DIR = "art" + const val PARTIAL_SUFFIX = ".part" + } +} + +fun interface ArtworkFetcher { + suspend fun fetch(url: String): ByteArray +} + +class OkHttpArtworkFetcher( + private val http: OkHttpClient, +) : ArtworkFetcher { + override suspend fun fetch(url: String): ByteArray = + withContext(Dispatchers.IO) { + val request = Request.Builder().url(url).build() + http.newCall(request).execute().use { response -> + if (!response.isSuccessful) throw IOException("Artwork request failed with ${response.code}") + response.body?.bytes() ?: throw IOException("Artwork response had no body") + } + } +} diff --git a/app/src/main/kotlin/com/subtracks/data/model/CoverArtRef.kt b/app/src/main/kotlin/com/subtracks/data/model/CoverArtRef.kt index 34c52e3f..39c2e19a 100644 --- a/app/src/main/kotlin/com/subtracks/data/model/CoverArtRef.kt +++ b/app/src/main/kotlin/com/subtracks/data/model/CoverArtRef.kt @@ -4,3 +4,9 @@ data class CoverArtRef( val url: String, val cacheKey: String, ) + +fun coverArtKey( + sourceId: Long, + coverArt: String, + thumbnail: Boolean, +): String = "$sourceId:$coverArt:$thumbnail" diff --git a/app/src/main/kotlin/com/subtracks/data/model/Download.kt b/app/src/main/kotlin/com/subtracks/data/model/Download.kt index d9cb3eba..69e503a5 100644 --- a/app/src/main/kotlin/com/subtracks/data/model/Download.kt +++ b/app/src/main/kotlin/com/subtracks/data/model/Download.kt @@ -40,6 +40,12 @@ data class SongDownload( get() = total.takeIf { it > 0 }?.let { (bytes.toFloat() / it).coerceIn(0f, 1f) } } +data class DownloadArtwork( + val sourceId: Long, + val albumCoverArt: String?, + val artistCoverArt: String?, +) + class DownloadStatusConverter { @ColumnTypeConverter fun fromDownloadStatus(status: DownloadStatus): String = status.name diff --git a/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt index 6e794b2a..c3f1fdcd 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt @@ -2,12 +2,15 @@ package com.subtracks.data.repo import android.net.Uri import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.download.ArtworkFetcher +import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.DownloadEngine import com.subtracks.data.download.EngineDownload import com.subtracks.data.download.EngineRequest import com.subtracks.data.download.EngineStatus import com.subtracks.data.model.DownloadStatus import com.subtracks.data.model.SongDownload +import com.subtracks.data.model.coverArtKey import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope @@ -27,6 +30,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeoutOrNull import java.io.File class DownloadRepository( @@ -34,6 +38,8 @@ class DownloadRepository( private val sourceRepository: SourceRepository, private val engine: DownloadEngine, private val downloadsDir: File, + private val artworkStore: ArtworkStore, + private val artworkFetcher: ArtworkFetcher, private val showMessage: (String) -> Unit = {}, private val dispatcher: CoroutineDispatcher = Dispatchers.IO, ) { @@ -84,19 +90,23 @@ class DownloadRepository( sourceId: Long, songId: String, ) = withContext(dispatcher) { + if (sourceRepository.activeSourceIdOnce() != sourceId) { + showMessage(NOT_ACTIVE) + return@withContext + } + if (alreadyDownloadingOrDownloaded(sourceId, songId)) return@withContext + if (sourceRepository.downloadUri(sourceId, songId) == null) { + showMessage(NO_ADDRESS) + return@withContext + } + // Artwork runs in our process and is fetched while the app is in the foreground; the + // media itself is handed to the platform, which keeps going without us. + runCatching { withTimeoutOrNull(ART_TIMEOUT_MS) { fetchArtwork(sourceId, songId) } } mutex.withLock { - if (sourceRepository.activeSourceIdOnce() != sourceId) { - showMessage("Can't download from a source that isn't active") - return@withLock - } + if (alreadyDownloadingOrDownloaded(sourceId, songId)) return@withLock val existing = db.downloadDao().find(sourceId, songId) - if (existing != null && existing.status != DownloadStatus.Failed) return@withLock existing?.engineId?.let(engine::cancel) - val url = sourceRepository.downloadUri(sourceId, songId) - if (url == null) { - showMessage(NO_ADDRESS) - return@withLock - } + val url = sourceRepository.downloadUri(sourceId, songId) ?: return@withLock enqueue(sourceId, songId, url) } } @@ -110,6 +120,7 @@ class DownloadRepository( row.engineId?.let(engine::cancel) file(sourceId, songId).delete() db.downloadDao().delete(sourceId, songId) + sweep(db.downloadDao().all()) } } @@ -144,6 +155,57 @@ class DownloadRepository( .flatMapLatest { sourceId -> if (sourceId == null) flowOf(emptyList()) else db.downloadDao().downloads(sourceId) } .map { rows -> rows.associateBy { it.songId } } + private suspend fun alreadyDownloadingOrDownloaded( + sourceId: Long, + songId: String, + ): Boolean = + db + .downloadDao() + .find(sourceId, songId) + ?.status + ?.let { it != DownloadStatus.Failed } == true + + private suspend fun fetchArtwork( + sourceId: Long, + songId: String, + ) { + val song = db.libraryDao().songOnce(sourceId, songId) + val coverArt = + listOfNotNull( + song?.albumId?.let { db.libraryDao().albumOnce(sourceId, it)?.coverArt }, + song?.artistId?.let { db.libraryDao().artistOnce(sourceId, it)?.coverArt }, + ).distinct() + coverArt.forEach { cover -> + storeArtwork(sourceId, cover, thumbnail = false) + storeArtwork(sourceId, cover, thumbnail = true) + } + } + + private suspend fun storeArtwork( + sourceId: Long, + coverArt: String, + thumbnail: Boolean, + ) { + val cacheKey = coverArtKey(sourceId, coverArt, thumbnail) + if (artworkStore.file(sourceId, cacheKey).exists()) return + val url = sourceRepository.networkCoverArt(sourceId, coverArt, thumbnail) ?: return + val bytes = runCatching { artworkFetcher.fetch(url) }.getOrNull() ?: return + if (bytes.isEmpty()) return + artworkStore.write(sourceId, cacheKey, bytes) + } + + private suspend fun neededArtwork(): Map> = + db + .downloadDao() + .artwork() + .groupBy { it.sourceId } + .mapValues { (sourceId, rows) -> + rows + .flatMap { row -> listOfNotNull(row.albumCoverArt, row.artistCoverArt) } + .flatMap { cover -> listOf(false, true).map { thumbnail -> coverArtKey(sourceId, cover, thumbnail) } } + .toSet() + } + private suspend fun enqueue( sourceId: Long, songId: String, @@ -234,13 +296,14 @@ class DownloadRepository( } } - private fun sweep(rows: List) { - val known = rows.map { it.sourceId to it.songId }.toHashSet() - // Artwork (a later block) lives in subdirectories and is swept separately. + private suspend fun sweep(rows: List) { + val knownSongs = rows.map { it.sourceId to it.songId }.toHashSet() + val knownArt = neededArtwork() downloadsDir.listFiles().orEmpty().filter { it.isDirectory }.forEach { sourceDir -> val sourceId = sourceDir.name.toLongOrNull() ?: return@forEach + artworkStore.sweep(sourceId, knownArt[sourceId].orEmpty()) sourceDir.listFiles().orEmpty().filter { it.isFile }.forEach { file -> - if ((sourceId to file.name) !in known) file.delete() + if ((sourceId to file.name) !in knownSongs) file.delete() } } } @@ -267,5 +330,7 @@ class DownloadRepository( const val MISSING_FILE = "The downloaded file is missing" const val NO_ADDRESS = "Can't download: the server address is unavailable" const val STOPPED = "The download stopped unexpectedly" + const val NOT_ACTIVE = "Can't download from a source that isn't active" + const val ART_TIMEOUT_MS = 10_000L } } diff --git a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt index 81b5c432..863f5ea9 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt @@ -1,10 +1,12 @@ package com.subtracks.data.repo import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.download.ArtworkStore import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.Source import com.subtracks.data.model.SubsonicConfig import com.subtracks.data.model.SubsonicSource +import com.subtracks.data.model.coverArtKey import com.subtracks.data.net.NetworkMode import com.subtracks.data.prefs.StreamQuality import com.subtracks.data.prefs.UserPreferences @@ -31,6 +33,7 @@ class SourceRepository( private val db: SubtracksDatabase, private val http: OkHttpClient, private val prefs: UserPreferences, + private val artworkStore: ArtworkStore, networkMode: Flow = flowOf(NetworkMode.Wifi), private val showMessage: (String) -> Unit = {}, ) { @@ -81,11 +84,20 @@ class SourceRepository( coverArt: String?, thumbnail: Boolean = false, ): CoverArtRef? { - val source = active ?: return null - val url = source.coverArtUri(coverArt, thumbnail)?.toString() ?: return null - return CoverArtRef(url = url, cacheKey = "${source.id}:$coverArt:$thumbnail") + val sourceId = activeSourceId ?: return null + if (coverArt == null) return null + val cacheKey = coverArtKey(sourceId, coverArt, thumbnail) + artworkStore.uri(sourceId, cacheKey)?.let { return CoverArtRef(url = it, cacheKey = cacheKey) } + val url = active?.coverArtUri(coverArt, thumbnail)?.toString() ?: return null + return CoverArtRef(url = url, cacheKey = cacheKey) } + fun networkCoverArt( + sourceId: Long, + coverArt: String, + thumbnail: Boolean, + ): String? = if (activeSourceId == sourceId) active?.coverArtUri(coverArt, thumbnail)?.toString() else null + fun streamUri( songId: String, durationMs: Long?, diff --git a/app/src/main/kotlin/com/subtracks/di/AppModule.kt b/app/src/main/kotlin/com/subtracks/di/AppModule.kt index 67d49c5e..9e3959c3 100644 --- a/app/src/main/kotlin/com/subtracks/di/AppModule.kt +++ b/app/src/main/kotlin/com/subtracks/di/AppModule.kt @@ -6,7 +6,10 @@ import android.os.Handler import android.os.Looper import android.widget.Toast import com.subtracks.data.db.createAndroidDatabase +import com.subtracks.data.download.ArtworkFetcher +import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.DownloadEngine +import com.subtracks.data.download.OkHttpArtworkFetcher import com.subtracks.data.download.SystemDownloadEngine import com.subtracks.data.net.networkMode import com.subtracks.data.prefs.createUserPreferences @@ -44,18 +47,22 @@ fun appModule( single { createAndroidDatabase(context) } single { http } single { createUserPreferences(context) } - single { SourceRepository(get(), get(), get(), networkMode = networkMode(context.applicationContext), showMessage = toast) } + single { SourceRepository(get(), get(), get(), get(), networkMode = networkMode(context.applicationContext), showMessage = toast) } single { LibraryRepository(get(), get(), toast) } single { ArtworkSeedRepository(get()) } single { SyncManager(get(), get(), get()) } single { QueueRepository(get()) } + single { ArtworkStore(downloadsRoot(context)) } + single { OkHttpArtworkFetcher(get()) } single { SystemDownloadEngine(context.applicationContext) } single { DownloadRepository( db = get(), sourceRepository = get(), engine = get(), - downloadsDir = File(context.getExternalFilesDir(Environment.DIRECTORY_MUSIC) ?: context.filesDir, "downloads"), + downloadsDir = downloadsRoot(context), + artworkStore = get(), + artworkFetcher = get(), showMessage = toast, ).also { it.start() } } @@ -70,3 +77,6 @@ fun appModule( viewModel { params -> PlaylistDetailViewModel(get(), get(), get(), get(), params.get()) } viewModel { QueueViewModel(get(), get()) } } + +private fun downloadsRoot(context: Context): File = + File(context.getExternalFilesDir(Environment.DIRECTORY_MUSIC) ?: context.filesDir, "downloads") diff --git a/app/src/test/kotlin/com/subtracks/data/download/ArtworkStoreTest.kt b/app/src/test/kotlin/com/subtracks/data/download/ArtworkStoreTest.kt new file mode 100644 index 00000000..73800e54 --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/data/download/ArtworkStoreTest.kt @@ -0,0 +1,65 @@ +package com.subtracks.data.download + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import java.io.File + +@RunWith(AndroidJUnit4::class) +class ArtworkStoreTest { + private val context = ApplicationProvider.getApplicationContext() + private val root = File(context.cacheDir, "art-${System.nanoTime()}") + private val store = ArtworkStore(root) + + @Test + fun storedBytesComeBackThroughTheFile() { + store.write(1, "1:art-1:false", byteArrayOf(1, 2, 3)) + + assertArrayEquals(byteArrayOf(1, 2, 3), store.file(1, "1:art-1:false").readBytes()) + assertTrue(store.uri(1, "1:art-1:false")!!.startsWith("file:")) + } + + @Test + fun anUnstoredKeyHasNoUri() { + assertNull(store.uri(1, "1:missing:false")) + } + + @Test + fun aServerSuppliedCoverArtIdCannotEscapeTheArtDirectory() { + val hostile = "1:x/../../../../escape:false" + + store.write(1, hostile, byteArrayOf(1)) + + assertTrue(store.file(1, hostile).canonicalPath.startsWith(root.canonicalPath + File.separator)) + } + + @Test + fun sweepKeepsWhatIsStillNeededAndDropsTheRest() { + store.write(1, "1:keep:false", byteArrayOf(1)) + store.write(1, "1:drop:false", byteArrayOf(1)) + store.write(2, "2:other:false", byteArrayOf(1)) + + store.sweep(1, setOf("1:keep:false")) + + assertTrue(store.file(1, "1:keep:false").exists()) + assertFalse(store.file(1, "1:drop:false").exists()) + assertTrue(store.file(2, "2:other:false").exists()) + } + + @Test + fun anUnwritableArtDirectoryIsReportedRatherThanThrown() { + File(root, "1/art").apply { + parentFile?.mkdirs() + writeText("not a directory") + } + + assertFalse(store.write(1, "1:art-1:false", byteArrayOf(1))) + assertFalse(store.file(1, "1:art-1:false").exists()) + } +} diff --git a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt index 3e9dbb80..5026c135 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt @@ -7,12 +7,17 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.download.ArtworkFetcher +import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine +import com.subtracks.data.model.Album +import com.subtracks.data.model.Artist import com.subtracks.data.model.DownloadStatus import com.subtracks.data.model.Song import com.subtracks.data.model.SongDownload import com.subtracks.data.model.Source import com.subtracks.data.model.SubsonicSource +import com.subtracks.data.model.coverArtKey import com.subtracks.data.prefs.fakeUserPreferences import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest @@ -21,12 +26,14 @@ import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import java.io.File +import java.io.IOException import java.util.concurrent.CopyOnWriteArrayList @RunWith(AndroidJUnit4::class) @@ -35,8 +42,17 @@ class DownloadRepositoryTest { private lateinit var sources: SourceRepository private lateinit var engine: FakeDownloadEngine private lateinit var repository: DownloadRepository + private lateinit var artwork: ArtworkStore private lateinit var dir: File private val messages = CopyOnWriteArrayList() + private val requestedArt = CopyOnWriteArrayList() + private var failArtworkFetch = false + private val fetcher = + ArtworkFetcher { url -> + requestedArt += url + if (failArtworkFetch) throw IOException("boom") + byteArrayOf(1, 2, 3) + } @Before fun setUp() { @@ -46,10 +62,20 @@ class DownloadRepositoryTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - sources = SourceRepository(db, OkHttpClient(), fakeUserPreferences()) + artwork = ArtworkStore(File(context.cacheDir, "art-${System.nanoTime()}")) + sources = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), artwork) engine = FakeDownloadEngine() dir = File(context.cacheDir, "downloads-${System.nanoTime()}") - repository = DownloadRepository(db, sources, engine, dir, showMessage = { messages += it }) + repository = + DownloadRepository( + db, + sources, + engine, + dir, + artworkStore = artwork, + artworkFetcher = fetcher, + showMessage = { messages += it }, + ) } @After @@ -270,6 +296,108 @@ class DownloadRepositoryTest { assertFalse(file(1, "orphan").exists()) } + @Test + fun startingADownloadAlsoStoresTheAlbumAndArtistArtwork() = + runTest { + seedLibrary() + + runBlocking { repository.download(1, "s1") } + + assertEquals(4, requestedArt.size) + listOf(ALBUM_ART, ARTIST_ART).forEach { cover -> + listOf(false, true).forEach { thumbnail -> + val key = coverArtKey(1, cover, thumbnail) + assertNotNull("expected stored art for $key", artwork.uri(1, key)) + } + } + } + + @Test + fun aSongWithoutArtworkStillDownloads() = + runTest { + seedLibrary() + runBlocking { + db.libraryDao().upsertAlbums(listOf(album(coverArt = null))) + db.libraryDao().upsertArtists(listOf(artist(coverArt = null))) + } + + runBlocking { repository.download(1, "s1") } + + assertTrue(requestedArt.isEmpty()) + assertEquals(1, engine.requests.size) + } + + @Test + fun aFailedArtworkFetchDoesNotStopTheDownload() = + runTest { + seedLibrary() + failArtworkFetch = true + + runBlocking { repository.download(1, "s1") } + + assertEquals(1, engine.requests.size) + assertEquals(DownloadStatus.Queued, row(1, "s1")?.status) + assertNull(artwork.uri(1, coverArtKey(1, ALBUM_ART, false))) + } + + @Test + fun artworkThatIsAlreadyStoredIsNotFetchedAgain() = + runTest { + seedLibrary() + listOf(ALBUM_ART, ARTIST_ART).forEach { cover -> + listOf(false, true).forEach { thumbnail -> + artwork.write(1, coverArtKey(1, cover, thumbnail), byteArrayOf(9)) + } + } + + runBlocking { repository.download(1, "s1") } + + assertTrue(requestedArt.isEmpty()) + assertEquals(1, engine.requests.size) + } + + @Test + fun removingTheLastDownloadOfAnAlbumRemovesItsArtwork() = + runTest { + seedLibrary() + runBlocking { repository.download(1, "s1") } + + runBlocking { repository.remove(1, "s1") } + + assertNull(artwork.uri(1, coverArtKey(1, ALBUM_ART, false))) + assertNull(artwork.uri(1, coverArtKey(1, ARTIST_ART, false))) + } + + @Test + fun artworkAnotherDownloadStillNeedsIsKept() = + runTest { + seedLibrary() + runBlocking { repository.download(1, "s1") } + runBlocking { repository.download(1, "s2") } + + runBlocking { repository.remove(1, "s1") } + + assertNotNull(artwork.uri(1, coverArtKey(1, ALBUM_ART, false))) + } + + private fun album(coverArt: String?) = + Album( + sourceId = 1, + id = "al1", + artistId = "ar1", + name = "Album", + albumArtist = "Artist", + created = 0, + coverArt = coverArt, + genre = null, + year = 2000, + starred = null, + songCount = 2, + ) + + private fun artist(coverArt: String?) = + Artist(sourceId = 1, id = "ar1", name = "Artist", albumCount = 1, starred = null, coverArt = coverArt) + private fun seedLibrary() { runBlocking { db.sourcesDao().upsertSource(Source(1, "source", "http://localhost:4533/", isActive = true, createdAt = 0)) @@ -290,8 +418,24 @@ class DownloadRepositoryTest { starred = null, genre = null, ), + Song( + sourceId = 1, + id = "s2", + albumId = "al1", + artistId = "ar1", + title = "Song 2", + album = "Album", + artist = "Artist", + duration = 100, + track = 2, + disc = 1, + starred = null, + genre = null, + ), ), ) + db.libraryDao().upsertAlbums(listOf(album(ALBUM_ART))) + db.libraryDao().upsertArtists(listOf(artist(ARTIST_ART))) } await { sources.downloadUri(1, "s1") != null } } @@ -323,4 +467,9 @@ class DownloadRepositoryTest { while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10) assertTrue("Timed out waiting for the expected download state", predicate()) } + + private companion object { + const val ALBUM_ART = "art-al1" + const val ARTIST_ART = "art-ar1" + } } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt index 5cbca415..24e38dd8 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt @@ -6,6 +6,7 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.download.ArtworkStore import com.subtracks.data.model.Song import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.source.StarType @@ -25,6 +26,7 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import java.io.File import java.util.concurrent.TimeUnit @RunWith(AndroidJUnit4::class) @@ -42,7 +44,7 @@ class LibraryRepositoryTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences()) + sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art"))) repository = LibraryRepository(db, sourceRepository, showMessage = { messages += it }) } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt index db67244e..e6d22d10 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt @@ -6,6 +6,9 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.download.ArtworkStore +import com.subtracks.data.model.CoverArtRef +import com.subtracks.data.model.coverArtKey import com.subtracks.data.prefs.fakeUserPreferences import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first @@ -22,11 +25,13 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import java.io.File @RunWith(AndroidJUnit4::class) class SourceRepositoryTest { private lateinit var db: SubtracksDatabase private lateinit var repository: SourceRepository + private lateinit var artwork: ArtworkStore private val messages = ArrayList() @Before @@ -37,7 +42,8 @@ class SourceRepositoryTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - repository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), showMessage = { messages += it }) + artwork = ArtworkStore(File(context.cacheDir, "art-${System.nanoTime()}")) + repository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), artwork, showMessage = { messages += it }) } @After @@ -95,6 +101,41 @@ class SourceRepositoryTest { } } + @Test + fun downloadedArtworkIsPreferredOverTheServer() = + runBlocking { + repository.addSource("server", "http://a.example/", "u", "p", true) + val sourceId = awaitActiveSourceId() + artwork.write(sourceId, coverArtKey(sourceId, "art-1", false), byteArrayOf(1, 2, 3)) + + val ref = awaitCoverArt("art-1") + + assertTrue("expected the stored file, was ${ref.url}", ref.url.startsWith("file:")) + assertEquals(coverArtKey(sourceId, "art-1", false), ref.cacheKey) + } + + @Test + fun coverArtFallsBackToTheServerWithoutDownloadedArtwork() = + runBlocking { + repository.addSource("server", "http://a.example/", "u", "p", true) + + val ref = awaitCoverArt("art-1") + + assertTrue("expected the server url, was ${ref.url}", ref.url.startsWith("http")) + } + + private suspend fun awaitActiveSourceId(): Long = withTimeout(5_000) { repository.activeSourceId().first { it != null }!! } + + private suspend fun awaitCoverArt(coverArt: String): CoverArtRef = + withTimeout(5_000) { + var ref = repository.coverArt(coverArt) + while (ref == null) { + delay(10) + ref = repository.coverArt(coverArt) + } + ref + } + private suspend fun withServer(block: suspend (MockWebServer) -> Unit) { val server = MockWebServer() server.start() diff --git a/app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt b/app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt index b3e199a9..f58710cb 100644 --- a/app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt @@ -6,6 +6,7 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.download.ArtworkStore import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.repo.QueueRepository import com.subtracks.data.repo.SourceRepository @@ -19,6 +20,7 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import java.io.File @RunWith(AndroidJUnit4::class) class SyncManagerTest { @@ -35,7 +37,7 @@ class SyncManagerTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences()) + sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art"))) queueRepository = QueueRepository(db) manager = SyncManager(db, sourceRepository, queueRepository) } diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index f3cb3ae2..b71001dd 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -7,6 +7,7 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine import com.subtracks.data.model.Album import com.subtracks.data.model.QueueEntry @@ -64,9 +65,17 @@ class PlaybackControllerTest { .build() prefs = fakeUserPreferences() networkMode = MutableStateFlow(NetworkMode.Wifi) - sources = SourceRepository(db, OkHttpClient(), prefs, networkMode = networkMode) + sources = SourceRepository(db, OkHttpClient(), prefs, ArtworkStore(File(context.cacheDir, "art")), networkMode = networkMode) queues = QueueRepository(db) - downloads = DownloadRepository(db, sources, FakeDownloadEngine(), File(context.cacheDir, "downloads")) + downloads = + DownloadRepository( + db, + sources, + FakeDownloadEngine(), + File(context.cacheDir, "downloads"), + artworkStore = ArtworkStore(File(context.cacheDir, "art")), + artworkFetcher = { ByteArray(0) }, + ) handle = FakePlayerHandle() controller = PlaybackController( diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt index abc2b358..1a88da11 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -6,6 +6,7 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine import com.subtracks.data.model.QueueKind import com.subtracks.data.model.Song @@ -55,9 +56,18 @@ class QueueViewModelTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - val sources = SourceRepository(db, OkHttpClient(), fakeUserPreferences()) + val sources = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art"))) queues = QueueRepository(db) - val downloads = DownloadRepository(db, sources, FakeDownloadEngine(), File(context.cacheDir, "downloads")) + val artwork = ArtworkStore(File(context.cacheDir, "art")) + val downloads = + DownloadRepository( + db, + sources, + FakeDownloadEngine(), + File(context.cacheDir, "downloads"), + artworkStore = artwork, + artworkFetcher = { ByteArray(0) }, + ) controller = PlaybackController(sources, queues, FakePlayerConnection(FakePlayerHandle()), downloads, dispatcher = dispatcher) viewModel = QueueViewModel(queues, controller) }