confirm the end of a full last page before failing the page cap

A full page at the cap is not proof of truncation, so a library whose
size is an exact multiple of the page size failed for no reason. Each
paged loop now keeps going one page past the cap and only throws when
that page comes back non-empty; a library up to the cap syncs normally.
Add a boundary test for the exact-cap case.

Also note the accepted per-batch observer invalidation churn in the sync
code and the architecture doc.
This commit is contained in:
austinried
2026-10-04 00:29:58 +00:00
parent c48ec670d8
commit 7b06b23860
4 changed files with 37 additions and 16 deletions
@@ -46,7 +46,7 @@ class SubsonicSource(
val (frequent, recent) = fetchRanks()
var offset = 0
var pages = 0
while (pages < maxPages) {
while (true) {
var count = 0
entityBatches(
method = "getAlbumList2",
@@ -67,10 +67,10 @@ class SubsonicSource(
)
}
if (count < PAGE_SIZE) break
offset += PAGE_SIZE
pages++
if (pages > maxPages) throw pageCapExceeded()
offset += PAGE_SIZE
}
checkPageCap(pages)
}.flowOn(Dispatchers.IO).buffer(1)
override fun songs(): Flow<List<Song>> =
@@ -138,7 +138,7 @@ class SubsonicSource(
flow {
var offset = 0
var pages = 0
while (pages < maxPages) {
while (true) {
var count = 0
entityBatches(
method = "search3",
@@ -158,17 +158,17 @@ class SubsonicSource(
emit(batch)
}
if (count < PAGE_SIZE) break
offset += PAGE_SIZE
pages++
if (pages > maxPages) throw pageCapExceeded()
offset += PAGE_SIZE
}
checkPageCap(pages)
}
private fun albumSongs(): Flow<List<Song>> =
flow {
var offset = 0
var pages = 0
while (pages < maxPages) {
while (true) {
val albums = ArrayList<String>(PAGE_SIZE)
entityBatches(
method = "getAlbumList2",
@@ -187,10 +187,10 @@ class SubsonicSource(
).collect { emit(it) }
}
if (albums.size < PAGE_SIZE) break
offset += PAGE_SIZE
pages++
if (pages > maxPages) throw pageCapExceeded()
offset += PAGE_SIZE
}
checkPageCap(pages)
}
private suspend fun fetchRanks(): Pair<Map<String, Long>, Map<String, Long>> = fetchRank("frequent") to fetchRank("recent")
@@ -200,7 +200,7 @@ class SubsonicSource(
val ranks = HashMap<String, Long>()
var offset = 0
var pages = 0
while (pages < maxPages) {
while (true) {
var index = 0
entityBatches(
method = "getAlbumList2",
@@ -215,10 +215,10 @@ class SubsonicSource(
}
}
if (index < PAGE_SIZE) break
offset += PAGE_SIZE
pages++
if (pages > maxPages) throw pageCapExceeded()
offset += PAGE_SIZE
}
checkPageCap(pages)
ranks
} catch (_: SubsonicException) {
emptyMap()
@@ -239,9 +239,7 @@ class SubsonicSource(
return supported
}
private fun checkPageCap(pages: Int) {
if (pages >= maxPages) throw SubsonicException(-1, "Library exceeds the $maxPages page cap")
}
private fun pageCapExceeded() = SubsonicException(-1, "Library exceeds the ${maxPages * PAGE_SIZE}-row page cap")
private fun <T> entityBatches(
method: String,
@@ -90,6 +90,8 @@ class SyncService(
library.deleteOrphanPlaylistSongs(source.id)
}
// ponytail: one commit per batch means Room notifies paged observers once per batch; gate
// UI observation on SyncStatus.Running if that churn is ever noticeable.
private suspend fun <T> write(
table: String,
columns: List<String>,
@@ -203,6 +203,27 @@ class SubsonicSourceTest {
assertEquals(true, failure.exceptionOrNull() is SubsonicException)
}
@Test
fun albumsAtThePageCapSucceedWhenTheNextPageIsEmpty() =
runBlocking {
server.dispatcher =
object : Dispatcher() {
override fun dispatch(request: RecordedRequest): MockResponse {
val url = request.requestUrl!!
if (url.encodedPath != "/rest/getAlbumList2.view") return MockResponse().setResponseCode(404)
if (url.queryParameter("type") != "newest") return MockResponse().setBody(emptyAlbumList())
return when (url.queryParameter("offset")) {
"0", "500" -> MockResponse().setBody(albumPage(url.queryParameter("offset")!!.toInt(), 500))
else -> MockResponse().setBody(emptyAlbumList())
}
}
}
val albums = SubsonicSource(1, client(), maxPages = 2).albums().toList().flatten()
assertEquals(1000, albums.size)
}
@Test
fun playlistEntriesWithoutIdsAreSkipped() =
runBlocking {