reconcile and notify downloads outside the active source

A source switched away from kept downloading on the platform while our poller
ignored it, so its rows froze as Running, the notification vanished and the only
Cancel went with it. The poll now reconciles every source's in-flight rows (the
engine reports status without a source) and the notification reads all of them,
which is also what cancelActive already acted on.

The grouped tab queries drop entities with nothing downloaded, since the UI
treats a missing entry and an all-zero one the same.

The indicator's two-value enum was used as a boolean, so it is one; and the
library screenshot now asserts the ring is drawn, rather than capturing an
image that would stay green if it disappeared.
This commit is contained in:
austinried
2026-10-04 00:30:03 +00:00
parent 47064e82f0
commit 7db6d1365f
9 changed files with 81 additions and 34 deletions
@@ -15,6 +15,9 @@ interface DownloadDao {
@Query("SELECT * FROM song_downloads WHERE sourceId = :sourceId") @Query("SELECT * FROM song_downloads WHERE sourceId = :sourceId")
fun downloads(sourceId: Long): Flow<List<SongDownload>> fun downloads(sourceId: Long): Flow<List<SongDownload>>
@Query("SELECT * FROM song_downloads WHERE status IN ('Queued', 'Running')")
fun activeDownloads(): Flow<List<SongDownload>>
@Query("SELECT * FROM song_downloads ORDER BY rowid") @Query("SELECT * FROM song_downloads ORDER BY rowid")
suspend fun all(): List<SongDownload> suspend fun all(): List<SongDownload>
@@ -67,7 +70,8 @@ interface DownloadDao {
"FROM songs " + "FROM songs " +
"LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " + "LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " +
"WHERE songs.sourceId = :sourceId AND songs.albumId IS NOT NULL " + "WHERE songs.sourceId = :sourceId AND songs.albumId IS NOT NULL " +
"GROUP BY songs.albumId", "GROUP BY songs.albumId " +
"HAVING downloaded > 0 OR downloading > 0",
) )
fun albumStatuses(sourceId: Long): Flow<List<EntityDownloadStatus>> fun albumStatuses(sourceId: Long): Flow<List<EntityDownloadStatus>>
@@ -80,7 +84,8 @@ interface DownloadDao {
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " + "LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " +
"WHERE songs.sourceId = :sourceId AND albums.artistId IS NOT NULL " + "WHERE songs.sourceId = :sourceId AND albums.artistId IS NOT NULL " +
"GROUP BY albums.artistId", "GROUP BY albums.artistId " +
"HAVING downloaded > 0 OR downloading > 0",
) )
fun artistStatuses(sourceId: Long): Flow<List<EntityDownloadStatus>> fun artistStatuses(sourceId: Long): Flow<List<EntityDownloadStatus>>
@@ -93,7 +98,8 @@ interface DownloadDao {
"JOIN songs ON songs.sourceId = playlist_songs.sourceId AND songs.id = playlist_songs.songId " + "JOIN songs ON songs.sourceId = playlist_songs.sourceId AND songs.id = playlist_songs.songId " +
"LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " + "LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " +
"WHERE playlist_songs.sourceId = :sourceId " + "WHERE playlist_songs.sourceId = :sourceId " +
"GROUP BY playlist_songs.playlistId", "GROUP BY playlist_songs.playlistId " +
"HAVING downloaded > 0 OR downloading > 0",
) )
fun playlistStatuses(sourceId: Long): Flow<List<EntityDownloadStatus>> fun playlistStatuses(sourceId: Long): Flow<List<EntityDownloadStatus>>
@@ -20,6 +20,8 @@ class DownloadCancelReceiver :
context: Context, context: Context,
intent: Intent, intent: Intent,
) { ) {
// The broadcast budget is short, and cancelActive waits for the repository mutex, which a
// large downloadAll holds for the whole insert pass; a huge queue could outlast it.
val pending = goAsync() val pending = goAsync()
CoroutineScope(SupervisorJob() + Dispatchers.IO).launch { CoroutineScope(SupervisorJob() + Dispatchers.IO).launch {
try { try {
@@ -31,8 +31,8 @@ class DownloadNotifier(
fun start() { fun start() {
createChannel() createChannel()
scope.launch { scope.launch {
repository.states().collect { rows -> repository.activeDownloads().collect { rows ->
val content = downloadNotification(rows.values) val content = downloadNotification(rows)
if (content == null) { if (content == null) {
manager.cancel(ID) manager.cancel(ID)
} else if (manager.areNotificationsEnabled()) { } else if (manager.areNotificationsEnabled()) {
@@ -82,6 +82,8 @@ class DownloadRepository(
fun states(): StateFlow<Map<String, SongDownload>> = statesFlow fun states(): StateFlow<Map<String, SongDownload>> = statesFlow
fun activeDownloads(): Flow<List<SongDownload>> = db.downloadDao().activeDownloads()
suspend fun awaitLoaded() { suspend fun awaitLoaded() {
if (started) loaded.await() if (started) loaded.await()
} }
@@ -539,8 +541,10 @@ class DownloadRepository(
delay(POLL_MS) delay(POLL_MS)
val promoted = val promoted =
mutex.withLock { mutex.withLock {
val sourceId = sourceRepository.activeSourceIdOnce() // Reconcile every source's in-flight rows, not just the active one: the
val active = db.downloadDao().all().filter { it.status.isActive && it.sourceId == sourceId } // engine reports status without a source, and a source switched away
// from keeps downloading, so its rows must not freeze as Running.
val active = db.downloadDao().all().filter { it.status.isActive }
if (active.isEmpty()) return@launch if (active.isEmpty()) return@launch
reconcileRows(active) reconcileRows(active)
promote() promote()
@@ -20,10 +20,7 @@ import com.subtracks.data.model.ListDownloadStatus
const val DOWNLOADING_INDICATOR_TAG = "Downloading" const val DOWNLOADING_INDICATOR_TAG = "Downloading"
enum class DownloadIndicator { Hidden, InProgress } internal fun ListDownloadStatus.isDownloading(): Boolean = total > 0 && downloading > 0
fun ListDownloadStatus.indicator(): DownloadIndicator =
if (total > 0 && downloading > 0) DownloadIndicator.InProgress else DownloadIndicator.Hidden
@Composable @Composable
fun ListDownloadIndicator( fun ListDownloadIndicator(
@@ -32,7 +29,7 @@ fun ListDownloadIndicator(
scrim: Boolean = false, scrim: Boolean = false,
) { ) {
val current = status ?: return val current = status ?: return
if (current.indicator() == DownloadIndicator.Hidden) return if (!current.isDownloading()) return
val progress = { current.downloaded.toFloat() / current.total } val progress = { current.downloaded.toFloat() / current.total }
if (scrim) { if (scrim) {
Surface( Surface(
@@ -126,14 +126,6 @@ fun MenuTarget.bulkRef(): Pair<DownloadList, String>? =
is MenuTarget.Song -> null is MenuTarget.Song -> null
} }
fun MenuTarget.listDownload(): ListDownloadStatus? =
when (this) {
is MenuTarget.Album -> download
is MenuTarget.Playlist -> download
is MenuTarget.Artist -> download
is MenuTarget.Song -> null
}
class ItemActions( class ItemActions(
val playNext: ((QueueRef) -> Unit)? = null, val playNext: ((QueueRef) -> Unit)? = null,
val addToQueue: ((QueueRef) -> Unit)? = null, val addToQueue: ((QueueRef) -> Unit)? = null,
@@ -337,7 +329,14 @@ private fun BulkDownloadItem(
dismiss: (() -> Unit) -> Unit, dismiss: (() -> Unit) -> Unit,
) { ) {
val bulk = actions.bulkDownload ?: return val bulk = actions.bulkDownload ?: return
for (action in (target.listDownload() ?: ListDownloadStatus()).actions()) { val status =
when (target) {
is MenuTarget.Album -> target.download
is MenuTarget.Playlist -> target.download
is MenuTarget.Artist -> target.download
is MenuTarget.Song -> null
}
for (action in (status ?: ListDownloadStatus()).actions()) {
when (action) { when (action) {
BulkDownloadAction.Delete -> MenuItem(Icons.Rounded.Delete, "Delete downloads") { dismiss { bulk(target, action) } } BulkDownloadAction.Delete -> MenuItem(Icons.Rounded.Delete, "Delete downloads") { dismiss { bulk(target, action) } }
BulkDownloadAction.Cancel -> MenuItem(Icons.Rounded.Cancel, "Cancel downloads") { dismiss { bulk(target, action) } } BulkDownloadAction.Cancel -> MenuItem(Icons.Rounded.Cancel, "Cancel downloads") { dismiss { bulk(target, action) } }
@@ -851,6 +851,40 @@ class DownloadRepositoryTest {
assertEquals(setOf("ar1" to "Artist"), songs.map { it.artistId to it.artistName }.toSet()) assertEquals(setOf("ar1" to "Artist"), songs.map { it.artistId to it.artistName }.toSet())
} }
@Test
fun activeDownloadsCoverEverySourceNotJustTheActiveOne() =
runTest {
seedLibrary()
runBlocking {
db.sourcesDao().upsertSource(Source(2, "other", "http://other/", isActive = false, createdAt = 0))
db.libraryDao().upsertSongs(
listOf(
Song(
sourceId = 2,
id = "o1",
albumId = "al2",
artistId = "ar2",
title = "Other",
album = "Other",
artist = "Other",
duration = 100,
track = 1,
disc = 1,
starred = null,
genre = null,
),
),
)
db.downloadDao().upsert(SongDownload(1, "s1", DownloadStatus.Running, engineId = 1))
db.downloadDao().upsert(SongDownload(1, "s2", DownloadStatus.Completed))
db.downloadDao().upsert(SongDownload(2, "o1", DownloadStatus.Queued))
}
val rows = runBlocking { repository.activeDownloads().first() }
assertEquals(setOf(1L to "s1", 2L to "o1"), rows.map { it.sourceId to it.songId }.toSet())
}
private fun requestedSongIds(): List<String> = engine.requests.map { it.second.path.substringAfterLast('/') } private fun requestedSongIds(): List<String> = engine.requests.map { it.second.path.substringAfterLast('/') }
private fun seedSongs(tracks: IntRange) { private fun seedSongs(tracks: IntRange) {
@@ -11,7 +11,9 @@ import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.test.assertCountEquals
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
import androidx.compose.ui.test.onAllNodesWithContentDescription
import androidx.compose.ui.test.onAllNodesWithTag import androidx.compose.ui.test.onAllNodesWithTag
import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onAllNodesWithText
import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.onRoot
@@ -41,6 +43,7 @@ import com.subtracks.data.prefs.StreamQuality
import com.subtracks.playback.PlaybackState import com.subtracks.playback.PlaybackState
import com.subtracks.playback.QueueItem import com.subtracks.playback.QueueItem
import com.subtracks.playback.RepeatMode import com.subtracks.playback.RepeatMode
import com.subtracks.ui.components.DOWNLOADING_INDICATOR_TAG
import com.subtracks.ui.library.ALBUM_COVER_TAG import com.subtracks.ui.library.ALBUM_COVER_TAG
import com.subtracks.ui.library.ARTIST_NAME_SCRIM_TAG import com.subtracks.ui.library.ARTIST_NAME_SCRIM_TAG
import com.subtracks.ui.library.AlbumDetailScreen import com.subtracks.ui.library.AlbumDetailScreen
@@ -154,6 +157,7 @@ class AppScreenshotTest {
} }
} }
awaitTag(ALBUM_COVER_TAG) awaitTag(ALBUM_COVER_TAG)
composeRule.onAllNodesWithContentDescription(DOWNLOADING_INDICATOR_TAG).assertCountEquals(1)
composeRule.onRoot().captureRoboImage("src/test/screenshots/library_albums_downloads.png") composeRule.onRoot().captureRoboImage("src/test/screenshots/library_albums_downloads.png")
} }
@@ -1,37 +1,38 @@
package com.subtracks.ui.components package com.subtracks.ui.components
import com.subtracks.data.model.ListDownloadStatus import com.subtracks.data.model.ListDownloadStatus
import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test import org.junit.Test
class DownloadIndicatorTest { class DownloadIndicatorTest {
@Test @Test
fun anUntouchedListShowsNothing() { fun anUntouchedListIsNotDownloading() {
assertEquals(DownloadIndicator.Hidden, ListDownloadStatus().indicator()) assertFalse(ListDownloadStatus().isDownloading())
} }
@Test @Test
fun aPopulatedButUnstartedListShowsNothing() { fun aPopulatedButUnstartedListIsNotDownloading() {
assertEquals(DownloadIndicator.Hidden, ListDownloadStatus(total = 4).indicator()) assertFalse(ListDownloadStatus(total = 4).isDownloading())
} }
@Test @Test
fun aQueuedButNotYetDownloadedListShowsProgress() { fun aQueuedButNotYetDownloadedListIsDownloading() {
assertEquals(DownloadIndicator.InProgress, ListDownloadStatus(total = 4, downloading = 4).indicator()) assertTrue(ListDownloadStatus(total = 4, downloading = 4).isDownloading())
} }
@Test @Test
fun aPartlyDownloadedIdleListShowsNothing() { fun aPartlyDownloadedIdleListIsNotDownloading() {
assertEquals(DownloadIndicator.Hidden, ListDownloadStatus(total = 4, downloaded = 2).indicator()) assertFalse(ListDownloadStatus(total = 4, downloaded = 2).isDownloading())
} }
@Test @Test
fun aPartlyDownloadedListWithSomethingInFlightShowsProgress() { fun aPartlyDownloadedListWithSomethingInFlightIsDownloading() {
assertEquals(DownloadIndicator.InProgress, ListDownloadStatus(total = 4, downloaded = 2, downloading = 1).indicator()) assertTrue(ListDownloadStatus(total = 4, downloaded = 2, downloading = 1).isDownloading())
} }
@Test @Test
fun aFullyDownloadedListShowsNothing() { fun aFullyDownloadedListIsNotDownloading() {
assertEquals(DownloadIndicator.Hidden, ListDownloadStatus(total = 4, downloaded = 4).indicator()) assertFalse(ListDownloadStatus(total = 4, downloaded = 4).isDownloading())
} }
} }