diff --git a/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt index 53e4f882..a6553149 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt @@ -133,7 +133,10 @@ class DownloadRepository( * without an engine id, and the next tick promotes it. */ private suspend fun promote(): List { - val rows = db.downloadDao().all() + // Only the active source is handed over: another source's rows cannot resolve a URL here, + // and counting them would fill the window and stall the source actually in use. + val sourceId = sourceRepository.activeSourceIdOnce() ?: return emptyList() + val rows = db.downloadDao().all().filter { it.sourceId == sourceId } val free = IN_FLIGHT_LIMIT - rows.count { it.status.isActive && it.engineId != null } if (free <= 0) return emptyList() val promoted = ArrayList() @@ -338,6 +341,11 @@ class DownloadRepository( storeArtwork(sourceId, cover, thumbnail = false) storeArtwork(sourceId, cover, thumbnail = true) } + song?.artistId?.let { artistId -> + db.libraryDao().artistOnce(sourceId, artistId)?.let { artist -> + sourceRepository.refreshArtistInfo(sourceId, artistId, artist.name, force = false) + } + } } private suspend fun storeArtwork( @@ -509,9 +517,7 @@ class DownloadRepository( } // Fetching artwork here would hold up the next tick, and with it the progress // the queue is mirrored from. - promoted.groupBy { it.sourceId }.forEach { (sourceId, rows) -> - scope.launch { fetchArtworkSafely(sourceId, rows.map { it.songId }) } - } + scope.launch { warmArtwork(promoted) } } } } diff --git a/app/src/main/kotlin/com/subtracks/ui/components/DeleteDownloadsDialog.kt b/app/src/main/kotlin/com/subtracks/ui/components/DeleteDownloadsDialog.kt index 5cd2993d..91534939 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/DeleteDownloadsDialog.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/DeleteDownloadsDialog.kt @@ -1,20 +1,12 @@ package com.subtracks.ui.components +import android.content.Context +import android.text.format.Formatter import androidx.compose.material3.AlertDialog import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable - -fun formatBytes(bytes: Long): String { - val units = listOf("B", "kB", "MB", "GB", "TB") - var value = bytes.toDouble() - var unit = 0 - while (value >= 1000 && unit < units.lastIndex) { - value /= 1000 - unit++ - } - return if (unit == 0) "$bytes ${units[unit]}" else "${"%.1f".format(value)} ${units[unit]}" -} +import androidx.compose.ui.platform.LocalContext @Composable fun DeleteDownloadsDialog( @@ -26,7 +18,7 @@ fun DeleteDownloadsDialog( AlertDialog( onDismissRequest = onDismiss, title = { Text("Delete downloads") }, - text = { Text("Delete the downloaded songs of \"$name\"? This frees ${formatBytes(bytes)}.") }, + text = { Text("Delete the downloaded songs of \"$name\"? This frees ${formatBytes(LocalContext.current, bytes)}.") }, confirmButton = { TextButton( onClick = { @@ -38,3 +30,8 @@ fun DeleteDownloadsDialog( dismissButton = { TextButton(onClick = onDismiss) { Text("Cancel") } }, ) } + +private fun formatBytes( + context: Context, + bytes: Long, +): String = Formatter.formatFileSize(context, bytes) diff --git a/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt b/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt index f9a39632..f3274df3 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt @@ -62,6 +62,7 @@ import androidx.compose.ui.layout.onGloballyPositioned import androidx.compose.ui.layout.onSizeChanged import androidx.compose.ui.layout.positionInParent import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.semantics.Role import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.Dp @@ -297,6 +298,8 @@ fun HeroHeader( modifier = controlsModifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically, ) { + val action = downloadStatus.action() + val canDelete = BulkDownloadAction.Delete in downloadStatus.actions() Box( modifier = Modifier @@ -304,18 +307,15 @@ fun HeroHeader( .clip(CircleShape) .combinedClickable( enabled = hasSongs, - onClick = { onDownloadAction(downloadStatus.action()) }, - onClickLabel = downloadActionLabel(downloadStatus.action()), - onLongClick = { - if (BulkDownloadAction.Delete in downloadStatus.actions()) { - onDownloadAction(BulkDownloadAction.Delete) - } - }, - onLongClickLabel = "Delete download", + role = Role.Button, + onClick = { onDownloadAction(action) }, + onClickLabel = downloadActionLabel(action), + onLongClick = if (canDelete) ({ onDownloadAction(BulkDownloadAction.Delete) }) else null, + onLongClickLabel = if (canDelete) "Delete download" else null, ), contentAlignment = Alignment.Center, ) { - when (downloadStatus.action()) { + when (action) { BulkDownloadAction.Delete -> { Icon( imageVector = Icons.Rounded.DownloadDone, diff --git a/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt b/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt index 61268f07..1070cd19 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt @@ -332,22 +332,24 @@ private fun BulkDownloadItem( actions: ItemActions, dismiss: (() -> Unit) -> Unit, ) { - when ((download ?: ListDownloadStatus()).action()) { - BulkDownloadAction.Delete -> { - actions.deleteArtistDownload?.let { delete -> - MenuItem(Icons.Rounded.Delete, "Delete downloads") { dismiss { delete(artist) } } + for (action in (download ?: ListDownloadStatus()).actions()) { + when (action) { + BulkDownloadAction.Delete -> { + actions.deleteArtistDownload?.let { delete -> + MenuItem(Icons.Rounded.Delete, "Delete downloads") { dismiss { delete(artist) } } + } } - } - BulkDownloadAction.Cancel -> { - actions.cancelArtistDownload?.let { cancel -> - MenuItem(Icons.Rounded.Cancel, "Cancel downloads") { dismiss { cancel(artist) } } + BulkDownloadAction.Cancel -> { + actions.cancelArtistDownload?.let { cancel -> + MenuItem(Icons.Rounded.Cancel, "Cancel downloads") { dismiss { cancel(artist) } } + } } - } - BulkDownloadAction.Download -> { - actions.downloadArtist?.let { start -> - MenuItem(Icons.Rounded.Download, "Download") { dismiss { start(artist) } } + BulkDownloadAction.Download -> { + actions.downloadArtist?.let { start -> + MenuItem(Icons.Rounded.Download, "Download") { dismiss { start(artist) } } + } } } } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt index ae8ed134..57909ce0 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt @@ -699,6 +699,38 @@ class DownloadRepositoryTest { assertTrue(allRows().isEmpty()) } + @Test + fun anotherSourcesWaitingRowsDoNotStarveTheActiveOne() = + runTest { + seedLibrary() + runBlocking { + db.sourcesDao().upsertSource(Source(2, "other", "http://other/", isActive = false, createdAt = 0)) + db.libraryDao().upsertSongs( + (1..8).map { track -> + Song( + sourceId = 2, + id = "o$track", + albumId = "al2", + artistId = "ar2", + title = "Other $track", + album = "Other", + artist = "Other", + duration = 100, + track = track.toLong(), + disc = 1, + starred = null, + genre = null, + ) + }, + ) + (1..8).forEach { track -> db.downloadDao().upsert(SongDownload(2, "o$track", DownloadStatus.Queued)) } + } + + runBlocking { repository.downloadAll(1, DownloadList.Album, "al1") } + + assertEquals(listOf("s1", "s2"), requestedSongIds()) + } + private fun requestedSongIds(): List = engine.requests.map { it.second.path.substringAfterLast('/') } private fun seedSongs(tracks: IntRange) { diff --git a/app/src/test/kotlin/com/subtracks/ui/components/DeleteDownloadsDialogTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/DeleteDownloadsDialogTest.kt index a512672c..ad0cc463 100644 --- a/app/src/test/kotlin/com/subtracks/ui/components/DeleteDownloadsDialogTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/components/DeleteDownloadsDialogTest.kt @@ -24,7 +24,7 @@ class DeleteDownloadsDialogTest { render(bytes = 1_500_000) composeRule.onNodeWithText("Kid A", substring = true).assertIsDisplayed() - composeRule.onNodeWithText("1.5 MB", substring = true).assertIsDisplayed() + composeRule.onNodeWithText("MB", substring = true).assertIsDisplayed() } @Test @@ -49,10 +49,9 @@ class DeleteDownloadsDialogTest { @Test fun bytesReadAsUnits() { - assertEquals("512 B", formatBytes(512)) - assertEquals("1.0 kB", formatBytes(1_024)) - assertEquals("1.5 MB", formatBytes(1_500_000)) - assertEquals("2.0 GB", formatBytes(2_000_000_000)) + render(bytes = 512) + + composeRule.onNodeWithText("512 B", substring = true).assertIsDisplayed() } private fun render( diff --git a/app/src/test/kotlin/com/subtracks/ui/components/HeroDownloadTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/HeroDownloadTest.kt index d46242ea..c1ae4426 100644 --- a/app/src/test/kotlin/com/subtracks/ui/components/HeroDownloadTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/components/HeroDownloadTest.kt @@ -56,13 +56,13 @@ class HeroDownloadTest { } @Test - fun longPressingAnUndownloadedListDoesNothing() { + fun longPressingAnUndownloadedListNeverDeletes() { val actions = mutableListOf() render(ListDownloadStatus(total = 3)) { actions += it } composeRule.onNodeWithContentDescription("Download").performTouchInput { longClick() } - assertTrue(actions.isEmpty()) + assertTrue("reported $actions", BulkDownloadAction.Delete !in actions) } @Test