diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 4d4078fd..404c2144 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -483,29 +483,6 @@ interface LibraryDao { albumId: String, ): Flow> - @Query( - "SELECT songs.id FROM songs " + - "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id AND sd.status = 'Completed' " + - "WHERE songs.sourceId = :sourceId AND songs.albumId = :albumId " + - "ORDER BY songs.disc, songs.track, songs.id", - ) - suspend fun downloadedAlbumSongIds( - sourceId: Long, - albumId: String, - ): List - - @Query( - "SELECT playlist_songs.songId FROM playlist_songs " + - "JOIN song_downloads sd ON sd.sourceId = playlist_songs.sourceId " + - "AND sd.songId = playlist_songs.songId AND sd.status = 'Completed' " + - "WHERE playlist_songs.sourceId = :sourceId AND playlist_songs.playlistId = :playlistId " + - "ORDER BY playlist_songs.position", - ) - suspend fun downloadedPlaylistSongIds( - sourceId: Long, - playlistId: String, - ): List - @Query("$PLAYLIST_SONGS_SELECT AND $DOWNLOADED_SONG$PLAYLIST_SONGS_ORDER") fun playlistSongsDownloaded( sourceId: Long, diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index e7638296..47d6e982 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -208,6 +208,15 @@ class LibraryRepository( db.libraryDao().songsByAlbum(sourceId, albumId) } + suspend fun albumSongOrdinal( + sourceId: Long, + albumId: String, + songId: String, + ): Long { + val index = db.queueDao().albumSongIds(sourceId, albumId).indexOf(songId) + return if (index < 0) 0L else index.toLong() + } + fun playlist( sourceId: Long, playlistId: String, diff --git a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt index c8d4173c..eca9a3e1 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -250,19 +250,6 @@ class QueueRepository( songId: String, ) = QueueEntry(position = 0, sourceId = sourceId, kind = QueueKind.Song, refId = songId) - suspend fun downloadedSongIds( - sourceId: Long, - kind: QueueKind, - refId: String, - ): List { - val library = db.libraryDao() - return when (kind) { - QueueKind.Album -> library.downloadedAlbumSongIds(sourceId, refId) - QueueKind.Playlist -> library.downloadedPlaylistSongIds(sourceId, refId) - QueueKind.Song -> emptyList() - } - } - suspend fun sourceName( kind: QueueKind, sourceId: Long, diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 363dc834..5546436c 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -146,7 +146,6 @@ class PlaybackController( private val offline: Boolean get() = sourceRepository.offline.value private var windowPositions = LongArray(0) private var windowFiltered = false - private var contextOverride: QueueContext? = null init { scope.launch { @@ -242,48 +241,9 @@ class PlaybackController( entry: QueueEntry, start: Long, disableShuffle: Boolean = false, - ) { - if (!offline) { - contextOverride = null - play(listOf(entry), start, disableShuffle) - return - } - scope.launch { - val entries = downloadedEntries(entry) - if (entries.isEmpty()) { - showMessage("Nothing downloaded for offline playback") - return@launch - } - contextOverride = QueueContext(entry.kind, entry.sourceId, entry.refId) - play(entries, start, disableShuffle = true) - } - } + ) = play(listOf(entry), start, disableShuffle) - private fun shuffleContext(entry: QueueEntry) { - if (!offline) { - contextOverride = null - shufflePlay(listOf(entry)) - return - } - scope.launch { - val entries = downloadedEntries(entry) - if (entries.isEmpty()) { - showMessage("Nothing downloaded for offline playback") - return@launch - } - contextOverride = QueueContext(entry.kind, entry.sourceId, entry.refId) - shufflePlay(entries) - } - } - - private suspend fun downloadedEntries(entry: QueueEntry): List { - if (entry.kind == QueueKind.Song) { - return if (downloads.localUri(entry.refId) != null) listOf(entry) else emptyList() - } - return queueRepository - .downloadedSongIds(entry.sourceId, entry.kind, entry.refId) - .map { QueueEntry(position = 0, sourceId = entry.sourceId, kind = QueueKind.Song, refId = it) } - } + private fun shuffleContext(entry: QueueEntry) = shufflePlay(listOf(entry)) private fun shufflePlay(entries: List) { if (player == null) { @@ -1024,7 +984,6 @@ class PlaybackController( windowEnd = -1 windowPositions = LongArray(0) windowFiltered = false - contextOverride = null lastPosition = null lastEdit = null player?.run { @@ -1393,7 +1352,6 @@ class PlaybackController( } private fun contextAt(position: Long?): QueueContext? { - contextOverride?.let { return it } val snapshot = snapshot ?: return null val play = position?.let { snapshot.anchorContextPlay(it) } ?: return null val entry = snapshot.locateContext(play)?.first ?: return null diff --git a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt index b7bd4eb0..740194ee 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt @@ -137,10 +137,10 @@ fun AlbumDetailScreen( artwork: ArtworkColors?, downloads: Map = emptyMap(), onBack: () -> Unit, - onSongClick: (Int) -> Unit, + onSongClick: (String) -> Unit, onSongLongClick: (MenuTarget) -> Unit = {}, onShuffle: () -> Unit = {}, - onPlay: () -> Unit = { onSongClick(0) }, + onPlay: () -> Unit = {}, downloadStatus: ListDownloadStatus = ListDownloadStatus(), onDownloadAction: (BulkDownloadAction) -> Unit = {}, onMore: () -> Unit = {}, @@ -204,7 +204,7 @@ fun AlbumDetailScreen( }, content = { rowModifier -> var lastDisc: Long? = null - songs.forEachIndexed { index, song -> + songs.forEach { song -> val disc = song.disc ?: 1L val discLabel = discLabels[disc] if (disc != lastDisc && (multiDisc || discLabel != null)) { @@ -221,7 +221,7 @@ fun AlbumDetailScreen( download = downloads[song.id], modifier = rowModifier.combinedClickable( - onClick = { onSongClick(index) }, + onClick = { onSongClick(song.id) }, onLongClick = { onSongLongClick(MenuTarget.Song(song, coverArt(album?.coverArt, true), downloads[song.id])) }, diff --git a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailViewModel.kt index 4bb06404..41445527 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailViewModel.kt @@ -79,9 +79,10 @@ class AlbumDetailViewModel( } } - fun play(startIndex: Int) { + fun play(songId: String) { viewModelScope.launch { - playbackController.playAlbum(sourceId.first(), albumId, startIndex.toLong()) + val id = sourceId.first() + playbackController.playAlbum(id, albumId, libraryRepository.albumSongOrdinal(id, albumId, songId)) } } diff --git a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt index 3701a055..1678d420 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt @@ -113,10 +113,10 @@ fun PlaylistDetailScreen( downloadStatus: ListDownloadStatus = ListDownloadStatus(), onDownloadAction: (BulkDownloadAction) -> Unit = {}, onBack: () -> Unit, - onSongClick: (Int) -> Unit, + onSongClick: (Long) -> Unit, onSongLongClick: (MenuTarget) -> Unit = {}, onShuffle: () -> Unit = {}, - onPlay: () -> Unit = { onSongClick(0) }, + onPlay: () -> Unit = {}, onMore: () -> Unit = {}, playingSongId: String? = null, modifier: Modifier = Modifier, @@ -155,7 +155,7 @@ fun PlaylistDetailScreen( download = downloads[item.song.id], modifier = rowModifier.combinedClickable( - onClick = { onSongClick(index) }, + onClick = { onSongClick(item.position) }, onLongClick = { onSongLongClick(MenuTarget.Song(item.song, coverArt(item.coverArt, true), downloads[item.song.id])) }, diff --git a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailViewModel.kt index b8f2cce8..3dfb8029 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailViewModel.kt @@ -81,9 +81,9 @@ class PlaylistDetailViewModel( thumbnail: Boolean, ): CoverArtRef? = sourceRepository.coverArt(coverArt, thumbnail) - fun play(startIndex: Int) { + fun play(position: Long) { viewModelScope.launch { - playbackController.playPlaylist(sourceId.first(), playlistId, startIndex.toLong()) + playbackController.playPlaylist(sourceId.first(), playlistId, position) } } diff --git a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt index ebd8cd7c..53528e78 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -375,8 +375,6 @@ class LibraryDaoTest { db.downloadDao().upsert(SongDownload(sourceId, "s3", DownloadStatus.Completed)) assertEquals(listOf("s1", "s3"), dao.songsByAlbumDownloaded(sourceId, "al-1").first().map { it.id }) - assertEquals(listOf("s1", "s3"), dao.downloadedAlbumSongIds(sourceId, "al-1")) - assertEquals(listOf("s1", "s3"), dao.downloadedPlaylistSongIds(sourceId, "pl-1")) assertEquals( listOf("s1", "s3"), dao.playlistSongsDownloaded(sourceId, "pl-1").page().map { it.song.id }, diff --git a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt index 1d18fe32..a007b322 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt @@ -130,6 +130,22 @@ class LibraryRepositoryTest { } } + @Test + fun albumSongOrdinalFollowsTheAlbumOrder() = + runBlocking { + val sourceId = sourceRepository.addSource("nav", "http://localhost/", "u", "p", false) + db.libraryDao().upsertSongs( + listOf( + seedSong(sourceId).copy(id = "s3", track = 3), + seedSong(sourceId).copy(id = "s1", track = 1), + seedSong(sourceId).copy(id = "s2", track = 2), + ), + ) + + assertEquals(0L, repository.albumSongOrdinal(sourceId, "al1", "s1")) + assertEquals(2L, repository.albumSongOrdinal(sourceId, "al1", "s3")) + } + private suspend fun withSource(block: suspend (MockWebServer) -> Unit) { val server = MockWebServer() server.start() diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 1ea842e1..1017926e 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -11,6 +11,8 @@ 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.Playlist +import com.subtracks.data.model.PlaylistSong import com.subtracks.data.model.QueueEntry import com.subtracks.data.model.QueueKind import com.subtracks.data.model.Song @@ -2495,6 +2497,68 @@ class PlaybackControllerTest { ) } + @Test + fun offlinePlayAlbumFiltersTheWindowToDownloadedSongs() { + 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 }) + assertEquals(listOf(QueueKind.Album), runBlocking { queues.snapshot().entries }.map { it.entry.kind }) + assertEquals(1, runBlocking { queues.snapshot().entries }.size) + } + + @Test + fun offlinePlaylistPlayKeepsASingleReferenceQueue() { + seedAlbum(3, sourceId = 1) + runBlocking { + db.libraryDao().upsertPlaylists( + listOf(Playlist(1, "pl1", "Playlist", comment = null, coverArt = null, songCount = 3, created = 0)), + ) + db.libraryDao().upsertPlaylistSongs( + listOf( + PlaylistSong(1, "pl1", "s1", 0), + PlaylistSong(1, "pl1", "s2", 1), + PlaylistSong(1, "pl1", "s3", 2), + ), + ) + } + markDownloaded(1, "s1") + markDownloaded(1, "s2") + markDownloaded(1, "s3") + setOffline() + + controller.playPlaylist(1, "pl1", 0) + + await { handle.items.map { it.id } == listOf("s1", "s2", "s3") } + assertEquals(listOf(QueueKind.Playlist), runBlocking { queues.snapshot().entries }.map { it.entry.kind }) + assertEquals(1, runBlocking { queues.snapshot().entries }.size) + } + + @Test + fun offlinePlayAlbumStartsAtTheOrdinalItIsGiven() { + seedAlbum(4, sourceId = 1) + markDownloaded(1, "s2") + markDownloaded(1, "s3") + setOffline() + + controller.playAlbum(1, "al1", 2) + + await { + controller.state.value.item + ?.id == "s3" + } + assertEquals( + "s3", + controller.state.value.item + ?.id, + ) + } + @Test fun offlinePlayAlbumKeepsTheAlbumAsThePlayingContext() { seedAlbum(4, sourceId = 1) @@ -2558,19 +2622,6 @@ class PlaybackControllerTest { ) } - @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) @@ -2682,17 +2733,14 @@ class PlaybackControllerTest { 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) + runBlocking { + downloads.reconcile() + db.downloadDao().upsert(SongDownload(sourceId, songId, DownloadStatus.Completed)) + file.parentFile?.mkdirs() + file.writeBytes(byteArrayOf(1)) + downloads.reconcile() } - assertTrue("Timed out marking $songId downloaded", downloads.localUri(songId) != null) + await { downloads.localUri(songId) != null } } private fun setOffline() {