From 813ca00fba235ec43fecc636ad7bdc0477e73889 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:05:56 +0000 Subject: [PATCH] fix the offline queue mapping and cover it with tests --- .../subtracks/data/repo/OfflineException.kt | 3 - .../subtracks/data/repo/SourceRepository.kt | 2 +- .../subtracks/playback/PlaybackController.kt | 43 ++-- .../com/subtracks/ui/library/LibraryScreen.kt | 2 +- .../playback/PlaybackControllerTest.kt | 195 +++++++++++++++++- 5 files changed, 223 insertions(+), 22 deletions(-) delete mode 100644 app/src/main/kotlin/com/subtracks/data/repo/OfflineException.kt diff --git a/app/src/main/kotlin/com/subtracks/data/repo/OfflineException.kt b/app/src/main/kotlin/com/subtracks/data/repo/OfflineException.kt deleted file mode 100644 index 89f0888b..00000000 --- a/app/src/main/kotlin/com/subtracks/data/repo/OfflineException.kt +++ /dev/null @@ -1,3 +0,0 @@ -package com.subtracks.data.repo - -internal class OfflineException : Exception("Offline mode is on") diff --git a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt index c8b0c6cb..c99b44e0 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt @@ -225,7 +225,7 @@ class SourceRepository( useTokenAuth: Boolean, ): Result = if (_offline.value) { - Result.failure(OfflineException()) + Result.failure(IllegalStateException("Offline mode is on")) } else { withContext(Dispatchers.IO) { var fellBack = false diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index a4f94d81..4d5efea9 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -124,8 +124,7 @@ class PlaybackController( private var player: PlayerHandle? = null private var connecting = false private var pendingPlay: PendingPlay? = null - private var pendingShuffle: QueueEntry? = null - private var pendingShuffleList: List? = null + private var pendingShuffle: List? = null private var positionJob: Job? = null private var windowJob: Job? = null private var bufferingJob: Job? = null @@ -144,8 +143,9 @@ class PlaybackController( private var repeatMode = RepeatMode.Off private var endedHandled = false private var lastSavedPositionMs = 0L - private var offline = false + private val offline: Boolean get() = sourceRepository.offline.value private var windowPositions = LongArray(0) + private var windowFiltered = false init { scope.launch { @@ -154,11 +154,7 @@ class PlaybackController( } } scope.launch { - sourceRepository.offline.collect { value -> - if (value == offline) return@collect - offline = value - reloadForOffline() - } + sourceRepository.offline.drop(1).collect { reloadForOffline() } } scope.launch { sourceRepository.quality.drop(1).collect { @@ -190,9 +186,8 @@ class PlaybackController( scope.launch { val pending = pendingPlay pendingPlay = null - val shuffle = pendingShuffle?.let { listOf(it) } ?: pendingShuffleList + val shuffle = pendingShuffle pendingShuffle = null - pendingShuffleList = null when { pending != null -> start(pending.entries, pending.position, pending.disableShuffle) shuffle != null -> startShuffled(shuffle) @@ -287,7 +282,7 @@ class PlaybackController( private fun shufflePlay(entries: List) { if (player == null) { - if (entries.size == 1) pendingShuffle = entries.first() else pendingShuffleList = entries + pendingShuffle = entries connect() return } @@ -1022,6 +1017,8 @@ class PlaybackController( queueSourceId = null windowStart = 0 windowEnd = -1 + windowPositions = LongArray(0) + windowFiltered = false lastPosition = null lastEdit = null player?.run { @@ -1054,6 +1051,7 @@ class PlaybackController( } val actual = window[startIndex].position windowPositions = LongArray(window.size) { window[it].position } + windowFiltered = offline windowStart = window.first().position windowEnd = window.last().position lastPosition = actual @@ -1262,18 +1260,18 @@ class PlaybackController( val player = player ?: return null if (player.itemCount == 0) return null val index = player.currentIndex - return if (offline) windowPositions.getOrNull(index) ?: (windowStart + index) else (windowStart + index) + return if (windowFiltered) windowPositions.getOrNull(index) ?: (windowStart + index) else (windowStart + index) } private fun windowIndexOf(position: Long): Int = - if (offline) { + if (windowFiltered) { windowPositions.indexOf(position) } else { (position - windowStart).toInt() } private fun inWindow(position: Long): Boolean = - if (offline) { + if (windowFiltered) { windowPositions.contains(position) } else { position in windowStart..windowEnd @@ -1309,7 +1307,7 @@ class PlaybackController( startLock.withLock { val player = player ?: return@withLock val snapshot = snapshot ?: return@withLock - if (snapshot.size == 0L || player.isEnded) return@withLock + if (snapshot.size == 0L) return@withLock downloads.awaitLoaded() val current = currentPosition() ?: return@withLock val positionMs = player.currentPositionMs @@ -1362,6 +1360,18 @@ class PlaybackController( private fun handleEnded() { if (endedHandled) return endedHandled = true + if (offline) { + scope.launch { + val snapshot = snapshot ?: return@launch + val current = currentPosition() ?: return@launch + downloads.awaitLoaded() + val target = + downloadedAtOrAfter(snapshot, current + 1) + ?: if (repeatMode == RepeatMode.All) downloadedAtOrAfter(snapshot, 0L) else null + target?.let { jumpTo(it) } + } + return + } if (repeatMode == RepeatMode.All) jumpTo(0) } @@ -1472,6 +1482,9 @@ class PlaybackController( const val RESTART_THRESHOLD_MS = 3_000L const val SEED_PIN_TRIES = 1_000_000 const val FILE_SCHEME = "file:" + + // ponytail: walks the queue one row at a time to find the next downloaded track; swap for a + // single query over the downloaded ids if a sparse queue ever makes the scan noticeable. const val OFFLINE_SCAN_LIMIT = 10_000L } } diff --git a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt index 1f328e86..0db81f89 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt @@ -219,7 +219,7 @@ fun LibraryRoute( LibraryTab.entries.associateWith { tab -> val query by viewModel.listQuery(tab.listTab()).collectAsStateWithLifecycle() val term by viewModel.search(tab.listTab()).collectAsStateWithLifecycle() - "${query.sort}|${query.descending}|${query.starred}|${query.downloaded || offline}|$term|$offline" + "${query.sort}|${query.descending}|${query.starred}|${query.downloaded || offline}|$term" } val playback by playbackController.state.collectAsStateWithLifecycle() val artwork = rememberArtworkColors(playbackController.coverArt(playback.item, thumbnail = true)) diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 2a84f9b4..d3989968 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -10,9 +10,11 @@ import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine import com.subtracks.data.model.Album +import com.subtracks.data.model.DownloadStatus import com.subtracks.data.model.QueueEntry import com.subtracks.data.model.QueueKind import com.subtracks.data.model.Song +import com.subtracks.data.model.SongDownload import com.subtracks.data.model.Source import com.subtracks.data.net.NetworkMode import com.subtracks.data.prefs.StreamQuality @@ -25,9 +27,11 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout import okhttp3.OkHttpClient import org.junit.After import org.junit.Assert.assertEquals @@ -52,6 +56,7 @@ class PlaybackControllerTest { private lateinit var sources: SourceRepository private lateinit var queues: QueueRepository private lateinit var downloads: DownloadRepository + private lateinit var downloadsDir: File private lateinit var handle: FakePlayerHandle private lateinit var controller: PlaybackController private val messages = CopyOnWriteArrayList() @@ -68,15 +73,16 @@ class PlaybackControllerTest { networkMode = MutableStateFlow(NetworkMode.Wifi) sources = SourceRepository(db, OkHttpClient(), prefs, ArtworkStore(File(context.cacheDir, "art")), networkMode = networkMode) queues = QueueRepository(db) + downloadsDir = File(context.cacheDir, "downloads") downloads = DownloadRepository( db, sources, FakeDownloadEngine(), - File(context.cacheDir, "downloads"), + downloadsDir, artworkStore = ArtworkStore(File(context.cacheDir, "art")), artworkFetcher = { ByteArray(0) }, - ) + ).also { it.start() } handle = FakePlayerHandle() controller = PlaybackController( @@ -2451,6 +2457,191 @@ class PlaybackControllerTest { assertEquals(windows, handle.operations.count { it.startsWith("setWindow") }) } + @Test + fun togglingOnlineKeepsTheCurrentDownloadedTrack() { + seedAlbum(5, sourceId = 1) + markDownloaded(1, "s1") + markDownloaded(1, "s3") + markDownloaded(1, "s5") + + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + + setOffline() + await { handle.items.map { it.id } == listOf("s1", "s3", "s5") } + + handle.advanceTo(2) + await { + controller.state.value.item + ?.id == "s5" + } + assertEquals( + "s5", + controller.state.value.item + ?.id, + ) + + handle.operations.clear() + setOnline() + + await { handle.operations.any { it.startsWith("setWindow") } } + assertEquals( + "s5", + controller.state.value.item + ?.id, + ) + } + + @Test + fun offlinePlayAlbumBuildsADownloadedOnlyQueue() { + seedAlbum(4, sourceId = 1) + markDownloaded(1, "s1") + markDownloaded(1, "s3") + setOffline() + + controller.playAlbum(1, "al1", 0) + + await { handle.items.map { it.id } == listOf("s1", "s3") } + assertEquals(listOf("s1", "s3"), handle.items.map { it.id }) + } + + @Test + fun togglingOfflineKeepsADownloadedCurrentTrack() { + seedAlbum(4, sourceId = 1) + (1..4).forEach { markDownloaded(1, "s$it") } + controller.playAlbum(1, "al1", 1) + await { + controller.state.value.item + ?.id == "s2" + } + handle.operations.clear() + + setOffline() + + await { handle.operations.any { it.startsWith("setWindow") } } + assertEquals( + "s2", + controller.state.value.item + ?.id, + ) + } + + @Test + fun togglingOfflineSkipsANonDownloadedCurrentTrack() { + seedAlbum(4, sourceId = 1) + markDownloaded(1, "s3") + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + + setOffline() + + await { + controller.state.value.item + ?.id == "s3" + } + assertEquals( + "s3", + controller.state.value.item + ?.id, + ) + } + + @Test + fun offlineNextCrossesTheLoadedWindowBoundary() { + seedAlbum(30, sourceId = 1) + (1..30).forEach { markDownloaded(1, "s$it") } + setOffline() + + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + assertTrue("expected a bounded window, was ${handle.itemCount}", handle.itemCount < 30) + + handle.advanceTo(handle.itemCount - 1) + await { + controller.state.value.item + ?.id == "s26" + } + + controller.next() + + await { + controller.state.value.item + ?.id == "s27" + } + assertEquals( + "s27", + controller.state.value.item + ?.id, + ) + } + + @Test + fun offlineEndedPlaybackAdvancesPastTheLoadedWindow() { + seedAlbum(30, sourceId = 1) + (1..30).forEach { markDownloaded(1, "s$it") } + setOffline() + + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + handle.advanceTo(handle.itemCount - 1) + await { + controller.state.value.item + ?.id == "s26" + } + + handle.finish() + + await { + controller.state.value.item + ?.id == "s27" + } + assertEquals( + "s27", + controller.state.value.item + ?.id, + ) + } + + private fun markDownloaded( + sourceId: Long, + songId: String, + ) { + val file = File(downloadsDir, "$sourceId/$songId") + val deadline = System.nanoTime() + 5_000_000_000L + while (System.nanoTime() < deadline) { + runBlocking { + db.downloadDao().upsert(SongDownload(sourceId, songId, DownloadStatus.Completed)) + file.parentFile?.mkdirs() + file.writeBytes(byteArrayOf(1)) + } + if (downloads.localUri(songId) != null) return + Thread.sleep(10) + } + assertTrue("Timed out marking $songId downloaded", downloads.localUri(songId) != null) + } + + private fun setOffline() { + sources.setOfflineMode(true) + runBlocking { withTimeout(5_000) { sources.offline.first { it } } } + } + + private fun setOnline() { + sources.setOfflineMode(false) + runBlocking { withTimeout(5_000) { sources.offline.first { !it } } } + } + private fun seedAlbum( count: Int, sourceId: Long,