From a97a5e0687a5a4f64087fbfc206cadf11fa30303 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:29:52 +0000 Subject: [PATCH] bound artwork seeds and size the now-playing prefetch --- .../com/subtracks/data/db/ArtworkSeedDao.kt | 12 ++ .../com/subtracks/data/db/Migrations.kt | 16 +++ .../subtracks/data/db/SubtracksDatabase.kt | 2 +- .../kotlin/com/subtracks/data/model/Models.kt | 15 ++- .../data/repo/ArtworkSeedRepository.kt | 15 ++- .../subtracks/playback/PlaybackController.kt | 5 +- .../subtracks/ui/playback/NowPlayingScreen.kt | 32 ++++-- .../subtracks/data/db/ArtworkSeedDaoTest.kt | 103 ++++++++++++++++++ .../playback/PlaybackControllerTest.kt | 34 ++++++ .../ui/playback/NowPlayingArtworkTest.kt | 39 +++++++ 10 files changed, 255 insertions(+), 18 deletions(-) create mode 100644 app/src/test/kotlin/com/subtracks/data/db/ArtworkSeedDaoTest.kt create mode 100644 app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingArtworkTest.kt diff --git a/app/src/main/kotlin/com/subtracks/data/db/ArtworkSeedDao.kt b/app/src/main/kotlin/com/subtracks/data/db/ArtworkSeedDao.kt index d45d80c8..a6c895d9 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/ArtworkSeedDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/ArtworkSeedDao.kt @@ -12,4 +12,16 @@ interface ArtworkSeedDao { @Upsert suspend fun upsert(seed: ArtworkSeed) + + @Query("UPDATE artwork_seeds SET lastUsed = :lastUsed WHERE cacheKey = :cacheKey") + suspend fun touch( + cacheKey: String, + lastUsed: Long, + ) + + @Query( + "DELETE FROM artwork_seeds WHERE cacheKey NOT IN " + + "(SELECT cacheKey FROM artwork_seeds ORDER BY lastUsed DESC LIMIT :keep)", + ) + suspend fun prune(keep: Int) } diff --git a/app/src/main/kotlin/com/subtracks/data/db/Migrations.kt b/app/src/main/kotlin/com/subtracks/data/db/Migrations.kt index eb6120d9..456f5de6 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/Migrations.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/Migrations.kt @@ -112,6 +112,21 @@ val MIGRATION_11_12 = } } +val MIGRATION_12_13 = + object : Migration(12, 13) { + override suspend fun migrate(connection: SQLiteConnection) { + connection.execSQL("DROP TABLE IF EXISTS `artwork_seeds`") + connection.execSQL( + "CREATE TABLE IF NOT EXISTS `artwork_seeds` (`cacheKey` TEXT NOT NULL, " + + "`primary` INTEGER NOT NULL, `secondary` INTEGER, `nameBusy` INTEGER, " + + "`sourceId` INTEGER NOT NULL, `lastUsed` INTEGER NOT NULL DEFAULT 0, " + + "PRIMARY KEY(`cacheKey`), " + + "FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + ) + connection.execSQL("CREATE INDEX IF NOT EXISTS `index_artwork_seeds_sourceId` ON `artwork_seeds` (`sourceId`)") + } + } + val MIGRATIONS: Array = arrayOf( MIGRATION_1_2, @@ -125,4 +140,5 @@ val MIGRATIONS: Array = MIGRATION_9_10, MIGRATION_10_11, MIGRATION_11_12, + MIGRATION_12_13, ) diff --git a/app/src/main/kotlin/com/subtracks/data/db/SubtracksDatabase.kt b/app/src/main/kotlin/com/subtracks/data/db/SubtracksDatabase.kt index 8161808a..9c3cb186 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/SubtracksDatabase.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/SubtracksDatabase.kt @@ -32,7 +32,7 @@ import com.subtracks.data.model.SubsonicSource ShuffleOrder::class, ArtworkSeed::class, ], - version = 12, + version = 13, exportSchema = true, ) @ColumnTypeConverters(QueueKindConverter::class) diff --git a/app/src/main/kotlin/com/subtracks/data/model/Models.kt b/app/src/main/kotlin/com/subtracks/data/model/Models.kt index 9721745c..9b163429 100644 --- a/app/src/main/kotlin/com/subtracks/data/model/Models.kt +++ b/app/src/main/kotlin/com/subtracks/data/model/Models.kt @@ -237,12 +237,25 @@ data class ShuffleOrder( val flatPosition: Long, ) -@Entity(tableName = "artwork_seeds") +@Entity( + tableName = "artwork_seeds", + foreignKeys = [ + ForeignKey( + entity = Source::class, + parentColumns = ["id"], + childColumns = ["sourceId"], + onDelete = ForeignKey.CASCADE, + ), + ], + indices = [Index("sourceId")], +) data class ArtworkSeed( @PrimaryKey val cacheKey: String, val primary: Int, val secondary: Int?, val nameBusy: Boolean? = null, + val sourceId: Long = 0, + @ColumnInfo(defaultValue = "0") val lastUsed: Long = 0, ) class QueueKindConverter { diff --git a/app/src/main/kotlin/com/subtracks/data/repo/ArtworkSeedRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/ArtworkSeedRepository.kt index 0c0c7f02..096323dd 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/ArtworkSeedRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/ArtworkSeedRepository.kt @@ -3,6 +3,8 @@ package com.subtracks.data.repo import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.model.ArtworkSeed +private const val MAX_STORED_SEEDS = 1024 + interface ArtworkSeedStore { suspend fun seed(cacheKey: String): ArtworkSeed? @@ -11,10 +13,19 @@ interface ArtworkSeedStore { class ArtworkSeedRepository( private val db: SubtracksDatabase, + private val maxStoredSeeds: Int = MAX_STORED_SEEDS, ) : ArtworkSeedStore { - override suspend fun seed(cacheKey: String): ArtworkSeed? = db.artworkSeedDao().seed(cacheKey) + override suspend fun seed(cacheKey: String): ArtworkSeed? { + val dao = db.artworkSeedDao() + val row = dao.seed(cacheKey) ?: return null + dao.touch(cacheKey, System.currentTimeMillis()) + return row + } override suspend fun save(seed: ArtworkSeed) { - db.artworkSeedDao().upsert(seed) + val sourceId = seed.cacheKey.substringBefore(':').toLongOrNull() ?: return + val dao = db.artworkSeedDao() + dao.upsert(seed.copy(sourceId = sourceId, lastUsed = System.currentTimeMillis())) + dao.prune(maxStoredSeeds) } } diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index d385302f..7b0ae6af 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -580,11 +580,10 @@ class PlaybackController( thumbnail: Boolean = false, ): CoverArtRef? = sourceRepository.coverArt(item?.coverArtId, thumbnail) - suspend fun upcomingCoverArt(thumbnail: Boolean = false): CoverArtRef? { + suspend fun upcomingItem(): QueueItem? { val snap = snapshot ?: return null val position = currentPosition() ?: return null - val next = queueRepository.itemAt(snap, position + 1) ?: return null - return sourceRepository.coverArt(next.toQueueItem().coverArtId, thumbnail) + return queueRepository.itemAt(snap, position + 1)?.toQueueItem() } private fun play( diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt index 49a1a026..e9d4d399 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt @@ -98,8 +98,10 @@ fun NowPlayingRoute( } LaunchedEffect(state.item?.id, state.hasNext) { if (!state.hasNext) return@LaunchedEffect - controller.upcomingCoverArt(thumbnail = true)?.let { ArtworkSeedCache.prefetch(context, it) } - controller.upcomingCoverArt()?.let { prefetchImage(context, it) } + controller.upcomingItem()?.let { next -> + controller.coverArt(next, thumbnail = true)?.let { ArtworkSeedCache.prefetch(context, it) } + controller.coverArt(next)?.let { prefetchImage(context, it) } + } } NowPlayingScreen( state = state, @@ -120,18 +122,26 @@ fun NowPlayingRoute( ) } -private fun prefetchImage( +internal fun prefetchImage( context: Context, ref: CoverArtRef, ) { - SingletonImageLoader.get(context).enqueue( - ImageRequest - .Builder(context) - .data(ref.url) - .memoryCacheKey(ref.cacheKey) - .diskCacheKey(ref.cacheKey) - .build(), - ) + SingletonImageLoader.get(context).enqueue(prefetchImageRequest(context, ref)) +} + +internal fun prefetchImageRequest( + context: Context, + ref: CoverArtRef, +): ImageRequest { + val metrics = context.resources.displayMetrics + val maxDimension = maxOf(metrics.widthPixels, metrics.heightPixels) + return ImageRequest + .Builder(context) + .data(ref.url) + .memoryCacheKey(ref.cacheKey) + .diskCacheKey(ref.cacheKey) + .size(maxDimension) + .build() } @OptIn(ExperimentalMaterial3Api::class) diff --git a/app/src/test/kotlin/com/subtracks/data/db/ArtworkSeedDaoTest.kt b/app/src/test/kotlin/com/subtracks/data/db/ArtworkSeedDaoTest.kt new file mode 100644 index 00000000..72091bcd --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/data/db/ArtworkSeedDaoTest.kt @@ -0,0 +1,103 @@ +package com.subtracks.data.db + +import android.content.Context +import androidx.room3.Room +import androidx.sqlite.driver.bundled.BundledSQLiteDriver +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.data.model.ArtworkSeed +import com.subtracks.data.model.Source +import com.subtracks.data.repo.ArtworkSeedRepository +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ArtworkSeedDaoTest { + private lateinit var db: SubtracksDatabase + + @Before + fun setUp() { + val context = ApplicationProvider.getApplicationContext() + db = + Room + .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) + .setDriver(BundledSQLiteDriver()) + .build() + } + + @After + fun tearDown() { + db.close() + } + + @Test + fun deletingASourceCascadesToItsSeeds() = + runTest { + val sourceId = source() + val dao = db.artworkSeedDao() + dao.upsert(ArtworkSeed(cacheKey = "$sourceId:cover:false", sourceId = sourceId, primary = 1, secondary = null)) + + db.sourcesDao().deleteSource(sourceId) + + assertNull(dao.seed("$sourceId:cover:false")) + } + + @Test + fun pruningKeepsTheMostRecentlyUsedSeeds() = + runTest { + val sourceId = source() + val dao = db.artworkSeedDao() + (1..5).forEach { index -> + dao.upsert( + ArtworkSeed( + cacheKey = "$sourceId:cover$index:false", + sourceId = sourceId, + primary = index, + secondary = null, + lastUsed = index.toLong(), + ), + ) + } + + dao.prune(keep = 2) + + val remaining = (1..5).mapNotNull { dao.seed("$sourceId:cover$it:false")?.cacheKey }.toSet() + assertEquals(setOf("$sourceId:cover5:false", "$sourceId:cover4:false"), remaining) + } + + @Test + fun savingDerivesTheSourceFromTheCacheKey() = + runTest { + val sourceId = source() + val repository = ArtworkSeedRepository(db) + + repository.save("$sourceId:cover:true", 10 to 20) + + val row = db.artworkSeedDao().seed("$sourceId:cover:true") + assertEquals(sourceId, row?.sourceId) + assertEquals(10, row?.primary) + assertEquals(20, row?.secondary) + } + + @Test + fun savingBeyondTheCapPrunesTheOldestSeeds() = + runTest { + val sourceId = source() + val repository = ArtworkSeedRepository(db, maxStoredSeeds = 2) + + repository.save("$sourceId:cover1:false", 1 to null) + repository.save("$sourceId:cover2:false", 2 to null) + repository.save("$sourceId:cover3:false", 3 to null) + + val stored = (1..3).count { db.artworkSeedDao().seed("$sourceId:cover$it:false") != null } + assertEquals(2, stored) + } + + private suspend fun source(): Long = + db.sourcesDao().insertSource(Source(name = "Navidrome", address = "http://localhost", isActive = true, createdAt = 0)) +} diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 2bb6e02d..33be4db5 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.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.model.Album import com.subtracks.data.model.QueueEntry import com.subtracks.data.model.QueueKind import com.subtracks.data.model.Song @@ -74,6 +75,39 @@ class PlaybackControllerTest { assertEquals(listOf("prepare", "play"), handle.operations.takeLast(2)) } + @Test + fun upcomingItemIsTheNextQueueItemWithItsCoverArt() { + seedAlbum(3, sourceId = 1) + runBlocking { + db.libraryDao().upsertAlbums( + listOf( + Album( + sourceId = 1, + id = "al1", + artistId = "ar1", + name = "Album", + albumArtist = "Artist", + created = 0, + coverArt = "cover-al1", + genre = null, + year = null, + starred = null, + songCount = 3, + frequentRank = null, + recentRank = null, + ), + ), + ) + } + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + + assertEquals("cover-al1", runBlocking { controller.upcomingItem()?.coverArtId }) + } + @Test fun playingTheSongsListQueuesTheWholeLibrary() { seedAlbum(60, sourceId = 1) diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingArtworkTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingArtworkTest.kt new file mode 100644 index 00000000..7c78cffc --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingArtworkTest.kt @@ -0,0 +1,39 @@ +package com.subtracks.ui.playback + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import coil3.SingletonImageLoader +import coil3.annotation.DelicateCoilApi +import coil3.size.Dimension +import com.subtracks.data.model.CoverArtRef +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith + +@OptIn(DelicateCoilApi::class) +@RunWith(AndroidJUnit4::class) +class NowPlayingArtworkTest { + @After + fun resetImageLoader() { + SingletonImageLoader.reset() + } + + @Test + fun prefetchingTheNextOriginalIsBoundedToTheScreen() = + runBlocking { + val context = ApplicationProvider.getApplicationContext() + val ref = CoverArtRef(url = "http://example.test/art", cacheKey = "1:art:false") + + val request = prefetchImageRequest(context, ref) + val resolved = request.sizeResolver.size() + val width = (resolved.width as Dimension.Pixels).px + + val metrics = context.resources.displayMetrics + assertEquals(maxOf(metrics.widthPixels, metrics.heightPixels), width) + assertEquals(ref.cacheKey, request.memoryCacheKey) + assertEquals(ref.cacheKey, request.diskCacheKey) + } +}