cancel a list in one go, and round the hero's touch feedback

- Cancelling a big playlist did per-row work: a binder cancel, two queries and a
  delete each, thousands of times, under the lock. The platform takes a list of
  ids in one call, so the cancel is now one binder call, one delete per chunk of
  rows and one sweep, which is both the several seconds and the stale button.
- Partial files are covered by the same sweep, which already deletes any file in
  a source directory that no row names; a test now proves a stray .part goes with
  the cancel.
- The hero's control was a plain clickable Box after the long-press work, so its
  splash was square; it clips to a circle the way an IconButton does.
This commit is contained in:
austinried
2026-10-04 00:30:03 +00:00
parent 3b6fb1341b
commit a57ca92250
7 changed files with 71 additions and 20 deletions
@@ -83,6 +83,12 @@ interface DownloadDao {
songId: String,
)
@Query("DELETE FROM song_downloads WHERE sourceId = :sourceId AND songId IN (:songIds)")
suspend fun deleteSongs(
sourceId: Long,
songIds: Collection<String>,
)
@Query("DELETE FROM song_downloads WHERE sourceId = :sourceId")
suspend fun deleteSource(sourceId: Long)
}
@@ -20,5 +20,5 @@ interface DownloadEngine {
fun download(id: Long): EngineDownload?
fun cancel(id: Long)
fun cancel(ids: List<Long>)
}
@@ -40,8 +40,8 @@ class SystemDownloadEngine(
}
}
override fun cancel(id: Long) {
manager().remove(id)
override fun cancel(ids: List<Long>) {
manager().remove(*ids.toLongArray())
}
private fun manager(): DownloadManager = context.getSystemService(DownloadManager::class.java)
@@ -119,7 +119,7 @@ class DownloadRepository(
): Boolean {
if (alreadyDownloadingOrDownloaded(sourceId, songId)) return false
val existing = db.downloadDao().find(sourceId, songId)
existing?.engineId?.let(engine::cancel)
existing?.engineId?.let { engine.cancel(listOf(it)) }
dir(sourceId).mkdirs()
file(sourceId, songId).delete()
db.downloadDao().upsert(SongDownload(sourceId = sourceId, songId = songId, status = DownloadStatus.Queued))
@@ -251,10 +251,7 @@ class DownloadRepository(
list: DownloadList,
refId: String,
) = withContext(dispatcher) {
mutex.withLock {
val removed = rowsFor(sourceId, list, refId).filter { it.status.isActive }.map { removeRow(sourceId, it.songId) }
if (removed.any { it }) sweep(db.downloadDao().all())
}
mutex.withLock { removeListRows(sourceId, list, refId) { it.status.isActive } }
}
suspend fun deleteAll(
@@ -262,13 +259,25 @@ class DownloadRepository(
list: DownloadList,
refId: String,
) = withContext(dispatcher) {
mutex.withLock {
val removed =
rowsFor(sourceId, list, refId)
.filter { it.status == DownloadStatus.Completed }
.map { removeRow(sourceId, it.songId) }
if (removed.any { it }) sweep(db.downloadDao().all())
}
mutex.withLock { removeListRows(sourceId, list, refId) { it.status == DownloadStatus.Completed } }
}
/**
* One binder call for the platform, one delete per chunk and one sweep, rather than a round of
* each per row: a playlist of thousands took seconds to cancel and left the button stale.
*/
private suspend fun removeListRows(
sourceId: Long,
list: DownloadList,
refId: String,
selecting: (SongDownload) -> Boolean,
) {
val rows = rowsFor(sourceId, list, refId).filter(selecting)
if (rows.isEmpty()) return
engine.cancel(rows.mapNotNull { it.engineId })
rows.forEach { file(sourceId, it.songId).delete() }
rows.map { it.songId }.chunked(DELETE_CHUNK).forEach { db.downloadDao().deleteSongs(sourceId, it) }
sweep(db.downloadDao().all())
}
suspend fun removeSource(sourceId: Long) =
@@ -278,7 +287,8 @@ class DownloadRepository(
.downloadDao()
.all()
.filter { it.sourceId == sourceId }
.forEach { row -> row.engineId?.let(engine::cancel) }
.mapNotNull { it.engineId }
.let(engine::cancel)
db.downloadDao().deleteSource(sourceId)
dir(sourceId).deleteRecursively()
}
@@ -407,7 +417,7 @@ class DownloadRepository(
songId: String,
): Boolean {
val row = db.downloadDao().find(sourceId, songId) ?: return false
row.engineId?.let(engine::cancel)
row.engineId?.let { engine.cancel(listOf(it)) }
file(sourceId, songId).delete()
db.downloadDao().delete(sourceId, songId)
return true
@@ -478,7 +488,7 @@ class DownloadRepository(
row: SongDownload,
message: String,
) {
row.engineId?.let(engine::cancel)
row.engineId?.let { engine.cancel(listOf(it)) }
file(row.sourceId, row.songId).delete()
db.downloadDao().upsert(row.copy(status = DownloadStatus.Failed, engineId = null, error = message))
}
@@ -542,6 +552,7 @@ class DownloadRepository(
const val NO_MEDIA = ".nomedia"
const val POLL_MS = 1_000L
const val IN_FLIGHT_LIMIT = 8
const val DELETE_CHUNK = 900
const val MISSING_FILE = "The downloaded file is missing"
const val NO_ADDRESS = "Can't download: the server address is unavailable"
const val EMPTY_LIST = "Nothing left to download from this list"
@@ -25,6 +25,7 @@ import androidx.compose.foundation.layout.width
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.LazyListScope
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.foundation.shape.CircleShape
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.automirrored.rounded.ArrowBack
@@ -53,6 +54,7 @@ import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.draw.drawBehind
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.graphicsLayer
@@ -299,6 +301,7 @@ fun HeroHeader(
modifier =
Modifier
.size(48.dp)
.clip(CircleShape)
.combinedClickable(
enabled = hasSongs,
onClick = { onDownloadAction(downloadStatus.action()) },
@@ -6,6 +6,7 @@ class FakeDownloadEngine : DownloadEngine {
val requests = mutableListOf<Pair<Long, EngineRequest>>()
val cancelled = mutableListOf<Long>()
var cancelCalls = 0
var enqueueDelayMs = 0L
var failEnqueueFor: String? = null
@@ -21,8 +22,9 @@ class FakeDownloadEngine : DownloadEngine {
override fun download(id: Long): EngineDownload? = downloads[id]
override fun cancel(id: Long) {
cancelled += id
override fun cancel(ids: List<Long>) {
cancelCalls++
cancelled += ids
}
fun running(
@@ -670,6 +670,35 @@ class DownloadRepositoryTest {
assertEquals(5L, repository.downloadedBytes(1, DownloadList.Artist, "ar1"))
}
@Test
fun cancellingAListRemovesPartialFilesToo() =
runTest {
seedLibrary()
runBlocking { repository.downloadAll(1, DownloadList.Album, "al1") }
writeFile(1, "s1", "half")
writeFile(1, "s1.part", "half")
writeFile(1, "s2", "half")
runBlocking { repository.cancelAll(1, DownloadList.Album, "al1") }
assertFalse(file(1, "s1").exists())
assertFalse(File(file(1, "s1").parentFile, "s1.part").exists())
assertFalse(file(1, "s2").exists())
}
@Test
fun cancellingAListCancelsThePlatformOnce() =
runTest {
seedLibrary()
seedSongs(3..40)
runBlocking { repository.downloadAll(1, DownloadList.Album, "al1") }
runBlocking { repository.cancelAll(1, DownloadList.Album, "al1") }
assertEquals(1, engine.cancelCalls)
assertTrue(allRows().isEmpty())
}
private fun requestedSongIds(): List<String> = engine.requests.map { it.second.path.substringAfterLast('/') }
private fun seedSongs(tracks: IntRange) {