address review: drop the dead disc query, pin the fetch bound, cover multi-page disc prune

This commit is contained in:
austinried
2026-10-04 00:29:59 +00:00
parent 34f15176d7
commit 0fb2ce2918
4 changed files with 38 additions and 6 deletions
@@ -82,9 +82,6 @@ interface LibraryDao {
limit: Int,
): List<String>
@Query("SELECT albumId, disc FROM discs WHERE sourceId = :sourceId")
suspend fun discKeys(sourceId: Long): List<DiscKey>
@Query(
"SELECT albumId, disc FROM discs WHERE sourceId = :sourceId " +
"AND (albumId > :afterAlbumId OR (albumId = :afterAlbumId AND disc > :afterDisc)) " +
@@ -282,11 +282,12 @@ class SubsonicSource(
private companion object {
const val PAGE_SIZE = 500
const val MAX_PAGES = 20_000
const val MAX_CONCURRENT_FETCHES = 4
const val THUMBNAIL_SIZE = 256
}
}
internal const val MAX_CONCURRENT_FETCHES = 4
private class PageCapExceeded(
message: String,
) : SubsonicException(-1, message)
@@ -312,7 +312,7 @@ class SubsonicSourceTest {
assertEquals(listOf(0L, 1L, 2L), batch.sortedBy { it.position }.map { it.position })
assertEquals((1..3).map { "$playlistId-s$it" }, batch.sortedBy { it.position }.map { it.songId })
}
assertTrue(maxInFlight.get() >= 2)
assertTrue(maxInFlight.get() in 2..MAX_CONCURRENT_FETCHES)
}
@Test
@@ -389,7 +389,7 @@ class SubsonicSourceTest {
val songs = SubsonicSource(1, client()).songs().toList().flatten()
assertEquals(4, songs.size)
assertTrue(maxInFlight.get() >= 2)
assertTrue(maxInFlight.get() in 2..MAX_CONCURRENT_FETCHES)
}
@Test
@@ -10,6 +10,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import com.subtracks.data.db.SubtracksDatabase
import com.subtracks.data.model.Album
import com.subtracks.data.model.Artist
import com.subtracks.data.model.DiscKey
import com.subtracks.data.model.Playlist
import com.subtracks.data.model.PlaylistSong
import com.subtracks.data.model.Song
@@ -542,6 +543,34 @@ class SyncServiceTest {
assertEquals(survivors.sorted(), db.libraryDao().artistIds(1).sorted())
}
@Test
fun partialDiscPruneAcrossManyPagesKeepsTheSurvivors() =
runTest {
insertSource()
val total = 1100
fun albumsWithDiscTitles(predicate: (Int) -> Boolean) =
(1..total).map { index ->
if (predicate(index)) {
album("al$index").copy(discTitles = mapOf(1L to "Disc $index"))
} else {
album("al$index")
}
}
val source = FakeMusicSource(albums = albumsWithDiscTitles { true })
SyncService(db, source).sync()
assertEquals(total, storedDiscKeys().size)
source.albums = albumsWithDiscTitles { it % 2 == 0 }
SyncService(db, source).sync()
val survivors = (1..total).filter { it % 2 == 0 }.map { "al$it" }.toSet()
assertEquals(survivors.size, storedDiscKeys().size)
assertEquals(survivors, storedDiscKeys().map { it.albumId }.toSet())
}
@Test
fun anIdenticalSyncDoesNotRewriteRows() =
runTest {
@@ -571,6 +600,11 @@ class SyncServiceTest {
}
}
private suspend fun storedDiscKeys(): List<DiscKey> =
db
.libraryDao()
.discKeysAfter(1, "", 0L, 100_000)
private suspend fun storedDiscs(albumId: String): Map<Long, String> =
db
.libraryDao()