diff --git a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicSource.kt b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicSource.kt index 676d61fe..fe8787d9 100644 --- a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicSource.kt +++ b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicSource.kt @@ -25,7 +25,9 @@ import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.Semaphore +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.sync.withPermit import kotlinx.coroutines.withContext import org.xml.sax.Attributes @@ -224,6 +226,23 @@ class SubsonicSource( private fun albumSongs(): Flow> = channelFlow { val permits = Semaphore(maxConcurrentFetches) + val lock = Mutex() + val pending = ArrayList(PAGE_SIZE) + + suspend fun accept(songs: List) { + lock + .withLock { + pending.addAll(songs) + if (pending.size < PAGE_SIZE) { + null + } else { + val batch = ArrayList(pending.subList(0, PAGE_SIZE)) + pending.subList(0, PAGE_SIZE).clear() + batch + } + }?.let { send(it) } + } + var offset = 0 var pages = 0 while (true) { @@ -251,7 +270,7 @@ class SubsonicSource( onChild = { song, name, attrs -> if (name == "genres") song.addGenre(attrs) }, ).collect { batch -> val accepted = batch.map { it.toSong() }.filter { it.id.isNotEmpty() } - if (accepted.isNotEmpty()) send(accepted) + accept(accepted) } } } @@ -265,7 +284,14 @@ class SubsonicSource( if (pages > maxPages) throw pageCapExceeded() offset += PAGE_SIZE } - } + val tail = + lock.withLock { + val held = pending.toList() + pending.clear() + held + } + if (tail.isNotEmpty()) send(tail) + }.buffer(1) private fun supportsEmptyQuerySearch(): Boolean { emptyQuerySearchSupported?.let { return it } diff --git a/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicSourceTest.kt b/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicSourceTest.kt index 83604f9e..5ae95b7c 100644 --- a/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicSourceTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicSourceTest.kt @@ -608,6 +608,40 @@ class SubsonicSourceTest { assertTrue(maxInFlight.get() in 2..DEFAULT_FETCH_CONCURRENCY) } + @Test + fun albumSongsAreEmittedInBoundedBatches() = + runBlocking { + val albumCount = 200 + val songsPerAlbum = 3 + server.dispatcher = + object : Dispatcher() { + override fun dispatch(request: RecordedRequest): MockResponse { + val url = request.requestUrl!! + return when (url.encodedPath) { + "/rest/search3.view" -> { + MockResponse().setBody(failed(10, "Required parameter is missing")) + } + + "/rest/getAlbumList2.view" -> { + MockResponse().setBody(albumListOf(albumCount)) + } + + "/rest/getAlbum.view" -> { + MockResponse().setBody(albumWithSongs(url.queryParameter("id")!!, songsPerAlbum)) + } + + else -> { + MockResponse().setResponseCode(404) + } + } + } + } + + val batches = SubsonicSource(1, client()).songs().toList().map { it.size } + + assertEquals(listOf(500, 100), batches) + } + @Test fun playlistEntriesWithoutIdsAreSkipped() = runBlocking { @@ -705,6 +739,13 @@ class SubsonicSourceTest { "" + "" + private fun albumWithSongs( + albumId: String, + count: Int, + ) = "" + + (1..count).joinToString("") { "" } + + "" + private fun failed( code: Int, message: String, diff --git a/docs/architecture.md b/docs/architecture.md index ad537746..c79bd3cb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -120,7 +120,7 @@ Images: grids and lists request a 256 px `getCoverArt` thumbnail; the 120 dp det A run is triggered by the user (pull-to-refresh or a Sync action) or by `AutoSync`, a foreground-only timer. While the app has a visible Activity `AutoSync` re-checks at most every 15 minutes (the configured interval, capped) whether `syncIntervalMinutes` has elapsed since the last attempt and, if the mode allows (`Off`, `Wi-Fi only` or `Always`, default `Wi-Fi only`), the app is online and a server is configured, it asks `SyncManager` for a silent sync; nothing runs while the app is backgrounded, and a missed interval catches up on the next open. Only the time of the last attempt is persisted, so a server that keeps failing is retried once per interval rather than on every poll. A silent sync still shows the progress notification and the Settings spinner, but never a toast. -Songs have a primary and a fallback fetch. The primary probes `search3` with an empty query once (memoized per source); if the server supports it, the library is paged through `search3` 500 songs at a time. Otherwise the fallback pages `getAlbumList2` and then issues one `getAlbum` per album — it works everywhere but costs a request per album instead of per 500 songs. Every paged loop stops on a short page and is capped at 1000 pages. Playlists are fetched whole, and their songs are stored by position so a playlist that got shorter (or was emptied) is trimmed at the end. +Songs have a primary and a fallback fetch. The primary probes `search3` with an empty query once (memoized per source); if the server supports it, the library is paged through `search3` 500 songs at a time. Otherwise the fallback pages `getAlbumList2` and then issues one `getAlbum` per album — it works everywhere but costs a request per album instead of per 500 songs, though it still coalesces the songs into 500-row batches before writing them. Every paged loop stops on a short page and is capped at 20,000 pages. Playlists are fetched whole, and their songs are stored by position so a playlist that got shorter (or was emptied) is trimmed at the end. Cleanup is the "everything I had, minus everything the server just sent" step described above: artists, albums, songs and playlists delete the leftover IDs in 500-row chunks, disc titles are deleted per `(album, disc)` for albums that still exist, and playlist entries beyond the new count are removed along with entries for playlists that disappeared.