download a song's artwork with it and use it before the network

Second block of #65.

- ArtworkStore keeps the album's and the artist's cover art, thumbnail and full,
  in downloads/<sourceId>/art, keyed by the cache key the rest of the app already
  uses for Coil and for palettes. Keys are base64url encoded because cover art ids
  come from the server and must not reach outside the art directory.
- Downloading a song fetches that artwork first, in our process and while the app
  is in the foreground, under a timeout, and hands the media to the platform
  afterwards whether or not the artwork arrived. A song whose album or artist has
  no art, or whose art fetch fails, still downloads.
- SourceRepository.coverArt resolves a stored file before the server, at the one
  point every CoverArt, the notification bitmap loader and the palette prefetch go
  through, so downloaded artwork works without the network.
- The sweep prunes art that no download references, so removing the last download
  of an album drops its art while art another download still needs stays.

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<Context>()
+    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())
+    }
+}

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<String>()
+    private val requestedArt = CopyOnWriteArrayList<String>()
+    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"
+    }
 }

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 })
     }
 

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<String>()
 
     @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()

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)
     }

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(

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)
     }